diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DefaultSynchronousMetricStorage.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DefaultSynchronousMetricStorage.java index 44c5050dbd4..3a359d75d23 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DefaultSynchronousMetricStorage.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/DefaultSynchronousMetricStorage.java @@ -89,6 +89,16 @@ public void recordLong(long value, Attributes attributes, Context context) { @Override public void recordDouble(double value, Attributes attributes, Context context) { + if (Double.isNaN(value)) { + logger.log( + Level.FINE, + "Instrument " + + metricDescriptor.getSourceInstrument().getName() + + " has recorded measurement Not-a-Number (NaN) value with attributes " + + attributes + + ". Dropping measurement."); + return; + } AggregatorHandle handle = getAggregatorHandle(attributes, context); handle.recordDouble(value, attributes, context); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurement.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurement.java index 6d0187b443b..27eeac81190 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurement.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurement.java @@ -139,6 +139,16 @@ public void record(double value, Attributes attributes) { logNoActiveReader(); return; } + if (Double.isNaN(value)) { + logger.log( + Level.FINE, + "Instrument " + + instrumentDescriptor.getName() + + " has recorded measurement Not-a-Number (NaN) value with attributes " + + attributes + + ". Dropping measurement."); + return; + } Measurement measurement; MemoryMode memoryMode = activeReader.getReader().getMemoryMode(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java index 215eaa2fe31..d733821972b 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleCounterTest.java @@ -16,6 +16,7 @@ import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.metrics.internal.state.DefaultSynchronousMetricStorage; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; @@ -166,6 +167,14 @@ void doubleCounterAdd_Monotonicity() { "Counters can only increase. Instrument testCounter has recorded a negative value."); } + @Test + @SuppressLogger(DefaultSynchronousMetricStorage.class) + void doubleCounterAdd_NaN() { + DoubleCounter doubleCounter = sdkMeter.counterBuilder("testCounter").ofDoubles().build(); + doubleCounter.add(Double.NaN); + assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); + } + @Test void stressTest() { DoubleCounter doubleCounter = sdkMeter.counterBuilder("testCounter").ofDoubles().build(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleGaugeTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleGaugeTest.java index 404ed738d83..cb13c2c9e95 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleGaugeTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleGaugeTest.java @@ -15,13 +15,15 @@ import io.opentelemetry.api.metrics.ObservableDoubleGauge; import io.opentelemetry.extension.incubator.metrics.DoubleGauge; import io.opentelemetry.extension.incubator.metrics.ExtendedDoubleGaugeBuilder; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.metrics.internal.state.DefaultSynchronousMetricStorage; +import io.opentelemetry.sdk.metrics.internal.state.SdkObservableMeasurement; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; import java.time.Duration; import java.util.stream.IntStream; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; /** Unit tests for {@link SdkDoubleGauge}. */ @@ -54,12 +56,20 @@ void set_PreventNullAttributes() { .hasMessage("attributes"); } + @Test + @SuppressLogger(DefaultSynchronousMetricStorage.class) + void set_NaN() { + DoubleGauge gauge = ((ExtendedDoubleGaugeBuilder) sdkMeter.gaugeBuilder("testGauge")).build(); + gauge.set(Double.NaN); + assertThat(cumulativeReader.collectAllMetrics()).hasSize(0); + } + @Test void observable_RemoveCallback() { ObservableDoubleGauge gauge = sdkMeter.gaugeBuilder("testGauge").buildWithCallback(measurement -> measurement.record(10)); - Assertions.assertThat(cumulativeReader.collectAllMetrics()) + assertThat(cumulativeReader.collectAllMetrics()) .satisfiesExactly( metric -> assertThat(metric) @@ -69,7 +79,16 @@ void observable_RemoveCallback() { gauge.close(); - Assertions.assertThat(cumulativeReader.collectAllMetrics()).hasSize(0); + assertThat(cumulativeReader.collectAllMetrics()).hasSize(0); + } + + @Test + @SuppressLogger(SdkObservableMeasurement.class) + void observable_NaN() { + sdkMeter + .gaugeBuilder("testGauge") + .buildWithCallback(measurement -> measurement.record(Double.NaN)); + assertThat(cumulativeReader.collectAllMetrics()).hasSize(0); } @Test diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java index 6862a1d8fc9..dd060736ed6 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java @@ -16,6 +16,7 @@ import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.metrics.internal.state.DefaultSynchronousMetricStorage; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; @@ -266,6 +267,14 @@ void doubleHistogramRecord_NonNegativeCheck() { "Histograms can only record non-negative values. Instrument testHistogram has recorded a negative value."); } + @Test + @SuppressLogger(DefaultSynchronousMetricStorage.class) + void doubleHistogramRecord_NaN() { + DoubleHistogram histogram = sdkMeter.histogramBuilder("testHistogram").build(); + histogram.record(Double.NaN); + assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); + } + @Test void stressTest() { DoubleHistogram doubleHistogram = sdkMeter.histogramBuilder("testHistogram").build(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleCounterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleCounterTest.java index a568a3ffb2d..cc1c887bf3a 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleCounterTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleCounterTest.java @@ -8,10 +8,13 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static org.assertj.core.api.Assertions.assertThat; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.ObservableDoubleCounter; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.metrics.internal.state.SdkObservableMeasurement; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; @@ -53,6 +56,20 @@ void removeCallback() { assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); } + @Test + @SuppressLogger(SdkObservableMeasurement.class) + void observable_NaN() { + InMemoryMetricReader sdkMeterReader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + sdkMeterProviderBuilder.registerMetricReader(sdkMeterReader).build(); + sdkMeterProvider + .get(getClass().getName()) + .counterBuilder("testObserver") + .ofDoubles() + .buildWithCallback(measurement -> measurement.record(Double.NaN)); + assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); + } + @Test void collectMetrics_NoRecords() { InMemoryMetricReader sdkMeterReader = InMemoryMetricReader.create(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleUpDownCounterTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleUpDownCounterTest.java index daf0dc0f18e..8b212220093 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleUpDownCounterTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkObservableDoubleUpDownCounterTest.java @@ -8,12 +8,15 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.ObservableDoubleUpDownCounter; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.metrics.internal.state.SdkObservableMeasurement; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; @@ -55,6 +58,20 @@ void removeCallback() { assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); } + @Test + @SuppressLogger(SdkObservableMeasurement.class) + void observable_NaN() { + InMemoryMetricReader sdkMeterReader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + sdkMeterProviderBuilder.registerMetricReader(sdkMeterReader).build(); + sdkMeterProvider + .get(getClass().getName()) + .upDownCounterBuilder("testCounter") + .ofDoubles() + .buildWithCallback(measurement -> measurement.record(Double.NaN)); + assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); + } + @Test void collectMetrics_NoRecords() { InMemoryMetricReader sdkMeterReader = InMemoryMetricReader.create(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurementTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurementTest.java index ca3590f1fd4..aaef2c841fe 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurementTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SdkObservableMeasurementTest.java @@ -7,11 +7,15 @@ import static io.opentelemetry.sdk.metrics.data.AggregationTemporality.CUMULATIVE; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import io.github.netmikey.logunit.api.LogCapturer; +import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.common.export.MemoryMode; import io.opentelemetry.sdk.metrics.InstrumentType; @@ -21,12 +25,18 @@ import io.opentelemetry.sdk.metrics.internal.export.RegisteredReader; import io.opentelemetry.sdk.metrics.internal.view.ViewRegistry; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; -import org.assertj.core.util.Lists; +import java.util.Arrays; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.ArgumentCaptor; +import org.slf4j.event.Level; @SuppressWarnings("rawtypes") -public class SdkObservableMeasurementTest { +class SdkObservableMeasurementTest { + + @RegisterExtension + final LogCapturer logs = + LogCapturer.create().captureForLogger(SdkObservableMeasurement.class.getName(), Level.DEBUG); private AsynchronousMetricStorage mockAsyncStorage1; private RegisteredReader registeredReader1; @@ -66,11 +76,11 @@ private void setup(MemoryMode memoryMode) { SdkObservableMeasurement.create( instrumentationScopeInfo, instrumentDescriptor, - Lists.newArrayList(mockAsyncStorage1, mockAsyncStorage2)); + Arrays.asList(mockAsyncStorage1, mockAsyncStorage2)); } @Test - public void testRecordLongImmutableData() { + void recordLong_ImmutableData() { setup(MemoryMode.IMMUTABLE_DATA); sdkObservableMeasurement.setActiveReader(registeredReader1, 0, 10); @@ -90,7 +100,7 @@ public void testRecordLongImmutableData() { } @Test - public void testRecordDoubleReturnImmutableData() { + void recordDouble_ImmutableData() { setup(MemoryMode.IMMUTABLE_DATA); sdkObservableMeasurement.setActiveReader(registeredReader1, 0, 10); @@ -110,7 +120,7 @@ public void testRecordDoubleReturnImmutableData() { } @Test - public void testRecordDoubleReturnReusableData() { + void recordDouble_ReusableData() { setup(MemoryMode.REUSABLE_DATA); sdkObservableMeasurement.setActiveReader(registeredReader1, 0, 10); @@ -142,7 +152,7 @@ public void testRecordDoubleReturnReusableData() { } @Test - public void testRecordLongReturnReusableData() { + void recordLong_ReusableData() { setup(MemoryMode.REUSABLE_DATA); sdkObservableMeasurement.setActiveReader(registeredReader1, 0, 10); @@ -172,4 +182,16 @@ public void testRecordLongReturnReusableData() { sdkObservableMeasurement.unsetActiveReader(); } } + + @Test + @SuppressLogger(SdkObservableMeasurement.class) + void recordDouble_NaN() { + setup(MemoryMode.REUSABLE_DATA); + sdkObservableMeasurement.setActiveReader(registeredReader1, 0, 10); + sdkObservableMeasurement.record(Double.NaN); + + verify(mockAsyncStorage1, never()).record(any()); + logs.assertContains( + "Instrument testCounter has recorded measurement Not-a-Number (NaN) value with attributes {}. Dropping measurement."); + } } diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SynchronousMetricStorageTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SynchronousMetricStorageTest.java index 96f4d6274b7..aa6fe1d3999 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SynchronousMetricStorageTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/state/SynchronousMetricStorageTest.java @@ -7,6 +7,7 @@ import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.attributeEntry; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -25,6 +26,7 @@ import io.opentelemetry.sdk.metrics.data.MetricData; import io.opentelemetry.sdk.metrics.internal.aggregator.Aggregator; import io.opentelemetry.sdk.metrics.internal.aggregator.AggregatorFactory; +import io.opentelemetry.sdk.metrics.internal.aggregator.EmptyMetricData; import io.opentelemetry.sdk.metrics.internal.descriptor.Advice; import io.opentelemetry.sdk.metrics.internal.descriptor.InstrumentDescriptor; import io.opentelemetry.sdk.metrics.internal.descriptor.MetricDescriptor; @@ -37,6 +39,7 @@ import io.opentelemetry.sdk.testing.time.TestClock; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.event.Level; @SuppressLogger(DefaultSynchronousMetricStorage.class) public class SynchronousMetricStorageTest { @@ -56,7 +59,8 @@ public class SynchronousMetricStorageTest { private static final int CARDINALITY_LIMIT = 25; @RegisterExtension - LogCapturer logs = LogCapturer.create().captureForType(DefaultSynchronousMetricStorage.class); + LogCapturer logs = + LogCapturer.create().captureForType(DefaultSynchronousMetricStorage.class, Level.DEBUG); private final RegisteredReader deltaReader = RegisteredReader.create(InMemoryMetricReader.createDelta(), ViewRegistry.create()); @@ -69,6 +73,25 @@ public class SynchronousMetricStorageTest { .createAggregator(DESCRIPTOR, ExemplarFilter.alwaysOff())); private final AttributesProcessor attributesProcessor = AttributesProcessor.noop(); + @Test + void recordDouble_NaN() { + DefaultSynchronousMetricStorage storage = + new DefaultSynchronousMetricStorage<>( + cumulativeReader, + METRIC_DESCRIPTOR, + aggregator, + attributesProcessor, + CARDINALITY_LIMIT); + + storage.recordDouble(Double.NaN, Attributes.empty(), Context.current()); + + logs.assertContains( + "Instrument name has recorded measurement Not-a-Number (NaN) value with attributes {}. Dropping measurement."); + verify(aggregator, never()).createHandle(); + assertThat(storage.collect(RESOURCE, INSTRUMENTATION_SCOPE_INFO, 0, 10)) + .isEqualTo(EmptyMetricData.getInstance()); + } + @Test void attributesProcessor_applied() { Attributes attributes = Attributes.builder().put("K", "V").build();