Add record_min_max option to exponential histogram aggregation

.changelog/5377.added

@@ -0,0 +1 @@
+`opentelemetry-sdk`: add `record_min_max` option to `ExponentialBucketHistogramAggregation`, matching the option already available on `ExplicitBucketHistogramAggregation` and required by the specification

.gitignore

@@ -36,6 +36,8 @@ coverage.xml
 .cache
 htmlcov
 
+.agent/
+
 # Translations
 *.mo
 

opentelemetry-sdk/src/opentelemetry/sdk/_configuration/_meter_provider.py

@@ -203,6 +203,13 @@ def _create_aggregation(config: AggregationConfig) -> Aggregation:
             kwargs["max_scale"] = (
                 config.base2_exponential_bucket_histogram.max_scale
             )
+        if (
+            config.base2_exponential_bucket_histogram.record_min_max
+            is not None
+        ):
+            kwargs["record_min_max"] = (
+                config.base2_exponential_bucket_histogram.record_min_max
+            )
         return ExponentialBucketHistogramAggregation(**kwargs)
     if config.last_value is not None:
         return LastValueAggregation()

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

@@ -623,6 +623,7 @@ def __init__(
         # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exponential-bucket-histogram-aggregation)
         max_size: int = 160,
         max_scale: int = 20,
+        record_min_max: bool = True,
     ):
         # max_size is the maximum capacity of the positive and negative
         # buckets.
