Fix float histogram bucket precision

[pratik-mahalle]

Fixes #6803

Changes

Fixed precision issues when using Histogram<float> with custom HistogramBucketBoundaries.

Problem: Float values like 0.025f were being converted to double with precision errors, resulting in bucket boundaries like 0.02500000037252903.

Solution: For float types, convert to string first (which preserves the intended decimal representation) then parse as double. This ensures clean bucket boundaries (e.g., 0.025f → 0.025).

Other numeric types (int, long, short, byte, double) continue using the existing Convert.ToDouble() approach.

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)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.65%. Comparing base (14ef30b) to head (19dbc57).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6866   +/-   ##
=======================================
  Coverage   88.65%   88.65%           
=======================================
  Files         263      263           
  Lines       12397    12414   +17     
=======================================
+ Hits        10990    11006   +16     
- Misses       1407     1408    +1     

Flag	Coverage Δ
unittests-Project-Experimental	88.59% <100.00%> (+<0.01%)	⬆️
unittests-Project-Stable	88.57% <100.00%> (+0.12%)	⬆️

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

Files with missing lines	Coverage Δ
...ntation/Serializer/ProtobufOtlpMetricSerializer.cs	98.93% <100.00%> (ø)
src/OpenTelemetry/Metrics/AggregatorStore.cs	87.86% <100.00%> (+0.26%)	⬆️
...c/OpenTelemetry/Metrics/HistogramExplicitBounds.cs	100.00% <100.00%> (ø)
...nTelemetry/Metrics/MetricPoint/HistogramBuckets.cs	100.00% <100.00%> (ø)
src/OpenTelemetry/Metrics/MetricStreamIdentity.cs	92.50% <100.00%> (+0.71%)	⬆️

... and 1 file with indirect coverage changes

[Copilot]

Pull request overview

This pull request fixes a floating-point precision issue when using Histogram<float> with custom HistogramBucketBoundaries. The problem occurred when float values like 0.025f were converted to double, resulting in precision errors like 0.02500000037252903 instead of the intended 0.025.

Changes:

• Modified float-to-double conversion for histogram bucket boundaries to preserve decimal precision
• Added comprehensive test to verify the precision fix works correctly

Reviewed changes

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

File	Description
src/OpenTelemetry/Metrics/MetricStreamIdentity.cs	Implemented float-to-string-to-double conversion to preserve decimal precision for float histogram boundaries
test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs	Added test to verify float histogram boundaries maintain proper precision

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

This test validates that exported ExplicitBound values look “clean”, but it doesn’t guard against a potential semantic regression where recording a value exactly equal to a float boundary (e.g., histogram.Record(0.025f) or 0.1f) no longer increments the expected bucket due to the float->double boundary conversion. Add an assertion that a boundary-equal float measurement lands in the correct (inclusive) bucket to catch this.

[Copilot]

The new float-handling path converts bucket boundaries from float -> string -> double. This changes the numeric boundary used for bucketing (e.g., 0.1f is recorded as 0.10000000149011612 via the float measurement callback, but the boundary becomes 0.1), which can shift measurements at/near the boundary into a different bucket (breaking the “upper bound inclusive” behavior for values equal to the provided float boundary). Consider keeping internal bucketing bounds as the exact float value (float-cast-to-double) and only “cleaning” the values at export/serialization time (or store separate arrays for lookup vs. exported explicit bounds).

Suggested change

	// For float types, convert to string first to preserve the intended decimal precision
	// and avoid floating-point representation issues (e.g., 0.025f -> 0.025 instead of 0.02500000037252903)
	explicitBucketBoundaries[i] = typeof(T) == typeof(float)
	? double.Parse(
	Convert.ToString(adviceExplicitBucketBoundaries[i], CultureInfo.InvariantCulture)!,
	CultureInfo.InvariantCulture)
	: Convert.ToDouble(
	adviceExplicitBucketBoundaries[i],
	CultureInfo.InvariantCulture);
	if (typeof(T) == typeof(float))
	{
	// For float types, preserve the exact numeric value by casting directly from float to double.
	// This avoids changing the numeric boundary (e.g., 0.10000000149011612f) while still promoting
	// to double for internal bucketing comparisons.
	explicitBucketBoundaries[i] = (double)(float)(object)adviceExplicitBucketBoundaries[i]!;
	}
	else
	{
	explicitBucketBoundaries[i] = Convert.ToDouble(
	adviceExplicitBucketBoundaries[i],
	CultureInfo.InvariantCulture);
	}

