Fix float histogram bucket precision by pratik-mahalle · Pull Request #6866 · open-telemetry/opentelemetry-dotnet

pratik-mahalle · 2026-02-01T16:03:33Z

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)

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

codecov · 2026-02-01T16:10:58Z

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
2515	4	2511	10

View the top 3 failed test(s) by shortest run time

OpenTelemetry.Metrics.Tests.MetricViewTests::HistogramWithFloatAdviceBoundaries_MaintainsPrecision

Stack Traces | 0.00554s run time

at OpenTelemetry.Metrics.Tests.MetricViewTests.HistogramWithFloatAdviceBoundaries_MaintainsPrecision() in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricViewTests.cs:line 831
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

OpenTelemetry.Metrics.Tests.MetricViewTests::HistogramWithFloatAdviceBoundaries_MaintainsPrecision

Stack Traces | 0.0148s run time

at OpenTelemetry.Metrics.Tests.MetricViewTests.HistogramWithFloatAdviceBoundaries_MaintainsPrecision() in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricViewTests.cs:line 831
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

OpenTelemetry.Metrics.Tests.MetricApiTests::MultithreadedSingleCounterTest

Stack Traces | 0.16s run time

at OpenTelemetry.Metrics.Tests.MetricApiTests.MultithreadedCounterTest[T](T deltaValueUpdatedByEachCall) in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricApiTests.cs:line 2019
   at OpenTelemetry.Metrics.Tests.MetricApiTests.MultithreadedSingleCounterTest() in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricApiTests.cs:line 1621
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

OpenTelemetry.Metrics.Tests.MetricApiTests::MultithreadedSingleCounterTest

Stack Traces | 0.16s run time

at OpenTelemetry.Metrics.Tests.MetricApiTests.MultithreadedCounterTest[T](T deltaValueUpdatedByEachCall) in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricApiTests.cs:line 2019
   at OpenTelemetry.Metrics.Tests.MetricApiTests.MultithreadedSingleCounterTest() in C:\a\opentelemetry-dotnet\opentelemetry-dotnet\test\OpenTelemetry.Tests\Metrics\MetricApiTests.cs:line 1621
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

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.

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

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 · 2026-02-08T12:43:16Z

test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs

+        // Record some values
+        histogram.Record(0.02f);
+        histogram.Record(0.5f);
+        histogram.Record(5.0f);
+
+        meterProvider.ForceFlush(MaxTimeToAllowForFlush);
+        Assert.Single(exportedItems);
+        var metric = exportedItems[0];
+
+        List<MetricPoint> metricPoints = [];
+        foreach (ref readonly var mp in metric.GetMetricPoints())
+        {
+            metricPoints.Add(mp);
+        }
+
+        Assert.Single(metricPoints);
+        var histogramPoint = metricPoints[0];
+
+        // Verify the bucket boundaries maintain proper precision
+        // The key assertion is that the boundaries should be clean decimal values
+        // not values with floating-point precision errors like 0.02500000037252903
+
+        // Expected clean boundaries (converted from float to double with proper precision)
+        var expectedBoundaries = new double[]
+        {
+            0.001, 0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10, 30, 60, 120,
+        };
+
+        var index = 0;
+        foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets())
+        {
+            if (index < expectedBoundaries.Length)
+            {
+                // Verify each boundary is the expected clean value
+                Assert.Equal(expectedBoundaries[index], histogramMeasurement.ExplicitBound);
+            }
+            else
+            {
+                // Verify the last bucket is positive infinity
+                Assert.Equal(double.PositiveInfinity, histogramMeasurement.ExplicitBound);
+            }
+
+            index++;
+        }
+
+        // Verify we got the expected number of buckets
+        Assert.Equal(expectedBoundaries.Length + 1, index);
+    }


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 · 2026-02-08T12:43:17Z

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs

+                // 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);


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);

}

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

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?

okay, Iets see, that works or not

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

#6869 is merged now, so this PR should be able to unskip that new test for floats.

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 ❌

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

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:

Apply the same fix for float values (if possible)

Adjust the test to use values for the measurements that fall in the right buckets

Okay, lets have an opinion from the others Maintainers first

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

src/OpenTelemetry/CHANGELOG.md

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

Fix float histogram bucket precision

8574b7b

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

pratik-mahalle requested a review from a team as a code owner February 1, 2026 16:03

github-actions bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Feb 1, 2026

martincostello reviewed Feb 3, 2026

View reviewed changes

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs Outdated Show resolved Hide resolved

pratik-mahalle added 2 commits February 3, 2026 16:53

Add test for float histogram bucket precision

8450645

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

fix the test failing

3f4f121

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

martincostello requested a review from a team February 3, 2026 12:15

cijothomas requested a review from Copilot February 5, 2026 19:29

Copilot started reviewing on behalf of cijothomas February 5, 2026 19:29 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs Outdated Show resolved Hide resolved

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs Outdated Show resolved Hide resolved

src/OpenTelemetry/Metrics/MetricStreamIdentity.cs Outdated Show resolved Hide resolved

pratik-mahalle added 2 commits February 8, 2026 17:55

updated as per copilot suggestion

63b1a56

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

chnages uses tostring

85a570c

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

pratik-mahalle requested a review from Copilot February 8, 2026 12:37

Copilot started reviewing on behalf of pratik-mahalle February 8, 2026 12:38 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

added expendedbound arrays

0f54960

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

martincostello reviewed Feb 9, 2026

View reviewed changes

src/OpenTelemetry/CHANGELOG.md Outdated Show resolved Hide resolved

pratik-mahalle and others added 3 commits February 9, 2026 17:02

remove the details from changelog

d5d8353

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

Merge branch 'main' into fix-histogram-float-precision

9d8ea54

unskip the test

8792c52

Signed-off-by: Pratik Mahalle <mahallepratik683@gmail.com>

Conversation

pratik-mahalle commented Feb 1, 2026

Changes

Merge requirement checklist

Uh oh!

codecov bot commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 4 Tests Failed:

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

pratik-mahalle Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Options:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Feb 1, 2026 •

edited

Loading

pratik-mahalle Feb 8, 2026 •

edited

Loading