diff --git a/.changelog/5847.added b/.changelog/5847.added new file mode 100644 index 00000000000..d8a7f83653a --- /dev/null +++ b/.changelog/5847.added @@ -0,0 +1 @@ +`opentelemetry-api`, `opentelemetry-sdk`: add experimental `_opt_in_advisory` metric instrument parameter and `View` `enabled` configuration parameter diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py index 183966b1b7b..1b61e8df4b4 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py @@ -1,7 +1,7 @@ # Copyright The OpenTelemetry Authors # SPDX-License-Identifier: Apache-2.0 -# pylint: disable=too-many-ancestors +# pylint: disable=too-many-ancestors,too-many-lines """ The OpenTelemetry metrics API describes the classes used to generate @@ -55,7 +55,7 @@ ObservableGauge, ObservableUpDownCounter, UpDownCounter, - _MetricsHistogramAdvisory, + _MetricsAdvisory, _ProxyCounter, _ProxyGauge, _ProxyHistogram, @@ -63,6 +63,8 @@ _ProxyObservableGauge, _ProxyObservableUpDownCounter, _ProxyUpDownCounter, + _validate_explicit_bucket_boundaries_advisory, + _validate_opt_in_advisory, ) from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider @@ -174,7 +176,7 @@ class _InstrumentRegistrationStatus: instrument_id: str already_registered: bool conflict: bool - current_advisory: _MetricsHistogramAdvisory | None + current_advisory: _MetricsAdvisory | None class Meter(ABC): @@ -194,7 +196,7 @@ def __init__( self._name = name self._version = version self._schema_url = schema_url - self._instrument_ids: dict[str, _MetricsHistogramAdvisory | None] = {} + self._instrument_ids: dict[str, _MetricsAdvisory | None] = {} self._instrument_ids_lock = Lock() @property @@ -224,7 +226,7 @@ def _register_instrument( type_: type, unit: str, description: str, - advisory: _MetricsHistogramAdvisory | None = None, + advisory: _MetricsAdvisory | None = None, ) -> _InstrumentRegistrationStatus: """ Register an instrument with the name, type, unit and description as @@ -285,6 +287,8 @@ def create_counter( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Counter: """Creates a `Counter` instrument @@ -301,6 +305,8 @@ def create_up_down_counter( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> UpDownCounter: """Creates an `UpDownCounter` instrument @@ -318,6 +324,8 @@ def create_observable_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableCounter: """Creates an `ObservableCounter` instrument @@ -463,6 +471,7 @@ def create_histogram( description: str = "", *, explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + _opt_in_advisory: bool | None = None, ) -> Histogram: """Creates a :class:`~opentelemetry.metrics.Histogram` instrument @@ -478,6 +487,8 @@ def create_gauge( # type: ignore # pylint: disable=no-self-use name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Gauge: # pyright: ignore[reportReturnType] """Creates a ``Gauge`` instrument @@ -496,6 +507,8 @@ def create_observable_gauge( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableGauge: """Creates an `ObservableGauge` instrument @@ -516,6 +529,8 @@ def create_observable_up_down_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableUpDownCounter: """Creates an `ObservableUpDownCounter` instrument @@ -562,11 +577,23 @@ def create_counter( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Counter: with self._lock: if self._real_meter: - return self._real_meter.create_counter(name, unit, description) - proxy = _ProxyCounter(name, unit, description) + return self._real_meter.create_counter( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) + proxy = _ProxyCounter( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -575,11 +602,23 @@ def create_up_down_counter( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> UpDownCounter: with self._lock: if self._real_meter: - return self._real_meter.create_up_down_counter(name, unit, description) - proxy = _ProxyUpDownCounter(name, unit, description) + return self._real_meter.create_up_down_counter( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) + proxy = _ProxyUpDownCounter( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -589,11 +628,25 @@ def create_observable_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableCounter: with self._lock: if self._real_meter: - return self._real_meter.create_observable_counter(name, callbacks, unit, description) - proxy = _ProxyObservableCounter(name, callbacks, unit=unit, description=description) + return self._real_meter.create_observable_counter( + name, + callbacks, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) + proxy = _ProxyObservableCounter( + name, + callbacks, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -604,6 +657,7 @@ def create_histogram( description: str = "", *, explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + _opt_in_advisory: bool | None = None, ) -> Histogram: with self._lock: if self._real_meter: @@ -612,8 +666,15 @@ def create_histogram( unit, description, explicit_bucket_boundaries_advisory=explicit_bucket_boundaries_advisory, + _opt_in_advisory=_opt_in_advisory, ) - proxy = _ProxyHistogram(name, unit, description, explicit_bucket_boundaries_advisory) + proxy = _ProxyHistogram( + name, + unit, + description, + explicit_bucket_boundaries_advisory, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -622,11 +683,23 @@ def create_gauge( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Gauge: with self._lock: if self._real_meter: - return self._real_meter.create_gauge(name, unit, description) - proxy = _ProxyGauge(name, unit, description) + return self._real_meter.create_gauge( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) + proxy = _ProxyGauge( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -636,11 +709,25 @@ def create_observable_gauge( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableGauge: with self._lock: if self._real_meter: - return self._real_meter.create_observable_gauge(name, callbacks, unit, description) - proxy = _ProxyObservableGauge(name, callbacks, unit=unit, description=description) + return self._real_meter.create_observable_gauge( + name, + callbacks, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) + proxy = _ProxyObservableGauge( + name, + callbacks, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -650,6 +737,8 @@ def create_observable_up_down_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableUpDownCounter: with self._lock: if self._real_meter: @@ -658,8 +747,15 @@ def create_observable_up_down_counter( callbacks, unit, description, + _opt_in_advisory=_opt_in_advisory, ) - proxy = _ProxyObservableUpDownCounter(name, callbacks, unit=unit, description=description) + proxy = _ProxyObservableUpDownCounter( + name, + callbacks, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) self._instruments.append(proxy) return proxy @@ -675,9 +771,17 @@ def create_counter( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Counter: """Returns a no-op Counter.""" - status = self._register_instrument(name, NoOpCounter, unit, description) + status = self._register_instrument( + name, + NoOpCounter, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -687,16 +791,29 @@ def create_counter( status, ) - return NoOpCounter(name, unit=unit, description=description) + return NoOpCounter( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def create_gauge( self, name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> Gauge: """Returns a no-op Gauge.""" - status = self._register_instrument(name, NoOpGauge, unit, description) + status = self._register_instrument( + name, + NoOpGauge, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -705,16 +822,29 @@ def create_gauge( description, status, ) - return NoOpGauge(name, unit=unit, description=description) + return NoOpGauge( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def create_up_down_counter( self, name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> UpDownCounter: """Returns a no-op UpDownCounter.""" - status = self._register_instrument(name, NoOpUpDownCounter, unit, description) + status = self._register_instrument( + name, + NoOpUpDownCounter, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -723,7 +853,12 @@ def create_up_down_counter( description, status, ) - return NoOpUpDownCounter(name, unit=unit, description=description) + return NoOpUpDownCounter( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def create_observable_counter( self, @@ -731,9 +866,17 @@ def create_observable_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableCounter: """Returns a no-op ObservableCounter.""" - status = self._register_instrument(name, NoOpObservableCounter, unit, description) + status = self._register_instrument( + name, + NoOpObservableCounter, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -747,6 +890,7 @@ def create_observable_counter( callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) def create_histogram( @@ -756,6 +900,7 @@ def create_histogram( description: str = "", *, explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + _opt_in_advisory: bool | None = None, ) -> Histogram: """Returns a no-op Histogram.""" status = self._register_instrument( @@ -763,7 +908,12 @@ def create_histogram( NoOpHistogram, unit, description, - _MetricsHistogramAdvisory(explicit_bucket_boundaries=explicit_bucket_boundaries_advisory), + _MetricsAdvisory( + opt_in=_validate_opt_in_advisory(_opt_in_advisory), + explicit_bucket_boundaries=_validate_explicit_bucket_boundaries_advisory( + explicit_bucket_boundaries_advisory + ), + ), ) if status.conflict: self._log_instrument_registration_conflict( @@ -778,6 +928,7 @@ def create_histogram( unit=unit, description=description, explicit_bucket_boundaries_advisory=explicit_bucket_boundaries_advisory, + _opt_in_advisory=_opt_in_advisory, ) def create_observable_gauge( @@ -786,9 +937,17 @@ def create_observable_gauge( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableGauge: """Returns a no-op ObservableGauge.""" - status = self._register_instrument(name, NoOpObservableGauge, unit, description) + status = self._register_instrument( + name, + NoOpObservableGauge, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -802,6 +961,7 @@ def create_observable_gauge( callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) def create_observable_up_down_counter( @@ -810,9 +970,17 @@ def create_observable_up_down_counter( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> ObservableUpDownCounter: """Returns a no-op ObservableUpDownCounter.""" - status = self._register_instrument(name, NoOpObservableUpDownCounter, unit, description) + status = self._register_instrument( + name, + NoOpObservableUpDownCounter, + unit, + description, + _MetricsAdvisory(opt_in=_validate_opt_in_advisory(_opt_in_advisory)), + ) if status.conflict: self._log_instrument_registration_conflict( name, @@ -826,6 +994,7 @@ def create_observable_up_down_counter( callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py index 5ebe05e891c..06c55eac340 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py +++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/instrument.py @@ -29,10 +29,50 @@ @dataclass(frozen=True) -class _MetricsHistogramAdvisory: +class _MetricsAdvisory: + opt_in: bool | None = None explicit_bucket_boundaries: Sequence[float] | None = None +_MetricsHistogramAdvisory = _MetricsAdvisory + + +def _validate_opt_in_advisory( + opt_in_advisory: bool | None, +) -> bool | None: + """Returns the advisory as a bool, otherwise logs a warning and returns + `None` so the advisory is ignored.""" + + if opt_in_advisory is None: + return None + + if isinstance(opt_in_advisory, bool): + return opt_in_advisory + + _logger.warning("_opt_in_advisory must be a bool") + return None + + +def _validate_explicit_bucket_boundaries_advisory( + explicit_bucket_boundaries_advisory: Sequence[float] | None, +) -> Sequence[float] | None: + """Returns the advisory if it is a sequence of numbers, otherwise logs a + warning and returns `None` so the advisory is ignored.""" + + if explicit_bucket_boundaries_advisory is None: + return None + + if isinstance(explicit_bucket_boundaries_advisory, Sequence): + try: + if all(isinstance(entry, (float, int)) for entry in explicit_bucket_boundaries_advisory): + return explicit_bucket_boundaries_advisory + except (KeyError, TypeError): + pass + + _logger.warning("explicit_bucket_boundaries_advisory must be a sequence of numbers") + return None + + @dataclass(frozen=True) class CallbackOptions: """Options for the callback @@ -59,6 +99,8 @@ def __init__( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: pass @@ -102,10 +144,13 @@ def __init__( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: self._name = name self._unit = unit self._description = description + self._opt_in_advisory = _opt_in_advisory self._real_instrument: InstrumentT | None = None def on_meter_set(self, meter: "metrics.Meter") -> None: @@ -128,8 +173,15 @@ def __init__( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit, description) + super().__init__( + name, + unit, + description, + _opt_in_advisory=_opt_in_advisory, + ) self._callbacks = callbacks @@ -147,8 +199,15 @@ def __init__( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) class Counter(Synchronous): @@ -179,8 +238,15 @@ def __init__( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def add( self, @@ -206,6 +272,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> Counter: self._name, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) @@ -241,8 +308,15 @@ def __init__( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def add( self, @@ -268,6 +342,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> UpDownCounter: self._name, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) @@ -286,12 +361,15 @@ def __init__( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: super().__init__( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) @@ -302,6 +380,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> ObservableCounter: self._callbacks, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) @@ -321,12 +400,15 @@ def __init__( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: super().__init__( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) @@ -340,6 +422,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> ObservableUpDownCou self._callbacks, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) @@ -356,6 +439,8 @@ def __init__( unit: str = "", description: str = "", explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + *, + _opt_in_advisory: bool | None = None, ) -> None: pass @@ -391,12 +476,15 @@ def __init__( unit: str = "", description: str = "", explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + *, + _opt_in_advisory: bool | None = None, ) -> None: super().__init__( name, unit=unit, description=description, explicit_bucket_boundaries_advisory=explicit_bucket_boundaries_advisory, + _opt_in_advisory=_opt_in_advisory, ) def record( @@ -415,8 +503,15 @@ def __init__( unit: str = "", description: str = "", explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) self._explicit_bucket_boundaries_advisory = explicit_bucket_boundaries_advisory def record( @@ -434,6 +529,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> Histogram: self._unit, self._description, explicit_bucket_boundaries_advisory=self._explicit_bucket_boundaries_advisory, + _opt_in_advisory=self._opt_in_advisory, ) @@ -453,12 +549,15 @@ def __init__( callbacks: Sequence[CallbackT] | None = None, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: super().__init__( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) @@ -472,6 +571,7 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> ObservableGauge: self._callbacks, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) @@ -507,8 +607,15 @@ def __init__( name: str, unit: str = "", description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> None: - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) def set( self, @@ -537,4 +644,5 @@ def _create_real_instrument(self, meter: "metrics.Meter") -> Gauge: self._name, self._unit, self._description, + _opt_in_advisory=self._opt_in_advisory, ) diff --git a/opentelemetry-api/tests/metrics/test_meter.py b/opentelemetry-api/tests/metrics/test_meter.py index b6e4146a68a..61ca36c5047 100644 --- a/opentelemetry-api/tests/metrics/test_meter.py +++ b/opentelemetry-api/tests/metrics/test_meter.py @@ -13,18 +13,37 @@ class ChildMeter(Meter): # pylint: disable=signature-differs - def create_counter(self, name, unit="", description=""): - super().create_counter(name, unit=unit, description=description) + def create_counter(self, name, unit="", description="", *, _opt_in_advisory=None): + super().create_counter( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) - def create_up_down_counter(self, name, unit="", description=""): - super().create_up_down_counter(name, unit=unit, description=description) + def create_up_down_counter(self, name, unit="", description="", *, _opt_in_advisory=None): + super().create_up_down_counter( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) - def create_observable_counter(self, name, callbacks, unit="", description=""): + def create_observable_counter( + self, + name, + callbacks, + unit="", + description="", + *, + _opt_in_advisory=None, + ): super().create_observable_counter( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) def create_histogram( @@ -34,31 +53,56 @@ def create_histogram( description="", *, explicit_bucket_boundaries_advisory=None, + _opt_in_advisory=None, ): super().create_histogram( name, unit=unit, description=description, explicit_bucket_boundaries_advisory=explicit_bucket_boundaries_advisory, + _opt_in_advisory=_opt_in_advisory, ) - def create_gauge(self, name, unit="", description=""): - super().create_gauge(name, unit=unit, description=description) + def create_gauge(self, name, unit="", description="", *, _opt_in_advisory=None): + super().create_gauge( + name, + unit=unit, + description=description, + _opt_in_advisory=_opt_in_advisory, + ) - def create_observable_gauge(self, name, callbacks, unit="", description=""): + def create_observable_gauge( + self, + name, + callbacks, + unit="", + description="", + *, + _opt_in_advisory=None, + ): super().create_observable_gauge( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) - def create_observable_up_down_counter(self, name, callbacks, unit="", description=""): + def create_observable_up_down_counter( + self, + name, + callbacks, + unit="", + description="", + *, + _opt_in_advisory=None, + ): super().create_observable_up_down_counter( name, callbacks, unit=unit, description=description, + _opt_in_advisory=_opt_in_advisory, ) @@ -99,16 +143,64 @@ def test_repeated_instrument_names_with_different_advisory(self): try: test_meter = NoOpMeter("name") + test_meter.create_counter("counter", _opt_in_advisory=True) + test_meter.create_up_down_counter("up_down_counter", _opt_in_advisory=True) + test_meter.create_observable_counter( + "observable_counter", + Mock(), + _opt_in_advisory=True, + ) test_meter.create_histogram("histogram", explicit_bucket_boundaries_advisory=[1.0]) + test_meter.create_histogram("histogram_opt_in", _opt_in_advisory=True) + test_meter.create_gauge("gauge", _opt_in_advisory=True) + test_meter.create_observable_gauge( + "observable_gauge", + Mock(), + _opt_in_advisory=True, + ) + test_meter.create_observable_up_down_counter( + "observable_up_down_counter", + Mock(), + _opt_in_advisory=True, + ) except Exception as error: # pylint: disable=broad-exception-caught self.fail(f"Unexpected exception raised {error}") + for method_name, instrument_name in [ + ("create_counter", "counter"), + ("create_up_down_counter", "up_down_counter"), + ("create_histogram", "histogram"), + ("create_histogram", "histogram_opt_in"), + ("create_gauge", "gauge"), + ]: + with self.assertLogs(level=WARNING): + getattr(test_meter, method_name)( + instrument_name, + ) + for instrument_name in [ - "histogram", + "observable_counter", + "observable_gauge", + "observable_up_down_counter", ]: with self.assertLogs(level=WARNING): getattr(test_meter, f"create_{instrument_name}")( instrument_name, + Mock(), + ) + + def test_opt_in_advisory_validation(self): + test_meter = NoOpMeter("name") + for idx, invalid_value in enumerate(("true", 123, [True])): + with self.subTest(invalid_value=invalid_value): + with self.assertLogs(level=WARNING) as log: + test_meter.create_counter( + f"counter_{idx}", + _opt_in_advisory=invalid_value, + ) + self.assertIn( + "_opt_in_advisory must be a bool", + log.output[0], ) def test_create_counter(self): diff --git a/opentelemetry-api/tests/metrics/test_meter_provider.py b/opentelemetry-api/tests/metrics/test_meter_provider.py index 38a202447fa..e6e48ebf0d6 100644 --- a/opentelemetry-api/tests/metrics/test_meter_provider.py +++ b/opentelemetry-api/tests/metrics/test_meter_provider.py @@ -230,15 +230,25 @@ def test_proxy_meter(self): real_meter_provider.get_meter.assert_called_once_with(meter_name, None, None) real_meter: Mock = real_meter_provider.get_meter() - real_meter.create_counter.assert_called_once_with(name, unit, description) - real_meter.create_up_down_counter.assert_called_once_with(name, unit, description) + real_meter.create_counter.assert_called_once_with(name, unit, description, _opt_in_advisory=None) + real_meter.create_up_down_counter.assert_called_once_with(name, unit, description, _opt_in_advisory=None) real_meter.create_histogram.assert_called_once_with( - name, unit, description, explicit_bucket_boundaries_advisory=None + name, + unit, + description, + explicit_bucket_boundaries_advisory=None, + _opt_in_advisory=None, + ) + real_meter.create_gauge.assert_called_once_with(name, unit, description, _opt_in_advisory=None) + real_meter.create_observable_counter.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=None + ) + real_meter.create_observable_up_down_counter.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=None + ) + real_meter.create_observable_gauge.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=None ) - real_meter.create_gauge.assert_called_once_with(name, unit, description) - real_meter.create_observable_counter.assert_called_once_with(name, [callback], unit, description) - real_meter.create_observable_up_down_counter.assert_called_once_with(name, [callback], unit, description) - real_meter.create_observable_gauge.assert_called_once_with(name, [callback], unit, description) # The synchronous instrument measurement methods should call through to # the real instruments @@ -299,3 +309,67 @@ def test_proxy_meter_with_real_meter(self) -> None: real_meter.create_observable_up_down_counter(), ) self.assertIs(observable_gauge, real_meter.create_observable_gauge()) + + def test_proxy_meter_forwards_opt_in_advisory(self) -> None: + meter_name = "foo" + proxy_meter: _ProxyMeter = _ProxyMeterProvider().get_meter(meter_name) + self.assertIsInstance(proxy_meter, _ProxyMeter) + name = "foo" + unit = "s" + description = "Foobar" + callback = Mock() + + proxy_meter.create_counter(name, unit=unit, description=description, _opt_in_advisory=True) + proxy_meter.create_up_down_counter(name, unit=unit, description=description, _opt_in_advisory=True) + proxy_meter.create_histogram( + name, + unit=unit, + description=description, + explicit_bucket_boundaries_advisory=[1.0, 2.0], + _opt_in_advisory=True, + ) + proxy_meter.create_gauge(name, unit=unit, description=description, _opt_in_advisory=True) + proxy_meter.create_observable_counter( + name, + callbacks=[callback], + unit=unit, + description=description, + _opt_in_advisory=True, + ) + proxy_meter.create_observable_up_down_counter( + name, + callbacks=[callback], + unit=unit, + description=description, + _opt_in_advisory=True, + ) + proxy_meter.create_observable_gauge( + name, + callbacks=[callback], + unit=unit, + description=description, + _opt_in_advisory=True, + ) + + real_meter_provider = Mock() + proxy_meter.on_set_meter_provider(real_meter_provider) + real_meter: Mock = real_meter_provider.get_meter() + real_meter.create_counter.assert_called_once_with(name, unit, description, _opt_in_advisory=True) + real_meter.create_up_down_counter.assert_called_once_with(name, unit, description, _opt_in_advisory=True) + real_meter.create_histogram.assert_called_once_with( + name, + unit, + description, + explicit_bucket_boundaries_advisory=[1.0, 2.0], + _opt_in_advisory=True, + ) + real_meter.create_gauge.assert_called_once_with(name, unit, description, _opt_in_advisory=True) + real_meter.create_observable_counter.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=True + ) + real_meter.create_observable_up_down_counter.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=True + ) + real_meter.create_observable_gauge.assert_called_once_with( + name, [callback], unit, description, _opt_in_advisory=True + ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py index 2a43dc299c7..08ff59c07a9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py @@ -25,6 +25,11 @@ ) from opentelemetry.metrics import UpDownCounter as APIUpDownCounter from opentelemetry.metrics import _Gauge as APIGauge +from opentelemetry.metrics._internal.instrument import ( + _MetricsAdvisory, + _validate_explicit_bucket_boundaries_advisory, + _validate_opt_in_advisory, +) from opentelemetry.sdk.environment_variables import ( OTEL_METRICS_EXEMPLAR_FILTER, OTEL_SDK_DISABLED, @@ -116,9 +121,18 @@ def _is_enabled(self) -> bool: def _set_meter_config(self, meter_config: _MeterConfig) -> None: self._meter_config.update(meter_config) - def create_counter(self, name, unit="", description="") -> APICounter: + def create_counter( + self, + name: str, + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, + ) -> APICounter: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _Counter, unit, description) + status = self._register_instrument(name, _Counter, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _Counter( name, @@ -127,6 +141,7 @@ def create_counter(self, name, unit="", description="") -> APICounter: unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -143,9 +158,18 @@ def create_counter(self, name, unit="", description="") -> APICounter: ) return instrument - def create_up_down_counter(self, name, unit="", description="") -> APIUpDownCounter: + def create_up_down_counter( + self, + name: str, + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, + ) -> APIUpDownCounter: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _UpDownCounter, unit, description) + status = self._register_instrument(name, _UpDownCounter, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _UpDownCounter( name, @@ -154,6 +178,7 @@ def create_up_down_counter(self, name, unit="", description="") -> APIUpDownCoun unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -172,13 +197,17 @@ def create_up_down_counter(self, name, unit="", description="") -> APIUpDownCoun def create_observable_counter( self, - name, + name: str, callbacks=None, - unit="", - description="", + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> APIObservableCounter: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _ObservableCounter, unit, description) + status = self._register_instrument(name, _ObservableCounter, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _ObservableCounter( name, @@ -188,6 +217,7 @@ def create_observable_counter( unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -214,22 +244,16 @@ def create_histogram( description: str = "", *, explicit_bucket_boundaries_advisory: Sequence[float] | None = None, + _opt_in_advisory: bool | None = None, ) -> APIHistogram: - if explicit_bucket_boundaries_advisory is not None: - invalid_advisory = False - if isinstance(explicit_bucket_boundaries_advisory, Sequence): - try: - invalid_advisory = not ( - all(isinstance(e, (float, int)) for e in explicit_bucket_boundaries_advisory) - ) - except (KeyError, TypeError): - invalid_advisory = True - else: - invalid_advisory = True - - if invalid_advisory: - explicit_bucket_boundaries_advisory = None - _logger.warning("explicit_bucket_boundaries_advisory must be a sequence of numbers") + explicit_bucket_boundaries_advisory = _validate_explicit_bucket_boundaries_advisory( + explicit_bucket_boundaries_advisory + ) + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory( + opt_in=_opt_in_advisory, + explicit_bucket_boundaries=explicit_bucket_boundaries_advisory, + ) with self._instrument_registration_lock: status = self._register_instrument( @@ -237,7 +261,7 @@ def create_histogram( _Histogram, unit, description, - explicit_bucket_boundaries_advisory, + advisory, ) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _Histogram( @@ -248,6 +272,7 @@ def create_histogram( description, explicit_bucket_boundaries_advisory, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -264,9 +289,18 @@ def create_histogram( ) return instrument - def create_gauge(self, name, unit="", description="") -> APIGauge: + def create_gauge( + self, + name: str, + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, + ) -> APIGauge: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _Gauge, unit, description) + status = self._register_instrument(name, _Gauge, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _Gauge( name, @@ -275,6 +309,7 @@ def create_gauge(self, name, unit="", description="") -> APIGauge: unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -291,9 +326,19 @@ def create_gauge(self, name, unit="", description="") -> APIGauge: ) return instrument - def create_observable_gauge(self, name, callbacks=None, unit="", description="") -> APIObservableGauge: + def create_observable_gauge( + self, + name: str, + callbacks=None, + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, + ) -> APIObservableGauge: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _ObservableGauge, unit, description) + status = self._register_instrument(name, _ObservableGauge, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _ObservableGauge( name, @@ -303,6 +348,7 @@ def create_observable_gauge(self, name, callbacks=None, unit="", description="") unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] @@ -323,10 +369,18 @@ def create_observable_gauge(self, name, callbacks=None, unit="", description="") return instrument def create_observable_up_down_counter( - self, name, callbacks=None, unit="", description="" + self, + name: str, + callbacks=None, + unit: str = "", + description: str = "", + *, + _opt_in_advisory: bool | None = None, ) -> APIObservableUpDownCounter: + _opt_in_advisory = _validate_opt_in_advisory(_opt_in_advisory) + advisory = _MetricsAdvisory(opt_in=_opt_in_advisory) with self._instrument_registration_lock: - status = self._register_instrument(name, _ObservableUpDownCounter, unit, description) + status = self._register_instrument(name, _ObservableUpDownCounter, unit, description, advisory) if not status.already_registered: self._instrument_id_instrument[status.instrument_id] = _ObservableUpDownCounter( name, @@ -336,6 +390,7 @@ def create_observable_up_down_counter( unit, description, _meter_config=self._meter_config, + _advisory=advisory, ) instrument = self._instrument_id_instrument[status.instrument_id] diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py index e9ac5eecce5..d304f676580 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py @@ -14,7 +14,9 @@ Aggregation, AggregationTemporality, DefaultAggregation, + DropAggregation, _Aggregation, + _DropAggregation, _SumAggregation, ) from opentelemetry.sdk.metrics._internal.instrument import _Instrument @@ -92,23 +94,26 @@ def __init__( self._instrument_class_aggregation = instrument_class_aggregation self._name = self._view._name or self._instrument.name self._description = self._view._description or self._instrument.description - if not isinstance(self._view._aggregation, DefaultAggregation): - self._aggregation = self._view._aggregation._create_aggregation( - self._instrument, - None, - self._view._exemplar_reservoir_factory, - 0, - ) + advisory = getattr(self._instrument, "_advisory", None) + opt_in = getattr(advisory, "opt_in", None) is True + self._aggregation_factory: Aggregation + if self._view._enabled is False or (opt_in and self._view._enabled is not True): + self._aggregation_factory = DropAggregation() + elif not isinstance(self._view._aggregation, DefaultAggregation): + self._aggregation_factory = self._view._aggregation else: - self._aggregation = self._instrument_class_aggregation[self._instrument.__class__]._create_aggregation( - self._instrument, - None, - self._view._exemplar_reservoir_factory, - 0, - ) + self._aggregation_factory = self._instrument_class_aggregation[self._instrument.__class__] + self._aggregation = self._aggregation_factory._create_aggregation( + self._instrument, + None, + self._view._exemplar_reservoir_factory, + 0, + ) def conflicts(self, other: "_ViewInstrumentMatch") -> bool: # pylint: disable=protected-access + if isinstance(self._aggregation, _DropAggregation) or isinstance(other._aggregation, _DropAggregation): + return False result = ( self._name == other._name @@ -135,6 +140,9 @@ def conflicts(self, other: "_ViewInstrumentMatch") -> bool: # pylint: disable=protected-access def consume_measurement(self, measurement: Measurement, should_sample_exemplar: bool = True) -> None: + if isinstance(self._aggregation, _DropAggregation): + return + attributes = {} if measurement.attributes: # Make a shallow copy since the user can mutate the dict after the fact. @@ -177,20 +185,12 @@ def _get_or_create_aggregation(self, aggr_key: _HashedAttributes, attributes: At start_time_unix_nano = self._previous_collection_start_nanos or time_ns() # pylint: disable=protected-access - if not isinstance(self._view._aggregation, DefaultAggregation): - aggregation = self._view._aggregation._create_aggregation( - self._instrument, - attributes, - self._view._exemplar_reservoir_factory, - start_time_unix_nano, - ) - else: - aggregation = self._instrument_class_aggregation[self._instrument.__class__]._create_aggregation( - self._instrument, - attributes, - self._view._exemplar_reservoir_factory, - start_time_unix_nano, - ) + aggregation = self._aggregation_factory._create_aggregation( + self._instrument, + attributes, + self._view._exemplar_reservoir_factory, + start_time_unix_nano, + ) self._attributes_aggregation[aggr_key] = aggregation return aggregation @@ -199,6 +199,9 @@ def collect( collection_aggregation_temporality: AggregationTemporality, collection_start_nanos: int, ) -> Sequence[DataPointT] | None: + if isinstance(self._aggregation, _DropAggregation): + return None + data_points: list[DataPointT] = [] with self._lock: aggregations = self._attributes_aggregation diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py index 872715b8955..df3dfde825a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py @@ -29,6 +29,7 @@ from opentelemetry.metrics import _Gauge as APIGauge from opentelemetry.metrics._internal.instrument import ( CallbackOptions, + _MetricsAdvisory, _MetricsHistogramAdvisory, ) from opentelemetry.sdk.metrics._internal.measurement import Measurement @@ -69,6 +70,7 @@ def __init__( description: str = "", *, _meter_config: _ProxyMeterConfig | None = None, + _advisory: _MetricsAdvisory | None = None, ): # pylint: disable=no-member result = self._check_name_unit_description(name, unit, description) @@ -91,7 +93,13 @@ def __init__( self.instrumentation_scope = instrumentation_scope self._measurement_consumer = measurement_consumer self._meter_config = _meter_config - super().__init__(name, unit=unit, description=description) + self._advisory = _advisory or _MetricsAdvisory() + super().__init__( + name, + unit=unit, + description=description, + _opt_in_advisory=self._advisory.opt_in, + ) def _is_enabled(self) -> bool: return self._meter_config is None or self._meter_config.is_enabled @@ -108,6 +116,7 @@ def __init__( description: str = "", *, _meter_config: _ProxyMeterConfig | None = None, + _advisory: _MetricsAdvisory | None = None, ): # pylint: disable=no-member result = self._check_name_unit_description(name, unit, description) @@ -130,7 +139,14 @@ def __init__( self.instrumentation_scope = instrumentation_scope self._measurement_consumer = measurement_consumer self._meter_config = _meter_config - super().__init__(name, callbacks, unit=unit, description=description) + self._advisory = _advisory or _MetricsAdvisory() + super().__init__( + name, + callbacks, + unit=unit, + description=description, + _opt_in_advisory=self._advisory.opt_in, + ) self._callbacks: list[CallbackT] = [] @@ -278,7 +294,10 @@ def __init__( explicit_bucket_boundaries_advisory: Sequence[float] | None = None, *, _meter_config: _ProxyMeterConfig | None = None, + _advisory: _MetricsAdvisory | None = None, ): + if _advisory is None: + _advisory = _MetricsHistogramAdvisory(explicit_bucket_boundaries=explicit_bucket_boundaries_advisory) super().__init__( name, unit=unit, @@ -286,8 +305,8 @@ def __init__( instrumentation_scope=instrumentation_scope, measurement_consumer=measurement_consumer, _meter_config=_meter_config, + _advisory=_advisory, ) - self._advisory = _MetricsHistogramAdvisory(explicit_bucket_boundaries=explicit_bucket_boundaries_advisory) def __new__(cls, *args, **kwargs): if cls is Histogram: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py index ea84bf25a0d..82d1fc87942 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py @@ -268,12 +268,16 @@ def _check_view_instrument_compatibility(view: View, instrument: _Instrument) -> result = True # pylint: disable=protected-access - if isinstance(instrument, Asynchronous) and isinstance( - view._aggregation, - ( - ExplicitBucketHistogramAggregation, - ExponentialBucketHistogramAggregation, - ), + if ( + view._enabled is not False + and isinstance(instrument, Asynchronous) + and isinstance( + view._aggregation, + ( + ExplicitBucketHistogramAggregation, + ExponentialBucketHistogramAggregation, + ), + ) ): _logger.warning( "View %s and instrument %s will produce semantic errors when matched, the view has not been applied.", diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py index d989d2c3097..41abe64c73b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/view.py @@ -97,6 +97,13 @@ class View: by the attribute ``otel.metric.overflow=true``. If `None` the cardinality limit of the metric reader is used, which defaults to 2000. Must be a positive integer. + enabled: This is a metric stream customizing attribute: whether the + metric stream is enabled. If `False`, the metric stream will use + `DropAggregation` regardless of `aggregation`. If `True`, the + metric stream will be enabled (including for instruments with + `_opt_in_advisory=True`). If `None`, instruments with + `_opt_in_advisory=True` will use `DropAggregation` and all other + instruments will be enabled. Must be a bool. This class is not intended to be subclassed by the user. """ @@ -117,6 +124,7 @@ def __init__( exemplar_reservoir_factory: Callable[[type[_Aggregation]], ExemplarReservoirBuilder] | None = None, instrument_unit: str | None = None, aggregation_cardinality_limit: int | None = None, + enabled: bool | None = None, ): if aggregation_cardinality_limit is not None and ( isinstance(aggregation_cardinality_limit, bool) @@ -127,6 +135,9 @@ def __init__( f"aggregation_cardinality_limit must be a positive integer, got {aggregation_cardinality_limit!r}" ) + if enabled is not None and not isinstance(enabled, bool): + raise ValueError(f"enabled must be a bool, got {enabled!r}") + if ( instrument_type is instrument_name @@ -165,6 +176,7 @@ def __init__( self._aggregation = aggregation or self._default_aggregation self._exemplar_reservoir_factory = exemplar_reservoir_factory or _default_reservoir_factory self._aggregation_cardinality_limit = aggregation_cardinality_limit + self._enabled = enabled # pylint: disable=too-many-return-statements # pylint: disable=too-many-branches diff --git a/opentelemetry-sdk/tests/metrics/integration_test/test_opt_in_and_view_enabled.py b/opentelemetry-sdk/tests/metrics/integration_test/test_opt_in_and_view_enabled.py new file mode 100644 index 00000000000..9e148d809bf --- /dev/null +++ b/opentelemetry-sdk/tests/metrics/integration_test/test_opt_in_and_view_enabled.py @@ -0,0 +1,245 @@ +# Copyright The OpenTelemetry Authors +# SPDX-License-Identifier: Apache-2.0 + +# pylint: disable=protected-access + +from logging import WARNING +from unittest import TestCase + +from opentelemetry.metrics import Observation +from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics.export import InMemoryMetricReader +from opentelemetry.sdk.metrics.view import ( + ExplicitBucketHistogramAggregation, + SumAggregation, + View, +) + + +def _create_all_instruments(meter, *, opt_in_advisory=None): + kwargs = {} + if opt_in_advisory is not None: + kwargs["_opt_in_advisory"] = opt_in_advisory + + counter = meter.create_counter("test_counter", **kwargs) + up_down_counter = meter.create_up_down_counter("test_up_down_counter", **kwargs) + histogram = meter.create_histogram("test_histogram", **kwargs) + gauge = meter.create_gauge("test_gauge", **kwargs) + observable_counter = meter.create_observable_counter( + "test_observable_counter", + callbacks=[lambda options: [Observation(1)]], + **kwargs, + ) + observable_up_down_counter = meter.create_observable_up_down_counter( + "test_observable_up_down_counter", + callbacks=[lambda options: [Observation(1)]], + **kwargs, + ) + observable_gauge = meter.create_observable_gauge( + "test_observable_gauge", + callbacks=[lambda options: [Observation(1)]], + **kwargs, + ) + + counter.add(1) + up_down_counter.add(1) + histogram.record(1) + gauge.set(1) + + return ( + counter, + up_down_counter, + histogram, + gauge, + observable_counter, + observable_up_down_counter, + observable_gauge, + ) + + +class TestOptInAdvisoryAndViewEnabled(TestCase): + def test_opt_in_advisory_true_disabled_by_default(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + meter = meter_provider.get_meter("testmeter") + + _create_all_instruments(meter, opt_in_advisory=True) + + self.assertIsNone(reader.get_metrics_data()) + + def test_opt_in_advisory_false_or_none_enabled_by_default(self): + for opt_in_advisory in (None, False): + with self.subTest(opt_in_advisory=opt_in_advisory): + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + meter = meter_provider.get_meter("testmeter") + + _create_all_instruments(meter, opt_in_advisory=opt_in_advisory) + + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + metrics = metrics_data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual(len(metrics), 7) + + def test_opt_in_advisory_true_matching_view_without_enabled_remains_disabled( + self, + ): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[ + View( + instrument_name="test_counter", + name="renamed_counter", + description="custom description", + attribute_keys={"k"}, + aggregation=SumAggregation(), + ), + View( + instrument_name="test_histogram", + aggregation=ExplicitBucketHistogramAggregation(boundaries=[1.0, 5.0]), + ), + View(instrument_name="test_*"), + ], + ) + meter = meter_provider.get_meter("testmeter") + + _create_all_instruments(meter, opt_in_advisory=True) + + self.assertIsNone(reader.get_metrics_data()) + + def test_opt_in_advisory_true_enabled_by_view(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[View(instrument_name="test_*", enabled=True)], + ) + meter = meter_provider.get_meter("testmeter") + + _create_all_instruments(meter, opt_in_advisory=True) + + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + metrics = metrics_data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual( + {metric.name for metric in metrics}, + { + "test_counter", + "test_up_down_counter", + "test_histogram", + "test_gauge", + "test_observable_counter", + "test_observable_up_down_counter", + "test_observable_gauge", + }, + ) + + def test_opt_in_histogram_enabled_by_view_preserves_bucket_advisory(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[View(instrument_name="testhistogram", enabled=True)], + ) + meter = meter_provider.get_meter("testmeter") + histogram = meter.create_histogram( + "testhistogram", + explicit_bucket_boundaries_advisory=[1.0, 2.0, 3.0], + _opt_in_advisory=True, + ) + histogram.record(2, {"label": "value"}) + + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + metric = metrics_data.resource_metrics[0].scope_metrics[0].metrics[0] + self.assertEqual(metric.name, "testhistogram") + self.assertEqual(metric.data.data_points[0].explicit_bounds, (1.0, 2.0, 3.0)) + + def test_view_enabled_false_disables_default_and_opt_in_instruments(self): + for opt_in_advisory in (None, False, True): + with self.subTest(opt_in_advisory=opt_in_advisory): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[ + View( + instrument_name="test_counter", + aggregation=SumAggregation(), + enabled=False, + ), + View(instrument_name="test_*", enabled=False), + ], + ) + meter = meter_provider.get_meter("testmeter") + + _create_all_instruments(meter, opt_in_advisory=opt_in_advisory) + + self.assertIsNone(reader.get_metrics_data()) + + def test_disable_all_views_and_reenable_selected_instrument(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[ + View(instrument_name="*", enabled=False), + View(instrument_name="enabled_counter", enabled=True), + ], + ) + meter = meter_provider.get_meter("testmeter") + disabled_counter = meter.create_counter("disabled_counter") + enabled_counter = meter.create_counter("enabled_counter", _opt_in_advisory=True) + disabled_counter.add(5) + enabled_counter.add(7) + + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + metrics = metrics_data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual(len(metrics), 1) + self.assertEqual(metrics[0].name, "enabled_counter") + self.assertEqual(metrics[0].data.data_points[0].value, 7) + + def test_invalid_opt_in_advisory_logs_warning_and_defaults_to_enabled(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider(metric_readers=[reader]) + meter = meter_provider.get_meter("testmeter") + + with self.assertLogs(level=WARNING) as log: + counter = meter.create_counter( + "test_counter", + _opt_in_advisory="invalid", # type: ignore[arg-type] + ) + self.assertIn("_opt_in_advisory must be a bool", log.output[0]) + + counter.add(3) + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + metrics = metrics_data.resource_metrics[0].scope_metrics[0].metrics + self.assertEqual(len(metrics), 1) + self.assertEqual(metrics[0].name, "test_counter") + self.assertEqual(metrics[0].data.data_points[0].value, 3) + + def test_disabled_streams_do_not_retain_per_attribute_aggregations(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[View(instrument_name="view_disabled_counter", enabled=False)], + ) + meter = meter_provider.get_meter("testmeter") + opt_in_counter = meter.create_counter("opt_in_counter", _opt_in_advisory=True) + view_disabled_counter = meter.create_counter("view_disabled_counter") + + for idx in range(10): + attrs = {"key": f"value_{idx}"} + opt_in_counter.add(1, attrs) + view_disabled_counter.add(1, attrs) + + self.assertIsNone(reader.get_metrics_data()) + + reader_storage = meter_provider._measurement_consumer._reader_storages[reader] + matches_by_name = { + instrument.name: matches + for instrument, matches in reader_storage._instrument_view_instrument_matches.items() + } + self.assertEqual(len(matches_by_name["opt_in_counter"]), 1) + self.assertEqual(matches_by_name["opt_in_counter"][0]._attributes_aggregation, {}) + self.assertEqual(len(matches_by_name["view_disabled_counter"]), 1) + self.assertEqual(matches_by_name["view_disabled_counter"][0]._attributes_aggregation, {}) diff --git a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py index 581e066216b..b08620d7c19 100644 --- a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py +++ b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py @@ -320,6 +320,42 @@ def test_conflicting_view_configuration(self): _DEFAULT_VIEW, ) + def test_conflicting_view_configuration_disabled_view_allowed(self): + for aggregation in ( + ExplicitBucketHistogramAggregation(), + ExponentialBucketHistogramAggregation(), + ): + observable_counter = _ObservableCounter( + "observable_counter", + Mock(), + [Mock()], + unit="unit", + description="description", + ) + disabled_view = View( + instrument_name="observable_counter", + aggregation=aggregation, + enabled=False, + ) + metric_reader_storage = MetricReaderStorage( + SdkConfiguration( + exemplar_filter=Mock(), + resource=Mock(), + views=(disabled_view,), + ), + MagicMock(**{"__getitem__.return_value": AggregationTemporality.CUMULATIVE}), + MagicMock(**{"__getitem__.return_value": DefaultAggregation()}), + ) + + with self.assertNoLogs("opentelemetry.sdk.metrics._internal.metric_reader_storage", level=WARNING): + metric_reader_storage.consume_measurement(Measurement(1, time_ns(), observable_counter, Context())) + + self.assertIs( + metric_reader_storage._instrument_view_instrument_matches[observable_counter][0]._view, + disabled_view, + ) + self.assertIsNone(metric_reader_storage.collect()) + def test_conflicting_view_configuration_exponential_histogram(self): observable_counter = _ObservableCounter( "observable_counter", @@ -753,3 +789,35 @@ def test_view_instrument_match_conflict_8(self): "will cause conflicting metrics", log.records[0].message, ) + + def test_view_instrument_match_no_conflict_when_disabled(self): + observable_counter_0 = _ObservableCounter( + "observable_counter_0", + Mock(), + [Mock()], + unit="unit", + description="description", + ) + observable_counter_1 = _ObservableCounter( + "observable_counter_1", + Mock(), + [Mock()], + unit="unit", + description="description", + ) + metric_reader_storage = MetricReaderStorage( + SdkConfiguration( + exemplar_filter=Mock(), + resource=Mock(), + views=( + View(instrument_name="observable_counter_0", name="foo", enabled=False), + View(instrument_name="observable_counter_1", name="foo", enabled=False), + ), + ), + MagicMock(**{"__getitem__.return_value": AggregationTemporality.CUMULATIVE}), + MagicMock(**{"__getitem__.return_value": DefaultAggregation()}), + ) + + with self.assertNoLogs("opentelemetry.sdk.metrics._internal.metric_reader_storage", level=WARNING): + metric_reader_storage.consume_measurement(Measurement(1, time_ns(), observable_counter_0, Context())) + metric_reader_storage.consume_measurement(Measurement(1, time_ns(), observable_counter_1, Context())) diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 47db7bc6489..2a7363036e9 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -637,13 +637,82 @@ def test_repeated_instrument_names(self): def test_repeated_instrument_names_with_different_advisory(self): with self.assertNotRaises(Exception): + self.meter.create_counter("counter", _opt_in_advisory=True) + self.meter.create_up_down_counter("up_down_counter", _opt_in_advisory=True) + self.meter.create_observable_counter("observable_counter", callbacks=[Mock()], _opt_in_advisory=True) self.meter.create_histogram("histogram", explicit_bucket_boundaries_advisory=[1.0]) + self.meter.create_histogram("histogram_opt_in", _opt_in_advisory=True) + self.meter.create_gauge("gauge", _opt_in_advisory=True) + self.meter.create_observable_gauge("observable_gauge", callbacks=[Mock()], _opt_in_advisory=True) + self.meter.create_observable_up_down_counter( + "observable_up_down_counter", + callbacks=[Mock()], + _opt_in_advisory=True, + ) + + for method_name, instrument_name in [ + ("create_counter", "counter"), + ("create_up_down_counter", "up_down_counter"), + ("create_histogram", "histogram"), + ("create_histogram", "histogram_opt_in"), + ("create_gauge", "gauge"), + ]: + with self.assertLogs(level=WARNING): + getattr(self.meter, method_name)(instrument_name) for instrument_name in [ - "histogram", + "observable_counter", + "observable_gauge", + "observable_up_down_counter", ]: with self.assertLogs(level=WARNING): - getattr(self.meter, f"create_{instrument_name}")(instrument_name) + getattr(self.meter, f"create_{instrument_name}")(instrument_name, callbacks=[Mock()]) + + def test_create_instruments_with_opt_in_advisory(self): + counter = self.meter.create_counter("counter", _opt_in_advisory=True) + self.assertTrue(counter._advisory.opt_in) + + up_down_counter = self.meter.create_up_down_counter("up_down_counter", _opt_in_advisory=False) + self.assertFalse(up_down_counter._advisory.opt_in) + + observable_counter = self.meter.create_observable_counter( + "observable_counter", callbacks=[Mock()], _opt_in_advisory=True + ) + self.assertTrue(observable_counter._advisory.opt_in) + + histogram = self.meter.create_histogram( + "histogram", + explicit_bucket_boundaries_advisory=[1.0, 2.0], + _opt_in_advisory=True, + ) + self.assertTrue(histogram._advisory.opt_in) + self.assertEqual(histogram._advisory.explicit_bucket_boundaries, [1.0, 2.0]) + + gauge = self.meter.create_gauge("gauge", _opt_in_advisory=True) + self.assertTrue(gauge._advisory.opt_in) + + observable_gauge = self.meter.create_observable_gauge( + "observable_gauge", callbacks=[Mock()], _opt_in_advisory=True + ) + self.assertTrue(observable_gauge._advisory.opt_in) + + observable_up_down_counter = self.meter.create_observable_up_down_counter( + "observable_up_down_counter", + callbacks=[Mock()], + _opt_in_advisory=True, + ) + self.assertTrue(observable_up_down_counter._advisory.opt_in) + + def test_create_instruments_opt_in_advisory_validation(self): + for idx, invalid_advisory in enumerate(("hello", 123, [True])): + with self.subTest(invalid_advisory=invalid_advisory): + with self.assertLogs(level=WARNING) as log: + counter = self.meter.create_counter( + f"counter_{idx}", + _opt_in_advisory=invalid_advisory, + ) + self.assertIsNone(counter._advisory.opt_in) + self.assertIn("_opt_in_advisory must be a bool", log.output[0]) def test_create_counter(self): counter = self.meter.create_counter("name", unit="unit", description="description") diff --git a/opentelemetry-sdk/tests/metrics/test_view.py b/opentelemetry-sdk/tests/metrics/test_view.py index 8d9a7f8bbef..db519c260c6 100644 --- a/opentelemetry-sdk/tests/metrics/test_view.py +++ b/opentelemetry-sdk/tests/metrics/test_view.py @@ -209,3 +209,19 @@ def test_view_name_with_concrete_instrument_name(self): self.assertEqual(view_with_counter._name, "custom_name") self.assertEqual(view_with_counter._instrument_name, "my_counter") self.assertIs(view_with_counter._instrument_type, Counter) + + def test_view_enabled(self): + default_view = View(instrument_name="my_counter") + self.assertIsNone(default_view._enabled) + + enabled_view = View(instrument_name="my_counter", enabled=True) + self.assertTrue(enabled_view._enabled) + + disabled_view = View(instrument_name="my_counter", enabled=False) + self.assertFalse(disabled_view._enabled) + + def test_invalid_view_enabled(self): + for enabled in (0, 1, "false", "true", 1.5): + with self.subTest(enabled=enabled): + with self.assertRaises(ValueError): + View(instrument_name="my_counter", enabled=enabled) diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index 8985a339d34..ba5d638d62c 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -10,6 +10,7 @@ from unittest.mock import MagicMock, Mock, patch from opentelemetry.context import Context +from opentelemetry.metrics._internal.instrument import _MetricsAdvisory from opentelemetry.sdk.metrics._internal._view_instrument_match import ( _hash_attributes, _ViewInstrumentMatch, @@ -171,8 +172,8 @@ def test_consume_measurement(self): {_hash_attributes({}): self.mock_created_aggregation}, ) - # Test that a drop aggregation is handled in the same way as any - # other aggregation. + # Test that a drop aggregation short-circuits without creating + # per-attribute aggregations. drop_aggregation = DropAggregation() view_instrument_match = _ViewInstrumentMatch( @@ -185,6 +186,7 @@ def test_consume_measurement(self): instrument=instrument1, instrument_class_aggregation=MagicMock(**{"__getitem__.return_value": DefaultAggregation()}), ) + self.assertIsInstance(view_instrument_match._aggregation, _DropAggregation) view_instrument_match.consume_measurement( Measurement( value=0, @@ -194,10 +196,54 @@ def test_consume_measurement(self): attributes=None, ) ) - self.assertIsInstance( - view_instrument_match._attributes_aggregation[_hash_attributes({})], - _DropAggregation, + self.assertEqual(view_instrument_match._attributes_aggregation, {}) + self.assertIsNone(view_instrument_match.collect(AggregationTemporality.CUMULATIVE, 0)) + + def test_disabled_stream_uses_drop_aggregation(self): + cases = ( + ( + "view_enabled_false", + View(instrument_name="instrument", enabled=False), + _Counter( + name="instrument", + instrumentation_scope=Mock(), + measurement_consumer=Mock(), + ), + ), + ( + "opt_in_default_disabled", + View(instrument_name="instrument"), + _Counter( + name="instrument", + instrumentation_scope=Mock(), + measurement_consumer=Mock(), + _advisory=_MetricsAdvisory(opt_in=True), + ), + ), ) + for case_name, view, instrument in cases: + with self.subTest(case=case_name): + match = _ViewInstrumentMatch( + view=view, + instrument=instrument, + instrument_class_aggregation={_Counter: DefaultAggregation()}, + ) + self.assertIsInstance(match._aggregation, _DropAggregation) + + for idx in range(5): + match.consume_measurement( + Measurement( + value=1, + time_unix_nano=time_ns(), + instrument=instrument, + context=Context(), + attributes={"key": f"val_{idx}"}, + ) + ) + + self.assertEqual(match._attributes_aggregation, {}) + self.assertIsNone(match.collect(AggregationTemporality.CUMULATIVE, 0)) + self.assertIsNone(match.collect(AggregationTemporality.DELTA, 0)) def test_collect(self): instrument1 = _Counter(