diff --git a/src/OpenTelemetry/Internal/InterlockedHelper.cs b/src/OpenTelemetry/Internal/InterlockedHelper.cs index f279dee5024..e8c6dcc02c3 100644 --- a/src/OpenTelemetry/Internal/InterlockedHelper.cs +++ b/src/OpenTelemetry/Internal/InterlockedHelper.cs @@ -16,7 +16,7 @@ public static void Add(ref double location, double value) var currentValue = Volatile.Read(ref location); var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); - if (!AreSame(returnedValue, currentValue)) + if (returnedValue != currentValue) { AddRare(ref location, value, returnedValue); } @@ -35,7 +35,7 @@ private static void AddRare(ref double location, double value, double currentVal sw.SpinOnce(); var returnedValue = Interlocked.CompareExchange(ref location, currentValue + value, currentValue); - if (AreSame(returnedValue, currentValue)) + if (returnedValue == currentValue) { break; } @@ -43,8 +43,4 @@ private static void AddRare(ref double location, double value, double currentVal currentValue = returnedValue; } } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static bool AreSame(double left, double right) - => BitConverter.DoubleToInt64Bits(left) == BitConverter.DoubleToInt64Bits(right); } diff --git a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs index c972ffad85e..0e177e1dec5 100644 --- a/src/OpenTelemetry/Metrics/MeterProviderSdk.cs +++ b/src/OpenTelemetry/Metrics/MeterProviderSdk.cs @@ -219,6 +219,16 @@ internal static void MeasurementRecordedDouble(Instrument instrument, double val return; } + if (!MathHelper.IsFinite(value) + && instrument is Counter + or UpDownCounter + or ObservableCounter + or ObservableUpDownCounter) + { + OpenTelemetrySdkEventSource.Log.MeasurementDropped(instrument?.Name ?? "UnknownInstrument", "Measurement value was invalid.", "Use a finite value for sum instruments (not NaN or Infinity)."); + return; + } + metricState.RecordMeasurementDouble(value, tags); } diff --git a/test/OpenTelemetry.Tests/Internal/InterlockedHelperTests.cs b/test/OpenTelemetry.Tests/Internal/InterlockedHelperTests.cs deleted file mode 100644 index 9da16b16ed4..00000000000 --- a/test/OpenTelemetry.Tests/Internal/InterlockedHelperTests.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -using Xunit; - -namespace OpenTelemetry.Internal.Tests; - -public class InterlockedHelperTests -{ - [Fact] - public async Task AddWhenCurrentValueIsNaNShouldNotHang() - { - var timeout = TimeSpan.FromSeconds(2); - var value = double.NaN; - - var task = Task.Run(() => InterlockedHelper.Add(ref value, 1d)); - -#if NET - await task.WaitAsync(timeout); -#else - using var cts = new CancellationTokenSource(timeout); - var completed = await Task.WhenAny(task, Task.Delay(timeout, cts.Token)) == task; - Assert.True(completed); -#endif - - Assert.True(double.IsNaN(value)); - } -} diff --git a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTests.cs b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTests.cs index 8f51b19d3a2..486df3e1cf8 100644 --- a/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTests.cs +++ b/test/OpenTelemetry.Tests/Metrics/MeterProviderSdkTests.cs @@ -104,4 +104,82 @@ void RunTest() } } } + + [Fact] + public void NonFiniteCounterDoubleMeasurementsAreDropped() + { + var meterName = Utils.GetCurrentMethodName(); + var exportedItems = new List(); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meterName) + .AddInMemoryExporter(exportedItems) + .Build(); + + using var meter = new Meter(meterName); + var counter = meter.CreateCounter("counter"); + + counter.Add(double.NaN); + counter.Add(double.PositiveInfinity); + counter.Add(double.NegativeInfinity); + counter.Add(2.5); + + Assert.True(meterProvider.ForceFlush()); + + var exportedMetric = Assert.Single(exportedItems); + var metric = new MetricSnapshot(exportedMetric); + var metricPoint = Assert.Single(metric.MetricPoints); + Assert.Equal(2.5, metricPoint.GetSumDouble()); + } + + [Fact] + public void NonFiniteUpDownCounterDoubleMeasurementsAreDropped() + { + var meterName = Utils.GetCurrentMethodName(); + var exportedItems = new List(); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meterName) + .AddInMemoryExporter(exportedItems) + .Build(); + + using var meter = new Meter(meterName); + var upDownCounter = meter.CreateUpDownCounter("updowncounter"); + + upDownCounter.Add(double.PositiveInfinity); + upDownCounter.Add(double.NegativeInfinity); + upDownCounter.Add(2.5); + + Assert.True(meterProvider.ForceFlush()); + + var exportedMetric = Assert.Single(exportedItems); + var metric = new MetricSnapshot(exportedMetric); + var metricPoint = Assert.Single(metric.MetricPoints); + Assert.Equal(2.5, metricPoint.GetSumDouble()); + } + + [Fact] + public void NaNHistogramMeasurementsAreNotDropped() + { + var meterName = Utils.GetCurrentMethodName(); + var exportedItems = new List(); + + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddMeter(meterName) + .AddInMemoryExporter(exportedItems) + .Build(); + + using var meter = new Meter(meterName); + var histogram = meter.CreateHistogram("histogram"); + + histogram.Record(18); + histogram.Record(double.NaN); + + Assert.True(meterProvider.ForceFlush()); + + var exportedMetric = Assert.Single(exportedItems); + var metric = new MetricSnapshot(exportedMetric); + var metricPoint = Assert.Single(metric.MetricPoints); + Assert.True(double.IsNaN(metricPoint.GetHistogramSum())); + } }