opentelemetry-sdk: drop NaN measurements at instrument level to prevent aggregation poisoning

.changelog/5336.fixed

@@ -0,0 +1 @@
+`opentelemetry-sdk`: drop NaN measurements at the instrument level to prevent permanent aggregation poisoning

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py

@@ -4,6 +4,7 @@
 # pylint: disable=too-many-ancestors, unused-import
 from __future__ import annotations
 
+import math
 from collections.abc import Generator, Iterable, Sequence
 from logging import getLogger
 from time import time_ns
@@ -163,6 +164,12 @@ def callback(
         for callback in self._callbacks:
             try:
                 for api_measurement in callback(callback_options):
+                    if math.isnan(api_measurement.value):
+                        _logger.warning(
+                            "Callback returned NaN for instrument %s, ignoring measurement.",
+                            self.name,
+                        )
+                        continue
                     yield Measurement(
                         api_measurement.value,
                         time_unix_nano=time_ns(),
@@ -192,6 +199,12 @@ def add(
             super().add(amount, attributes=attributes, context=context)
             return
 
+        if math.isnan(amount):
+            _logger.warning(
+                "Add amount is NaN on Counter %s, ignoring measurement.",
+                self.name,
+            )
+            return
         if amount < 0:
             _logger.warning(
                 "Add amount must be non-negative on Counter %s.", self.name
@@ -225,6 +238,12 @@ def add(
             super().add(amount, attributes=attributes, context=context)
             return
 
+        if math.isnan(amount):
+            _logger.warning(
+                "Add amount is NaN on UpDownCounter %s, ignoring measurement.",
+                self.name,
+            )
+            return
         time_unix_nano = time_ns()
         self._measurement_consumer.consume_measurement(
             Measurement(
@@ -294,6 +313,12 @@ def record(
             super().record(amount, attributes=attributes, context=context)
             return
 
+        if math.isnan(amount):
+            _logger.warning(
+                "Record amount is NaN on Histogram %s, ignoring measurement.",
+                self.name,
+            )
+            return
         if amount < 0:
             _logger.warning(
                 "Record amount must be non-negative on Histogram %s.",
@@ -328,6 +353,12 @@ def set(
             super().set(amount, attributes=attributes, context=context)
             return
 
+        if math.isnan(amount):
+            _logger.warning(
+                "Set amount is NaN on Gauge %s, ignoring measurement.",
+                self.name,
+            )
+            return
         time_unix_nano = time_ns()
         self._measurement_consumer.consume_measurement(
             Measurement(

opentelemetry-sdk/tests/metrics/test_instrument.py

@@ -51,6 +51,13 @@ def test_add_non_monotonic(self):
             counter.add(-1.0)
         mc.consume_measurement.assert_not_called()
 
+    def test_add_nan(self):
+        mc = Mock()
+        counter = _Counter("name", Mock(), mc)
+        with self.assertLogs(level=WARNING):
+            counter.add(float("nan"))
+        mc.consume_measurement.assert_not_called()
+
     def test_disallow_direct_counter_creation(self):
         with self.assertRaises(TypeError):
             # pylint: disable=abstract-class-instantiated
@@ -70,6 +77,13 @@ def test_add_non_monotonic(self):
         counter.add(-1.0)
         mc.consume_measurement.assert_called_once()
 
+    def test_add_nan(self):
+        mc = Mock()
+        counter = _UpDownCounter("name", Mock(), mc)
+        with self.assertLogs(level=WARNING):
+            counter.add(float("nan"))
+        mc.consume_measurement.assert_not_called()
+
     def test_disallow_direct_up_down_counter_creation(self):
         with self.assertRaises(TypeError):
             # pylint: disable=abstract-class-instantiated
@@ -300,6 +314,21 @@ def test_generator_multiple_generator_callback(self):
             ],
         )
 
+    def test_nan_callback_value_is_dropped(self):
+        def nan_callback(options: CallbackOptions):
+            return [
+                Observation(float("nan"), attributes=TEST_ATTRIBUTES),
+                Observation(1, attributes=TEST_ATTRIBUTES),
+            ]
+
+        observable_gauge = _ObservableGauge(
+            "name", Mock(), Mock(), [nan_callback]
+        )
+        with self.assertLogs(level=WARNING):
+            measurements = list(observable_gauge.callback(CallbackOptions()))
+        self.assertEqual(len(measurements), 1)
+        self.assertEqual(measurements[0].value, 1)
+
     def test_disallow_direct_observable_gauge_creation(self):
         with self.assertRaises(TypeError):
             # pylint: disable=abstract-class-instantiated
@@ -392,6 +421,13 @@ def test_set(self):
         gauge.set(1.0)
         mc.consume_measurement.assert_called_once()
 
+    def test_set_nan(self):
+        mc = Mock()
+        gauge = _Gauge("name", Mock(), mc)
+        with self.assertLogs(level=WARNING):
+            gauge.set(float("nan"))
+        mc.consume_measurement.assert_not_called()
+
     def test_disallow_direct_counter_creation(self):
         with self.assertRaises(TypeError):
             # pylint: disable=abstract-class-instantiated
@@ -487,6 +523,13 @@ def test_record_non_monotonic(self):
             hist.record(-1.0)
         mc.consume_measurement.assert_not_called()
 
+    def test_record_nan(self):
+        mc = Mock()
+        hist = _Histogram("name", Mock(), mc)
+        with self.assertLogs(level=WARNING):
+            hist.record(float("nan"))
+        mc.consume_measurement.assert_not_called()
+
     def test_disallow_direct_histogram_creation(self):
         with self.assertRaises(TypeError):
             # pylint: disable=abstract-class-instantiated