Skip to content

[Exporter.Prometheus] Fix number formatting - #7589

Merged
martincostello merged 8 commits into
open-telemetry:mainfrom
martincostello:gh-7585
Aug 12, 2026
Merged

martincostello merged 8 commits into
open-telemetry:mainfrom
martincostello:gh-7585

Conversation

@martincostello

Copy link
Copy Markdown
Member

Fixes #7585

Changes

Fix incorrectly formatted floating point numbers caused by a misunderstanding of Canonical Numbers.

Specifically this part:

A warning to implementers in C and other languages that share its printf implementation: The standard precision of %f, %e and %g is only six significant digits. 17 significant digits are required for full precision, e.g. printf("%.17g", d).

I took it to mean to use G17 to format things, but that over-specifies the precision and causes non-exact round-tripping issues as described in the linked discussion.

Re-running the benchmarks locally don't show any regression in terms of runtime or allocation due to the use of stackalloc.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

Add a clarifying comment as suggested by Copilot review.
Comment thread src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md Outdated
Comment thread src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md Outdated
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Aug 4, 2026

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-08-12 22:39 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.52%. Comparing base (312c1df) to head (708707f).
⚠️ Report is 17 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7589   +/-   ##
=======================================
  Coverage   90.51%   90.52%           
=======================================
  Files         292      292           
  Lines       16197    16215   +18     
=======================================
+ Hits        14661    14678   +17     
- Misses       1536     1537    +1     
Flag Coverage Δ
unittests-PowerShellScripts 87.43% <ø> (ø)
unittests-Project-Experimental 90.61% <100.00%> (+0.02%) ⬆️
unittests-Project-Stable 90.59% <100.00%> (+0.16%) ⬆️
unittests-Solution 90.59% <100.00%> (+0.02%) ⬆️
unittests-UnstableCoreLibraries-Experimental 51.18% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ernal/Shared/Serialization/TextFormatSerializer.cs 98.81% <100.00%> (+0.02%) ⬆️

... and 3 files with indirect coverage changes

@github-actions github-actions Bot added pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package labels Aug 4, 2026
@martincostello
martincostello marked this pull request as ready for review August 5, 2026 08:10
@martincostello
martincostello requested a review from a team as a code owner August 5, 2026 08:10
Copilot AI lite review requested due to automatic review settings August 5, 2026 08:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes floating-point number formatting in the Prometheus exporters’ text serialization to use the shortest round-trippable representation for sample values, and to correctly render OpenMetrics canonical numbers for histogram le and summary quantile label values. It updates unit/integration/fuzz tests and changelogs to lock in the corrected behavior and prevent regressions.

Changes:

  • Updated Prometheus text serialization to avoid G17-style over-precision padding and to correctly render canonical numbers for le/quantile.
  • Added/updated unit, integration, fuzz tests and snapshot baselines to validate round-tripping and boundary rendering.
  • Added CHANGELOG entries for both Prometheus.HttpListener and Prometheus.AspNetCore packages.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramWithFractionalBoundaries_useOpenMetrics=True.verified.txt New snapshot baseline for histogram fractional boundary rendering (OpenMetrics).
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramWithFractionalBoundaries_useOpenMetrics=False.verified.txt New snapshot baseline for histogram fractional boundary rendering (Prometheus text).
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusSerializerTests.cs Updates numeric formatting expectations and adds regression tests for round-tripping/precision.
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusIntegrationTests.cs Adjusts integration test bucket boundaries to cover more rendered shapes.
test/OpenTelemetry.Exporter.Prometheus.HttpListener.FuzzTests/PrometheusSerializerFuzzTests.cs Updates fuzz property to validate round-tripping without unnecessary padding.
test/OpenTelemetry.Exporter.Prometheus.AspNetCore.Tests/PrometheusIntegrationTests.cs Adds assertions to validate bucket boundary round-tripping through Prometheus API.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/Shared/Serialization/TextFormatSerializer.cs Core implementation change: shortest round-trippable formatting and corrected canonical number rendering.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md Adds Unreleased entries describing formatting/canonical-number fixes.
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md Adds Unreleased entries describing formatting/canonical-number fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add a timeout to test container start-up and dispose.
- Remove redundant port binding.
- Do not wait indefinitely for Prometheus test container logs.
- Collect hang dumps from tests.
@github-actions github-actions Bot added the infra Infra work - CI/CD, code coverage, linters label Aug 5, 2026
@martincostello martincostello added the keep-open Prevents issues and pull requests being closed as stale label Aug 7, 2026
@martincostello martincostello mentioned this pull request Aug 11, 2026
4 tasks
@martincostello martincostello added the review-priority Should be prioritized for review label Aug 11, 2026
@martincostello
martincostello added this pull request to the merge queue Aug 12, 2026
Merged via the queue into open-telemetry:main with commit 0d098dd Aug 12, 2026
107 checks passed
@martincostello
martincostello deleted the gh-7585 branch August 12, 2026 22:38
This was referenced Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infra Infra work - CI/CD, code coverage, linters pkg:OpenTelemetry.Exporter.Prometheus.AspNetCore Issues related to OpenTelemetry.Exporter.Prometheus.AspNetCore NuGet package pkg:OpenTelemetry.Exporter.Prometheus.HttpListener Issues related to OpenTelemetry.Exporter.Prometheus.HttpListener NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants