Select histogram aggregation with environment variables

Fixes #3264
open-telemetry · Apr 19, 2023 · fa1a167 · fa1a167
1 parent e003062
commit fa1a167
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+- Enable selection of histogram aggregation via environment variable
+  ([#3265](https://github.com/open-telemetry/opentelemetry-python/pull/3265))
 - Move Protobuf encoding to its own package
   ([#3169](https://github.com/open-telemetry/opentelemetry-python/pull/3169))
 - Add experimental feature to detect resource detectors in auto instrumentation

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py b/opentelemetry-sdk/src/opentelemetry/sdk/environment_variables.py
@@ -650,7 +650,7 @@
 The :envvar:`OTEL_METRICS_EXEMPLAR_FILTER` is the filter for which measurements can become Exemplars.
 """
 
-_OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION = (
+OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION = (
     "OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION"
 )
 """

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py
@@ -19,6 +19,7 @@
 from enum import IntEnum
 from logging import getLogger
 from math import inf
+from os import environ
 from threading import Lock
 from typing import Generic, List, Optional, Sequence, TypeVar
 
@@ -33,6 +34,9 @@
     Synchronous,
     UpDownCounter,
 )
+from opentelemetry.sdk.environment_variables import (
+    OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
+)
 from opentelemetry.sdk.metrics._internal.exponential_histogram.buckets import (
     Buckets,
 )
@@ -927,6 +931,29 @@ def _create_aggregation(
             )
 
         if isinstance(instrument, Histogram):
+
+            histogram_type = environ.get(
+                OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
+                "explicit_bucket_histogram",
+            )
+
+            if histogram_type == "base2_exponential_bucket_histogram":
+
+                return _ExponentialBucketHistogramAggregation(
+                    attributes, start_time_unix_nano
+                )
+
+            if histogram_type != "explicit_bucket_histogram":
+
+                _logger.warning(
+                    (
+                        "Invalid value for %s: %s, using explicit bucket "
+                        "histogram aggregation"
+                    ),
+                    OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
+                    histogram_type,
+                )
+
             return _ExplicitBucketHistogramAggregation(
                 attributes, start_time_unix_nano
             )

diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py
@@ -12,14 +12,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from logging import WARNING
 from math import inf
+from os import environ
 from time import sleep
 from typing import Union
 from unittest import TestCase
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
+from opentelemetry.metrics import Histogram
+from opentelemetry.sdk.environment_variables import (
+    OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION,
+)
 from opentelemetry.sdk.metrics._internal.aggregation import (
     _ExplicitBucketHistogramAggregation,
+    _ExponentialBucketHistogramAggregation,
     _LastValueAggregation,
     _SumAggregation,
 )
@@ -541,3 +548,60 @@ def test_observable_gauge(self):
             0,
         )
         self.assertIsInstance(aggregation, _LastValueAggregation)
+
+    def test_exponential_explicit_bucket_histogram(self):
+
+        histogram = Mock(spec=Histogram)
+        default_aggregation = DefaultAggregation()
+
+        self.assertIsInstance(
+            default_aggregation._create_aggregation(histogram, Mock(), Mock()),
+            _ExplicitBucketHistogramAggregation,
+        )
+
+        with patch.dict(
+            environ,
+            {
+                OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "base2_exponential_bucket_histogram"
+            },
+        ):
+            self.assertIsInstance(
+                default_aggregation._create_aggregation(
+                    histogram, Mock(), Mock()
+                ),
+                _ExponentialBucketHistogramAggregation,
+            )
+
+        with patch.dict(
+            environ,
+            {OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "abc"},
+        ):
+            with self.assertLogs(level=WARNING) as log:
+                self.assertIsInstance(
+                    default_aggregation._create_aggregation(
+                        histogram, Mock(), Mock()
+                    ),
+                    _ExplicitBucketHistogramAggregation,
+                )
+            self.assertEqual(
+                log.output[0],
+                (
+                    "WARNING:opentelemetry.sdk.metrics._internal.aggregation:"
+                    "Invalid value for OTEL_EXPORTER_OTLP_METRICS_DEFAULT_"
+                    "HISTOGRAM_AGGREGATION: abc, using explicit bucket "
+                    "histogram aggregation"
+                ),
+            )
+
+        with patch.dict(
+            environ,
+            {
+                OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION: "explicit_bucket_histogram"
+            },
+        ):
+            self.assertIsInstance(
+                default_aggregation._create_aggregation(
+                    histogram, Mock(), Mock()
+                ),
+                _ExplicitBucketHistogramAggregation,
+            )