Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed untyped Prometheus receiver bug, updated tests and updated Design.md #1194

Conversation

Nicolas-MacBeth
Copy link
Contributor

Description:
Made textparse.MetricTypeUnknown metrics convert to gauge by default to fix a Prometheus Receiver bug where untyped metrics were being dropped.

Link to tracking Issue:
Issue link with much more details: #1103

Testing:
Modified the tests for the new type the metrics are being converted to.

Documentation:
Added a small line in the design documentation stating that we default untyped metrics to gauge

Thanks @asuresh4 and @bogdandrutu

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 25, 2020

CLA Check
The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Jun 25, 2020

Codecov Report

Merging #1194 into master will increase coverage by 0.00%.
The diff coverage is 92.30%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1194   +/-   ##
=======================================
  Coverage   87.56%   87.57%           
=======================================
  Files         203      203           
  Lines       14555    14562    +7     
=======================================
+ Hits        12745    12752    +7     
- Misses       1370     1371    +1     
+ Partials      440      439    -1     
Impacted Files Coverage Δ
config/configtls/configtls.go 95.65% <86.66%> (-4.35%) ⬇️
config/configgrpc/configgrpc.go 100.00% <100.00%> (ø)
receiver/jaegerreceiver/factory.go 91.52% <100.00%> (ø)
receiver/opencensusreceiver/config.go 94.44% <100.00%> (ø)
receiver/otlpreceiver/config.go 94.44% <100.00%> (ø)
...iver/prometheusreceiver/internal/metricsbuilder.go 100.00% <100.00%> (ø)
translator/internaldata/resource_to_oc.go 76.47% <0.00%> (+2.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b3f0d7...1744e61. Read the comment docs.

@bogdandrutu bogdandrutu merged commit bd886e8 into open-telemetry:master Jun 25, 2020
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
…gn.md (open-telemetry#1194)

* Fixed untyped Prometheus receiver bug, updated tests and updated Design.md

* fixed a small comment
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
* Remove http status text

* Updated changelog

* More descriptive changelog

Co-authored-by: Tyler Yahn <[email protected]>

Co-authored-by: Tyler Yahn <[email protected]>
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this pull request Oct 9, 2024
)

* Enable external configmap checksum

Fixes [collector] No collector reload when using external configmap open-telemetry#1194

* Update examples for `opentelemetry-collector`

* Rename option to `configFile` for better consistency

* Template checksum annotation for all Deployment modes

* Add example for external configmap

* Hint with option name that config is a chart Template

* Remove unneeded example

* Adhere to existing user-config convention

* Update Collector chart schema to include new property

* Regenerate Collector examples

* Revert "Regenerate Collector examples"

This reverts commit 5e6d8f885e46dc1e74ef2bd312c75d1e3c0fca89.

* Remove configmap from local testing

* Generate annotation checksums by inferring ConfigMap name from the value of `existingName`

* Remove configmap from local testing

* Run `make generate-examples CHARTS=opentelemetry-collector`

* Bring back `configMap.existingPath` option

There doesn't seem to be a way to get at rendered template contents in Helm. This key allows a user to specify the path to template file that will be used to create `configMap.existingName`, in cases like mine where we want to wrap this chart in a chart that feeds in custom configmap contents.

* Regen examples to match upstream

* Improve comments based on PR comments

* Bump chart

* Update examples for chart version `102.1`

* Prefer `SHOULD`

Co-authored-by: Tyler Helmuth <[email protected]>

---------

Co-authored-by: Tyler Helmuth <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants