diff --git a/OpenTelemetry.slnx b/OpenTelemetry.slnx index 7e3e7ca4875..971293feae7 100644 --- a/OpenTelemetry.slnx +++ b/OpenTelemetry.slnx @@ -237,6 +237,7 @@ + diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index b2dd3221336..7a074122016 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -17,6 +17,10 @@ Notes](../../RELEASENOTES.md). * Update `OpenTelemetrySdkEventSource` to support the W3C randomness flag. ([#7301](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7301)) +* **Breaking Change** Explicit histogram boundaries no longer allow more than + 10 million values. + ([#7165](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7165)) + ## 1.15.3 Released 2026-Apr-21 diff --git a/src/OpenTelemetry/Metrics/HistogramExplicitBounds.cs b/src/OpenTelemetry/Metrics/HistogramExplicitBounds.cs index bd4b6b06731..317370a1dd7 100644 --- a/src/OpenTelemetry/Metrics/HistogramExplicitBounds.cs +++ b/src/OpenTelemetry/Metrics/HistogramExplicitBounds.cs @@ -2,7 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +#if NET +using System.Numerics; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; +using System.Runtime.Intrinsics.Arm; +using System.Runtime.Intrinsics.X86; +#endif using System.Runtime.CompilerServices; +using OpenTelemetry.Internal; namespace OpenTelemetry.Metrics; @@ -10,37 +18,19 @@ internal sealed class HistogramExplicitBounds { internal const int DefaultBoundaryCountForBinarySearch = 50; - private readonly BucketLookupNode? bucketLookupTreeRoot; - private readonly Func findHistogramBucketIndex; + private const int RadixLookupBitCount = 12; + private const int RadixLinearSearchThreshold = 32; + + private readonly RadixBucketLookup? radixBucketLookup; public HistogramExplicitBounds(double[] bounds, double[]? displayBounds = null) { this.Bounds = CleanUpInfinitiesFromExplicitBounds(bounds); this.DisplayBounds = displayBounds != null ? CleanUpInfinitiesFromExplicitBounds(displayBounds) : null; - this.findHistogramBucketIndex = this.FindBucketIndexLinear; if (this.Bounds.Length >= DefaultBoundaryCountForBinarySearch) { - this.bucketLookupTreeRoot = ConstructBalancedBST(this.Bounds, 0, this.Bounds.Length); - this.findHistogramBucketIndex = this.FindBucketIndexBinary; - - static BucketLookupNode? ConstructBalancedBST(double[] values, int min, int max) - { - if (min == max) - { - return null; - } - - var median = min + ((max - min) / 2); - return new BucketLookupNode - { - Index = median, - UpperBoundInclusive = values[median], - LowerBoundExclusive = median > 0 ? values[median - 1] : double.NegativeInfinity, - Left = ConstructBalancedBST(values, min, median), - Right = ConstructBalancedBST(values, median + 1, max), - }; - } + this.radixBucketLookup = new(this.Bounds); } } @@ -56,8 +46,42 @@ public HistogramExplicitBounds(double[] bounds, double[]? displayBounds = null) [MethodImpl(MethodImplOptions.AggressiveInlining)] public int FindBucketIndex(double value) - => this.findHistogramBucketIndex(value); + { + if (double.IsNaN(value)) + { + return this.Bounds.Length; + } + if (this.Bounds.Length == 0) + { + return 0; + } + + if (value <= this.Bounds[0]) + { + return 0; + } + + if (value > this.Bounds[this.Bounds.Length - 1]) + { + return this.Bounds.Length; + } + + if (this.radixBucketLookup != null) + { + (var start, var end) = this.radixBucketLookup.GetBucketSearchRange(value); + + return start == end + ? start + : end - start > RadixLinearSearchThreshold + ? this.FindBucketIndexBinary(value, start, end) + : this.FindBucketIndexLinear(value, start, end); + } + + return this.FindBucketIndexLinear(value, 0, this.Bounds.Length); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static double[] CleanUpInfinitiesFromExplicitBounds(double[] bounds) { for (var i = 0; i < bounds.Length; i++) @@ -71,59 +95,209 @@ private static double[] CleanUpInfinitiesFromExplicitBounds(double[] bounds) return bounds; } +#if NET [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int FindBucketIndexBinary(double value) + private static int FindBucketIndexLinearSimd(ReadOnlySpan bounds, double value) { - var current = this.bucketLookupTreeRoot; + var index = 0; - do + if (Avx.IsSupported && bounds.Length >= Vector256.Count) { - if (value <= current!.LowerBoundExclusive) + ref var searchSpace = ref MemoryMarshal.GetReference(bounds); + var valueVector = Vector256.Create(value); + var lastStart = bounds.Length - Vector256.Count; + + while (index <= lastStart) { - current = current.Left; + var boundsVector = Vector256.LoadUnsafe(ref searchSpace, (nuint)index); + var compare = Avx.CompareLessThanOrEqual(valueVector, boundsVector); + var mask = Avx.MoveMask(compare); + + if (mask != 0) + { + return index + BitOperations.TrailingZeroCount((uint)mask); + } + + index += Vector256.Count; } - else if (value > current.UpperBoundInclusive) + } + else if (Sse2.IsSupported && bounds.Length >= Vector128.Count) + { + ref var searchSpace = ref MemoryMarshal.GetReference(bounds); + var valueVector = Vector128.Create(value); + var lastStart = bounds.Length - Vector128.Count; + + while (index <= lastStart) { - current = current.Right; + var boundsVector = Vector128.LoadUnsafe(ref searchSpace, (nuint)index); + var compare = Sse2.CompareLessThanOrEqual(valueVector, boundsVector); + var mask = Sse2.MoveMask(compare); + + if (mask != 0) + { + return index + BitOperations.TrailingZeroCount((uint)mask); + } + + index += Vector128.Count; } - else + } + else if (AdvSimd.Arm64.IsSupported && bounds.Length >= Vector128.Count) + { + ref var searchSpace = ref MemoryMarshal.GetReference(bounds); + var valueVector = Vector128.Create(value); + var lastStart = bounds.Length - Vector128.Count; + + while (index <= lastStart) { - return current.Index; + var boundsVector = Vector128.LoadUnsafe(ref searchSpace, (nuint)index); + var compare = AdvSimd.Arm64.CompareLessThanOrEqual(valueVector, boundsVector).AsUInt64(); + + if (compare.GetElement(0) != 0) + { + return index; + } + + if (compare.GetElement(1) != 0) + { + return index + 1; + } + + index += Vector128.Count; + } + } + + for (; index < bounds.Length; index++) + { + if (value <= bounds[index]) + { + return index; } } - while (current != null); - Debug.Assert(this.Bounds != null, "ExplicitBounds was null."); + return -1; + } +#endif + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static ulong ToSortableBits(double value) + { + var bits = (ulong)BitConverter.DoubleToInt64Bits(value); + return (bits & 0x8000_0000_0000_0000UL) == 0 + ? bits ^ 0x8000_0000_0000_0000UL + : ~bits; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int FindBucketIndexBinary(double value, int start, int end) + { + var bounds = this.Bounds; + var left = start; + var right = end - 1; + + while (left <= right) + { + var middle = left + ((right - left) / 2); + + if (value <= bounds[middle]) + { + right = middle - 1; + } + else + { + left = middle + 1; + } + } - return this.Bounds!.Length; + return left; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int FindBucketIndexLinear(double value) + private int FindBucketIndexLinear(double value, int start, int end) { - int i; - for (i = 0; i < this.Bounds.Length; i++) +#if NET + if (!double.IsNaN(value)) + { + var index = FindBucketIndexLinearSimd(this.Bounds.AsSpan(start, end - start), value); + if (index >= 0) + { + return start + index; + } + } +#endif + + var bounds = this.Bounds; + + for (var i = start; i < end; i++) { // Upper bound is inclusive - if (value <= this.Bounds[i]) + if (value <= bounds[i]) { - break; + return i; } } - return i; + return end; } - private sealed class BucketLookupNode + private sealed class RadixBucketLookup { - public double UpperBoundInclusive { get; set; } + private readonly int[] bucketSearchStarts; + private readonly int keyMask; + private readonly int shift; + + public RadixBucketLookup(double[] bounds) + { + Debug.Assert(bounds.Length > 0, "bounds was empty"); + + var firstKey = ToSortableBits(bounds[0]); + var lastKey = ToSortableBits(bounds[bounds.Length - 1]); + var commonPrefixLength = MathHelper.LeadingZero64((long)(firstKey ^ lastKey)); + var radixBits = Math.Min(RadixLookupBitCount, 64 - commonPrefixLength); + + if (radixBits == 0) + { + this.bucketSearchStarts = [0, bounds.Length]; + this.keyMask = 0; + this.shift = 0; + } + else + { + var bucketCount = 1 << radixBits; + this.bucketSearchStarts = new int[bucketCount + 1]; + this.keyMask = bucketCount - 1; + this.shift = 64 - commonPrefixLength - radixBits; + + var boundaryIndex = 0; + + for (var key = 0; key < bucketCount; key++) + { + this.bucketSearchStarts[key] = boundaryIndex; + + while (boundaryIndex < bounds.Length && this.GetKey(ToSortableBits(bounds[boundaryIndex])) == key) + { + boundaryIndex++; + } + } - public double LowerBoundExclusive { get; set; } + this.bucketSearchStarts[bucketCount] = bounds.Length; + } + } - public int Index { get; set; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public (int Start, int End) GetBucketSearchRange(double value) + { + if (double.IsNaN(value)) + { + var end = this.bucketSearchStarts[this.bucketSearchStarts.Length - 1]; + return (end, end); + } - public BucketLookupNode? Left { get; set; } + var key = this.GetKey(ToSortableBits(value)); + return (this.bucketSearchStarts[key], this.bucketSearchStarts[key + 1]); + } - public BucketLookupNode? Right { get; set; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private int GetKey(ulong sortableBits) + => this.keyMask == 0 ? 0 : (int)((sortableBits >> this.shift) & (uint)this.keyMask); } } diff --git a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs index b3f99b105ce..22dccf9109f 100644 --- a/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs +++ b/src/OpenTelemetry/Metrics/MetricStreamIdentity.cs @@ -232,6 +232,10 @@ public bool Equals(MetricStreamIdentity other) return null; } + ExplicitBucketHistogramConfiguration.ThrowIfBoundaryCountExceedsLimit( + adviceExplicitBucketBoundaries.Count, + nameof(histogram)); + if (typeof(T) == typeof(double)) { var boundaries = (IReadOnlyList)adviceExplicitBucketBoundaries; diff --git a/src/OpenTelemetry/Metrics/View/ExplicitBucketHistogramConfiguration.cs b/src/OpenTelemetry/Metrics/View/ExplicitBucketHistogramConfiguration.cs index 661ce42629d..e4525b7b832 100644 --- a/src/OpenTelemetry/Metrics/View/ExplicitBucketHistogramConfiguration.cs +++ b/src/OpenTelemetry/Metrics/View/ExplicitBucketHistogramConfiguration.cs @@ -8,6 +8,8 @@ namespace OpenTelemetry.Metrics; /// public class ExplicitBucketHistogramConfiguration : HistogramConfiguration { + internal const int MaxBoundaryCount = 10_000_000; + #pragma warning disable CA1819 // Properties should not return arrays /// /// Gets or sets the optional boundaries of the histogram metric stream. @@ -21,6 +23,7 @@ public class ExplicitBucketHistogramConfiguration : HistogramConfiguration /// calculated. /// A null value would result in default bucket boundaries being /// used. + /// The array must not contain more than 10,000,000 values. /// /// Note: A copy is made of the provided array. /// @@ -33,6 +36,8 @@ public double[]? Boundaries { if (value != null) { + ThrowIfBoundaryCountExceedsLimit(value.Length, nameof(value)); + if (!IsSortedAndDistinct(value)) { throw new ArgumentException($"Histogram boundaries are invalid. Histogram boundaries must be in ascending order with distinct values.", nameof(value)); @@ -49,6 +54,17 @@ public double[]? Boundaries internal double[]? CopiedBoundaries { get; private set; } + internal static void ThrowIfBoundaryCountExceedsLimit(int boundaryCount, string? paramName) + { + if (boundaryCount > MaxBoundaryCount) + { + throw new ArgumentOutOfRangeException( + paramName, + boundaryCount, + $"Histogram boundaries are invalid. Maximum supported boundary count is {MaxBoundaryCount}."); + } + } + private static bool IsSortedAndDistinct(double[] values) { for (var i = 1; i < values.Length; i++) diff --git a/src/OpenTelemetry/OpenTelemetry.csproj b/src/OpenTelemetry/OpenTelemetry.csproj index cb11daff2df..3acaee9bad5 100644 --- a/src/OpenTelemetry/OpenTelemetry.csproj +++ b/src/OpenTelemetry/OpenTelemetry.csproj @@ -39,6 +39,7 @@ + diff --git a/test/Benchmarks/Metrics/HistogramExplicitBoundsBenchmarks.cs b/test/Benchmarks/Metrics/HistogramExplicitBoundsBenchmarks.cs new file mode 100644 index 00000000000..7cbd748889e --- /dev/null +++ b/test/Benchmarks/Metrics/HistogramExplicitBoundsBenchmarks.cs @@ -0,0 +1,101 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using BenchmarkDotNet.Attributes; +using OpenTelemetry.Metrics; + +namespace Benchmarks.Metrics; + +public class HistogramExplicitBoundsBenchmarks +{ + private HistogramExplicitBounds? emptyBounds; + private HistogramExplicitBounds? finiteBounds; + private HistogramExplicitBounds? infiniteBounds; + private double exactBoundaryValue; + private double inRangeValue; + + [Params(10, 49, 50, 1000)] + public int BoundCount { get; set; } + + [Params("PositiveOnly", "MixedSigned")] + public string Layout { get; set; } = string.Empty; + + [GlobalSetup] + public void Setup() + { + var finite = CreateBounds(this.BoundCount, this.Layout); + var infinite = CreateInfiniteBounds(finite); + + this.emptyBounds = new(Array.Empty()); + this.finiteBounds = new(finite); + this.infiniteBounds = new(infinite); + + var midpointIndex = finite.Length / 2; + this.exactBoundaryValue = finite[midpointIndex]; + this.inRangeValue = midpointIndex == 0 + ? finite[0] + : finite[midpointIndex - 1] + ((finite[midpointIndex] - finite[midpointIndex - 1]) / 2); + } + + [Benchmark] + public int LookupExactBoundary() => this.finiteBounds!.FindBucketIndex(this.exactBoundaryValue); + + [Benchmark] + public int LookupInRangeValue() => this.finiteBounds!.FindBucketIndex(this.inRangeValue); + + [Benchmark] + public int LookupNegativeInfinity() => this.finiteBounds!.FindBucketIndex(double.NegativeInfinity); + + [Benchmark] + public int LookupPositiveInfinity() => this.finiteBounds!.FindBucketIndex(double.PositiveInfinity); + + [Benchmark] + public int LookupNaN() => this.finiteBounds!.FindBucketIndex(double.NaN); + + [Benchmark] + public int LookupWithInfiniteBounds() => this.infiniteBounds!.FindBucketIndex(this.inRangeValue); + + [Benchmark] + public int LookupEmptyBounds() => this.emptyBounds!.FindBucketIndex(42); + + private static double[] CreateBounds(int count, string layout) + => layout == "MixedSigned" + ? CreateMixedSignedBounds(count) + : CreatePositiveOnlyBounds(count); + + private static double[] CreateInfiniteBounds(double[] finiteBounds) + { + var infiniteBounds = new double[finiteBounds.Length + 2]; + infiniteBounds[0] = double.NegativeInfinity; + Array.Copy(finiteBounds, 0, infiniteBounds, 1, finiteBounds.Length); + infiniteBounds[infiniteBounds.Length - 1] = double.PositiveInfinity; + return infiniteBounds; + } + + private static double[] CreateMixedSignedBounds(int count) + { + var bounds = new double[count]; + var start = -5000.0; + var step = 10000.0 / count; + + for (var i = 0; i < count; i++) + { + bounds[i] = start + (i * step); + } + + return bounds; + } + + private static double[] CreatePositiveOnlyBounds(int count) + { + var bounds = new double[count]; + var step = 10000.0 / count; + + for (var i = 0; i < count; i++) + { + bounds[i] = i * step; + } + + return bounds; + } +} diff --git a/test/OpenTelemetry.FuzzTests/Metrics/HistogramExplicitBoundsFuzzTests.cs b/test/OpenTelemetry.FuzzTests/Metrics/HistogramExplicitBoundsFuzzTests.cs new file mode 100644 index 00000000000..336c0832f76 --- /dev/null +++ b/test/OpenTelemetry.FuzzTests/Metrics/HistogramExplicitBoundsFuzzTests.cs @@ -0,0 +1,168 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using FsCheck; +using FsCheck.Fluent; +using FsCheck.Xunit; +using OpenTelemetry.Metrics; + +namespace OpenTelemetry.FuzzTests; + +public class HistogramExplicitBoundsFuzzTests +{ + [Property(MaxTest = 100)] + public Property SmallBoundarySetsMatchScalarBaseline() => Prop.ForAll( + CreateScenarioArbitrary(0, HistogramExplicitBounds.DefaultBoundaryCountForBinarySearch - 1), + (scenario) => MatchesScalarBaseline(scenario.Item1, scenario.Item2)); + + [Property(MaxTest = 100)] + public Property LargeBoundarySetsMatchScalarBaseline() => Prop.ForAll( + CreateScenarioArbitrary(HistogramExplicitBounds.DefaultBoundaryCountForBinarySearch, 256), + (scenario) => MatchesScalarBaseline(scenario.Item1, scenario.Item2)); + + private static Arbitrary> CreateScenarioArbitrary(int minBoundaryCount, int maxBoundaryCount) + { + var gen = + from boundaryCount in Gen.Choose(minBoundaryCount, maxBoundaryCount) + from start in Gen.Choose(-1000, 1000) + from scale in Gen.Choose(1, 16) + from increments in Gen.ArrayOf(Gen.Choose(1, 1000), boundaryCount) + from includeNegativeInfinity in Gen.Elements(true, false) + from includePositiveInfinity in Gen.Elements(true, false) + select CreateScenario( + boundaryCount, + start, + scale, + increments, + includeNegativeInfinity, + includePositiveInfinity); + + return gen.ToArbitrary(); + } + + private static bool MatchesScalarBaseline(double[] rawBoundaries, double[] testValues) + { + var histogramExplicitBounds = new HistogramExplicitBounds(rawBoundaries); + var expectedBounds = CleanBoundaries(rawBoundaries); + + if (!expectedBounds.SequenceEqual(histogramExplicitBounds.Bounds)) + { + return false; + } + + foreach (var value in testValues) + { + if (histogramExplicitBounds.FindBucketIndex(value) != FindBucketIndexScalar(expectedBounds, value)) + { + return false; + } + } + + return true; + } + + private static Tuple CreateScenario( + int boundaryCount, + int start, + int scale, + int[] increments, + bool includeNegativeInfinity, + bool includePositiveInfinity) + { + var finiteBoundaries = new double[boundaryCount]; + double current = start; + + for (var i = 0; i < boundaryCount; i++) + { + current += increments[i] / (double)scale; + finiteBoundaries[i] = current; + } + + var rawBoundaryCount = boundaryCount + + (includeNegativeInfinity ? 1 : 0) + + (includePositiveInfinity ? 1 : 0); + + var rawBoundaries = new double[rawBoundaryCount]; + var rawBoundaryIndex = 0; + + if (includeNegativeInfinity) + { + rawBoundaries[rawBoundaryIndex++] = double.NegativeInfinity; + } + + Array.Copy(finiteBoundaries, 0, rawBoundaries, rawBoundaryIndex, finiteBoundaries.Length); + rawBoundaryIndex += finiteBoundaries.Length; + + if (includePositiveInfinity) + { + rawBoundaries[rawBoundaryIndex] = double.PositiveInfinity; + } + + return Tuple.Create(rawBoundaries, CreateTestValues(finiteBoundaries)); + } + + private static double[] CreateTestValues(double[] cleanedBoundaries) + { + var values = new List(cleanedBoundaries.Length + 8) + { + double.NaN, + double.NegativeInfinity, + double.PositiveInfinity, + 0.0, + }; + + if (cleanedBoundaries.Length == 0) + { + values.Add(-1.0); + values.Add(1.0); + return [.. values]; + } + + values.Add(cleanedBoundaries[0] - 1.0); + values.Add(cleanedBoundaries[0]); + + var step = Math.Max(1, cleanedBoundaries.Length / 16); + + for (var i = step; i < cleanedBoundaries.Length; i += step) + { + values.Add(cleanedBoundaries[i]); + values.Add(cleanedBoundaries[i - 1] + ((cleanedBoundaries[i] - cleanedBoundaries[i - 1]) / 2)); + } + + values.Add(cleanedBoundaries[cleanedBoundaries.Length - 1]); + values.Add(cleanedBoundaries[cleanedBoundaries.Length - 1] + 1.0); + + return [.. values]; + } + + private static double[] CleanBoundaries(double[] rawBoundaries) + { + for (var i = 0; i < rawBoundaries.Length; i++) + { + if (double.IsNegativeInfinity(rawBoundaries[i]) || double.IsPositiveInfinity(rawBoundaries[i])) + { + return [.. rawBoundaries.Where(b => !double.IsNegativeInfinity(b) && !double.IsPositiveInfinity(b))]; + } + } + + return rawBoundaries; + } + + private static int FindBucketIndexScalar(double[] bounds, double value) + { + if (double.IsNaN(value)) + { + return bounds.Length; + } + + for (var i = 0; i < bounds.Length; i++) + { + if (value <= bounds[i]) + { + return i; + } + } + + return bounds.Length; + } +} diff --git a/test/OpenTelemetry.FuzzTests/OpenTelemetry.FuzzTests.csproj b/test/OpenTelemetry.FuzzTests/OpenTelemetry.FuzzTests.csproj new file mode 100644 index 00000000000..17ecc182b2b --- /dev/null +++ b/test/OpenTelemetry.FuzzTests/OpenTelemetry.FuzzTests.csproj @@ -0,0 +1,15 @@ + + + + $(TargetFrameworksForTests) + + + + + + + + + + + diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs index dcac57e9f84..0f271e9d49a 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs @@ -155,6 +155,80 @@ public void HistogramBinaryBucketTest() } } + [Fact] + public void HistogramLargeBucketLookupHandlesMixedSignsAndNaN() + { + var values = new double[HistogramExplicitBounds.DefaultBoundaryCountForBinarySearch * 4]; + + for (var i = 0; i < values.Length; i++) + { + values[i] = i - (values.Length / 2); + } + + var boundaries = new HistogramExplicitBounds(values); + + Assert.Equal(0, boundaries.FindBucketIndex(double.NegativeInfinity)); + Assert.Equal(0, boundaries.FindBucketIndex(values[0])); + Assert.Equal(values.Length / 2, boundaries.FindBucketIndex(0)); + Assert.Equal(values.Length - 1, boundaries.FindBucketIndex(values[values.Length - 1])); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.PositiveInfinity)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.NaN)); + } + + [Fact] + public void HistogramLargeBucketLookupHandlesPositiveOnlyBoundsAndInfiniteInputBounds() + { + var values = new double[HistogramExplicitBounds.DefaultBoundaryCountForBinarySearch * 4]; + + for (var i = 0; i < values.Length; i++) + { + values[i] = i; + } + + var rawBounds = new double[values.Length + 2]; + rawBounds[0] = double.NegativeInfinity; + Array.Copy(values, 0, rawBounds, 1, values.Length); + rawBounds[rawBounds.Length - 1] = double.PositiveInfinity; + + var boundaries = new HistogramExplicitBounds(rawBounds); + var midpoint = values.Length / 2; + var inRangeValue = values[midpoint - 1] + ((values[midpoint] - values[midpoint - 1]) / 2); + + Assert.Equal(0, boundaries.FindBucketIndex(double.NegativeInfinity)); + Assert.Equal(midpoint, boundaries.FindBucketIndex(values[midpoint])); + Assert.Equal(midpoint, boundaries.FindBucketIndex(inRangeValue)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.PositiveInfinity)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.NaN)); + } + + [Fact] + public void HistogramLargeBucketLookupHandlesDegenerateNaNBounds() + { + // Ensure the radix lookup fallback works when all bounds collapse to the same sortable key. + var values = Enumerable.Repeat(double.NaN, HistogramExplicitBounds.DefaultBoundaryCountForBinarySearch).ToArray(); + + var boundaries = new HistogramExplicitBounds(values); + + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.NegativeInfinity)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(0)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.PositiveInfinity)); + Assert.Equal(values.Length, boundaries.FindBucketIndex(double.NaN)); + } + + [Fact] + public void HistogramExplicitBoundsCleansInfiniteDisplayBounds() + { + var boundaries = new HistogramExplicitBounds( + [double.NegativeInfinity, 1, 2, double.PositiveInfinity], + [double.NegativeInfinity, 10, 20, double.PositiveInfinity]); + + Assert.NotNull(boundaries.Bounds); + Assert.NotNull(boundaries.DisplayBounds); + + Assert.Equal([1d, 2d], boundaries.Bounds); + Assert.Equal([10d, 20d], boundaries.DisplayBounds); + } + [Fact] public void HistogramWithOnlySumCount() { @@ -459,7 +533,9 @@ internal void ExponentialMaxScaleConfigWorks(int? maxScale) } [Theory] +#pragma warning disable xUnit1045 // Avoid using TheoryData type arguments that might not be serializable [MemberData(nameof(HistogramBoundaryTestCase.HistogramInfinityBoundariesTestCases))] +#pragma warning restore xUnit1045 // Avoid using TheoryData type arguments that might not be serializable internal void HistogramBucketBoundariesTest(HistogramBoundaryTestCase boundaryTestCase) { // Arrange diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index d4ef2e7636e..cca46f0a52a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -187,6 +187,21 @@ public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] bo Assert.Contains("Histogram boundaries must be in ascending order with distinct values", ex.Message, StringComparison.Ordinal); } + [Fact] + public void HistogramBoundaryCountValidationThrowsArgumentException() + { + var boundaryCount = ExplicitBucketHistogramConfiguration.MaxBoundaryCount + 1; + + var ex = Assert.Throws( + () => ExplicitBucketHistogramConfiguration.ThrowIfBoundaryCountExceedsLimit( + boundaryCount, + "value")); + + Assert.Contains("Maximum supported boundary count is", ex.Message, StringComparison.Ordinal); + Assert.Equal("value", ex.ParamName); + Assert.Equal(boundaryCount, ex.ActualValue); + } + [Theory] [InlineData(-1)] [InlineData(0)]