diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 2b3bd30138f..42161e7968e 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Recording a `double.NaN` value on a histogram increases the count of the + greatest bucket and does not affect the sum. + ([#3279](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3279)) + * Exposed public setters for `LogRecord.State`, `LogRecord.StateValues`, and `LogRecord.FormattedMessage`. ([#3217](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3217)) diff --git a/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs b/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs index 58bfc245b10..bf2f896d404 100644 --- a/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs +++ b/src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs @@ -30,7 +30,7 @@ public class ExplicitBucketHistogramConfiguration : MetricStreamConfiguration /// Requirements: /// /// The array must be in ascending order with distinct - /// values. + /// values. double.NaN is not allowed /// An empty array would result in no histogram buckets being /// calculated. /// A null value would result in default bucket boundaries being @@ -58,7 +58,7 @@ public double[] Boundaries { if (!IsSortedAndDistinct(value)) { - throw new ArgumentException($"Histogram boundaries are invalid. Histogram boundaries must be in ascending order with distinct values.", nameof(value)); + throw new ArgumentException($"Histogram boundaries are invalid. Histogram boundaries must be in ascending order with distinct values. double.NaN is not allowed.", nameof(value)); } double[] copy = new double[value.Length]; @@ -76,9 +76,14 @@ public double[] Boundaries private static bool IsSortedAndDistinct(double[] values) { + if (double.IsNaN(values[0])) + { + return false; + } + for (int i = 1; i < values.Length; i++) { - if (values[i] <= values[i - 1]) + if (double.IsNaN(values[i]) || values[i] <= values[i - 1]) { return false; } diff --git a/src/OpenTelemetry/Metrics/MetricPoint.cs b/src/OpenTelemetry/Metrics/MetricPoint.cs index f848c319594..42a3f00a45f 100644 --- a/src/OpenTelemetry/Metrics/MetricPoint.cs +++ b/src/OpenTelemetry/Metrics/MetricPoint.cs @@ -324,13 +324,16 @@ internal void Update(double number) case AggregationType.Histogram: { - int i; - for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) + int i = this.histogramBuckets.ExplicitBounds.Length; + if (!double.IsNaN(number)) { - // Upper bound is inclusive - if (number <= this.histogramBuckets.ExplicitBounds[i]) + for (i = 0; i < this.histogramBuckets.ExplicitBounds.Length; i++) { - break; + // Upper bound is inclusive + if (number <= this.histogramBuckets.ExplicitBounds[i]) + { + break; + } } } @@ -342,8 +345,11 @@ internal void Update(double number) unchecked { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; this.histogramBuckets.RunningBucketCounts[i]++; + if (!double.IsNaN(number)) + { + this.histogramBuckets.RunningSum += number; + } } this.histogramBuckets.IsCriticalSectionOccupied = 0; @@ -366,7 +372,10 @@ internal void Update(double number) unchecked { this.runningValue.AsLong++; - this.histogramBuckets.RunningSum += number; + if (!double.IsNaN(number)) + { + this.histogramBuckets.RunningSum += number; + } } this.histogramBuckets.IsCriticalSectionOccupied = 0; diff --git a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs index 0664dc18c00..2e7052b773a 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricTestData.cs @@ -44,6 +44,7 @@ public static IEnumerable ValidInstrumentNames public static IEnumerable InvalidHistogramBoundaries => new List { + new object[] { new double[] { double.NaN } }, new object[] { new double[] { 0, 0 } }, new object[] { new double[] { 1, 0 } }, new object[] { new double[] { 0, 1, 1, 2 } }, diff --git a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs index 3a12cd4ac05..b9e271806ef 100644 --- a/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MetricViewTests.cs @@ -204,7 +204,7 @@ public void AddViewWithInvalidHistogramBoundsThrowsArgumentException(double[] bo var ex = Assert.Throws(() => Sdk.CreateMeterProviderBuilder() .AddView("name1", new ExplicitBucketHistogramConfiguration { Boundaries = boundaries })); - Assert.Contains("Histogram boundaries must be in ascending order with distinct values", ex.Message); + Assert.Contains("Histogram boundaries must be in ascending order with distinct values. double.NaN is not allowed", ex.Message); } [Theory] @@ -501,7 +501,7 @@ public void ViewToProduceCustomHistogramBound() .AddInMemoryExporter(exportedItems) .Build(); - var histogram = meter.CreateHistogram("MyHistogram"); + var histogram = meter.CreateHistogram("MyHistogram"); histogram.Record(-10); histogram.Record(0); histogram.Record(1); @@ -509,6 +509,7 @@ public void ViewToProduceCustomHistogramBound() histogram.Record(10); histogram.Record(11); histogram.Record(19); + histogram.Record(double.NaN); meterProvider.ForceFlush(MaxTimeToAllowForFlush); Assert.Equal(2, exportedItems.Count); var metricDefault = exportedItems[0]; @@ -530,11 +531,11 @@ public void ViewToProduceCustomHistogramBound() var sum = histogramPoint.GetHistogramSum(); Assert.Equal(40, sum); - Assert.Equal(7, count); + Assert.Equal(8, count); int index = 0; int actualCount = 0; - var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 0 }; + var expectedBucketCounts = new long[] { 2, 1, 2, 2, 0, 0, 0, 0, 0, 0, 1 }; foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) { Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount); @@ -557,11 +558,11 @@ public void ViewToProduceCustomHistogramBound() sum = histogramPoint.GetHistogramSum(); Assert.Equal(40, sum); - Assert.Equal(7, count); + Assert.Equal(8, count); index = 0; actualCount = 0; - expectedBucketCounts = new long[] { 5, 2, 0 }; + expectedBucketCounts = new long[] { 5, 2, 1 }; foreach (var histogramMeasurement in histogramPoint.GetHistogramBuckets()) { Assert.Equal(expectedBucketCounts[index], histogramMeasurement.BucketCount);