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

Fix exemplars being added to gauge metrics in the prometheus exporter #5912

Merged
merged 12 commits into from
Oct 24, 2024

Conversation

trthomps
Copy link
Contributor

Prometheus Gauge metrics don't support exemplars and while addGaugeMetric() doesn't add them, addSumMetric() will if the metric is monotonic. This causes the prometheus client to throw an error:

* error collecting metric Desc{fqName: "http_server_request_body_size_bytes", help: "Measures size of RPC request messages (uncompressed).", constLabels: {}, variableLabels: {net_protocol_name,net_protocol_version,http_method,http_route,http_scheme,net_host_name,net_host_port,otel_scope_name,otel_scope_version}}: cannot inject exemplar into Gauge, Summary or Untyped

Copy link

linux-foundation-easycla bot commented Oct 22, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@trthomps trthomps marked this pull request as draft October 22, 2024 17:34
@trthomps trthomps force-pushed the fix-prometheus-gauge-exemplars branch from e713527 to 7f366df Compare October 22, 2024 17:38
@dashpole
Copy link
Contributor

Would be good to have a test

Copy link

codecov bot commented Oct 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.6%. Comparing base (30c4a9a) to head (3700b79).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #5912   +/-   ##
=====================================
  Coverage   84.6%   84.6%           
=====================================
  Files        272     272           
  Lines      22837   22839    +2     
=====================================
+ Hits       19322   19325    +3     
+ Misses      3171    3170    -1     
  Partials     344     344           

see 2 files with indirect coverage changes

@trthomps trthomps force-pushed the fix-prometheus-gauge-exemplars branch from 45c3a10 to 2d16850 Compare October 22, 2024 20:27
@trthomps trthomps marked this pull request as ready for review October 22, 2024 20:27
@trthomps
Copy link
Contributor Author

Would be good to have a test

@dashpole Added a test, let me know if you know of a better way to test this, I did step through the debugger and exemplars are getting added to the DataPoint.

CHANGELOG.md Outdated Show resolved Hide resolved
exporters/prometheus/exporter.go Outdated Show resolved Hide resolved
exporters/prometheus/exporter_test.go Outdated Show resolved Hide resolved
trthomps and others added 4 commits October 23, 2024 10:54
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: David Ashpole <dashpole@google.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
@trthomps
Copy link
Contributor Author

Thanks so much for working with me on this @dashpole.

@dmathieu dmathieu merged commit 664a075 into open-telemetry:main Oct 24, 2024
32 checks passed
@trthomps trthomps deleted the fix-prometheus-gauge-exemplars branch October 24, 2024 16:30
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.

3 participants