record_min_max=False exports histogram min=+Infinity and max=-Infinity

.changelog/5566.fixed

@@ -0,0 +1 @@
+`opentelemetry-sdk`: leave histogram `min`/`max` unset when `record_min_max` is disabled, instead of exporting the `+Inf`/`-Inf` sentinels

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

@@ -495,8 +495,8 @@ def collect(
         with self._lock:
             value = self._value
             sum_ = self._sum
-            min_ = self._min
-            max_ = self._max
+            min_ = self._min if self._record_min_max else None
+            max_ = self._max if self._record_min_max else None
 
             self._value = None
             self._sum = 0
@@ -539,8 +539,12 @@ def collect(
                         previous_value_element,
                     ) in zip(value, self._previous_value)
                 ]
-                self._previous_min = min(min_, self._previous_min)
-                self._previous_max = max(max_, self._previous_max)
+                if self._record_min_max:
+                    self._previous_min = min(min_, self._previous_min)
+                    self._previous_max = max(max_, self._previous_max)
+                else:
+                    self._previous_min = None
+                    self._previous_max = None
                 self._previous_sum = sum_ + self._previous_sum
 
                 return HistogramDataPoint(
@@ -764,8 +768,8 @@ def collect(
             value_positive = self._value_positive
             value_negative = self._value_negative
             sum_ = self._sum
-            min_ = self._min
-            max_ = self._max
+            min_ = self._min if self._record_min_max else None
+            max_ = self._max if self._record_min_max else None
             count = self._count
             zero_count = self._zero_count
             scale = self._scale
@@ -924,8 +928,12 @@ def collect(
                     collection_aggregation_temporality,
                 )
 
-                self._previous_min = min(min_, self._previous_min)
-                self._previous_max = max(max_, self._previous_max)
+                if self._record_min_max:
+                    self._previous_min = min(min_, self._previous_min)
+                    self._previous_max = max(max_, self._previous_max)
+                else:
+                    self._previous_min = None
+                    self._previous_max = None
                 self._previous_sum = sum_ + self._previous_sum
                 self._previous_count = count + self._previous_count
                 self._previous_zero_count = zero_count + self._previous_zero_count

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

@@ -3,6 +3,8 @@
 
 # pylint: disable=unused-import
 
+from __future__ import annotations
+
 from collections.abc import Sequence
 from dataclasses import asdict, dataclass, field
 from json import dumps, loads
@@ -44,8 +46,8 @@ class HistogramDataPoint:
     sum: int | float
     bucket_counts: Sequence[int]
     explicit_bounds: Sequence[float]
-    min: float
-    max: float
+    min: float | None
+    max: float | None
     exemplars: Sequence[Exemplar] = field(default_factory=list)
 
     def to_json(self, indent: int | None = 4) -> str:
@@ -75,8 +77,8 @@ class ExponentialHistogramDataPoint:
     positive: Buckets
     negative: Buckets
     flags: int
-    min: float
-    max: float
+    min: float | None
+    max: float | None
     exemplars: Sequence[Exemplar] = field(default_factory=list)
 
     def to_json(self, indent: int | None = 4) -> str:

opentelemetry-sdk/tests/metrics/test_histogram_record_min_max.py

@@ -0,0 +1,163 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+"""With record_min_max disabled the min/max fields must be absent, not sentinels.
+
+The aggregators seed min/max with +inf/-inf. If those sentinels reach a data
+point they are exported as present OTLP fields, producing a histogram whose
+minimum is +Infinity and maximum is -Infinity, and `to_json` emits the literals
+`Infinity` / `-Infinity`, which are not valid JSON.
+"""
+
+import json
+import unittest
+
+from opentelemetry.exporter.otlp.proto.common.metrics_encoder import (
+    encode_metrics,
+)
+from opentelemetry.sdk.metrics import Histogram as HistogramInstrument
+from opentelemetry.sdk.metrics import MeterProvider
+from opentelemetry.sdk.metrics.export import (
+    AggregationTemporality,
+    InMemoryMetricReader,
+)
+from opentelemetry.sdk.metrics.view import (
+    ExplicitBucketHistogramAggregation,
+    ExponentialBucketHistogramAggregation,
+    View,
+)
+
+
+def _reject_non_json_constants(constant):
+    raise ValueError(f"{constant!r} is not valid JSON")
+
+
+class _HistogramCase:
+    """Drive one histogram aggregation through a reader."""
+
+    def __init__(self, aggregation, temporality=AggregationTemporality.CUMULATIVE):
+        self.reader = InMemoryMetricReader(
+            preferred_temporality={HistogramInstrument: temporality},
+        )
+        self.provider = MeterProvider(
+            metric_readers=[self.reader],
+            views=[View(instrument_name="hist", aggregation=aggregation)],
+        )
+        self.histogram = self.provider.get_meter(__name__).create_histogram("hist")
+
+    def collect(self):
+        data = self.reader.get_metrics_data()
+        return data.resource_metrics[0].scope_metrics[0].metrics[0].data.data_points[0]
+
+    def collect_encoded(self):
+        data = self.reader.get_metrics_data()
+        metric = encode_metrics(data).resource_metrics[0].scope_metrics[0].metrics[0]
+        return getattr(metric, metric.WhichOneof("data")).data_points[0]
+
+    def shutdown(self):
+        self.provider.shutdown()
+
+
+class TestRecordMinMaxDisabled(unittest.TestCase):
+    AGGREGATIONS = {
+        "explicit": ExplicitBucketHistogramAggregation,
+        "exponential": ExponentialBucketHistogramAggregation,
+    }
+
+    def test_data_point_leaves_min_and_max_unset(self):
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=False))
+                case.histogram.record(5)
+                case.histogram.record(50)
+                point = case.collect()
+                self.assertIsNone(point.min)
+                self.assertIsNone(point.max)
+                self.assertEqual(point.sum, 55)
+                self.assertEqual(point.count, 2)
+                case.shutdown()
+
+    def test_otlp_fields_are_absent(self):
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=False))
+                case.histogram.record(5)
+                point = case.collect_encoded()
+                self.assertFalse(point.HasField("min"))
+                self.assertFalse(point.HasField("max"))
+                case.shutdown()
+
+    def test_to_json_is_valid_json(self):
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=False))
+                case.histogram.record(5)
+                payload = case.collect().to_json()
+                json.loads(payload, parse_constant=_reject_non_json_constants)
+                case.shutdown()
+
+    def test_cumulative_collections_stay_unset(self):
+        """The delta-to-cumulative merge must not resurrect the sentinels."""
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=False))
+                case.histogram.record(5)
+                case.collect()
+                case.histogram.record(50)
+                point = case.collect()
+                self.assertIsNone(point.min)
+                self.assertIsNone(point.max)
+                self.assertEqual(point.sum, 55)
+                case.shutdown()
+
+    def test_delta_temporality_leaves_min_and_max_unset(self):
+        case = _HistogramCase(
+            ExplicitBucketHistogramAggregation(record_min_max=False),
+            temporality=AggregationTemporality.DELTA,
+        )
+        case.histogram.record(5)
+        point = case.collect()
+        self.assertIsNone(point.min)
+        self.assertIsNone(point.max)
+        case.shutdown()
+
+
+class TestRecordMinMaxEnabled(unittest.TestCase):
+    """The default must keep working exactly as before."""
+
+    AGGREGATIONS = {
+        "explicit": ExplicitBucketHistogramAggregation,
+        "exponential": ExponentialBucketHistogramAggregation,
+    }
+
+    def test_min_and_max_are_recorded(self):
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=True))
+                case.histogram.record(5)
+                case.histogram.record(50)
+                point = case.collect()
+                self.assertEqual(point.min, 5)
+                self.assertEqual(point.max, 50)
+                case.shutdown()
+
+    def test_cumulative_min_and_max_span_collections(self):
+        for name, aggregation in self.AGGREGATIONS.items():
+            with self.subTest(aggregation=name):
+                case = _HistogramCase(aggregation(record_min_max=True))
+                case.histogram.record(50)
+                case.collect()
+                case.histogram.record(5)
+                point = case.collect()
+                self.assertEqual(point.min, 5)
+                self.assertEqual(point.max, 50)
+                case.shutdown()
+
+    def test_otlp_fields_are_present(self):
+        case = _HistogramCase(ExplicitBucketHistogramAggregation(record_min_max=True))
+        case.histogram.record(5)
+        point = case.collect_encoded()
+        self.assertTrue(point.HasField("min"))
+        self.assertTrue(point.HasField("max"))
+        self.assertEqual(point.min, 5)
+        case.shutdown()