[pratik-mahalle]

The current fix (converting float→string→double) changes the numeric boundary used for bucketing, which could cause boundary-equal measurements to land in the wrong bucket:

When recording 0.025f, the value becomes 0.02500000037252903 (float cast to double)
With this fix, the boundary becomes 0.025 (clean double)
Result: 0.02500000037252903 > 0.025, so the measurement lands in the NEXT bucket

Options:

• Fix at export time only - Keep exact float values for internal bucketing, but clean them when exporting/serializing bucket labels
• Document as expected behavior - Users should use Histogram with double boundaries to avoid precision issues

What should I do ? @martincostello @cijothomas

[martincostello]

I added some additional tests in #6869 to cover different histogram types (here), and there's assertions for buckets included in those.

Maybe add those to this PR and see what the behaviour is with MultithreadedSingleCounterTest unskipped?

[pratik-mahalle]

okay, Iets see, that works or not

[pratik-mahalle]

See @martincostello https://github.com/open-telemetry/opentelemetry-dotnet/actions/runs/21823389316/job/62963644437?pr=6866

[pratik-mahalle]

I've made the requested changes, but there's a critical issue that needs your decision:

The Problem
The current fix (converting float→string→double) changes bucketing behavior:

When recording histogram.Record(0.025f), the value becomes 0.02500000037252903 (float cast to double)
With this fix, the boundary becomes the clean 0.025
Result: 0.02500000037252903 > 0.025, so the measurement lands in the NEXT bucket ❌

[pratik-mahalle]

see https://github.com/open-telemetry/opentelemetry-dotnet/actions/runs/21860044391/job/63086519937?pr=6866

[martincostello]

Feels to me like that's an unavoidable consequence of using float because it's inherently not as precise.

While we can make the behaviour for the boundaries more natural with the conversion fix, I'm not sure it makes sense to do that to record the measurements themselves. We could do the same fix there to convert float to double, but there would be a performance overhead.

Let's see what other reviewers think, but possible options could be:

1. Apply the same fix for float values (if possible)
2. Adjust the test to use values for the measurements that fall in the right buckets

[pratik-mahalle]

Okay, lets have an opinion from the others Maintainers first

[pratik-mahalle]

Have a look on the new changes @martincostello

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[pratik-mahalle]

/unstale

[rajkumar-rangaraj]

I will review it this week.

[rajkumar-rangaraj]

Thanks for working on this @pratik-mahalle! The fix addresses a real user pain point, but I have a concern about the current approach that I think we need to address before merging.

Bucketing Correctness Regression

The current fix cleans up bucket boundaries at setup time in MetricStreamIdentity, but the recorded measurement values still go through a raw float → double cast. This creates a mismatch:

// Boundary after this PR (cleaned):
0.025

// Value when user calls histogram.Record(0.025f):
(double)0.025f = 0.02500000037252903

// Result: 0.02500000037252903 > 0.025 → lands in NEXT bucket ❌

Before this PR, both the boundary and the recorded value had the same float-to-double imprecision, so they matched correctly. After this PR, every float measurement that exactly equals a boundary will be off by one bucket. This is a semantic regression, the labels look better, but the data is wrong.

This is the same concern both Copilot and @martincostello raised in the earlier review threads. As Martin noted, applying the same float→string→double conversion on the measurement recording path would fix the consistency but introduces a performance overhead on what is a hot path.

We could consider fixing at export/serialization time, not at boundary setup.