@@ -671,6 +672,7 @@ def __init__(
         self._start_time_unix_nano = start_time_unix_nano
         self._max_size = max_size
         self._max_scale = max_scale
+        self._record_min_max = record_min_max
 
         self._value_positive = None
         self._value_negative = None
@@ -709,8 +711,9 @@ def aggregate(
 
             self._sum += measurement_value
 
-            self._min = min(self._min, measurement_value)
-            self._max = max(self._max, measurement_value)
+            if self._record_min_max:
+                self._min = min(self._min, measurement_value)
+                self._max = max(self._max, measurement_value)
 
             self._count += 1
 
@@ -1307,13 +1310,28 @@ def _create_aggregation(
 
 
 class ExponentialBucketHistogramAggregation(Aggregation):
+    """This aggregation informs the SDK to collect:
+
+    - Count of Measurement values falling using a base-2 exponential formula.
+    - Arithmetic sum of Measurement values in population. This SHOULD NOT be collected when used with instruments that record negative measurements, e.g. UpDownCounter or ObservableGauge.
+    - Min (optional) Measurement value in population.
+    - Max (optional) Measurement value in population.
+
+    Args:
+        max_size: Maximum number of buckets in each of the positive and negative ranges, not counting the special zero bucket.
+        max_scale: Maximum scale factor.
+        record_min_max: Whether to record min and max.
+    """
+
     def __init__(
         self,
         max_size: int = 160,
         max_scale: int = 20,
+        record_min_max: bool = True,
     ):
         self._max_size = max_size
         self._max_scale = max_scale
+        self._record_min_max = record_min_max
 
     def _create_aggregation(
         self,
@@ -1339,6 +1357,7 @@ def _create_aggregation(
             start_time_unix_nano,
             max_size=self._max_size,
             max_scale=self._max_scale,
+            record_min_max=self._record_min_max,
         )
 
 

opentelemetry-sdk/tests/_configuration/test_meter_provider.py

@@ -23,9 +23,6 @@
 from opentelemetry.sdk._configuration.models import (
     ConsoleMetricExporter as ConsoleMetricExporterConfig,
 )
-from opentelemetry.sdk._configuration.models import (
-    ExemplarFilter as ExemplarFilterConfig,
-)
 from opentelemetry.sdk._configuration.models import (
     ExperimentalOtlpFileMetricExporter as ExperimentalOtlpFileMetricExporterConfig,
 )
@@ -71,8 +68,6 @@
     View as ViewConfig,
 )
 from opentelemetry.sdk.metrics import (
-    AlwaysOffExemplarFilter,
-    AlwaysOnExemplarFilter,
     Counter,
     Histogram,
     MeterProvider,
@@ -934,6 +929,29 @@ def test_stream_aggregation_base2_exponential_with_params(self):
             view._aggregation, ExponentialBucketHistogramAggregation
         )
 
+    def test_stream_aggregation_base2_exponential_record_min_max(self):
+        for record_min_max, expected in [
+            (True, True),
+            (False, False),
+            (None, True),
+        ]:
+            with self.subTest(record_min_max=record_min_max):
+                view = self._get_view(
+                    self._make_view_config(
+                        stream_kwargs={
+                            "aggregation": AggregationConfig(
+                                base2_exponential_bucket_histogram=Base2Config(
+                                    record_min_max=record_min_max
+                                )
+                            )
+                        }
+                    )
+                )
+                self.assertIsInstance(
+                    view._aggregation, ExponentialBucketHistogramAggregation
+                )
+                self.assertEqual(view._aggregation._record_min_max, expected)
+
     def test_stream_aggregation_last_value(self):
         view = self._get_view(
             self._make_view_config(
@@ -957,39 +975,3 @@ def test_stream_aggregation_default(self):
             )
         )
         self.assertIsInstance(view._aggregation, DefaultAggregation)
-
-
-class TestExemplarFilter(unittest.TestCase):
-    @staticmethod
-    def _make_config(exemplar_filter):
-        return MeterProviderConfig(readers=[], exemplar_filter=exemplar_filter)
-
-    def test_always_on(self):
-        provider = create_meter_provider(
-            self._make_config(ExemplarFilterConfig.always_on)
-        )
-        self.assertIsInstance(
-            provider._sdk_config.exemplar_filter, AlwaysOnExemplarFilter
-        )
-
-    def test_always_off(self):
-        provider = create_meter_provider(
-            self._make_config(ExemplarFilterConfig.always_off)
-        )
-        self.assertIsInstance(
-            provider._sdk_config.exemplar_filter, AlwaysOffExemplarFilter
-        )
-
-    def test_trace_based(self):
-        provider = create_meter_provider(
-            self._make_config(ExemplarFilterConfig.trace_based)
-        )
-        self.assertIsInstance(
-            provider._sdk_config.exemplar_filter, TraceBasedExemplarFilter
-        )
-
-    def test_absent_defaults_to_trace_based(self):
-        provider = create_meter_provider(MeterProviderConfig(readers=[]))
-        self.assertIsInstance(
-            provider._sdk_config.exemplar_filter, TraceBasedExemplarFilter
-        )

opentelemetry-sdk/tests/_configuration/test_meter_provider_exemplar_filter.py

@@ -0,0 +1,58 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+# Tests access private members of SDK classes to assert correct configuration.
+# pylint: disable=protected-access
+
+import unittest
+
+from opentelemetry.sdk._configuration._meter_provider import (
+    create_meter_provider,
+)
+from opentelemetry.sdk._configuration.models import (
+    ExemplarFilter as ExemplarFilterConfig,
+)
+from opentelemetry.sdk._configuration.models import (
+    MeterProvider as MeterProviderConfig,
+)
+from opentelemetry.sdk.metrics import (
+    AlwaysOffExemplarFilter,
+    AlwaysOnExemplarFilter,
+    TraceBasedExemplarFilter,
+)
+
+
+class TestExemplarFilter(unittest.TestCase):
+    @staticmethod
+    def _make_config(exemplar_filter):
+        return MeterProviderConfig(readers=[], exemplar_filter=exemplar_filter)
+
+    def test_always_on(self):
+        provider = create_meter_provider(
+            self._make_config(ExemplarFilterConfig.always_on)
+        )
+        self.assertIsInstance(
+            provider._sdk_config.exemplar_filter, AlwaysOnExemplarFilter
+        )
+
+    def test_always_off(self):
+        provider = create_meter_provider(
+            self._make_config(ExemplarFilterConfig.always_off)
+        )
+        self.assertIsInstance(
+            provider._sdk_config.exemplar_filter, AlwaysOffExemplarFilter
+        )
+
+    def test_trace_based(self):
+        provider = create_meter_provider(
+            self._make_config(ExemplarFilterConfig.trace_based)
+        )
+        self.assertIsInstance(
+            provider._sdk_config.exemplar_filter, TraceBasedExemplarFilter
+        )
+
+    def test_absent_defaults_to_trace_based(self):
+        provider = create_meter_provider(MeterProviderConfig(readers=[]))
+        self.assertIsInstance(
+            provider._sdk_config.exemplar_filter, TraceBasedExemplarFilter
+        )

opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py

@@ -7,7 +7,7 @@
 from inspect import currentframe
 from itertools import permutations
 from logging import WARNING
-from math import ldexp
+from math import inf, ldexp
 from random import Random, randrange
 from sys import float_info, maxsize
 from time import time_ns
@@ -112,6 +112,65 @@ def test_create_aggregation(self, mock_logarithm_mapping):
 
         mock_logarithm_mapping.assert_called_with(100)
 
+    def test_create_aggregation_record_min_max(self):
+        for record_min_max, expected in [
+            (None, True),
+            (True, True),
+            (False, False),
+        ]:
+            with self.subTest(record_min_max=record_min_max):
+                kwargs = (
+                    {}
+                    if record_min_max is None
+                    else {"record_min_max": record_min_max}
+                )
+                exponential_bucket_histogram_aggregation = (
+                    ExponentialBucketHistogramAggregation(**kwargs)
+                )._create_aggregation(Mock(), Mock(), Mock(), Mock())
+
+                self.assertEqual(
+                    exponential_bucket_histogram_aggregation._record_min_max,
+                    expected,
+                )
+
+    def test_min_max(self):
+        """
+        `record_min_max` indicates the aggregator to record the minimum and
+        maximum value in the population
+        """
+
+        now = time_ns()
+
+        for record_min_max, expected_min, expected_max in [
+            (True, 1, 9999),
+            (False, inf, -inf),
+        ]:
+            with self.subTest(record_min_max=record_min_max):
+                ctx = Context()
+                exponential_histogram_aggregation = (
+                    _ExponentialBucketHistogramAggregation(
+                        Mock(),
+                        _default_reservoir_factory(
+                            _ExponentialBucketHistogramAggregation
+                        ),
+                        AggregationTemporality.CUMULATIVE,
+                        0,
+                        record_min_max=record_min_max,
+                    )
+                )
+
+                for value in [2, 4, 1, 9999]:
+                    exponential_histogram_aggregation.aggregate(
+                        Measurement(value, now, Mock(), ctx)
+                    )
+
+                self.assertEqual(
+                    exponential_histogram_aggregation._min, expected_min
+                )
+                self.assertEqual(
+                    exponential_histogram_aggregation._max, expected_max
+                )
+
     def assertInEpsilon(self, first, second, epsilon):
         self.assertLessEqual(first, (second * (1 + epsilon)))
         self.assertGreaterEqual(first, (second * (1 - epsilon)))