diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py index 1ef8dcfaa6..4326aaa080 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py @@ -11,13 +11,15 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import weakref from atexit import register, unregister +from collections.abc import Sequence from logging import getLogger from os import environ from threading import Lock from time import time_ns -from typing import Optional, Sequence +from typing import Optional # This kind of import is needed to avoid Sphinx errors. import opentelemetry.sdk.metrics @@ -63,7 +65,7 @@ from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationScope from opentelemetry.util._once import Once -from opentelemetry.util.types import Attributes +from opentelemetry.util.types import Attributes, MetricsInstrumentAdvisory _logger = getLogger(__name__) @@ -196,7 +198,29 @@ def create_observable_counter( self._instrument_id_instrument[instrument_id] = instrument return instrument - def create_histogram(self, name, unit="", description="") -> APIHistogram: + def create_histogram( + self, + name: str, + unit: str = "", + description: str = "", + advisory: MetricsInstrumentAdvisory = None, + ) -> APIHistogram: + if advisory is not None: + raise_error = False + try: + boundaries = advisory["ExplicitBucketBoundaries"] + raise_error = not ( + boundaries + and all(isinstance(e, (int, float)) for e in boundaries) + ) + except (KeyError, TypeError): + raise_error = True + + if raise_error: + raise ValueError( + "Advisory must be a dict with ExplicitBucketBoundaries key containing a sequence of numbers" + ) + ( is_instrument_registered, instrument_id, @@ -223,6 +247,7 @@ def create_histogram(self, name, unit="", description="") -> APIHistogram: self._measurement_consumer, unit, description, + advisory, ) with self._instrument_id_instrument_lock: self._instrument_id_instrument[instrument_id] = instrument diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index a579e3072e..fd592c0d30 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -437,6 +437,25 @@ def collect( ) +_DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES: Sequence[float] = ( + 0.0, + 5.0, + 10.0, + 25.0, + 50.0, + 75.0, + 100.0, + 250.0, + 500.0, + 750.0, + 1000.0, + 2500.0, + 5000.0, + 7500.0, + 10000.0, +) + + class _ExplicitBucketHistogramAggregation(_Aggregation[HistogramPoint]): def __init__( self, @@ -444,25 +463,14 @@ def __init__( instrument_aggregation_temporality: AggregationTemporality, start_time_unix_nano: int, reservoir_builder: ExemplarReservoirBuilder, - boundaries: Sequence[float] = ( - 0.0, - 5.0, - 10.0, - 25.0, - 50.0, - 75.0, - 100.0, - 250.0, - 500.0, - 750.0, - 1000.0, - 2500.0, - 5000.0, - 7500.0, - 10000.0, - ), + boundaries: Optional[Sequence[float]] = None, record_min_max: bool = True, ): + if boundaries is None: + boundaries = ( + _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES + ) + super().__init__( attributes, reservoir_builder=partial( @@ -1268,6 +1276,11 @@ def _create_aggregation( ) if isinstance(instrument, Histogram): + boundaries: Optional[Sequence[float]] = ( + instrument.advisory.get("ExplicitBucketBoundaries") + if instrument.advisory is not None + else None + ) return _ExplicitBucketHistogramAggregation( attributes, reservoir_builder=reservoir_factory( @@ -1276,6 +1289,7 @@ def _create_aggregation( instrument_aggregation_temporality=( AggregationTemporality.DELTA ), + boundaries=boundaries, start_time_unix_nano=start_time_unix_nano, ) @@ -1347,25 +1361,13 @@ class ExplicitBucketHistogramAggregation(Aggregation): def __init__( self, - boundaries: Sequence[float] = ( - 0.0, - 5.0, - 10.0, - 25.0, - 50.0, - 75.0, - 100.0, - 250.0, - 500.0, - 750.0, - 1000.0, - 2500.0, - 5000.0, - 7500.0, - 10000.0, - ), + boundaries: Optional[Sequence[float]] = None, record_min_max: bool = True, ) -> None: + if boundaries is None: + boundaries = ( + _DEFAULT_EXPLICIT_BUCKET_HISTOGRAM_AGGREGATION_BOUNDARIES + ) self._boundaries = boundaries self._record_min_max = record_min_max diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py index c93f83a4e6..a898f151fe 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py @@ -34,6 +34,7 @@ from opentelemetry.metrics._internal.instrument import CallbackOptions from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.util.instrumentation import InstrumentationScope +from opentelemetry.util.types import MetricsInstrumentAdvisory _logger = getLogger(__name__) @@ -219,6 +220,24 @@ def __new__(cls, *args, **kwargs): class Histogram(_Synchronous, APIHistogram): + def __init__( + self, + name: str, + instrumentation_scope: InstrumentationScope, + measurement_consumer: "opentelemetry.sdk.metrics.MeasurementConsumer", + unit: str = "", + description: str = "", + advisory: MetricsInstrumentAdvisory = None, + ): + super().__init__( + name, + unit=unit, + description=description, + instrumentation_scope=instrumentation_scope, + measurement_consumer=measurement_consumer, + ) + self.advisory = advisory + def __new__(cls, *args, **kwargs): if cls is Histogram: raise TypeError("Histogram must be instantiated via a meter.") diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 4a625908af..48cc6e32d3 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -15,6 +15,7 @@ # pylint: disable=protected-access from math import inf +from random import randint from time import sleep, time_ns from typing import Union from unittest import TestCase @@ -628,6 +629,22 @@ def test_histogram(self): ) self.assertIsInstance(aggregation, _ExplicitBucketHistogramAggregation) + def test_histogram_with_advisory(self): + boundaries = [randint(0, 1000)] + aggregation = self.default_aggregation._create_aggregation( + _Histogram( + "name", + Mock(), + Mock(), + advisory={"ExplicitBucketBoundaries": boundaries}, + ), + Mock(), + _default_reservoir_factory, + 0, + ) + self.assertIsInstance(aggregation, _ExplicitBucketHistogramAggregation) + self.assertEqual(aggregation._boundaries, tuple(boundaries)) + def test_gauge(self): aggregation = self.default_aggregation._create_aggregation( _Gauge( diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 4ba0c2fde8..9b2d87bb19 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -500,6 +500,38 @@ def test_create_histogram(self): self.assertIsInstance(histogram, Histogram) self.assertEqual(histogram.name, "name") + def test_create_histogram_with_advisory(self): + histogram = self.meter.create_histogram( + "name", + unit="unit", + description="description", + advisory={"ExplicitBucketBoundaries": [0, 1, 2]}, + ) + + self.assertIsInstance(histogram, Histogram) + self.assertEqual(histogram.name, "name") + self.assertEqual( + histogram.advisory, {"ExplicitBucketBoundaries": [0, 1, 2]} + ) + + def test_create_histogram_advisory_validation(self): + advisories = [ + {"ExplicitBucketBoundaries": None}, + {"ExplicitBucketBoundaries": []}, + {}, + [], + {"ExplicitBucketBoundaries": [1, 2.0, "3"]}, + ] + for advisory in advisories: + with self.subTest(advisory=advisory): + with self.assertRaises(ValueError): + self.meter.create_histogram( + "name", + unit="unit", + description="description", + advisory=advisory, + ) + def test_create_observable_gauge(self): observable_gauge = self.meter.create_observable_gauge( "name", callbacks=[Mock()], unit="unit", description="description"