Clean up infinite explicit bucket bounds to prevent duplicate histogram bucket output (#6290)

Co-authored-by: Alan West <[email protected]>

Author: VanyaChe

src/OpenTelemetry/Metrics/MetricPoint/HistogramBuckets.cs

@@ -33,6 +33,7 @@ public class HistogramBuckets
 
     internal HistogramBuckets(double[]? explicitBounds)
     {
+        explicitBounds = CleanUpInfinitiesFromExplicitBounds(explicitBounds);
         this.ExplicitBounds = explicitBounds;
         this.findHistogramBucketIndex = this.FindBucketIndexLinear;
         if (explicitBounds != null && explicitBounds.Length >= DefaultBoundaryCountForBinarySearch)
@@ -158,6 +159,9 @@ internal void Snapshot(bool outputDelta)
         }
     }
 
+    private static double[]? CleanUpInfinitiesFromExplicitBounds(double[]? explicitBounds) => explicitBounds
+        ?.Where(b => !double.IsNegativeInfinity(b) && !double.IsPositiveInfinity(b)).ToArray();
+
     /// <summary>
     /// Enumerates the elements of a <see cref="HistogramBuckets"/>.
     /// </summary>

test/OpenTelemetry.Extensions.Hosting.Tests/OpenTelemetry.Extensions.Hosting.Tests.csproj

@@ -23,6 +23,7 @@
     IMetricsBuilder/IMetricsListener API added at the host level in .NET 8
     instead of the direct lower-level MeterListener API added in .NET 6. -->
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Metrics\AggregatorTests.cs" Link="Includes\Metrics\AggregatorTests.cs" />
+    <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Metrics\HistogramBoundaryTestCase.cs" Link="Includes\Metrics\HistogramBoundaryTestCase.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Metrics\HostingMeterProviderBuilder.cs" Link="Includes\Metrics\HostingMeterProviderBuilder.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Metrics\KnownHistogramBuckets.cs" Link="Includes\Metrics\KnownHistogramBuckets.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Metrics\MetricApiTests.cs" Link="Includes\Metrics\MetricApiTests.cs" />

test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs

@@ -21,6 +21,8 @@ public AggregatorTests()
         this.aggregatorStore = new(MetricStreamIdentity, AggregationType.HistogramWithBuckets, AggregationTemporality.Cumulative, 1024);
     }
 
+    public static TheoryData<HistogramBoundaryTestCase> HistogramInfinityBoundariesTestCases => HistogramBoundaryTestCase.HistogramInfinityBoundariesTestCases();
+
     [Fact]
     public void HistogramDistributeToAllBucketsDefault()
     {
@@ -450,6 +452,40 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale)
         Assert.Equal(expectedScale, metricPoint.GetExponentialHistogramData().Scale);
     }
 
+    [Theory]
+    [MemberData(nameof(HistogramBoundaryTestCase.HistogramInfinityBoundariesTestCases))]
+    internal void HistogramBucketBoundariesTest(HistogramBoundaryTestCase boundaryTestCase)
+    {
+        // Arrange
+        var histogramPoint = new MetricPoint(this.aggregatorStore, AggregationType.HistogramWithBuckets, null, boundaryTestCase.InputBoundaries, Metric.DefaultExponentialHistogramMaxBuckets, Metric.DefaultExponentialHistogramMaxScale);
+        var expectedTotalBuckets = boundaryTestCase.ExpectedBucketCounts.Length;
+
+        // Act
+        foreach (var value in boundaryTestCase.InputValues)
+        {
+            histogramPoint.Update(value);
+        }
+
+        histogramPoint.TakeSnapshot(true);
+
+        // Assert
+        var count = histogramPoint.GetHistogramCount();
+        Assert.Equal(boundaryTestCase.InputValues.Length, count);
+
+        int bucketIndex = 0;
+        int actualBucketCount = 0;
+
+        foreach (var histogramBucket in histogramPoint.GetHistogramBuckets())
+        {
+            Assert.Equal(boundaryTestCase.ExpectedBucketCounts[bucketIndex], histogramBucket.BucketCount);
+            Assert.Equal(boundaryTestCase.ExpectedBucketBounds[bucketIndex], histogramBucket.ExplicitBound);
+            bucketIndex++;
+            actualBucketCount++;
+        }
+
+        Assert.Equal(expectedTotalBuckets, actualBucketCount);
+    }
+
     private static void HistogramSnapshotThread(object? obj)
     {
         var args = obj as ThreadArguments;

test/OpenTelemetry.Tests/Metrics/HistogramBoundaryTestCase.cs

@@ -0,0 +1,70 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+using Xunit;
+
+namespace OpenTelemetry.Metrics.Tests;
+
+#pragma warning disable CA1819 // Properties should not return arrays
+#pragma warning disable CA1515 // Consider making public types internal
+public class HistogramBoundaryTestCase(
+#pragma warning restore CA1515 // Consider making public types internal
+    string testName,
+    double[] inputBoundaries,
+    double[] inputValues,
+    long[] expectedBucketCounts,
+    double[] expectedBucketBounds)
+{
+    public double[] InputBoundaries { get; } = inputBoundaries;
+
+    public double[] InputValues { get; } = inputValues;
+
+    public long[] ExpectedBucketCounts { get; } = expectedBucketCounts;
+
+    public double[] ExpectedBucketBounds { get; } = expectedBucketBounds;
+
+    public string TestName { get; set; } = testName;
+
+    public static TheoryData<HistogramBoundaryTestCase> HistogramInfinityBoundariesTestCases()
+    {
+        var data = new TheoryData<HistogramBoundaryTestCase>
+        {
+            new(
+                testName: "Custom boundaries with no infinity in explicit boundaries",
+                inputBoundaries: [0, 10],
+                inputValues: [-10, 0, 5, 10, 100],
+                expectedBucketCounts: [2, 2, 1],
+                expectedBucketBounds: [0, 10, double.PositiveInfinity]),
+
+            new(
+                testName: "Custom boundaries with positive infinity",
+                inputBoundaries: [0, double.PositiveInfinity],
+                inputValues: [-10, 0, 10, 100],
+                expectedBucketCounts: [2, 2],
+                expectedBucketBounds: [0, double.PositiveInfinity]),
+
+            new(
+                testName: "Custom boundaries with negative infinity",
+                inputBoundaries: [double.NegativeInfinity, 0, 10],
+                inputValues: [-100, -10, 0, 5, 10, 100],
+                expectedBucketCounts: [3, 2, 1],
+                expectedBucketBounds: [0, 10, double.PositiveInfinity]),
+
+            new(
+                testName: "Custom boundaries with both infinities",
+                inputBoundaries: [double.NegativeInfinity, 0, 10, double.PositiveInfinity],
+                inputValues: [-100, -10, 0, 5, 10, 100],
+                expectedBucketCounts: [3, 2, 1],
+                expectedBucketBounds: [0, 10, double.PositiveInfinity]),
+
+            new(
+                testName: "Custom boundaries with infinities only",
+                inputBoundaries: [double.NegativeInfinity, double.PositiveInfinity],
+                inputValues: [-10, 0, 10],
+                expectedBucketCounts: [3],
+                expectedBucketBounds: [double.PositiveInfinity]),
+        };
+
+        return data;
+    }
+}