[Exporter.Prometheus] Fix spec-compliance issues#7454
Conversation
- Fix double unit suffixes in metric names when using OpenMetrics. - Fix incorrect handling of leading digits in metric names for OpenMetrics. - Fix incorrect `Content-Type` for PrometheusText.
Add PR numbers.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7454 +/- ##
==========================================
- Coverage 89.94% 89.85% -0.10%
==========================================
Files 277 277
Lines 14637 14642 +5
==========================================
- Hits 13165 13156 -9
- Misses 1472 1486 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Pull request overview
This PR targets spec-compliance fixes in the Prometheus exporters (HttpListener + ASP.NET Core), primarily around OpenMetrics metric naming and scrape response Content-Type generation.
Changes:
- Adjust OpenMetrics metric name sanitization (leading digits handling) and unit-suffix behavior for counters.
- Update
Content-Typegeneration logic and expand/adjust protocol-related test coverage. - Add changelog entries for both Prometheus exporter packages.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusProtocolTests.cs | Adds GetContentType/ToString protocol tests (currently asserts PrometheusText omits charset). |
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs | Updates OpenMetrics naming tests (leading digits + unit/_total interactions). |
| test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusHttpListenerTests.cs | Updates integration expectations for scrape Content-Type. |
| test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusIntegrationTests.cs | Updates ASP.NET Core scrape test Content-Type expectation. |
| test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusExporterMiddlewareTests.cs | Updates middleware integration expectations for scrape Content-Type. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/PrometheusProtocol.cs | Changes Content-Type composition (charset now conditional on OpenMetrics). |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/PrometheusMetric.cs | Fixes OpenMetrics unit suffix duplication for counters; adjusts leading-digit escaping behavior. |
| src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md | Records the spec-compliance fixes. |
| src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md | Records the spec-compliance fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Revert the changes to the `Content-Type` header's `charset`.
Fix issue with counter suffixes.
Rename tests to be less specific.
Fix changes that didn't get reverted.
Kielek
left a comment
There was a problem hiding this comment.
Pushed some direct feedback, please double check and merge if you are fine.
Changes
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)