diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index b1c24c09a97..cddb469c8e2 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -6,6 +6,12 @@ Notes](../../RELEASENOTES.md). ## Unreleased +* Non-finite histogram measurements (`NaN`, `+Inf`, `-Inf`) are now dropped + rather than recorded, so they can no longer permanently corrupt the sum, + min, and max of a cumulative histogram stream. This aligns the explicit-bucket + and exponential histograms with the metrics SDK specification. + ([#7759](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7759)) + ## 1.21.0-rc.2 Released 2026-Nov-09 diff --git a/src/OpenTelemetry/Metrics/AggregatorStore.cs b/src/OpenTelemetry/Metrics/AggregatorStore.cs index 2422ce88324..3848207fa0a 100644 --- a/src/OpenTelemetry/Metrics/AggregatorStore.cs +++ b/src/OpenTelemetry/Metrics/AggregatorStore.cs @@ -56,6 +56,7 @@ internal sealed class AggregatorStore private readonly MetricPoint[] metricPoints; private readonly int[] currentMetricPointBatch; private readonly AggregationType aggType; + private readonly bool isHistogramAggregation; private readonly HistogramExplicitBounds histogramExplicitBounds; private readonly int exponentialHistogramMaxSize; private readonly int exponentialHistogramMaxScale; @@ -112,6 +113,15 @@ internal AggregatorStore( this.exponentialHistogramMaxScale = metricStreamIdentity.ExponentialHistogramMaxScale; this.StartTimeExclusive = DateTimeOffset.UtcNow; this.ExemplarReservoirFactory = exemplarReservoirFactory; + + this.isHistogramAggregation = aggType is + AggregationType.Histogram + or AggregationType.HistogramWithMinMax + or AggregationType.HistogramWithBuckets + or AggregationType.HistogramWithMinMaxBuckets + or AggregationType.Base2ExponentialHistogram + or AggregationType.Base2ExponentialHistogramWithMinMax; + if (metricStreamIdentity.TagKeys != null) { this.updateLongCallback = this.UpdateLongCustomTags; @@ -249,6 +259,14 @@ internal void Update(long value, ReadOnlySpan> tag internal void Update(double value, ReadOnlySpan> tags) { + if (this.isHistogramAggregation && (double.IsNaN(value) || double.IsInfinity(value))) + { + // Reject non-finite histogram measurements before tag lookup: the value is always + // dropped, so a unique tag set carrying only non-finite values must not consume a + // cardinality slot that a subsequent, valid tag set could need. + return; + } + try { this.updateDoubleCallback(value, tags); diff --git a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs index 38dff19f317..66c569e9708 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs @@ -1011,6 +1011,13 @@ internal void NullifyMetricPointState() private void UpdateHistogram(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { + if (double.IsNaN(number) || double.IsInfinity(number)) + { + // Non-finite values would poison RunningSum for the lifetime of a cumulative stream. + this.CompleteUpdateWithoutMeasurement(); + return; + } + var histogramBuckets = this.mpComponents!.HistogramBuckets!; // Offer the exemplar before the value is added. The status below is set in the same critical @@ -1043,6 +1050,14 @@ private void UpdateHistogram(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { + if (double.IsNaN(number) || double.IsInfinity(number)) + { + // Non-finite values would poison RunningSum/RunningMin/RunningMax for the lifetime of + // a cumulative stream. + this.CompleteUpdateWithoutMeasurement(); + return; + } + var histogramBuckets = this.mpComponents!.HistogramBuckets!; // Offer the exemplar before the update, as for UpdateHistogram(). @@ -1068,6 +1083,13 @@ private void UpdateHistogramWithMinMax(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { + if (double.IsNaN(number) || double.IsInfinity(number)) + { + // Non-finite values would poison RunningSum for the lifetime of a cumulative stream. + this.CompleteUpdateWithoutMeasurement(); + return; + } + var histogramBuckets = this.mpComponents!.HistogramBuckets!; var bucketIndex = histogramBuckets.FindBucketIndex(number); @@ -1093,6 +1115,14 @@ private void UpdateHistogramWithBuckets(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { + if (double.IsNaN(number) || double.IsInfinity(number)) + { + // Non-finite values would poison RunningSum/RunningMin/RunningMax for the lifetime of + // a cumulative stream. + this.CompleteUpdateWithoutMeasurement(); + return; + } + var histogramBuckets = this.mpComponents!.HistogramBuckets!; var bucketIndex = histogramBuckets.FindBucketIndex(number); @@ -1121,8 +1151,10 @@ private void UpdateHistogramWithBucketsAndMinMax(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { - if (number < 0) + if (number < 0 || double.IsNaN(number) || double.IsInfinity(number)) { + // Negative values are rejected by this SDK path; non-finite values would also + // poison RunningSum/RunningMin/RunningMax for the lifetime of a cumulative stream. this.CompleteUpdateWithoutMeasurement(); return; } @@ -1150,8 +1182,10 @@ private void UpdateBase2ExponentialHistogram(double number, ReadOnlySpan> tags = default, bool offerExemplar = false) { - if (number < 0) + if (number < 0 || double.IsNaN(number) || double.IsInfinity(number)) { + // Negatives are undefined for exponential histograms; non-finite values would also + // poison RunningSum/RunningMin/RunningMax for the lifetime of a cumulative stream. this.CompleteUpdateWithoutMeasurement(); return; } diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramInfinities.verified.txt b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramInfinities.verified.txt index f094a15ebfa..0dae9bae288 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramInfinities.verified.txt +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramInfinities.verified.txt @@ -14,6 +14,6 @@ test_histogram_bucket{otel_scope_name="HistogramInfinities",le="2500"} 1 test_histogram_bucket{otel_scope_name="HistogramInfinities",le="5000"} 1 test_histogram_bucket{otel_scope_name="HistogramInfinities",le="7500"} 1 test_histogram_bucket{otel_scope_name="HistogramInfinities",le="10000"} 1 -test_histogram_bucket{otel_scope_name="HistogramInfinities",le="+Inf"} 3 -test_histogram_sum{otel_scope_name="HistogramInfinities"} +Inf -test_histogram_count{otel_scope_name="HistogramInfinities"} 3 +test_histogram_bucket{otel_scope_name="HistogramInfinities",le="+Inf"} 1 +test_histogram_sum{otel_scope_name="HistogramInfinities"} 18 +test_histogram_count{otel_scope_name="HistogramInfinities"} 1 diff --git a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramNaN.verified.txt b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramNaN.verified.txt index f115c7fbd8f..29f91f9256e 100644 --- a/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramNaN.verified.txt +++ b/test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/snapshots/PrometheusSerializer.HistogramNaN.verified.txt @@ -14,6 +14,6 @@ test_histogram_bucket{otel_scope_name="HistogramNaN",le="2500"} 1 test_histogram_bucket{otel_scope_name="HistogramNaN",le="5000"} 1 test_histogram_bucket{otel_scope_name="HistogramNaN",le="7500"} 1 test_histogram_bucket{otel_scope_name="HistogramNaN",le="10000"} 1 -test_histogram_bucket{otel_scope_name="HistogramNaN",le="+Inf"} 3 -test_histogram_sum{otel_scope_name="HistogramNaN"} NaN -test_histogram_count{otel_scope_name="HistogramNaN"} 3 +test_histogram_bucket{otel_scope_name="HistogramNaN",le="+Inf"} 1 +test_histogram_sum{otel_scope_name="HistogramNaN"} 18 +test_histogram_count{otel_scope_name="HistogramNaN"} 1 diff --git a/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs b/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs index 3b2c77733a1..3eeb34a0872 100644 --- a/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/AggregatorTests.cs @@ -21,6 +21,81 @@ public AggregatorTests() public static TheoryData HistogramInfinityBoundariesTestCases => HistogramBoundaryTestCase.HistogramInfinityBoundariesTestCases(); + public static TheoryData NonFiniteHistogramMeasurementTestCases() + { + var data = new TheoryData(); + + // Typed as an integer as AggregationType is internal + AggregationType[] aggregationTypes = + [ + AggregationType.Histogram, + AggregationType.HistogramWithMinMax, + AggregationType.HistogramWithBuckets, + AggregationType.HistogramWithMinMaxBuckets, + AggregationType.Base2ExponentialHistogram, + AggregationType.Base2ExponentialHistogramWithMinMax, + ]; + + double[] nonFiniteValues = [double.NaN, double.PositiveInfinity, double.NegativeInfinity]; + + foreach (var aggregationType in aggregationTypes) + { + foreach (var nonFinite in nonFiniteValues) + { + data.Add((int)aggregationType, nonFinite, false); + data.Add((int)aggregationType, nonFinite, true); + } + } + + return data; + } + + [Theory] + [MemberData(nameof(NonFiniteHistogramMeasurementTestCases))] + public void NonFiniteMeasurementIsIgnoredByHistogram(int aggregationTypeValue, double nonFinite, bool outputDelta) + { + var aggregationType = (AggregationType)aggregationTypeValue; + var boundaries = new HistogramExplicitBounds(Metric.DefaultHistogramBounds); + var temporality = outputDelta ? AggregationTemporality.Delta : AggregationTemporality.Cumulative; + var aggregatorStore = new AggregatorStore(MetricStreamIdentity, aggregationType, temporality, 1024); + var histogramPoint = new MetricPoint(aggregatorStore, aggregationType, null, boundaries, Metric.DefaultExponentialHistogramMaxBuckets, Metric.DefaultExponentialHistogramMaxScale); + + histogramPoint.Update(1); + histogramPoint.Update(nonFinite); + histogramPoint.Update(2); + histogramPoint.Update(3); + + histogramPoint.TakeSnapshot(outputDelta); + + // The non-finite measurement is dropped: only the three finite values are counted... + Assert.Equal(3, histogramPoint.GetHistogramCount()); + + // ...and the sum is exactly 1 + 2 + 3, not poisoned to NaN/Infinity. + Assert.Equal(6, histogramPoint.GetHistogramSum()); + + if (aggregationType is AggregationType.HistogramWithMinMax or + AggregationType.HistogramWithMinMaxBuckets or + AggregationType.Base2ExponentialHistogramWithMinMax) + { + // Min/max are not poisoned to NaN/Infinity either. + Assert.True(histogramPoint.TryGetHistogramMinMaxValues(out var min, out var max)); + Assert.Equal(1, min); + Assert.Equal(3, max); + } + + if (aggregationType is AggregationType.HistogramWithBuckets or AggregationType.HistogramWithMinMaxBuckets) + { + // The non-finite measurement was not distributed into any explicit bucket. + var bucketedCount = 0L; + foreach (var bucket in histogramPoint.GetHistogramBuckets()) + { + bucketedCount += bucket.BucketCount; + } + + Assert.Equal(3, bucketedCount); + } + } + [Fact] public void HistogramDistributeToAllBucketsDefault() { diff --git a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs index a5d5be6be88..d5ba9cf347b 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricOverflowAttributeTests.cs @@ -422,4 +422,51 @@ public void EveryMetricPointSlotIsUsedExactlyOnceUpToTheCardinalityLimit(MetricR var overflowMetricPoint = Assert.Single(metricPoints, mp => mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); Assert.Equal(100, overflowMetricPoint.GetSumLong()); } + + [Theory] + [InlineData(MetricReaderTemporalityPreference.Delta)] + [InlineData(MetricReaderTemporalityPreference.Cumulative)] + public void NonFiniteHistogramMeasurementsDoNotConsumeCardinality(MetricReaderTemporalityPreference temporalityPreference) + { + var exportedItems = new List(); + + using var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("TestHistogram"); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meter.Name) + .AddView(histogram.Name, new ExplicitBucketHistogramConfiguration { CardinalityLimit = 3 }) + .AddInMemoryExporter(exportedItems, metricReaderOptions => metricReaderOptions.TemporalityPreference = temporalityPreference) + .Build(); + + histogram.Record(double.NaN, new KeyValuePair("Key", 1)); + histogram.Record(double.PositiveInfinity, new KeyValuePair("Key", 2)); + histogram.Record(double.NegativeInfinity, new KeyValuePair("Key", 3)); + + histogram.Record(42, new KeyValuePair("Key", 4)); + histogram.Record(42, new KeyValuePair("Key", 5)); + histogram.Record(42, new KeyValuePair("Key", 6)); + + meterProvider.ForceFlush(); + + Assert.Single(exportedItems); + var metric = exportedItems[0]; + + var metricPoints = new List(); + foreach (ref readonly var mp in metric.GetMetricPoints()) + { + metricPoints.Add(mp); + } + + // The non-finite measurements never occupied a MetricPoint, so no overflow occurred. + Assert.DoesNotContain(metricPoints, mp => mp.Tags.Count != 0 && mp.Tags.KeyAndValues[0].Key == "otel.metric.overflow"); + + var taggedMetricPoints = metricPoints.Where(mp => mp.Tags.Count != 0).ToList(); + Assert.Equal(3, taggedMetricPoints.Count); + foreach (var mp in taggedMetricPoints) + { + Assert.Equal(1, mp.GetHistogramCount()); + Assert.Equal(42, mp.GetHistogramSum()); + } + } } diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs index 5c28d2f33f0..14ecca90eb2 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs @@ -42,8 +42,7 @@ internal static class MetricTestData public static TheoryData ValidHistogramMinMax => new() { { new double[] { -10.0, 0.0, 1.0, 9.0, 10.0, 11.0, 19.0 }, new HistogramConfiguration(), -10.0, 19.0 }, - { new double[] { double.NegativeInfinity }, new HistogramConfiguration(), double.NegativeInfinity, double.NegativeInfinity }, - { new double[] { double.NegativeInfinity, 0.0, double.PositiveInfinity }, new HistogramConfiguration(), double.NegativeInfinity, double.PositiveInfinity }, + { new double[] { double.NegativeInfinity, 0.0, double.PositiveInfinity }, new HistogramConfiguration(), 0.0, 0.0 }, { new double[] { 1.0 }, new HistogramConfiguration(), 1.0, 1.0 }, { new double[] { 5.0, 100.0, 4.0, 101.0, -2.0, 97.0 }, new ExplicitBucketHistogramConfiguration { Boundaries = [10.0, 20.0] }, -2.0, 101.0 }, { new double[] { 5.0, 100.0, 4.0, 101.0, -2.0, 97.0 }, new Base2ExponentialBucketHistogramConfiguration(), 4.0, 101.0 }, diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 08a1e0a54de..dc9a2b65627 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -1030,6 +1030,31 @@ public void HistogramMinMaxNotPresent(double[] values, HistogramConfiguration hi Assert.False(histogramPoint.TryGetHistogramMinMaxValues(out var _, out var _)); } + [Theory] + [InlineData(MetricReaderTemporalityPreference.Cumulative)] + [InlineData(MetricReaderTemporalityPreference.Delta)] + public void AllNonFiniteHistogramMeasurementsProduceNoMetricPoints(MetricReaderTemporalityPreference temporalityPreference) + { + using var meter = new Meter(Utils.GetCurrentMethodName()); + var histogram = meter.CreateHistogram("MyHistogram"); + var exportedItems = new List(); + + using var container = BuildMeterProvider(out var meterProvider, builder => builder + .AddMeter(meter.Name) + .AddInMemoryExporter(exportedItems, o => o.TemporalityPreference = temporalityPreference)); + + histogram.Record(double.NaN); + histogram.Record(double.PositiveInfinity); + histogram.Record(double.NegativeInfinity); + + meterProvider.ForceFlush(MaxTimeToAllowForFlush); + + // Every measurement was non-finite and dropped, so the stream never had a valid + // measurement recorded against it: nothing is exported, rather than an "empty" + // histogram with a count and sum of zero. + Assert.Empty(exportedItems); + } + [Fact] public void ViewToSelectTagKeys() {