From 082d55355500e9a6e46a801f92e5f5b9e2d054b0 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Fri, 24 Sep 2021 18:17:11 +0200 Subject: [PATCH 01/48] Adds metrics API (#1887) * Adds metric prototype Fixes #1835 * Fix docs * Add API metrics doc * Add missing docs * Add files * Adding docs * Refactor to _initialize * Refactor initialize * Add more documentation * Add exporter test * Add process * Fix tests * Try to add aggregator_class argument Tests are failing here * Fix instrument parent classes * Test default aggregator * WIP * Add prototype test * Tests passing again * Use right counters * All tests passing * Rearrange instrument storage * Fix tests * Add HTTP server test * WIP * WIP * Add prototype * WIP * Fail the test * WIP * WIP * WIP * WIP * Add views * Discard instruments via views * Fix tests * WIP * WIP * Fix lint * WIP * Fix test * Fix lint * Fix method * Fix lint * Mypy workaround * Skip if 3.6 * Fix lint * Add reason * Fix 3.6 * Fix run * Fix lint * Remove SDK metrics * Remove SDK docs * Remove metrics * Remove assertnotraises mixin * Revert sdk docs conf * Remove SDK env var changes * Fix unit checking * Define positional-only arguments * Add Metrics plans * Add API tests * WIP * WIP test * WIP * WIP * WIP * Set provider test passing * Use a fixture * Add test for get_provider * Rename tests * WIP * WIP * WIP * WIP * Remove non specific requirement * Add meter requirements * Put all meter provider tests in one file * Add meter tests * Make attributes be passed as a dictionary * Make some interfaces private * Log an error instead * Remove ASCII flag * Add CHANGELOG entry * Add instrument tests * All tests passing * Add test * Add name tests * Add unit tests * Add description tests * Add counter tests * Add more tests * Add Histogram tests * Add observable gauge tests * Add updowncounter tests * Add observableupdowncounter tests * Fix lint * Fix docs * Fix lint * Ignore mypy * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Remove useless pylint skip * Add locks to meter and meterprovider * Add lock to instruments * Fix fixmes * Fix lint * Add documentation placeholder * Remove blank line as requested. * Do not override Rlock * Remove unecessary super calls * Add missing super calls * Remove plan files * Add missing parameters * Rename observe to callback * Fix lint * Rename to secure_instrument_name * Remove locks * Fix lint * Remove args and kwargs * Remove implementation that gives meters access to meter provider * Allow creating async instruments with either a callback function or generator * add additional test with callback form of observable counter * add a test/example that reads measurements from proc stat * implement cpu time integration test with generator too Co-authored-by: Aaron Abbott --- .../src/opentelemetry/metrics/__init__.py | 384 ++++++++++++++++++ .../src/opentelemetry/metrics/instrument.py | 234 +++++++++++ .../src/opentelemetry/metrics/measurement.py | 39 ++ 3 files changed, 657 insertions(+) create mode 100644 opentelemetry-api/src/opentelemetry/metrics/__init__.py create mode 100644 opentelemetry-api/src/opentelemetry/metrics/instrument.py create mode 100644 opentelemetry-api/src/opentelemetry/metrics/measurement.py diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py new file mode 100644 index 00000000000..83d210e063b --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -0,0 +1,384 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# pylint: disable=too-many-ancestors +# type: ignore + +# FIXME enhance the documentation of this module +""" +This module provides abstract and concrete (but noop) classes that can be used +to generate metrics. +""" + + +from abc import ABC, abstractmethod +from logging import getLogger +from os import environ +from typing import Optional, cast + +from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER +from opentelemetry.metrics.instrument import ( + Counter, + DefaultCounter, + DefaultHistogram, + DefaultObservableCounter, + DefaultObservableGauge, + DefaultObservableUpDownCounter, + DefaultUpDownCounter, + Histogram, + ObservableCounter, + ObservableGauge, + ObservableUpDownCounter, + UpDownCounter, +) +from opentelemetry.util._providers import _load_provider + +_logger = getLogger(__name__) + + +class MeterProvider(ABC): + @abstractmethod + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + if name is None or name == "": + _logger.warning("Invalid name: %s", name) + + +class _DefaultMeterProvider(MeterProvider): + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + super().get_meter(name, version=version, schema_url=schema_url) + return _DefaultMeter(name, version=version, schema_url=schema_url) + + +class ProxyMeterProvider(MeterProvider): + def get_meter( + self, + name, + version=None, + schema_url=None, + ) -> "Meter": + if _METER_PROVIDER: + return _METER_PROVIDER.get_meter( + name, version=version, schema_url=schema_url + ) + return ProxyMeter(name, version=version, schema_url=schema_url) + + +class Meter(ABC): + def __init__(self, name, version=None, schema_url=None): + super().__init__() + self._name = name + self._version = version + self._schema_url = schema_url + self._instrument_names = set() + + @property + def name(self): + return self._name + + @property + def version(self): + return self._version + + @property + def schema_url(self): + return self._schema_url + + def _secure_instrument_name(self, name): + name = name.lower() + + if name in self._instrument_names: + _logger.error("Instrument name %s has been used already", name) + + else: + self._instrument_names.add(name) + + @abstractmethod + def create_counter(self, name, unit="", description="") -> Counter: + self._secure_instrument_name(name) + + @abstractmethod + def create_up_down_counter( + self, name, unit="", description="" + ) -> UpDownCounter: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_counter( + self, name, callback, unit="", description="" + ) -> ObservableCounter: + """Creates an observable counter instrument + + An observable counter observes a monotonically increasing count by + calling a provided callback which returns multiple + :class:`~opentelemetry.metrics.measurement.Measurement`. + + For example, an observable counter could be used to report system CPU + time periodically. Here is a basic implementation:: + + def cpu_time_callback() -> Iterable[Measurement]: + measurements = [] + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) + measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) + measurements.append(Measurement(int(states[2]) // 100, {"cpu": cpu, "state": "system"})) + # ... other states + return measurements + + meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_callback, + unit="s", + description="CPU time" + ) + + To reduce memory usage, you can use generator callbacks instead of + building the full list:: + + def cpu_time_callback() -> Iterable[Measurement]: + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + yield Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"}) + yield Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"}) + # ... other states + + Alternatively, you can pass a generator directly instead of a callback, + which should return iterables of + :class:`~opentelemetry.metrics.measurement.Measurement`:: + + def cpu_time_callback(states_to_include: set[str]) -> Iterable[Iterable[Measurement]]: + while True: + measurements = [] + with open("/proc/stat") as procstat: + procstat.readline() # skip the first line + for line in procstat: + if not line.startswith("cpu"): break + cpu, *states = line.split() + if "user" in states_to_include: + measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) + if "nice" in states_to_include: + measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) + # ... other states + yield measurements + + meter.create_observable_counter( + "system.cpu.time", + callback=cpu_time_callback({"user", "system"}), + unit="s", + description="CPU time" + ) + + Args: + name: The name of the instrument to be created + callback: A callback that returns an iterable of + :class:`~opentelemetry.metrics.measurement.Measurement`. + Alternatively, can be a generator that yields iterables of + :class:`~opentelemetry.metrics.measurement.Measurement`. + unit: The unit for measurements this instrument reports. For + example, ``By`` for bytes. UCUM units are recommended. + description: A description for this instrument and what it measures. + """ + + self._secure_instrument_name(name) + + @abstractmethod + def create_histogram(self, name, unit="", description="") -> Histogram: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_gauge( + self, name, callback, unit="", description="" + ) -> ObservableGauge: + self._secure_instrument_name(name) + + @abstractmethod + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ) -> ObservableUpDownCounter: + self._secure_instrument_name(name) + + +class ProxyMeter(Meter): + def __init__( + self, + name, + version=None, + schema_url=None, + ): + super().__init__(name, version=version, schema_url=schema_url) + self._real_meter: Optional[Meter] = None + self._noop_meter = _DefaultMeter( + name, version=version, schema_url=schema_url + ) + + @property + def _meter(self) -> Meter: + if self._real_meter is not None: + return self._real_meter + + if _METER_PROVIDER: + self._real_meter = _METER_PROVIDER.get_meter( + self._name, + self._version, + ) + return self._real_meter + return self._noop_meter + + def create_counter(self, *args, **kwargs) -> Counter: + return self._meter.create_counter(*args, **kwargs) + + def create_up_down_counter(self, *args, **kwargs) -> UpDownCounter: + return self._meter.create_up_down_counter(*args, **kwargs) + + def create_observable_counter(self, *args, **kwargs) -> ObservableCounter: + return self._meter.create_observable_counter(*args, **kwargs) + + def create_histogram(self, *args, **kwargs) -> Histogram: + return self._meter.create_histogram(*args, **kwargs) + + def create_observable_gauge(self, *args, **kwargs) -> ObservableGauge: + return self._meter.create_observable_gauge(*args, **kwargs) + + def create_observable_up_down_counter( + self, *args, **kwargs + ) -> ObservableUpDownCounter: + return self._meter.create_observable_up_down_counter(*args, **kwargs) + + +class _DefaultMeter(Meter): + def create_counter(self, name, unit="", description="") -> Counter: + super().create_counter(name, unit=unit, description=description) + return DefaultCounter(name, unit=unit, description=description) + + def create_up_down_counter( + self, name, unit="", description="" + ) -> UpDownCounter: + super().create_up_down_counter( + name, unit=unit, description=description + ) + return DefaultUpDownCounter(name, unit=unit, description=description) + + def create_observable_counter( + self, name, callback, unit="", description="" + ) -> ObservableCounter: + super().create_observable_counter( + name, callback, unit=unit, description=description + ) + return DefaultObservableCounter( + name, + callback, + unit=unit, + description=description, + ) + + def create_histogram(self, name, unit="", description="") -> Histogram: + super().create_histogram(name, unit=unit, description=description) + return DefaultHistogram(name, unit=unit, description=description) + + def create_observable_gauge( + self, name, callback, unit="", description="" + ) -> ObservableGauge: + super().create_observable_gauge( + name, callback, unit=unit, description=description + ) + return DefaultObservableGauge( + name, + callback, + unit=unit, + description=description, + ) + + def create_observable_up_down_counter( + self, name, callback, unit="", description="" + ) -> ObservableUpDownCounter: + super().create_observable_up_down_counter( + name, callback, unit=unit, description=description + ) + return DefaultObservableUpDownCounter( + name, + callback, + unit=unit, + description=description, + ) + + +_METER_PROVIDER = None +_PROXY_METER_PROVIDER = None + + +def get_meter( + name: str, + version: str = "", + meter_provider: Optional[MeterProvider] = None, +) -> "Meter": + """Returns a `Meter` for use by the given instrumentation library. + + This function is a convenience wrapper for + opentelemetry.trace.MeterProvider.get_meter. + + If meter_provider is omitted the current configured one is used. + """ + if meter_provider is None: + meter_provider = get_meter_provider() + return meter_provider.get_meter(name, version) + + +def set_meter_provider(meter_provider: MeterProvider) -> None: + """Sets the current global :class:`~.MeterProvider` object. + + This can only be done once, a warning will be logged if any furter attempt + is made. + """ + global _METER_PROVIDER # pylint: disable=global-statement + + if _METER_PROVIDER is not None: + _logger.warning("Overriding of current MeterProvider is not allowed") + return + + _METER_PROVIDER = meter_provider + + +def get_meter_provider() -> MeterProvider: + """Gets the current global :class:`~.MeterProvider` object.""" + # pylint: disable=global-statement + global _METER_PROVIDER + global _PROXY_METER_PROVIDER + + if _METER_PROVIDER is None: + if OTEL_PYTHON_METER_PROVIDER not in environ.keys(): + if _PROXY_METER_PROVIDER is None: + _PROXY_METER_PROVIDER = ProxyMeterProvider() + return _PROXY_METER_PROVIDER + + _METER_PROVIDER = cast( + "MeterProvider", + _load_provider(OTEL_PYTHON_METER_PROVIDER, "meter_provider"), + ) + return _METER_PROVIDER diff --git a/opentelemetry-api/src/opentelemetry/metrics/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/instrument.py new file mode 100644 index 00000000000..5d382056408 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/instrument.py @@ -0,0 +1,234 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# pylint: disable=too-many-ancestors + +# type: ignore + +from abc import ABC, abstractmethod +from collections import abc as collections_abc +from logging import getLogger +from re import compile as compile_ +from typing import Callable, Generator, Iterable, Union + +from opentelemetry.metrics.measurement import Measurement + +_TInstrumentCallback = Callable[[], Iterable[Measurement]] +_TInstrumentCallbackGenerator = Generator[Iterable[Measurement], None, None] +TCallback = Union[_TInstrumentCallback, _TInstrumentCallbackGenerator] + + +_logger = getLogger(__name__) + + +class Instrument(ABC): + + _name_regex = compile_(r"[a-zA-Z][-.\w]{0,62}") + + @property + def name(self): + return self._name + + @property + def unit(self): + return self._unit + + @property + def description(self): + return self._description + + @abstractmethod + def __init__(self, name, unit="", description=""): + + if name is None or self._name_regex.fullmatch(name) is None: + _logger.error("Invalid instrument name %s", name) + + else: + self._name = name + + if unit is None: + self._unit = "" + elif len(unit) > 63: + _logger.error("unit must be 63 characters or shorter") + + elif any(ord(character) > 127 for character in unit): + _logger.error("unit must only contain ASCII characters") + else: + self._unit = unit + + if description is None: + description = "" + + self._description = description + + +class Synchronous(Instrument): + pass + + +class Asynchronous(Instrument): + @abstractmethod + def __init__( + self, + name, + callback: TCallback, + *args, + unit="", + description="", + **kwargs + ): + super().__init__( + name, *args, unit=unit, description=description, **kwargs + ) + + if isinstance(callback, collections_abc.Callable): + self._callback = callback + elif isinstance(callback, collections_abc.Generator): + self._callback = self._wrap_generator_callback(callback) + else: + _logger.error("callback must be a callable or generator") + + def _wrap_generator_callback( + self, + generator_callback: _TInstrumentCallbackGenerator, + ) -> _TInstrumentCallback: + """Wraps a generator style callback into a callable one""" + has_items = True + + def inner() -> Iterable[Measurement]: + nonlocal has_items + if not has_items: + return [] + + try: + return next(generator_callback) + except StopIteration: + has_items = False + _logger.error( + "callback generator for instrument %s ran out of measurements", + self._name, + ) + return [] + + return inner + + def callback(self): + measurements = self._callback() + if not isinstance(measurements, collections_abc.Iterable): + _logger.error( + "Callback must return an iterable of Measurement, got %s", + type(measurements), + ) + return + for measurement in measurements: + if not isinstance(measurement, Measurement): + _logger.error( + "Callback must return an iterable of Measurement, " + "iterable contained type %s", + type(measurement), + ) + yield measurement + + +class _Adding(Instrument): + pass + + +class _Grouping(Instrument): + pass + + +class _Monotonic(_Adding): + pass + + +class _NonMonotonic(_Adding): + pass + + +class Counter(_Monotonic, Synchronous): + @abstractmethod + def add(self, amount, attributes=None): + if amount < 0: + _logger.error("Amount must be non-negative") + + +class DefaultCounter(Counter): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def add(self, amount, attributes=None): + return super().add(amount, attributes=attributes) + + +class UpDownCounter(_NonMonotonic, Synchronous): + @abstractmethod + def add(self, amount, attributes=None): + pass + + +class DefaultUpDownCounter(UpDownCounter): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def add(self, amount, attributes=None): + return super().add(amount, attributes=attributes) + + +class ObservableCounter(_Monotonic, Asynchronous): + def callback(self): + measurements = super().callback() + + for measurement in measurements: + if measurement.value < 0: + _logger.error("Amount must be non-negative") + yield measurement + + +class DefaultObservableCounter(ObservableCounter): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) + + +class ObservableUpDownCounter(_NonMonotonic, Asynchronous): + + pass + + +class DefaultObservableUpDownCounter(ObservableUpDownCounter): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) + + +class Histogram(_Grouping, Synchronous): + @abstractmethod + def record(self, amount, attributes=None): + pass + + +class DefaultHistogram(Histogram): + def __init__(self, name, unit="", description=""): + super().__init__(name, unit=unit, description=description) + + def record(self, amount, attributes=None): + return super().record(amount, attributes=attributes) + + +class ObservableGauge(_Grouping, Asynchronous): + pass + + +class DefaultObservableGauge(ObservableGauge): + def __init__(self, name, callback, unit="", description=""): + super().__init__(name, callback, unit=unit, description=description) diff --git a/opentelemetry-api/src/opentelemetry/metrics/measurement.py b/opentelemetry-api/src/opentelemetry/metrics/measurement.py new file mode 100644 index 00000000000..6b5b081c266 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/metrics/measurement.py @@ -0,0 +1,39 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# pylint: disable=too-many-ancestors +# type:ignore + + +from abc import ABC, abstractmethod + + +class Measurement(ABC): + @property + def value(self): + return self._value + + @property + def attributes(self): + return self._attributes + + @abstractmethod + def __init__(self, value, attributes=None): + self._value = value + self._attributes = attributes + + +class DefaultMeasurement(Measurement): + def __init__(self, value, attributes=None): + super().__init__(value, attributes=attributes) From 162acb95898a4de27f0738320f03573b618b5a37 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Wed, 29 Sep 2021 12:13:16 -0400 Subject: [PATCH 02/48] Make measurement a concrete class (#2153) * Make Measurement a concrete class * comments * update changelog --- .../src/opentelemetry/metrics/measurement.py | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/measurement.py b/opentelemetry-api/src/opentelemetry/metrics/measurement.py index 6b5b081c266..e8fd9725bb8 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/measurement.py +++ b/opentelemetry-api/src/opentelemetry/metrics/measurement.py @@ -12,28 +12,41 @@ # See the License for the specific language governing permissions and # limitations under the License. -# pylint: disable=too-many-ancestors -# type:ignore +from typing import Union +from opentelemetry.util.types import Attributes -from abc import ABC, abstractmethod +class Measurement: + """A measurement observed in an asynchronous instrument + + Return/yield instances of this class from asynchronous instrument callbacks. + + Args: + value: The float or int measured value + attributes: The measurement's attributes + """ + + def __init__( + self, value: Union[int, float], attributes: Attributes = None + ) -> None: + self._value = value + self._attributes = attributes -class Measurement(ABC): @property - def value(self): + def value(self) -> Union[float, int]: return self._value @property - def attributes(self): + def attributes(self) -> Attributes: return self._attributes - @abstractmethod - def __init__(self, value, attributes=None): - self._value = value - self._attributes = attributes - + def __eq__(self, other: object) -> bool: + return ( + isinstance(other, Measurement) + and self.value == other.value + and self.attributes == other.attributes + ) -class DefaultMeasurement(Measurement): - def __init__(self, value, attributes=None): - super().__init__(value, attributes=attributes) + def __repr__(self) -> str: + return f"Measurement(value={self.value}, attributes={self.attributes})" From 22b561d95657ecbc5ae3a0e1c03ddbd10b2aaa73 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 14 Oct 2021 12:46:02 -0400 Subject: [PATCH 03/48] Return proxy instruments from ProxyMeter (#2169) --- .../src/opentelemetry/metrics/__init__.py | 192 +++++++++++++----- .../src/opentelemetry/metrics/instrument.py | 94 ++++++++- 2 files changed, 233 insertions(+), 53 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 83d210e063b..c353a0e0032 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -25,7 +25,8 @@ from abc import ABC, abstractmethod from logging import getLogger from os import environ -from typing import Optional, cast +from threading import Lock +from typing import List, Optional, cast from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER from opentelemetry.metrics.instrument import ( @@ -41,7 +42,15 @@ ObservableGauge, ObservableUpDownCounter, UpDownCounter, + _ProxyCounter, + _ProxyHistogram, + _ProxyInstrument, + _ProxyObservableCounter, + _ProxyObservableGauge, + _ProxyObservableUpDownCounter, + _ProxyUpDownCounter, ) +from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider _logger = getLogger(__name__) @@ -70,18 +79,33 @@ def get_meter( return _DefaultMeter(name, version=version, schema_url=schema_url) -class ProxyMeterProvider(MeterProvider): +class _ProxyMeterProvider(MeterProvider): + def __init__(self) -> None: + self._lock = Lock() + self._meters: List[_ProxyMeter] = [] + self._real_meter_provider: Optional[MeterProvider] = None + def get_meter( self, name, version=None, schema_url=None, ) -> "Meter": - if _METER_PROVIDER: - return _METER_PROVIDER.get_meter( - name, version=version, schema_url=schema_url - ) - return ProxyMeter(name, version=version, schema_url=schema_url) + with self._lock: + if self._real_meter_provider is not None: + return self._real_meter_provider.get_meter( + name, version, schema_url + ) + + meter = _ProxyMeter(name, version=version, schema_url=schema_url) + self._meters.append(meter) + return meter + + def on_set_meter_provider(self, meter_provider: MeterProvider) -> None: + with self._lock: + self._real_meter_provider = meter_provider + for meter in self._meters: + meter.on_set_meter_provider(meter_provider) class Meter(ABC): @@ -225,7 +249,7 @@ def create_observable_up_down_counter( self._secure_instrument_name(name) -class ProxyMeter(Meter): +class _ProxyMeter(Meter): def __init__( self, name, @@ -233,43 +257,101 @@ def __init__( schema_url=None, ): super().__init__(name, version=version, schema_url=schema_url) + self._lock = Lock() + self._instruments: List[_ProxyInstrument] = [] self._real_meter: Optional[Meter] = None - self._noop_meter = _DefaultMeter( - name, version=version, schema_url=schema_url + + def on_set_meter_provider(self, meter_provider: MeterProvider) -> None: + """Called when a real meter provider is set on the creating _ProxyMeterProvider + + Creates a real backing meter for this instance and notifies all created + instruments so they can create real backing instruments. + """ + real_meter = meter_provider.get_meter( + self._name, self._version, self._schema_url ) - @property - def _meter(self) -> Meter: - if self._real_meter is not None: - return self._real_meter - - if _METER_PROVIDER: - self._real_meter = _METER_PROVIDER.get_meter( - self._name, - self._version, - ) - return self._real_meter - return self._noop_meter + with self._lock: + self._real_meter = real_meter + # notify all proxy instruments of the new meter so they can create + # real instruments to back themselves + for instrument in self._instruments: + instrument.on_meter_set(real_meter) - def create_counter(self, *args, **kwargs) -> Counter: - return self._meter.create_counter(*args, **kwargs) + def create_counter(self, name, unit="", description="") -> Counter: + with self._lock: + if self._real_meter: + return self._real_meter.create_counter(name, unit, description) + proxy = _ProxyCounter(name, unit, description) + self._instruments.append(proxy) + return proxy - def create_up_down_counter(self, *args, **kwargs) -> UpDownCounter: - return self._meter.create_up_down_counter(*args, **kwargs) + def create_up_down_counter( + self, name, unit="", description="" + ) -> UpDownCounter: + with self._lock: + if self._real_meter: + return self._real_meter.create_up_down_counter( + name, unit, description + ) + proxy = _ProxyUpDownCounter(name, unit, description) + self._instruments.append(proxy) + return proxy - def create_observable_counter(self, *args, **kwargs) -> ObservableCounter: - return self._meter.create_observable_counter(*args, **kwargs) + def create_observable_counter( + self, name, callback, unit="", description="" + ) -> ObservableCounter: + with self._lock: + if self._real_meter: + return self._real_meter.create_observable_counter( + name, callback, unit, description + ) + proxy = _ProxyObservableCounter( + name, callback, unit=unit, description=description + ) + self._instruments.append(proxy) + return proxy - def create_histogram(self, *args, **kwargs) -> Histogram: - return self._meter.create_histogram(*args, **kwargs) + def create_histogram(self, name, unit="", description="") -> Histogram: + with self._lock: + if self._real_meter: + return self._real_meter.create_histogram( + name, unit, description + ) + proxy = _ProxyHistogram(name, unit, description) + self._instruments.append(proxy) + return proxy - def create_observable_gauge(self, *args, **kwargs) -> ObservableGauge: - return self._meter.create_observable_gauge(*args, **kwargs) + def create_observable_gauge( + self, name, callback, unit="", description="" + ) -> ObservableGauge: + with self._lock: + if self._real_meter: + return self._real_meter.create_observable_gauge( + name, callback, unit, description + ) + proxy = _ProxyObservableGauge( + name, callback, unit=unit, description=description + ) + self._instruments.append(proxy) + return proxy def create_observable_up_down_counter( - self, *args, **kwargs + self, name, callback, unit="", description="" ) -> ObservableUpDownCounter: - return self._meter.create_observable_up_down_counter(*args, **kwargs) + with self._lock: + if self._real_meter: + return self._real_meter.create_observable_up_down_counter( + name, + callback, + unit, + description, + ) + proxy = _ProxyObservableUpDownCounter( + name, callback, unit=unit, description=description + ) + self._instruments.append(proxy) + return proxy class _DefaultMeter(Meter): @@ -329,8 +411,9 @@ def create_observable_up_down_counter( ) -_METER_PROVIDER = None -_PROXY_METER_PROVIDER = None +_METER_PROVIDER_SET_ONCE = Once() +_METER_PROVIDER: Optional[MeterProvider] = None +_PROXY_METER_PROVIDER = _ProxyMeterProvider() def get_meter( @@ -350,35 +433,40 @@ def get_meter( return meter_provider.get_meter(name, version) +def _set_meter_provider(meter_provider: MeterProvider, log: bool) -> None: + def set_mp() -> None: + global _METER_PROVIDER # pylint: disable=global-statement + _METER_PROVIDER = meter_provider + + # gives all proxies real instruments off the newly set meter provider + _PROXY_METER_PROVIDER.on_set_meter_provider(meter_provider) + + did_set = _METER_PROVIDER_SET_ONCE.do_once(set_mp) + + if log and not did_set: + _logger.warning("Overriding of current MeterProvider is not allowed") + + def set_meter_provider(meter_provider: MeterProvider) -> None: """Sets the current global :class:`~.MeterProvider` object. This can only be done once, a warning will be logged if any furter attempt is made. """ - global _METER_PROVIDER # pylint: disable=global-statement - - if _METER_PROVIDER is not None: - _logger.warning("Overriding of current MeterProvider is not allowed") - return - - _METER_PROVIDER = meter_provider + _set_meter_provider(meter_provider, log=True) def get_meter_provider() -> MeterProvider: """Gets the current global :class:`~.MeterProvider` object.""" - # pylint: disable=global-statement - global _METER_PROVIDER - global _PROXY_METER_PROVIDER if _METER_PROVIDER is None: if OTEL_PYTHON_METER_PROVIDER not in environ.keys(): - if _PROXY_METER_PROVIDER is None: - _PROXY_METER_PROVIDER = ProxyMeterProvider() return _PROXY_METER_PROVIDER - _METER_PROVIDER = cast( - "MeterProvider", - _load_provider(OTEL_PYTHON_METER_PROVIDER, "meter_provider"), + meter_provider: MeterProvider = _load_provider( + OTEL_PYTHON_METER_PROVIDER, "meter_provider" ) - return _METER_PROVIDER + _set_meter_provider(meter_provider, log=False) + + # _METER_PROVIDER will have been set by one thread + return cast("MeterProvider", _METER_PROVIDER) diff --git a/opentelemetry-api/src/opentelemetry/metrics/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/instrument.py index 5d382056408..7ff4de4b6eb 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/instrument.py +++ b/opentelemetry-api/src/opentelemetry/metrics/instrument.py @@ -27,6 +27,7 @@ _TInstrumentCallback = Callable[[], Iterable[Measurement]] _TInstrumentCallbackGenerator = Generator[Iterable[Measurement], None, None] TCallback = Union[_TInstrumentCallback, _TInstrumentCallbackGenerator] +InstrumentT = TypeVar("InstrumentT", bound="Instrument") _logger = getLogger(__name__) @@ -73,6 +74,32 @@ def __init__(self, name, unit="", description=""): self._description = description +class _ProxyInstrument(ABC, Generic[InstrumentT]): + def __init__(self, name, unit, description) -> None: + self._name = name + self._unit = unit + self._description = description + self._real_instrument: Optional[InstrumentT] = None + + def on_meter_set(self, meter: "metrics.Meter") -> None: + """Called when a real meter is set on the creating _ProxyMeter""" + + # We don't need any locking on proxy instruments because it's OK if some + # measurements get dropped while a real backing instrument is being + # created. + self._real_instrument = self._create_real_instrument(meter) + + @abstractmethod + def _create_real_instrument(self, meter: "metrics.Meter") -> InstrumentT: + """Create an instance of the real instrument. Implement this.""" + + +class _ProxyAsynchronousInstrument(_ProxyInstrument[InstrumentT]): + def __init__(self, name, callback, unit, description) -> None: + super().__init__(name, unit, description) + self._callback = callback + + class Synchronous(Instrument): pass @@ -172,6 +199,15 @@ def add(self, amount, attributes=None): return super().add(amount, attributes=attributes) +class _ProxyCounter(_ProxyInstrument[Counter], Counter): + def add(self, amount, attributes=None): + if self._real_instrument: + self._real_instrument.add(amount, attributes) + + def _create_real_instrument(self, meter: "metrics.Meter") -> Counter: + return meter.create_counter(self._name, self._unit, self._description) + + class UpDownCounter(_NonMonotonic, Synchronous): @abstractmethod def add(self, amount, attributes=None): @@ -186,6 +222,17 @@ def add(self, amount, attributes=None): return super().add(amount, attributes=attributes) +class _ProxyUpDownCounter(_ProxyInstrument[UpDownCounter], UpDownCounter): + def add(self, amount, attributes=None): + if self._real_instrument: + self._real_instrument.add(amount, attributes) + + def _create_real_instrument(self, meter: "metrics.Meter") -> UpDownCounter: + return meter.create_up_down_counter( + self._name, self._unit, self._description + ) + + class ObservableCounter(_Monotonic, Asynchronous): def callback(self): measurements = super().callback() @@ -201,8 +248,18 @@ def __init__(self, name, callback, unit="", description=""): super().__init__(name, callback, unit=unit, description=description) -class ObservableUpDownCounter(_NonMonotonic, Asynchronous): +class _ProxyObservableCounter( + _ProxyAsynchronousInstrument[ObservableCounter], ObservableCounter +): + def _create_real_instrument( + self, meter: "metrics.Meter" + ) -> ObservableCounter: + return meter.create_observable_counter( + self._name, self._callback, self._unit, self._description + ) + +class ObservableUpDownCounter(_NonMonotonic, Asynchronous): pass @@ -211,6 +268,18 @@ def __init__(self, name, callback, unit="", description=""): super().__init__(name, callback, unit=unit, description=description) +class _ProxyObservableUpDownCounter( + _ProxyAsynchronousInstrument[ObservableUpDownCounter], + ObservableUpDownCounter, +): + def _create_real_instrument( + self, meter: "metrics.Meter" + ) -> ObservableUpDownCounter: + return meter.create_observable_up_down_counter( + self._name, self._callback, self._unit, self._description + ) + + class Histogram(_Grouping, Synchronous): @abstractmethod def record(self, amount, attributes=None): @@ -225,6 +294,17 @@ def record(self, amount, attributes=None): return super().record(amount, attributes=attributes) +class _ProxyHistogram(_ProxyInstrument[Histogram], Histogram): + def record(self, amount, attributes=None): + if self._real_instrument: + self._real_instrument.record(amount, attributes) + + def _create_real_instrument(self, meter: "metrics.Meter") -> Histogram: + return meter.create_histogram( + self._name, self._unit, self._description + ) + + class ObservableGauge(_Grouping, Asynchronous): pass @@ -232,3 +312,15 @@ class ObservableGauge(_Grouping, Asynchronous): class DefaultObservableGauge(ObservableGauge): def __init__(self, name, callback, unit="", description=""): super().__init__(name, callback, unit=unit, description=description) + + +class _ProxyObservableGauge( + _ProxyAsynchronousInstrument[ObservableGauge], + ObservableGauge, +): + def _create_real_instrument( + self, meter: "metrics.Meter" + ) -> ObservableGauge: + return meter.create_observable_gauge( + self._name, self._callback, self._unit, self._description + ) From 6e8b1a771dadc20e9449d76651125adb173c063b Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 20:14:42 +0200 Subject: [PATCH 04/48] Merge main 4 (#2236) --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 2999b7fe434..68ade9e8cac 100644 --- a/tox.ini +++ b/tox.ini @@ -99,7 +99,7 @@ changedir = exporter-zipkin-combined: exporter/opentelemetry-exporter-zipkin/tests exporter-zipkin-proto-http: exporter/opentelemetry-exporter-zipkin-proto-http/tests exporter-zipkin-json: exporter/opentelemetry-exporter-zipkin-json/tests - + propagator-b3: propagator/opentelemetry-propagator-b3/tests propagator-jaeger: propagator/opentelemetry-propagator-jaeger/tests From 4b448d8682d83d9a4fa3e7d7de146a99540a074c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 19 Oct 2021 11:45:53 +0200 Subject: [PATCH 05/48] Add MeterProvider and Meter to the SDK Fixes #2200 --- .../src/opentelemetry/sdk/metrics/__init__.py | 169 ++++++++++++++++++ .../src/opentelemetry/sdk/metrics/api.py | 48 +++++ 2 files changed, 217 insertions(+) create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py new file mode 100644 index 00000000000..cc767c6ab1b --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -0,0 +1,169 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# pylint: disable=function-redefined,too-many-ancestors + +from atexit import register +from logging import getLogger +from typing import Optional + +from opentelemetry.metrics import Meter, MeterProvider +from opentelemetry.metrics.instrument import ( + Counter, + Histogram, + ObservableCounter, + ObservableGauge, + ObservableUpDownCounter, + UpDownCounter, +) +from opentelemetry.sdk.metrics.api import MetricExporter, MetricReader, View +from opentelemetry.sdk.resources import Resource +from opentelemetry.sdk.util.instrumentation import InstrumentationInfo + +_logger = getLogger(__name__) + + +class Meter(Meter): + def __init__(self, instrumentation_info: InstrumentationInfo): + super().__init__(instrumentation_info) + self._instrumentation_info = instrumentation_info + self._meter_provider = None + + def create_counter(self, name, unit=None, description=None) -> Counter: + pass + + def create_up_down_counter( + self, name, unit=None, description=None + ) -> UpDownCounter: + pass + + def create_observable_counter( + self, name, callback, unit=None, description=None + ) -> ObservableCounter: + pass + + def create_histogram(self, name, unit=None, description=None) -> Histogram: + pass + + def create_observable_gauge( + self, name, callback, unit=None, description=None + ) -> ObservableGauge: + pass + + def create_observable_up_down_counter( + self, name, callback, unit=None, description=None + ) -> ObservableUpDownCounter: + pass + + +class MeterProvider(MeterProvider): + """See `opentelemetry.metrics.Provider`.""" + + def __init__( + self, + resource: Resource = Resource.create({}), + shutdown_on_exit: bool = True, + ): + self._resource = resource + self._atexit_handler = None + + if shutdown_on_exit: + self._atexit_handler = register(self.shutdown) + + self._metric_readers = [] + self._metric_exporters = [] + self._views = [] + self._shutdown = False + + @property + def metric_readers(self): + return self._metric_readers + + @property + def metric_exporters(self): + return self._metric_exporters + + @property + def views(self): + return self._views + + @property + def resource(self) -> Resource: + return self._resource + + def get_meter( + self, + name: str, + version: Optional[str] = None, + schema_url: Optional[str] = None, + ) -> Meter: + meter = Meter(InstrumentationInfo(name, version, schema_url)) + + meter._meter_provider = self # pylint: disable=protected-access + + return meter + + def shutdown(self): + + if self._shutdown: + _logger.warning("shutdown can only be called once") + return False + + result = True + + for metric_reader in self._metric_readers: + result = result and metric_reader.shutdown() + + for metric_exporter in self._metric_exporters: + result = result and metric_exporter.shutdown() + + self._shutdown = True + + return result + + def force_flush(self) -> bool: + result = True + + for metric_reader in self._metric_readers: + result = result and metric_reader.force_flush() + + for metric_exporter in self._metric_exporters: + result = result and metric_exporter.force_flush() + + return result + + def register_metric_reader(self, metric_reader: "MetricReader") -> None: + self._metric_readers.append(metric_reader) + + def register_metric_exporter( + self, metric_exporter: "MetricExporter" + ) -> None: + self._metric_exporters.append(metric_exporter) + + def register_view(self, view: "View") -> None: + self._views.append(view) + + +class MetricReader(MetricReader): + def collect(self): + pass + + +class MetricExporter(MetricExporter): + def export(self): + pass + + +class View(View): + pass diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py new file mode 100644 index 00000000000..43ad9479b72 --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py @@ -0,0 +1,48 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +# pylint: disable=function-redefined,too-many-ancestors + +from abc import ABC, abstractmethod +from logging import getLogger + +_logger = getLogger(__name__) + + +class MetricReader(ABC): + def __init__(self): + self._shutdown = False + + @abstractmethod + def collect(self): + pass + + def shutdown(self): + self._shutdown = True + + +class MetricExporter(ABC): + def __init__(self): + self._shutdown = False + + @abstractmethod + def export(self): + pass + + def shutdown(self): + self._shutdown = True + + +class View(ABC): + pass From aa2b1f07753b6401ab70c2555173b3900485c085 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 12:23:39 +0200 Subject: [PATCH 06/48] Add FIXMEs --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index cc767c6ab1b..6c5d9a3887d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -41,29 +41,35 @@ def __init__(self, instrumentation_info: InstrumentationInfo): self._meter_provider = None def create_counter(self, name, unit=None, description=None) -> Counter: + # FIXME implement this method pass def create_up_down_counter( self, name, unit=None, description=None ) -> UpDownCounter: + # FIXME implement this method pass def create_observable_counter( self, name, callback, unit=None, description=None ) -> ObservableCounter: + # FIXME implement this method pass def create_histogram(self, name, unit=None, description=None) -> Histogram: + # FIXME implement this method pass def create_observable_gauge( self, name, callback, unit=None, description=None ) -> ObservableGauge: + # FIXME implement this method pass def create_observable_up_down_counter( self, name, callback, unit=None, description=None ) -> ObservableUpDownCounter: + # FIXME implement this method pass From 881b04a0d24d3a549c40045a2e6557f5cb1c25be Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 12:25:08 +0200 Subject: [PATCH 07/48] Fix docstring --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 6c5d9a3887d..9d2c1ae0bd2 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -74,7 +74,7 @@ def create_observable_up_down_counter( class MeterProvider(MeterProvider): - """See `opentelemetry.metrics.Provider`.""" + """See `opentelemetry.metrics.MeterProvider`.""" def __init__( self, From e3816ebfb91f4af365005aa786e44fab70d3183b Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 12:53:05 +0200 Subject: [PATCH 08/48] Add FIXME --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 9d2c1ae0bd2..a1b64a236bf 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -121,6 +121,7 @@ def get_meter( return meter def shutdown(self): + # FIXME implement a timeout if self._shutdown: _logger.warning("shutdown can only be called once") From 611ef1f67edf080fb8205483eb2e8ee50ac7a11b Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 12:58:40 +0200 Subject: [PATCH 09/48] Fix meter return --- .../src/opentelemetry/sdk/metrics/__init__.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index a1b64a236bf..5eab0b9d001 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -114,6 +114,13 @@ def get_meter( version: Optional[str] = None, schema_url: Optional[str] = None, ) -> Meter: + + if self._shutdown: + _logger.warning( + "A shutdown `MeterProvider` can not provide a `Meter`" + ) + return + meter = Meter(InstrumentationInfo(name, version, schema_url)) meter._meter_provider = self # pylint: disable=protected-access From c7f0daed723ae36f073b6152a975affad60bf30d Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 13:04:30 +0200 Subject: [PATCH 10/48] Log an error if a force flush fails --- .../src/opentelemetry/sdk/metrics/__init__.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 5eab0b9d001..25283f1cfa0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -147,15 +147,26 @@ def shutdown(self): return result def force_flush(self) -> bool: - result = True + metric_reader_result = True + metric_exporter_result = True for metric_reader in self._metric_readers: - result = result and metric_reader.force_flush() + metric_reader_result = ( + metric_reader_result and metric_reader.force_flush() + ) + + if not metric_reader_result: + _logger.warning("Unable to force flush all metric readers") for metric_exporter in self._metric_exporters: - result = result and metric_exporter.force_flush() + metric_exporter_result = ( + metric_exporter_result and metric_exporter.force_flush() + ) - return result + if not metric_exporter_result: + _logger.warning("Unable to force flush all metric exporters") + + return metric_reader_result and metric_exporter_result def register_metric_reader(self, metric_reader: "MetricReader") -> None: self._metric_readers.append(metric_reader) From 0ab82ba3b8605d05f3dd464e8dba325f305ece70 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 13:09:44 +0200 Subject: [PATCH 11/48] Add FIXME --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 25283f1cfa0..acdb2552eb2 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -147,6 +147,9 @@ def shutdown(self): return result def force_flush(self) -> bool: + + # FIXME implement a timeout + metric_reader_result = True metric_exporter_result = True From a01198fb54a31ef2cfdcb669a9eba9a8b44b2d5c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 13:18:40 +0200 Subject: [PATCH 12/48] Fix lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index acdb2552eb2..333b9c625c4 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -119,7 +119,7 @@ def get_meter( _logger.warning( "A shutdown `MeterProvider` can not provide a `Meter`" ) - return + return None meter = Meter(InstrumentationInfo(name, version, schema_url)) From 558c9acf856895355cf3dca59840bc229c7b4acc Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 27 Oct 2021 16:54:07 +0200 Subject: [PATCH 13/48] Remove SDK API module --- .../src/opentelemetry/sdk/metrics/__init__.py | 22 +++++++-- .../src/opentelemetry/sdk/metrics/api.py | 48 ------------------- 2 files changed, 18 insertions(+), 52 deletions(-) delete mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 333b9c625c4..dd04236af20 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -17,6 +17,7 @@ from atexit import register from logging import getLogger from typing import Optional +from abc import ABC, abstractmethod from opentelemetry.metrics import Meter, MeterProvider from opentelemetry.metrics.instrument import ( @@ -27,7 +28,6 @@ ObservableUpDownCounter, UpDownCounter, ) -from opentelemetry.sdk.metrics.api import MetricExporter, MetricReader, View from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo @@ -183,15 +183,29 @@ def register_view(self, view: "View") -> None: self._views.append(view) -class MetricReader(MetricReader): +class MetricReader(ABC): + def __init__(self): + self._shutdown = False + + @abstractmethod def collect(self): pass + def shutdown(self): + self._shutdown = True + -class MetricExporter(MetricExporter): +class MetricExporter(ABC): + def __init__(self): + self._shutdown = False + + @abstractmethod def export(self): pass + def shutdown(self): + self._shutdown = True + -class View(View): +class View: pass diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py deleted file mode 100644 index 43ad9479b72..00000000000 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/api.py +++ /dev/null @@ -1,48 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -# pylint: disable=function-redefined,too-many-ancestors - -from abc import ABC, abstractmethod -from logging import getLogger - -_logger = getLogger(__name__) - - -class MetricReader(ABC): - def __init__(self): - self._shutdown = False - - @abstractmethod - def collect(self): - pass - - def shutdown(self): - self._shutdown = True - - -class MetricExporter(ABC): - def __init__(self): - self._shutdown = False - - @abstractmethod - def export(self): - pass - - def shutdown(self): - self._shutdown = True - - -class View(ABC): - pass From 3c119a235936ec1ac7f15a3b0dd0ce0a0b41356e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 27 Oct 2021 17:25:36 +0200 Subject: [PATCH 14/48] Unregister --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index dd04236af20..cbfbebe85ef 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -14,7 +14,7 @@ # pylint: disable=function-redefined,too-many-ancestors -from atexit import register +from atexit import register, unregister from logging import getLogger from typing import Optional from abc import ABC, abstractmethod @@ -144,6 +144,10 @@ def shutdown(self): self._shutdown = True + if self._atexit_handler is not None: + unregister(self._atexit_handler) + self._atexit_handler = None + return result def force_flush(self) -> bool: From 899c064a2b8ab6900179159af2371f3dc2780a21 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:33:02 +0200 Subject: [PATCH 15/48] Fix API names --- .../src/opentelemetry/sdk/metrics/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index cbfbebe85ef..0f171422e79 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -19,7 +19,9 @@ from typing import Optional from abc import ABC, abstractmethod -from opentelemetry.metrics import Meter, MeterProvider +from opentelemetry.metrics import ( + Meter as APIMeter, MeterProvider as APIMeterProvider +) from opentelemetry.metrics.instrument import ( Counter, Histogram, @@ -34,7 +36,7 @@ _logger = getLogger(__name__) -class Meter(Meter): +class Meter(APIMeter): def __init__(self, instrumentation_info: InstrumentationInfo): super().__init__(instrumentation_info) self._instrumentation_info = instrumentation_info @@ -73,7 +75,7 @@ def create_observable_up_down_counter( pass -class MeterProvider(MeterProvider): +class MeterProvider(APIMeterProvider): """See `opentelemetry.metrics.MeterProvider`.""" def __init__( From bdce73606572a3225f06a1cd76a08c0b9d8afd70 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:45:40 +0200 Subject: [PATCH 16/48] Return _DefaultMeter --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 0f171422e79..9ab28f5d0dd 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -20,7 +20,7 @@ from abc import ABC, abstractmethod from opentelemetry.metrics import ( - Meter as APIMeter, MeterProvider as APIMeterProvider + Meter as APIMeter, MeterProvider as APIMeterProvider, _DefaultMeter ) from opentelemetry.metrics.instrument import ( Counter, @@ -121,7 +121,7 @@ def get_meter( _logger.warning( "A shutdown `MeterProvider` can not provide a `Meter`" ) - return None + return _DefaultMeter(name, version=version, schema_url=schema_url) meter = Meter(InstrumentationInfo(name, version, schema_url)) From 8cff4cacd32d023175211a3797a26d47f384ffe7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:48:05 +0200 Subject: [PATCH 17/48] Remove properties --- .../src/opentelemetry/sdk/metrics/__init__.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 9ab28f5d0dd..8ed813fca28 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -94,22 +94,6 @@ def __init__( self._views = [] self._shutdown = False - @property - def metric_readers(self): - return self._metric_readers - - @property - def metric_exporters(self): - return self._metric_exporters - - @property - def views(self): - return self._views - - @property - def resource(self) -> Resource: - return self._resource - def get_meter( self, name: str, From f01920747142b41849fcaa2d943ca2a2288676dd Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:51:00 +0200 Subject: [PATCH 18/48] Pass MeterProvider as a parameter to __init__ --- .../src/opentelemetry/sdk/metrics/__init__.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 8ed813fca28..f10cb064111 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -37,10 +37,14 @@ class Meter(APIMeter): - def __init__(self, instrumentation_info: InstrumentationInfo): + def __init__( + self, + instrumentation_info: InstrumentationInfo, + meter_provider: APIMeterProvider + ): super().__init__(instrumentation_info) self._instrumentation_info = instrumentation_info - self._meter_provider = None + self._meter_provider = meter_provider def create_counter(self, name, unit=None, description=None) -> Counter: # FIXME implement this method @@ -107,11 +111,7 @@ def get_meter( ) return _DefaultMeter(name, version=version, schema_url=schema_url) - meter = Meter(InstrumentationInfo(name, version, schema_url)) - - meter._meter_provider = self # pylint: disable=protected-access - - return meter + return Meter(InstrumentationInfo(name, version, schema_url), self) def shutdown(self): # FIXME implement a timeout From 07fdeacc9bdb0ef09b9875c10fc252c590199045 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:56:34 +0200 Subject: [PATCH 19/48] Add FIXMEs --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index f10cb064111..bcb34ba53d3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -182,6 +182,7 @@ def collect(self): pass def shutdown(self): + # FIXME this will need a Once wrapper self._shutdown = True @@ -194,6 +195,7 @@ def export(self): pass def shutdown(self): + # FIXME this will need a Once wrapper self._shutdown = True From 32b67e82ca805cca239fad33d527a267c3356b67 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 12:59:52 +0200 Subject: [PATCH 20/48] Add FIXMEs --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index bcb34ba53d3..e11928f6cdc 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -162,14 +162,17 @@ def force_flush(self) -> bool: return metric_reader_result and metric_exporter_result def register_metric_reader(self, metric_reader: "MetricReader") -> None: + # FIXME protect this method against race conditions self._metric_readers.append(metric_reader) def register_metric_exporter( self, metric_exporter: "MetricExporter" ) -> None: + # FIXME protect this method against race conditions self._metric_exporters.append(metric_exporter) def register_view(self, view: "View") -> None: + # FIXME protect this method against race conditions self._views.append(view) From cb3ed60541d655c195264268eeb9ca012db5fc2b Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 28 Oct 2021 13:33:09 +0200 Subject: [PATCH 21/48] Fix lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index e11928f6cdc..384e52512a9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -14,14 +14,14 @@ # pylint: disable=function-redefined,too-many-ancestors +from abc import ABC, abstractmethod from atexit import register, unregister from logging import getLogger from typing import Optional -from abc import ABC, abstractmethod -from opentelemetry.metrics import ( - Meter as APIMeter, MeterProvider as APIMeterProvider, _DefaultMeter -) +from opentelemetry.metrics import Meter as APIMeter +from opentelemetry.metrics import MeterProvider as APIMeterProvider +from opentelemetry.metrics import _DefaultMeter from opentelemetry.metrics.instrument import ( Counter, Histogram, @@ -40,7 +40,7 @@ class Meter(APIMeter): def __init__( self, instrumentation_info: InstrumentationInfo, - meter_provider: APIMeterProvider + meter_provider: APIMeterProvider, ): super().__init__(instrumentation_info) self._instrumentation_info = instrumentation_info @@ -204,3 +204,13 @@ def shutdown(self): class View: pass + + +class ConsoleMetricExporter(MetricExporter): + def export(self): + pass + + +class SDKMetricReader(MetricReader): + def collect(self): + pass From 325e9043dba081cecb28f12109a080780566ff97 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 21 Oct 2021 16:06:32 +0200 Subject: [PATCH 22/48] Add Aggregation to the metrics SDK Fixes #2229 --- .../opentelemetry/sdk/metrics/aggregation.py | 117 ++++++++ .../opentelemetry/sdk/metrics/instrument.py | 103 +++++++ .../tests/metrics/test_aggregation.py | 265 ++++++++++++++++++ tox.ini | 1 + 4 files changed, 486 insertions(+) create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py create mode 100644 opentelemetry-sdk/tests/metrics/test_aggregation.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py new file mode 100644 index 00000000000..7f63ef0cee7 --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py @@ -0,0 +1,117 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +from abc import ABC, abstractmethod +from collections import OrderedDict +from math import inf + +from opentelemetry.proto.metrics.v1.metrics_pb2 import ( + AGGREGATION_TEMPORALITY_CUMULATIVE, + AGGREGATION_TEMPORALITY_DELTA +) + + +class Aggregation(ABC): + + def __init__(self): + self._value = 0 + + @property + def value(self): + return self._value + + @abstractmethod + def aggregate(self, value): + pass + + @abstractmethod + def collect(self): + pass + + +class NoneAggregation(Aggregation): + """ + This aggregation drops all instrument measurements. + """ + + def aggregate(self, value): + pass + + def collect(self): + return self._value + + +class SumAggregation(Aggregation): + """ + This aggregation collects data for the SDK sum metric point. + """ + + def __init__(self, temporality=AGGREGATION_TEMPORALITY_CUMULATIVE): + super().__init__() + self._temporality = temporality + + def aggregate(self, value): + self._value = self._value + value + + def collect(self): + value = self._value + if self._temporality == AGGREGATION_TEMPORALITY_DELTA: + self._value = 0 + + return value + + +class LastValueAggregation(Aggregation): + + """ + This aggregation collects data for the SDK sum metric point. + """ + + def aggregate(self, value): + self._value = value + + def collect(self): + return self._value + + +class ExplicitBucketHistogramAggregation(Aggregation): + + """ + This aggregation collects data for the SDK sum metric point. + """ + + def __init__( + self, + temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, + boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf) + ): + super().__init__() + self._temporality = temporality + self._boundaries = boundaries + self._value = OrderedDict([(key, 0) for key in boundaries]) + + def aggregate(self, value): + for key in self._value.keys(): + + if value < key: + self._value[key] = self._value[key] + value + + break + + def collect(self): + value = self._value + if self._temporality == AGGREGATION_TEMPORALITY_DELTA: + self._value = OrderedDict([(key, 0) for key in self._boundaries]) + + return value diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py new file mode 100644 index 00000000000..1543dde0704 --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py @@ -0,0 +1,103 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +from opentelemetry.metrics.instrument import ( + Counter, + ObservableCounter, + UpDownCounter, + ObservableUpDownCounter, + Histogram, + ObservableGauge +) + +from opentelemetry.sdk.metrics.aggregation import ( + SumAggregation, + LastValueAggregation, + ExplicitBucketHistogramAggregation, +) + + +class Counter(Counter): + + def __init__( + self, + aggregation=SumAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() + + def add(self, amount, attributes=None): + pass + + +class UpDownCounter(UpDownCounter): + + def __init__( + self, + aggregation=SumAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() + + def add(self, amount, attributes=None): + pass + + +class ObservableCounter(ObservableCounter): + + def __init__( + self, + aggregation=SumAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() + + +class ObservableUpDownCounter(ObservableUpDownCounter): + + def __init__( + self, + aggregation=SumAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() + + +class Histogram(Histogram): + + def __init__( + self, + aggregation=ExplicitBucketHistogramAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() + + def add(self, amount, attributes=None): + pass + + +class ObservableGauge(ObservableGauge): + + def __init__( + self, + aggregation=LastValueAggregation, + aggregation_config={} + ): + self._aggregation = aggregation(**aggregation_config) + super().__init__() diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py new file mode 100644 index 00000000000..01c461556e6 --- /dev/null +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -0,0 +1,265 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + + +from unittest import TestCase +from math import inf + +from opentelemetry.sdk.metrics.aggregation import ( + NoneAggregation, + SumAggregation, + LastValueAggregation, + ExplicitBucketHistogramAggregation +) +from opentelemetry.proto.metrics.v1.metrics_pb2 import ( + AGGREGATION_TEMPORALITY_CUMULATIVE, + AGGREGATION_TEMPORALITY_DELTA +) + + +class TestNoneAggregation(TestCase): + + def test_aggregate(self): + """ + `NoneAggregation` drops all measurements. + """ + + none_aggregation = NoneAggregation() + + none_aggregation.aggregate(1) + none_aggregation.aggregate(2) + none_aggregation.aggregate(3) + + self.assertIs(none_aggregation.value, 0) + + +class TestSumAggregation(TestCase): + + def test_default_temporality(self): + """ + `SumAggregation` default temporality is + `AGGREGATION_TEMPORALITY_CUMULATIVE`. + """ + + sum_aggregation = SumAggregation() + self.assertEqual( + sum_aggregation._temporality, + AGGREGATION_TEMPORALITY_CUMULATIVE + ) + sum_aggregation = SumAggregation( + temporality=AGGREGATION_TEMPORALITY_DELTA + ) + self.assertEqual( + sum_aggregation._temporality, + AGGREGATION_TEMPORALITY_DELTA + ) + + def test_aggregate(self): + """ + `SumAggregation` collects data for sum metric points + """ + + sum_aggregation = SumAggregation() + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 6) + + def test_delta_temporality(self): + """ + `SumAggregation` supports delta temporality + """ + + sum_aggregation = SumAggregation(AGGREGATION_TEMPORALITY_DELTA) + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 6) + self.assertEqual(sum_aggregation.collect(), 6) + self.assertEqual(sum_aggregation.value, 0) + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 6) + self.assertEqual(sum_aggregation.collect(), 6) + self.assertEqual(sum_aggregation.value, 0) + + def test_cumulative_temporality(self): + """ + `SumAggregation` supports cumulative temporality + """ + + sum_aggregation = SumAggregation(AGGREGATION_TEMPORALITY_CUMULATIVE) + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 6) + self.assertEqual(sum_aggregation.collect(), 6) + self.assertEqual(sum_aggregation.value, 6) + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 12) + self.assertEqual(sum_aggregation.collect(), 12) + self.assertEqual(sum_aggregation.value, 12) + + +class TestLastValueAggregation(TestCase): + + def test_aggregate(self): + """ + `LastValueAggregation` collects data for gauge metric points with delta + temporality + """ + + last_value_aggregation = LastValueAggregation() + + last_value_aggregation.aggregate(1) + self.assertEqual(last_value_aggregation.value, 1) + + last_value_aggregation.aggregate(2) + self.assertEqual(last_value_aggregation.value, 2) + + last_value_aggregation.aggregate(3) + self.assertEqual(last_value_aggregation.value, 3) + + +class TestExplicitBucketHistogramAggregation(TestCase): + + def test_default_temporality(self): + """ + `ExplicitBucketHistogramAggregation` default temporality is + `AGGREGATION_TEMPORALITY_CUMULATIVE`. + """ + + explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation() + self.assertEqual( + explicit_bucket_histogram_aggregation._temporality, + AGGREGATION_TEMPORALITY_CUMULATIVE + ) + explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation( + temporality=AGGREGATION_TEMPORALITY_DELTA + ) + self.assertEqual( + explicit_bucket_histogram_aggregation._temporality, + AGGREGATION_TEMPORALITY_DELTA + ) + + def test_aggregate(self): + """ + `ExplicitBucketHistogramAggregation` collects data for explicit_bucket_histogram metric points + """ + + explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation() + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + self.assertEqual(explicit_bucket_histogram_aggregation.value[0], -1) + self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2) + self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 15) + self.assertEqual( + explicit_bucket_histogram_aggregation.value[inf], 9999 + ) + + def test_delta_temporality(self): + """ + `ExplicitBucketHistogramAggregation` supports delta temporality + """ + + explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_DELTA) + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + result = explicit_bucket_histogram_aggregation.collect() + + self.assertEqual(result[0], -1) + self.assertEqual(result[5], 2) + self.assertEqual(result[10], 15) + self.assertEqual(result[inf], 9999) + + self.assertEqual(explicit_bucket_histogram_aggregation.value[0], 0) + self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 0) + self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 0) + self.assertEqual( + explicit_bucket_histogram_aggregation.value[inf], 0 + ) + + def test_cumulative_temporality(self): + """ + `ExplicitBucketHistogramAggregation` supports cumulative temporality + """ + + explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_CUMULATIVE) + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + result = explicit_bucket_histogram_aggregation.collect() + + self.assertEqual(result[0], -1) + self.assertEqual(result[5], 2) + self.assertEqual(result[10], 15) + self.assertEqual(result[inf], 9999) + + self.assertEqual(explicit_bucket_histogram_aggregation.value[0], -1) + self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2) + self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 15) + self.assertEqual( + explicit_bucket_histogram_aggregation.value[inf], 9999 + ) + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + result = explicit_bucket_histogram_aggregation.collect() + + self.assertEqual(result[0], -1 * 2) + self.assertEqual(result[5], 2 * 2) + self.assertEqual(result[10], 15 * 2) + self.assertEqual(result[inf], 9999 * 2) + + self.assertEqual( + explicit_bucket_histogram_aggregation.value[0], -1 * 2 + ) + self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2 * 2) + self.assertEqual( + explicit_bucket_histogram_aggregation.value[10], 15 * 2 + ) + self.assertEqual( + explicit_bucket_histogram_aggregation.value[inf], 9999 * 2 + ) diff --git a/tox.ini b/tox.ini index 68ade9e8cac..6e27ff132c0 100644 --- a/tox.ini +++ b/tox.ini @@ -111,6 +111,7 @@ commands_pre = opentelemetry: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-semantic-conventions {toxinidir}/opentelemetry-sdk {toxinidir}/tests/opentelemetry-test-utils protobuf: pip install {toxinidir}/opentelemetry-proto + sdk: pip install {toxinidir}/opentelemetry-proto getting-started: pip install requests==2.26.0 flask==2.0.1 getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" From ab5d7537bbddf8712d9b29dba58b0fb0259c4102 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 13:24:39 +0200 Subject: [PATCH 23/48] lint fix wip --- .../opentelemetry/sdk/metrics/aggregation.py | 5 +- .../opentelemetry/sdk/metrics/instrument.py | 47 ++++------------ .../tests/metrics/test_aggregation.py | 56 ++++++++++--------- 3 files changed, 43 insertions(+), 65 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py index 7f63ef0cee7..1a592db1a84 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py @@ -18,12 +18,11 @@ from opentelemetry.proto.metrics.v1.metrics_pb2 import ( AGGREGATION_TEMPORALITY_CUMULATIVE, - AGGREGATION_TEMPORALITY_DELTA + AGGREGATION_TEMPORALITY_DELTA, ) class Aggregation(ABC): - def __init__(self): self._value = 0 @@ -94,7 +93,7 @@ class ExplicitBucketHistogramAggregation(Aggregation): def __init__( self, temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, - boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf) + boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf), ): super().__init__() self._temporality = temporality diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py index 1543dde0704..bc5c37bb0ef 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py @@ -14,27 +14,21 @@ from opentelemetry.metrics.instrument import ( Counter, + Histogram, ObservableCounter, - UpDownCounter, + ObservableGauge, ObservableUpDownCounter, - Histogram, - ObservableGauge + UpDownCounter, ) - from opentelemetry.sdk.metrics.aggregation import ( - SumAggregation, - LastValueAggregation, ExplicitBucketHistogramAggregation, + LastValueAggregation, + SumAggregation, ) class Counter(Counter): - - def __init__( - self, - aggregation=SumAggregation, - aggregation_config={} - ): + def __init__(self, aggregation=SumAggregation, aggregation_config={}): self._aggregation = aggregation(**aggregation_config) super().__init__() @@ -43,12 +37,7 @@ def add(self, amount, attributes=None): class UpDownCounter(UpDownCounter): - - def __init__( - self, - aggregation=SumAggregation, - aggregation_config={} - ): + def __init__(self, aggregation=SumAggregation, aggregation_config={}): self._aggregation = aggregation(**aggregation_config) super().__init__() @@ -57,33 +46,22 @@ def add(self, amount, attributes=None): class ObservableCounter(ObservableCounter): - - def __init__( - self, - aggregation=SumAggregation, - aggregation_config={} - ): + def __init__(self, aggregation=SumAggregation, aggregation_config={}): self._aggregation = aggregation(**aggregation_config) super().__init__() class ObservableUpDownCounter(ObservableUpDownCounter): - - def __init__( - self, - aggregation=SumAggregation, - aggregation_config={} - ): + def __init__(self, aggregation=SumAggregation, aggregation_config={}): self._aggregation = aggregation(**aggregation_config) super().__init__() class Histogram(Histogram): - def __init__( self, aggregation=ExplicitBucketHistogramAggregation, - aggregation_config={} + aggregation_config={}, ): self._aggregation = aggregation(**aggregation_config) super().__init__() @@ -93,11 +71,8 @@ def add(self, amount, attributes=None): class ObservableGauge(ObservableGauge): - def __init__( - self, - aggregation=LastValueAggregation, - aggregation_config={} + self, aggregation=LastValueAggregation, aggregation_config={} ): self._aggregation = aggregation(**aggregation_config) super().__init__() diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 01c461556e6..29a4dde5326 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -13,23 +13,22 @@ # limitations under the License. -from unittest import TestCase from math import inf +from unittest import TestCase +from opentelemetry.proto.metrics.v1.metrics_pb2 import ( + AGGREGATION_TEMPORALITY_CUMULATIVE, + AGGREGATION_TEMPORALITY_DELTA, +) from opentelemetry.sdk.metrics.aggregation import ( + ExplicitBucketHistogramAggregation, + LastValueAggregation, NoneAggregation, SumAggregation, - LastValueAggregation, - ExplicitBucketHistogramAggregation -) -from opentelemetry.proto.metrics.v1.metrics_pb2 import ( - AGGREGATION_TEMPORALITY_CUMULATIVE, - AGGREGATION_TEMPORALITY_DELTA ) class TestNoneAggregation(TestCase): - def test_aggregate(self): """ `NoneAggregation` drops all measurements. @@ -45,7 +44,6 @@ def test_aggregate(self): class TestSumAggregation(TestCase): - def test_default_temporality(self): """ `SumAggregation` default temporality is @@ -54,15 +52,13 @@ def test_default_temporality(self): sum_aggregation = SumAggregation() self.assertEqual( - sum_aggregation._temporality, - AGGREGATION_TEMPORALITY_CUMULATIVE + sum_aggregation._temporality, AGGREGATION_TEMPORALITY_CUMULATIVE ) sum_aggregation = SumAggregation( temporality=AGGREGATION_TEMPORALITY_DELTA ) self.assertEqual( - sum_aggregation._temporality, - AGGREGATION_TEMPORALITY_DELTA + sum_aggregation._temporality, AGGREGATION_TEMPORALITY_DELTA ) def test_aggregate(self): @@ -126,7 +122,6 @@ def test_cumulative_temporality(self): class TestLastValueAggregation(TestCase): - def test_aggregate(self): """ `LastValueAggregation` collects data for gauge metric points with delta @@ -146,24 +141,27 @@ def test_aggregate(self): class TestExplicitBucketHistogramAggregation(TestCase): - def test_default_temporality(self): """ `ExplicitBucketHistogramAggregation` default temporality is `AGGREGATION_TEMPORALITY_CUMULATIVE`. """ - explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation() + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation() + ) self.assertEqual( explicit_bucket_histogram_aggregation._temporality, - AGGREGATION_TEMPORALITY_CUMULATIVE + AGGREGATION_TEMPORALITY_CUMULATIVE, ) - explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation( - temporality=AGGREGATION_TEMPORALITY_DELTA + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation( + temporality=AGGREGATION_TEMPORALITY_DELTA + ) ) self.assertEqual( explicit_bucket_histogram_aggregation._temporality, - AGGREGATION_TEMPORALITY_DELTA + AGGREGATION_TEMPORALITY_DELTA, ) def test_aggregate(self): @@ -171,7 +169,9 @@ def test_aggregate(self): `ExplicitBucketHistogramAggregation` collects data for explicit_bucket_histogram metric points """ - explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation() + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation() + ) explicit_bucket_histogram_aggregation.aggregate(-1) explicit_bucket_histogram_aggregation.aggregate(2) @@ -191,7 +191,9 @@ def test_delta_temporality(self): `ExplicitBucketHistogramAggregation` supports delta temporality """ - explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_DELTA) + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_DELTA) + ) explicit_bucket_histogram_aggregation.aggregate(-1) explicit_bucket_histogram_aggregation.aggregate(2) @@ -209,16 +211,18 @@ def test_delta_temporality(self): self.assertEqual(explicit_bucket_histogram_aggregation.value[0], 0) self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 0) self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 0) - self.assertEqual( - explicit_bucket_histogram_aggregation.value[inf], 0 - ) + self.assertEqual(explicit_bucket_histogram_aggregation.value[inf], 0) def test_cumulative_temporality(self): """ `ExplicitBucketHistogramAggregation` supports cumulative temporality """ - explicit_bucket_histogram_aggregation = ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_CUMULATIVE) + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation( + AGGREGATION_TEMPORALITY_CUMULATIVE + ) + ) explicit_bucket_histogram_aggregation.aggregate(-1) explicit_bucket_histogram_aggregation.aggregate(2) From 26e103d6f5fb833f3fd2bfc094fc598247270fb9 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 14:01:21 +0200 Subject: [PATCH 24/48] Fix lint --- .../opentelemetry/sdk/metrics/instrument.py | 68 ++++++++++++++++--- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py index bc5c37bb0ef..15f240b698e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py @@ -12,6 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=function-redefined +# pylint: disable=dangerous-default-value +# Classes in this module use dictionaries as default arguments. This is +# considered dangerous by pylint because the default dictionary is shared by +# all instances. Implementations of these classes must not make any change to +# this default dictionary in __init__. + from opentelemetry.metrics.instrument import ( Counter, Histogram, @@ -28,43 +35,76 @@ class Counter(Counter): - def __init__(self, aggregation=SumAggregation, aggregation_config={}): + def __init__( + self, + name, + unit="", + description="", + aggregation=SumAggregation, + aggregation_config={}, + ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, unit=unit, description=description) def add(self, amount, attributes=None): pass class UpDownCounter(UpDownCounter): - def __init__(self, aggregation=SumAggregation, aggregation_config={}): + def __init__( + self, + name, + unit="", + description="", + aggregation=SumAggregation, + aggregation_config={}, + ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, unit=unit, description=description) def add(self, amount, attributes=None): pass class ObservableCounter(ObservableCounter): - def __init__(self, aggregation=SumAggregation, aggregation_config={}): + def __init__( + self, + name, + callback, + unit="", + description="", + aggregation=SumAggregation, + aggregation_config={}, + ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, callback, unit=unit, description=description) class ObservableUpDownCounter(ObservableUpDownCounter): - def __init__(self, aggregation=SumAggregation, aggregation_config={}): + def __init__( + self, + name, + callback, + unit="", + description="", + aggregation=SumAggregation, + aggregation_config={}, + ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, callback, unit=unit, description=description) class Histogram(Histogram): def __init__( self, + name, + unit="", + description="", aggregation=ExplicitBucketHistogramAggregation, aggregation_config={}, ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, unit=unit, description=description) def add(self, amount, attributes=None): pass @@ -72,7 +112,13 @@ def add(self, amount, attributes=None): class ObservableGauge(ObservableGauge): def __init__( - self, aggregation=LastValueAggregation, aggregation_config={} + self, + name, + callback, + unit="", + description="", + aggregation=LastValueAggregation, + aggregation_config={}, ): self._aggregation = aggregation(**aggregation_config) - super().__init__() + super().__init__(name, callback, unit=unit, description=description) From 49e82b6a30be506409f0f3cc88c2756e09176840 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 14:03:29 +0200 Subject: [PATCH 25/48] Add proto to setup.cfg --- opentelemetry-sdk/setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-sdk/setup.cfg b/opentelemetry-sdk/setup.cfg index 92c3ddbcef5..fdbd0202e92 100644 --- a/opentelemetry-sdk/setup.cfg +++ b/opentelemetry-sdk/setup.cfg @@ -45,6 +45,7 @@ include_package_data = True install_requires = opentelemetry-api == 1.7.1 opentelemetry-semantic-conventions == 0.26b1 + opentelemetry-proto == 1.7.1 [options.packages.find] where = src From a58e1f97817b1b5b8a7ee1fe9f441e0ff95a538e Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Mon, 25 Oct 2021 14:24:02 +0200 Subject: [PATCH 26/48] Add timestamp for last value --- .../src/opentelemetry/sdk/metrics/aggregation.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py index 1a592db1a84..066847d9ad9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py @@ -20,6 +20,7 @@ AGGREGATION_TEMPORALITY_CUMULATIVE, AGGREGATION_TEMPORALITY_DELTA, ) +from opentelemetry.util._time import _time_ns class Aggregation(ABC): @@ -77,8 +78,13 @@ class LastValueAggregation(Aggregation): This aggregation collects data for the SDK sum metric point. """ + def __init__(self): + super().__init__() + self._timestamp = _time_ns() + def aggregate(self, value): self._value = value + self._timestamp = _time_ns() def collect(self): return self._value From deb696f9bad6720d1b513aba45f745f368e1e857 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 2 Nov 2021 18:07:15 +0100 Subject: [PATCH 27/48] Rename modules to be private --- .../src/opentelemetry/metrics/__init__.py | 472 ------------------ .../src/opentelemetry/metrics/instrument.py | 326 ------------ .../src/opentelemetry/metrics/measurement.py | 52 -- .../sdk/{metrics => _metrics}/aggregation.py | 0 .../sdk/{metrics => _metrics}/instrument.py | 0 .../src/opentelemetry/sdk/metrics/__init__.py | 216 -------- 6 files changed, 1066 deletions(-) delete mode 100644 opentelemetry-api/src/opentelemetry/metrics/__init__.py delete mode 100644 opentelemetry-api/src/opentelemetry/metrics/instrument.py delete mode 100644 opentelemetry-api/src/opentelemetry/metrics/measurement.py rename opentelemetry-sdk/src/opentelemetry/sdk/{metrics => _metrics}/aggregation.py (100%) rename opentelemetry-sdk/src/opentelemetry/sdk/{metrics => _metrics}/instrument.py (100%) delete mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py deleted file mode 100644 index c353a0e0032..00000000000 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ /dev/null @@ -1,472 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -# pylint: disable=too-many-ancestors -# type: ignore - -# FIXME enhance the documentation of this module -""" -This module provides abstract and concrete (but noop) classes that can be used -to generate metrics. -""" - - -from abc import ABC, abstractmethod -from logging import getLogger -from os import environ -from threading import Lock -from typing import List, Optional, cast - -from opentelemetry.environment_variables import OTEL_PYTHON_METER_PROVIDER -from opentelemetry.metrics.instrument import ( - Counter, - DefaultCounter, - DefaultHistogram, - DefaultObservableCounter, - DefaultObservableGauge, - DefaultObservableUpDownCounter, - DefaultUpDownCounter, - Histogram, - ObservableCounter, - ObservableGauge, - ObservableUpDownCounter, - UpDownCounter, - _ProxyCounter, - _ProxyHistogram, - _ProxyInstrument, - _ProxyObservableCounter, - _ProxyObservableGauge, - _ProxyObservableUpDownCounter, - _ProxyUpDownCounter, -) -from opentelemetry.util._once import Once -from opentelemetry.util._providers import _load_provider - -_logger = getLogger(__name__) - - -class MeterProvider(ABC): - @abstractmethod - def get_meter( - self, - name, - version=None, - schema_url=None, - ) -> "Meter": - if name is None or name == "": - _logger.warning("Invalid name: %s", name) - - -class _DefaultMeterProvider(MeterProvider): - def get_meter( - self, - name, - version=None, - schema_url=None, - ) -> "Meter": - super().get_meter(name, version=version, schema_url=schema_url) - return _DefaultMeter(name, version=version, schema_url=schema_url) - - -class _ProxyMeterProvider(MeterProvider): - def __init__(self) -> None: - self._lock = Lock() - self._meters: List[_ProxyMeter] = [] - self._real_meter_provider: Optional[MeterProvider] = None - - def get_meter( - self, - name, - version=None, - schema_url=None, - ) -> "Meter": - with self._lock: - if self._real_meter_provider is not None: - return self._real_meter_provider.get_meter( - name, version, schema_url - ) - - meter = _ProxyMeter(name, version=version, schema_url=schema_url) - self._meters.append(meter) - return meter - - def on_set_meter_provider(self, meter_provider: MeterProvider) -> None: - with self._lock: - self._real_meter_provider = meter_provider - for meter in self._meters: - meter.on_set_meter_provider(meter_provider) - - -class Meter(ABC): - def __init__(self, name, version=None, schema_url=None): - super().__init__() - self._name = name - self._version = version - self._schema_url = schema_url - self._instrument_names = set() - - @property - def name(self): - return self._name - - @property - def version(self): - return self._version - - @property - def schema_url(self): - return self._schema_url - - def _secure_instrument_name(self, name): - name = name.lower() - - if name in self._instrument_names: - _logger.error("Instrument name %s has been used already", name) - - else: - self._instrument_names.add(name) - - @abstractmethod - def create_counter(self, name, unit="", description="") -> Counter: - self._secure_instrument_name(name) - - @abstractmethod - def create_up_down_counter( - self, name, unit="", description="" - ) -> UpDownCounter: - self._secure_instrument_name(name) - - @abstractmethod - def create_observable_counter( - self, name, callback, unit="", description="" - ) -> ObservableCounter: - """Creates an observable counter instrument - - An observable counter observes a monotonically increasing count by - calling a provided callback which returns multiple - :class:`~opentelemetry.metrics.measurement.Measurement`. - - For example, an observable counter could be used to report system CPU - time periodically. Here is a basic implementation:: - - def cpu_time_callback() -> Iterable[Measurement]: - measurements = [] - with open("/proc/stat") as procstat: - procstat.readline() # skip the first line - for line in procstat: - if not line.startswith("cpu"): break - cpu, *states = line.split() - measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) - measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) - measurements.append(Measurement(int(states[2]) // 100, {"cpu": cpu, "state": "system"})) - # ... other states - return measurements - - meter.create_observable_counter( - "system.cpu.time", - callback=cpu_time_callback, - unit="s", - description="CPU time" - ) - - To reduce memory usage, you can use generator callbacks instead of - building the full list:: - - def cpu_time_callback() -> Iterable[Measurement]: - with open("/proc/stat") as procstat: - procstat.readline() # skip the first line - for line in procstat: - if not line.startswith("cpu"): break - cpu, *states = line.split() - yield Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"}) - yield Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"}) - # ... other states - - Alternatively, you can pass a generator directly instead of a callback, - which should return iterables of - :class:`~opentelemetry.metrics.measurement.Measurement`:: - - def cpu_time_callback(states_to_include: set[str]) -> Iterable[Iterable[Measurement]]: - while True: - measurements = [] - with open("/proc/stat") as procstat: - procstat.readline() # skip the first line - for line in procstat: - if not line.startswith("cpu"): break - cpu, *states = line.split() - if "user" in states_to_include: - measurements.append(Measurement(int(states[0]) // 100, {"cpu": cpu, "state": "user"})) - if "nice" in states_to_include: - measurements.append(Measurement(int(states[1]) // 100, {"cpu": cpu, "state": "nice"})) - # ... other states - yield measurements - - meter.create_observable_counter( - "system.cpu.time", - callback=cpu_time_callback({"user", "system"}), - unit="s", - description="CPU time" - ) - - Args: - name: The name of the instrument to be created - callback: A callback that returns an iterable of - :class:`~opentelemetry.metrics.measurement.Measurement`. - Alternatively, can be a generator that yields iterables of - :class:`~opentelemetry.metrics.measurement.Measurement`. - unit: The unit for measurements this instrument reports. For - example, ``By`` for bytes. UCUM units are recommended. - description: A description for this instrument and what it measures. - """ - - self._secure_instrument_name(name) - - @abstractmethod - def create_histogram(self, name, unit="", description="") -> Histogram: - self._secure_instrument_name(name) - - @abstractmethod - def create_observable_gauge( - self, name, callback, unit="", description="" - ) -> ObservableGauge: - self._secure_instrument_name(name) - - @abstractmethod - def create_observable_up_down_counter( - self, name, callback, unit="", description="" - ) -> ObservableUpDownCounter: - self._secure_instrument_name(name) - - -class _ProxyMeter(Meter): - def __init__( - self, - name, - version=None, - schema_url=None, - ): - super().__init__(name, version=version, schema_url=schema_url) - self._lock = Lock() - self._instruments: List[_ProxyInstrument] = [] - self._real_meter: Optional[Meter] = None - - def on_set_meter_provider(self, meter_provider: MeterProvider) -> None: - """Called when a real meter provider is set on the creating _ProxyMeterProvider - - Creates a real backing meter for this instance and notifies all created - instruments so they can create real backing instruments. - """ - real_meter = meter_provider.get_meter( - self._name, self._version, self._schema_url - ) - - with self._lock: - self._real_meter = real_meter - # notify all proxy instruments of the new meter so they can create - # real instruments to back themselves - for instrument in self._instruments: - instrument.on_meter_set(real_meter) - - def create_counter(self, name, unit="", description="") -> Counter: - with self._lock: - if self._real_meter: - return self._real_meter.create_counter(name, unit, description) - proxy = _ProxyCounter(name, unit, description) - self._instruments.append(proxy) - return proxy - - def create_up_down_counter( - self, name, unit="", description="" - ) -> UpDownCounter: - with self._lock: - if self._real_meter: - return self._real_meter.create_up_down_counter( - name, unit, description - ) - proxy = _ProxyUpDownCounter(name, unit, description) - self._instruments.append(proxy) - return proxy - - def create_observable_counter( - self, name, callback, unit="", description="" - ) -> ObservableCounter: - with self._lock: - if self._real_meter: - return self._real_meter.create_observable_counter( - name, callback, unit, description - ) - proxy = _ProxyObservableCounter( - name, callback, unit=unit, description=description - ) - self._instruments.append(proxy) - return proxy - - def create_histogram(self, name, unit="", description="") -> Histogram: - with self._lock: - if self._real_meter: - return self._real_meter.create_histogram( - name, unit, description - ) - proxy = _ProxyHistogram(name, unit, description) - self._instruments.append(proxy) - return proxy - - def create_observable_gauge( - self, name, callback, unit="", description="" - ) -> ObservableGauge: - with self._lock: - if self._real_meter: - return self._real_meter.create_observable_gauge( - name, callback, unit, description - ) - proxy = _ProxyObservableGauge( - name, callback, unit=unit, description=description - ) - self._instruments.append(proxy) - return proxy - - def create_observable_up_down_counter( - self, name, callback, unit="", description="" - ) -> ObservableUpDownCounter: - with self._lock: - if self._real_meter: - return self._real_meter.create_observable_up_down_counter( - name, - callback, - unit, - description, - ) - proxy = _ProxyObservableUpDownCounter( - name, callback, unit=unit, description=description - ) - self._instruments.append(proxy) - return proxy - - -class _DefaultMeter(Meter): - def create_counter(self, name, unit="", description="") -> Counter: - super().create_counter(name, unit=unit, description=description) - return DefaultCounter(name, unit=unit, description=description) - - def create_up_down_counter( - self, name, unit="", description="" - ) -> UpDownCounter: - super().create_up_down_counter( - name, unit=unit, description=description - ) - return DefaultUpDownCounter(name, unit=unit, description=description) - - def create_observable_counter( - self, name, callback, unit="", description="" - ) -> ObservableCounter: - super().create_observable_counter( - name, callback, unit=unit, description=description - ) - return DefaultObservableCounter( - name, - callback, - unit=unit, - description=description, - ) - - def create_histogram(self, name, unit="", description="") -> Histogram: - super().create_histogram(name, unit=unit, description=description) - return DefaultHistogram(name, unit=unit, description=description) - - def create_observable_gauge( - self, name, callback, unit="", description="" - ) -> ObservableGauge: - super().create_observable_gauge( - name, callback, unit=unit, description=description - ) - return DefaultObservableGauge( - name, - callback, - unit=unit, - description=description, - ) - - def create_observable_up_down_counter( - self, name, callback, unit="", description="" - ) -> ObservableUpDownCounter: - super().create_observable_up_down_counter( - name, callback, unit=unit, description=description - ) - return DefaultObservableUpDownCounter( - name, - callback, - unit=unit, - description=description, - ) - - -_METER_PROVIDER_SET_ONCE = Once() -_METER_PROVIDER: Optional[MeterProvider] = None -_PROXY_METER_PROVIDER = _ProxyMeterProvider() - - -def get_meter( - name: str, - version: str = "", - meter_provider: Optional[MeterProvider] = None, -) -> "Meter": - """Returns a `Meter` for use by the given instrumentation library. - - This function is a convenience wrapper for - opentelemetry.trace.MeterProvider.get_meter. - - If meter_provider is omitted the current configured one is used. - """ - if meter_provider is None: - meter_provider = get_meter_provider() - return meter_provider.get_meter(name, version) - - -def _set_meter_provider(meter_provider: MeterProvider, log: bool) -> None: - def set_mp() -> None: - global _METER_PROVIDER # pylint: disable=global-statement - _METER_PROVIDER = meter_provider - - # gives all proxies real instruments off the newly set meter provider - _PROXY_METER_PROVIDER.on_set_meter_provider(meter_provider) - - did_set = _METER_PROVIDER_SET_ONCE.do_once(set_mp) - - if log and not did_set: - _logger.warning("Overriding of current MeterProvider is not allowed") - - -def set_meter_provider(meter_provider: MeterProvider) -> None: - """Sets the current global :class:`~.MeterProvider` object. - - This can only be done once, a warning will be logged if any furter attempt - is made. - """ - _set_meter_provider(meter_provider, log=True) - - -def get_meter_provider() -> MeterProvider: - """Gets the current global :class:`~.MeterProvider` object.""" - - if _METER_PROVIDER is None: - if OTEL_PYTHON_METER_PROVIDER not in environ.keys(): - return _PROXY_METER_PROVIDER - - meter_provider: MeterProvider = _load_provider( - OTEL_PYTHON_METER_PROVIDER, "meter_provider" - ) - _set_meter_provider(meter_provider, log=False) - - # _METER_PROVIDER will have been set by one thread - return cast("MeterProvider", _METER_PROVIDER) diff --git a/opentelemetry-api/src/opentelemetry/metrics/instrument.py b/opentelemetry-api/src/opentelemetry/metrics/instrument.py deleted file mode 100644 index 7ff4de4b6eb..00000000000 --- a/opentelemetry-api/src/opentelemetry/metrics/instrument.py +++ /dev/null @@ -1,326 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -# pylint: disable=too-many-ancestors - -# type: ignore - -from abc import ABC, abstractmethod -from collections import abc as collections_abc -from logging import getLogger -from re import compile as compile_ -from typing import Callable, Generator, Iterable, Union - -from opentelemetry.metrics.measurement import Measurement - -_TInstrumentCallback = Callable[[], Iterable[Measurement]] -_TInstrumentCallbackGenerator = Generator[Iterable[Measurement], None, None] -TCallback = Union[_TInstrumentCallback, _TInstrumentCallbackGenerator] -InstrumentT = TypeVar("InstrumentT", bound="Instrument") - - -_logger = getLogger(__name__) - - -class Instrument(ABC): - - _name_regex = compile_(r"[a-zA-Z][-.\w]{0,62}") - - @property - def name(self): - return self._name - - @property - def unit(self): - return self._unit - - @property - def description(self): - return self._description - - @abstractmethod - def __init__(self, name, unit="", description=""): - - if name is None or self._name_regex.fullmatch(name) is None: - _logger.error("Invalid instrument name %s", name) - - else: - self._name = name - - if unit is None: - self._unit = "" - elif len(unit) > 63: - _logger.error("unit must be 63 characters or shorter") - - elif any(ord(character) > 127 for character in unit): - _logger.error("unit must only contain ASCII characters") - else: - self._unit = unit - - if description is None: - description = "" - - self._description = description - - -class _ProxyInstrument(ABC, Generic[InstrumentT]): - def __init__(self, name, unit, description) -> None: - self._name = name - self._unit = unit - self._description = description - self._real_instrument: Optional[InstrumentT] = None - - def on_meter_set(self, meter: "metrics.Meter") -> None: - """Called when a real meter is set on the creating _ProxyMeter""" - - # We don't need any locking on proxy instruments because it's OK if some - # measurements get dropped while a real backing instrument is being - # created. - self._real_instrument = self._create_real_instrument(meter) - - @abstractmethod - def _create_real_instrument(self, meter: "metrics.Meter") -> InstrumentT: - """Create an instance of the real instrument. Implement this.""" - - -class _ProxyAsynchronousInstrument(_ProxyInstrument[InstrumentT]): - def __init__(self, name, callback, unit, description) -> None: - super().__init__(name, unit, description) - self._callback = callback - - -class Synchronous(Instrument): - pass - - -class Asynchronous(Instrument): - @abstractmethod - def __init__( - self, - name, - callback: TCallback, - *args, - unit="", - description="", - **kwargs - ): - super().__init__( - name, *args, unit=unit, description=description, **kwargs - ) - - if isinstance(callback, collections_abc.Callable): - self._callback = callback - elif isinstance(callback, collections_abc.Generator): - self._callback = self._wrap_generator_callback(callback) - else: - _logger.error("callback must be a callable or generator") - - def _wrap_generator_callback( - self, - generator_callback: _TInstrumentCallbackGenerator, - ) -> _TInstrumentCallback: - """Wraps a generator style callback into a callable one""" - has_items = True - - def inner() -> Iterable[Measurement]: - nonlocal has_items - if not has_items: - return [] - - try: - return next(generator_callback) - except StopIteration: - has_items = False - _logger.error( - "callback generator for instrument %s ran out of measurements", - self._name, - ) - return [] - - return inner - - def callback(self): - measurements = self._callback() - if not isinstance(measurements, collections_abc.Iterable): - _logger.error( - "Callback must return an iterable of Measurement, got %s", - type(measurements), - ) - return - for measurement in measurements: - if not isinstance(measurement, Measurement): - _logger.error( - "Callback must return an iterable of Measurement, " - "iterable contained type %s", - type(measurement), - ) - yield measurement - - -class _Adding(Instrument): - pass - - -class _Grouping(Instrument): - pass - - -class _Monotonic(_Adding): - pass - - -class _NonMonotonic(_Adding): - pass - - -class Counter(_Monotonic, Synchronous): - @abstractmethod - def add(self, amount, attributes=None): - if amount < 0: - _logger.error("Amount must be non-negative") - - -class DefaultCounter(Counter): - def __init__(self, name, unit="", description=""): - super().__init__(name, unit=unit, description=description) - - def add(self, amount, attributes=None): - return super().add(amount, attributes=attributes) - - -class _ProxyCounter(_ProxyInstrument[Counter], Counter): - def add(self, amount, attributes=None): - if self._real_instrument: - self._real_instrument.add(amount, attributes) - - def _create_real_instrument(self, meter: "metrics.Meter") -> Counter: - return meter.create_counter(self._name, self._unit, self._description) - - -class UpDownCounter(_NonMonotonic, Synchronous): - @abstractmethod - def add(self, amount, attributes=None): - pass - - -class DefaultUpDownCounter(UpDownCounter): - def __init__(self, name, unit="", description=""): - super().__init__(name, unit=unit, description=description) - - def add(self, amount, attributes=None): - return super().add(amount, attributes=attributes) - - -class _ProxyUpDownCounter(_ProxyInstrument[UpDownCounter], UpDownCounter): - def add(self, amount, attributes=None): - if self._real_instrument: - self._real_instrument.add(amount, attributes) - - def _create_real_instrument(self, meter: "metrics.Meter") -> UpDownCounter: - return meter.create_up_down_counter( - self._name, self._unit, self._description - ) - - -class ObservableCounter(_Monotonic, Asynchronous): - def callback(self): - measurements = super().callback() - - for measurement in measurements: - if measurement.value < 0: - _logger.error("Amount must be non-negative") - yield measurement - - -class DefaultObservableCounter(ObservableCounter): - def __init__(self, name, callback, unit="", description=""): - super().__init__(name, callback, unit=unit, description=description) - - -class _ProxyObservableCounter( - _ProxyAsynchronousInstrument[ObservableCounter], ObservableCounter -): - def _create_real_instrument( - self, meter: "metrics.Meter" - ) -> ObservableCounter: - return meter.create_observable_counter( - self._name, self._callback, self._unit, self._description - ) - - -class ObservableUpDownCounter(_NonMonotonic, Asynchronous): - pass - - -class DefaultObservableUpDownCounter(ObservableUpDownCounter): - def __init__(self, name, callback, unit="", description=""): - super().__init__(name, callback, unit=unit, description=description) - - -class _ProxyObservableUpDownCounter( - _ProxyAsynchronousInstrument[ObservableUpDownCounter], - ObservableUpDownCounter, -): - def _create_real_instrument( - self, meter: "metrics.Meter" - ) -> ObservableUpDownCounter: - return meter.create_observable_up_down_counter( - self._name, self._callback, self._unit, self._description - ) - - -class Histogram(_Grouping, Synchronous): - @abstractmethod - def record(self, amount, attributes=None): - pass - - -class DefaultHistogram(Histogram): - def __init__(self, name, unit="", description=""): - super().__init__(name, unit=unit, description=description) - - def record(self, amount, attributes=None): - return super().record(amount, attributes=attributes) - - -class _ProxyHistogram(_ProxyInstrument[Histogram], Histogram): - def record(self, amount, attributes=None): - if self._real_instrument: - self._real_instrument.record(amount, attributes) - - def _create_real_instrument(self, meter: "metrics.Meter") -> Histogram: - return meter.create_histogram( - self._name, self._unit, self._description - ) - - -class ObservableGauge(_Grouping, Asynchronous): - pass - - -class DefaultObservableGauge(ObservableGauge): - def __init__(self, name, callback, unit="", description=""): - super().__init__(name, callback, unit=unit, description=description) - - -class _ProxyObservableGauge( - _ProxyAsynchronousInstrument[ObservableGauge], - ObservableGauge, -): - def _create_real_instrument( - self, meter: "metrics.Meter" - ) -> ObservableGauge: - return meter.create_observable_gauge( - self._name, self._callback, self._unit, self._description - ) diff --git a/opentelemetry-api/src/opentelemetry/metrics/measurement.py b/opentelemetry-api/src/opentelemetry/metrics/measurement.py deleted file mode 100644 index e8fd9725bb8..00000000000 --- a/opentelemetry-api/src/opentelemetry/metrics/measurement.py +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -from typing import Union - -from opentelemetry.util.types import Attributes - - -class Measurement: - """A measurement observed in an asynchronous instrument - - Return/yield instances of this class from asynchronous instrument callbacks. - - Args: - value: The float or int measured value - attributes: The measurement's attributes - """ - - def __init__( - self, value: Union[int, float], attributes: Attributes = None - ) -> None: - self._value = value - self._attributes = attributes - - @property - def value(self) -> Union[float, int]: - return self._value - - @property - def attributes(self) -> Attributes: - return self._attributes - - def __eq__(self, other: object) -> bool: - return ( - isinstance(other, Measurement) - and self.value == other.value - and self.attributes == other.attributes - ) - - def __repr__(self) -> str: - return f"Measurement(value={self.value}, attributes={self.attributes})" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py similarity index 100% rename from opentelemetry-sdk/src/opentelemetry/sdk/metrics/aggregation.py rename to opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py similarity index 100% rename from opentelemetry-sdk/src/opentelemetry/sdk/metrics/instrument.py rename to opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py deleted file mode 100644 index 384e52512a9..00000000000 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ /dev/null @@ -1,216 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - -# pylint: disable=function-redefined,too-many-ancestors - -from abc import ABC, abstractmethod -from atexit import register, unregister -from logging import getLogger -from typing import Optional - -from opentelemetry.metrics import Meter as APIMeter -from opentelemetry.metrics import MeterProvider as APIMeterProvider -from opentelemetry.metrics import _DefaultMeter -from opentelemetry.metrics.instrument import ( - Counter, - Histogram, - ObservableCounter, - ObservableGauge, - ObservableUpDownCounter, - UpDownCounter, -) -from opentelemetry.sdk.resources import Resource -from opentelemetry.sdk.util.instrumentation import InstrumentationInfo - -_logger = getLogger(__name__) - - -class Meter(APIMeter): - def __init__( - self, - instrumentation_info: InstrumentationInfo, - meter_provider: APIMeterProvider, - ): - super().__init__(instrumentation_info) - self._instrumentation_info = instrumentation_info - self._meter_provider = meter_provider - - def create_counter(self, name, unit=None, description=None) -> Counter: - # FIXME implement this method - pass - - def create_up_down_counter( - self, name, unit=None, description=None - ) -> UpDownCounter: - # FIXME implement this method - pass - - def create_observable_counter( - self, name, callback, unit=None, description=None - ) -> ObservableCounter: - # FIXME implement this method - pass - - def create_histogram(self, name, unit=None, description=None) -> Histogram: - # FIXME implement this method - pass - - def create_observable_gauge( - self, name, callback, unit=None, description=None - ) -> ObservableGauge: - # FIXME implement this method - pass - - def create_observable_up_down_counter( - self, name, callback, unit=None, description=None - ) -> ObservableUpDownCounter: - # FIXME implement this method - pass - - -class MeterProvider(APIMeterProvider): - """See `opentelemetry.metrics.MeterProvider`.""" - - def __init__( - self, - resource: Resource = Resource.create({}), - shutdown_on_exit: bool = True, - ): - self._resource = resource - self._atexit_handler = None - - if shutdown_on_exit: - self._atexit_handler = register(self.shutdown) - - self._metric_readers = [] - self._metric_exporters = [] - self._views = [] - self._shutdown = False - - def get_meter( - self, - name: str, - version: Optional[str] = None, - schema_url: Optional[str] = None, - ) -> Meter: - - if self._shutdown: - _logger.warning( - "A shutdown `MeterProvider` can not provide a `Meter`" - ) - return _DefaultMeter(name, version=version, schema_url=schema_url) - - return Meter(InstrumentationInfo(name, version, schema_url), self) - - def shutdown(self): - # FIXME implement a timeout - - if self._shutdown: - _logger.warning("shutdown can only be called once") - return False - - result = True - - for metric_reader in self._metric_readers: - result = result and metric_reader.shutdown() - - for metric_exporter in self._metric_exporters: - result = result and metric_exporter.shutdown() - - self._shutdown = True - - if self._atexit_handler is not None: - unregister(self._atexit_handler) - self._atexit_handler = None - - return result - - def force_flush(self) -> bool: - - # FIXME implement a timeout - - metric_reader_result = True - metric_exporter_result = True - - for metric_reader in self._metric_readers: - metric_reader_result = ( - metric_reader_result and metric_reader.force_flush() - ) - - if not metric_reader_result: - _logger.warning("Unable to force flush all metric readers") - - for metric_exporter in self._metric_exporters: - metric_exporter_result = ( - metric_exporter_result and metric_exporter.force_flush() - ) - - if not metric_exporter_result: - _logger.warning("Unable to force flush all metric exporters") - - return metric_reader_result and metric_exporter_result - - def register_metric_reader(self, metric_reader: "MetricReader") -> None: - # FIXME protect this method against race conditions - self._metric_readers.append(metric_reader) - - def register_metric_exporter( - self, metric_exporter: "MetricExporter" - ) -> None: - # FIXME protect this method against race conditions - self._metric_exporters.append(metric_exporter) - - def register_view(self, view: "View") -> None: - # FIXME protect this method against race conditions - self._views.append(view) - - -class MetricReader(ABC): - def __init__(self): - self._shutdown = False - - @abstractmethod - def collect(self): - pass - - def shutdown(self): - # FIXME this will need a Once wrapper - self._shutdown = True - - -class MetricExporter(ABC): - def __init__(self): - self._shutdown = False - - @abstractmethod - def export(self): - pass - - def shutdown(self): - # FIXME this will need a Once wrapper - self._shutdown = True - - -class View: - pass - - -class ConsoleMetricExporter(MetricExporter): - def export(self): - pass - - -class SDKMetricReader(MetricReader): - def collect(self): - pass From 904c7d9f685dfc2a35571bdbfe75d38aa070d110 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 2 Nov 2021 18:43:02 +0100 Subject: [PATCH 28/48] Fix paths --- .../src/opentelemetry/sdk/_metrics/instrument.py | 4 ++-- opentelemetry-sdk/tests/metrics/test_aggregation.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 15f240b698e..7ad885afc61 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -19,7 +19,7 @@ # all instances. Implementations of these classes must not make any change to # this default dictionary in __init__. -from opentelemetry.metrics.instrument import ( +from opentelemetry._metrics.instrument import ( Counter, Histogram, ObservableCounter, @@ -27,7 +27,7 @@ ObservableUpDownCounter, UpDownCounter, ) -from opentelemetry.sdk.metrics.aggregation import ( +from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, SumAggregation, diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 29a4dde5326..e4d36f203cc 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -20,7 +20,7 @@ AGGREGATION_TEMPORALITY_CUMULATIVE, AGGREGATION_TEMPORALITY_DELTA, ) -from opentelemetry.sdk.metrics.aggregation import ( +from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, NoneAggregation, From 50e708ae7cee35cda4537ed73a3a75248e841083 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 10 Nov 2021 15:36:28 -0600 Subject: [PATCH 29/48] Set value in concrete classes init --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 066847d9ad9..39ee4ea02f0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -24,9 +24,6 @@ class Aggregation(ABC): - def __init__(self): - self._value = 0 - @property def value(self): return self._value @@ -45,6 +42,10 @@ class NoneAggregation(Aggregation): This aggregation drops all instrument measurements. """ + def __init__(self): + super().__init__() + self._value = None + def aggregate(self, value): pass @@ -59,6 +60,7 @@ class SumAggregation(Aggregation): def __init__(self, temporality=AGGREGATION_TEMPORALITY_CUMULATIVE): super().__init__() + self._value = 0 self._temporality = temporality def aggregate(self, value): @@ -80,6 +82,7 @@ class LastValueAggregation(Aggregation): def __init__(self): super().__init__() + self._value = None self._timestamp = _time_ns() def aggregate(self, value): From b157bb72eb21f15092d3ac906aa5e23955523dce Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 10 Nov 2021 18:29:38 -0600 Subject: [PATCH 30/48] Fix test --- opentelemetry-sdk/tests/metrics/test_aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index e4d36f203cc..9bdc87a9eea 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -40,7 +40,7 @@ def test_aggregate(self): none_aggregation.aggregate(2) none_aggregation.aggregate(3) - self.assertIs(none_aggregation.value, 0) + self.assertIs(none_aggregation.value, None) class TestSumAggregation(TestCase): From 3d5a779bfaeb596bf9d534cf29436c9547850591 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 10 Nov 2021 18:42:04 -0600 Subject: [PATCH 31/48] Fix lint --- opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 39ee4ea02f0..def87135f03 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -26,7 +26,7 @@ class Aggregation(ABC): @property def value(self): - return self._value + return self._value # pylint: disable=no-member @abstractmethod def aggregate(self, value): From f1b9529d5d41d43cf56d9ebd29117a13cba9e03a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 10 Nov 2021 19:08:06 -0600 Subject: [PATCH 32/48] Remove temporalities --- .../opentelemetry/sdk/_metrics/aggregation.py | 33 +--- .../tests/metrics/test_aggregation.py | 172 ------------------ 2 files changed, 3 insertions(+), 202 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index def87135f03..1ef5a9c699d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -16,10 +16,6 @@ from collections import OrderedDict from math import inf -from opentelemetry.proto.metrics.v1.metrics_pb2 import ( - AGGREGATION_TEMPORALITY_CUMULATIVE, - AGGREGATION_TEMPORALITY_DELTA, -) from opentelemetry.util._time import _time_ns @@ -32,9 +28,8 @@ def value(self): def aggregate(self, value): pass - @abstractmethod def collect(self): - pass + return self._value # pylint: disable=no-member class NoneAggregation(Aggregation): @@ -49,30 +44,19 @@ def __init__(self): def aggregate(self, value): pass - def collect(self): - return self._value - class SumAggregation(Aggregation): """ This aggregation collects data for the SDK sum metric point. """ - def __init__(self, temporality=AGGREGATION_TEMPORALITY_CUMULATIVE): + def __init__(self): super().__init__() self._value = 0 - self._temporality = temporality def aggregate(self, value): self._value = self._value + value - def collect(self): - value = self._value - if self._temporality == AGGREGATION_TEMPORALITY_DELTA: - self._value = 0 - - return value - class LastValueAggregation(Aggregation): @@ -89,9 +73,6 @@ def aggregate(self, value): self._value = value self._timestamp = _time_ns() - def collect(self): - return self._value - class ExplicitBucketHistogramAggregation(Aggregation): @@ -101,11 +82,10 @@ class ExplicitBucketHistogramAggregation(Aggregation): def __init__( self, - temporality=AGGREGATION_TEMPORALITY_CUMULATIVE, boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf), + record_min_max=True, ): super().__init__() - self._temporality = temporality self._boundaries = boundaries self._value = OrderedDict([(key, 0) for key in boundaries]) @@ -116,10 +96,3 @@ def aggregate(self, value): self._value[key] = self._value[key] + value break - - def collect(self): - value = self._value - if self._temporality == AGGREGATION_TEMPORALITY_DELTA: - self._value = OrderedDict([(key, 0) for key in self._boundaries]) - - return value diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 9bdc87a9eea..3a092529360 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -16,10 +16,6 @@ from math import inf from unittest import TestCase -from opentelemetry.proto.metrics.v1.metrics_pb2 import ( - AGGREGATION_TEMPORALITY_CUMULATIVE, - AGGREGATION_TEMPORALITY_DELTA, -) from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, @@ -44,23 +40,6 @@ def test_aggregate(self): class TestSumAggregation(TestCase): - def test_default_temporality(self): - """ - `SumAggregation` default temporality is - `AGGREGATION_TEMPORALITY_CUMULATIVE`. - """ - - sum_aggregation = SumAggregation() - self.assertEqual( - sum_aggregation._temporality, AGGREGATION_TEMPORALITY_CUMULATIVE - ) - sum_aggregation = SumAggregation( - temporality=AGGREGATION_TEMPORALITY_DELTA - ) - self.assertEqual( - sum_aggregation._temporality, AGGREGATION_TEMPORALITY_DELTA - ) - def test_aggregate(self): """ `SumAggregation` collects data for sum metric points @@ -74,52 +53,6 @@ def test_aggregate(self): self.assertEqual(sum_aggregation.value, 6) - def test_delta_temporality(self): - """ - `SumAggregation` supports delta temporality - """ - - sum_aggregation = SumAggregation(AGGREGATION_TEMPORALITY_DELTA) - - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(2) - sum_aggregation.aggregate(3) - - self.assertEqual(sum_aggregation.value, 6) - self.assertEqual(sum_aggregation.collect(), 6) - self.assertEqual(sum_aggregation.value, 0) - - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(2) - sum_aggregation.aggregate(3) - - self.assertEqual(sum_aggregation.value, 6) - self.assertEqual(sum_aggregation.collect(), 6) - self.assertEqual(sum_aggregation.value, 0) - - def test_cumulative_temporality(self): - """ - `SumAggregation` supports cumulative temporality - """ - - sum_aggregation = SumAggregation(AGGREGATION_TEMPORALITY_CUMULATIVE) - - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(2) - sum_aggregation.aggregate(3) - - self.assertEqual(sum_aggregation.value, 6) - self.assertEqual(sum_aggregation.collect(), 6) - self.assertEqual(sum_aggregation.value, 6) - - sum_aggregation.aggregate(1) - sum_aggregation.aggregate(2) - sum_aggregation.aggregate(3) - - self.assertEqual(sum_aggregation.value, 12) - self.assertEqual(sum_aggregation.collect(), 12) - self.assertEqual(sum_aggregation.value, 12) - class TestLastValueAggregation(TestCase): def test_aggregate(self): @@ -141,29 +74,6 @@ def test_aggregate(self): class TestExplicitBucketHistogramAggregation(TestCase): - def test_default_temporality(self): - """ - `ExplicitBucketHistogramAggregation` default temporality is - `AGGREGATION_TEMPORALITY_CUMULATIVE`. - """ - - explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation() - ) - self.assertEqual( - explicit_bucket_histogram_aggregation._temporality, - AGGREGATION_TEMPORALITY_CUMULATIVE, - ) - explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation( - temporality=AGGREGATION_TEMPORALITY_DELTA - ) - ) - self.assertEqual( - explicit_bucket_histogram_aggregation._temporality, - AGGREGATION_TEMPORALITY_DELTA, - ) - def test_aggregate(self): """ `ExplicitBucketHistogramAggregation` collects data for explicit_bucket_histogram metric points @@ -185,85 +95,3 @@ def test_aggregate(self): self.assertEqual( explicit_bucket_histogram_aggregation.value[inf], 9999 ) - - def test_delta_temporality(self): - """ - `ExplicitBucketHistogramAggregation` supports delta temporality - """ - - explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation(AGGREGATION_TEMPORALITY_DELTA) - ) - - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) - - result = explicit_bucket_histogram_aggregation.collect() - - self.assertEqual(result[0], -1) - self.assertEqual(result[5], 2) - self.assertEqual(result[10], 15) - self.assertEqual(result[inf], 9999) - - self.assertEqual(explicit_bucket_histogram_aggregation.value[0], 0) - self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 0) - self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 0) - self.assertEqual(explicit_bucket_histogram_aggregation.value[inf], 0) - - def test_cumulative_temporality(self): - """ - `ExplicitBucketHistogramAggregation` supports cumulative temporality - """ - - explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation( - AGGREGATION_TEMPORALITY_CUMULATIVE - ) - ) - - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) - - result = explicit_bucket_histogram_aggregation.collect() - - self.assertEqual(result[0], -1) - self.assertEqual(result[5], 2) - self.assertEqual(result[10], 15) - self.assertEqual(result[inf], 9999) - - self.assertEqual(explicit_bucket_histogram_aggregation.value[0], -1) - self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2) - self.assertEqual(explicit_bucket_histogram_aggregation.value[10], 15) - self.assertEqual( - explicit_bucket_histogram_aggregation.value[inf], 9999 - ) - - explicit_bucket_histogram_aggregation.aggregate(-1) - explicit_bucket_histogram_aggregation.aggregate(2) - explicit_bucket_histogram_aggregation.aggregate(7) - explicit_bucket_histogram_aggregation.aggregate(8) - explicit_bucket_histogram_aggregation.aggregate(9999) - - result = explicit_bucket_histogram_aggregation.collect() - - self.assertEqual(result[0], -1 * 2) - self.assertEqual(result[5], 2 * 2) - self.assertEqual(result[10], 15 * 2) - self.assertEqual(result[inf], 9999 * 2) - - self.assertEqual( - explicit_bucket_histogram_aggregation.value[0], -1 * 2 - ) - self.assertEqual(explicit_bucket_histogram_aggregation.value[5], 2 * 2) - self.assertEqual( - explicit_bucket_histogram_aggregation.value[10], 15 * 2 - ) - self.assertEqual( - explicit_bucket_histogram_aggregation.value[inf], 9999 * 2 - ) From 46dad807bbbbea8c799a6c64ceb8c49da0598300 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 11 Nov 2021 09:03:19 -0600 Subject: [PATCH 33/48] Use frozenset as key --- .../opentelemetry/sdk/_metrics/instrument.py | 98 ++++++++++++++----- 1 file changed, 74 insertions(+), 24 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 7ad885afc61..b671439b0b9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -34,23 +34,36 @@ ) -class Counter(Counter): +class _Instrument: def __init__( self, name, unit="", description="", - aggregation=SumAggregation, + aggregation=None, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) + self._attributes_aggregations = {} + self._aggregation = aggregation + self._aggregation_config = aggregation_config + aggregation(**aggregation_config) super().__init__(name, unit=unit, description=description) + +class _Synchronous(_Instrument): + def add(self, amount, attributes=None): - pass + + attributes = frozenset(attributes.items()) + if attributes not in self._attributes_aggregations.keys(): + + self._attributes_aggregations[attributes] = ( + self._aggregation(**self._aggregation_config) + ) + self._attributes_aggregators[attributes].aggregate(amount) -class UpDownCounter(UpDownCounter): +class Counter(_Synchronous, Counter): def __init__( self, name, @@ -59,14 +72,34 @@ def __init__( aggregation=SumAggregation, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) - super().__init__(name, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) - def add(self, amount, attributes=None): - pass +class UpDownCounter(_Synchronous, UpDownCounter): + def __init__( + self, + name, + unit="", + description="", + aggregation=SumAggregation, + aggregation_config={}, + ): + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) -class ObservableCounter(ObservableCounter): + +class ObservableCounter(_Instrument, ObservableCounter): def __init__( self, name, @@ -76,11 +109,16 @@ def __init__( aggregation=SumAggregation, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) - super().__init__(name, callback, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) -class ObservableUpDownCounter(ObservableUpDownCounter): +class ObservableUpDownCounter(_Instrument, ObservableUpDownCounter): def __init__( self, name, @@ -90,11 +128,16 @@ def __init__( aggregation=SumAggregation, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) - super().__init__(name, callback, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) -class Histogram(Histogram): +class Histogram(_Synchronous, Histogram): def __init__( self, name, @@ -103,14 +146,16 @@ def __init__( aggregation=ExplicitBucketHistogramAggregation, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) - super().__init__(name, unit=unit, description=description) - - def add(self, amount, attributes=None): - pass + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) -class ObservableGauge(ObservableGauge): +class ObservableGauge(_Instrument, ObservableGauge): def __init__( self, name, @@ -120,5 +165,10 @@ def __init__( aggregation=LastValueAggregation, aggregation_config={}, ): - self._aggregation = aggregation(**aggregation_config) - super().__init__(name, callback, unit=unit, description=description) + super().__init__( + name, + unit=unit, + description=description, + aggregation=aggregation, + aggregation_config=aggregation_config + ) From 2c8e8935362b6188b5e22535a9fc0110ca26988c Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 11 Nov 2021 15:23:42 -0600 Subject: [PATCH 34/48] Test instruments --- .../opentelemetry/sdk/_metrics/instrument.py | 6 ++- .../tests/metrics/test_instrument.py | 48 +++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 opentelemetry-sdk/tests/metrics/test_instrument.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index b671439b0b9..59fc98e142b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -47,20 +47,22 @@ def __init__( self._aggregation = aggregation self._aggregation_config = aggregation_config aggregation(**aggregation_config) - super().__init__(name, unit=unit, description=description) class _Synchronous(_Instrument): def add(self, amount, attributes=None): + if attributes is None: + raise Exception("Missing attributes") + attributes = frozenset(attributes.items()) if attributes not in self._attributes_aggregations.keys(): self._attributes_aggregations[attributes] = ( self._aggregation(**self._aggregation_config) ) - self._attributes_aggregators[attributes].aggregate(amount) + self._attributes_aggregations[attributes].aggregate(amount) class Counter(_Synchronous, Counter): diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py new file mode 100644 index 00000000000..19a163c5739 --- /dev/null +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -0,0 +1,48 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + + +from unittest import TestCase + +from opentelemetry.sdk._metrics.instrument import ( + _Synchronous +) +from opentelemetry.sdk._metrics.aggregation import ( + SumAggregation +) + + +class Test_Synchronous(TestCase): + def test_add(self): + """ + Test that `_Synchronous.add` can handle multiple aggregations + """ + + synchronous = _Synchronous("synchronous", aggregation=SumAggregation) + + synchronous.add(1, {"name0": "value0"}) + synchronous.add(1, {"name1": "value1"}) + + self.assertIsInstance( + synchronous._attributes_aggregations[ + frozenset({("name0", "value0")}) + ], + SumAggregation + ) + self.assertIsInstance( + synchronous._attributes_aggregations[ + frozenset({("name1", "value1")}) + ], + SumAggregation + ) From 2d9e0c3e13d1d458b5bd231f16a770ad7d6f4664 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Sun, 14 Nov 2021 20:51:51 -0600 Subject: [PATCH 35/48] Handle min, max and sum in explicit bucket histogram aggregator --- .../opentelemetry/sdk/_metrics/aggregation.py | 51 ++++++++++++++++--- .../opentelemetry/sdk/_metrics/instrument.py | 19 ++++--- .../tests/metrics/test_aggregation.py | 45 ++++++++++++++-- .../tests/metrics/test_instrument.py | 12 ++--- tox.ini | 3 +- 5 files changed, 100 insertions(+), 30 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 1ef5a9c699d..100734b224b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -14,10 +14,14 @@ from abc import ABC, abstractmethod from collections import OrderedDict +from logging import getLogger from math import inf +from opentelemetry._metrics.instrument import _Monotonic from opentelemetry.util._time import _time_ns +_logger = getLogger(__name__) + class Aggregation(ABC): @property @@ -37,8 +41,7 @@ class NoneAggregation(Aggregation): This aggregation drops all instrument measurements. """ - def __init__(self): - super().__init__() + def __init__(self, instrument): self._value = None def aggregate(self, value): @@ -50,8 +53,7 @@ class SumAggregation(Aggregation): This aggregation collects data for the SDK sum metric point. """ - def __init__(self): - super().__init__() + def __init__(self, instrument): self._value = 0 def aggregate(self, value): @@ -64,8 +66,7 @@ class LastValueAggregation(Aggregation): This aggregation collects data for the SDK sum metric point. """ - def __init__(self): - super().__init__() + def __init__(self, instrument): self._value = None self._timestamp = _time_ns() @@ -82,14 +83,52 @@ class ExplicitBucketHistogramAggregation(Aggregation): def __init__( self, + instrument, + *args, boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf), record_min_max=True, ): super().__init__() self._boundaries = boundaries self._value = OrderedDict([(key, 0) for key in boundaries]) + self._min = inf + self._max = -inf + self._sum = 0 + self._instrument = instrument + self._record_min_max = record_min_max + + @property + def min(self): + if not self._record_min_max: + _logger.warning("Min is not being recorded") + + return self._min + + @property + def max(self): + if not self._record_min_max: + _logger.warning("Max is not being recorded") + + return self._max + + @property + def sum(self): + if isinstance(self._instrument, _Monotonic): + return self._sum + + _logger.warning( + "Sum is not filled out when the associated " + "instrument is not monotonic" + ) def aggregate(self, value): + if self._record_min_max: + self._min = min(self._min, value) + self._max = max(self._max, value) + + if isinstance(self._instrument, _Monotonic): + self._sum += value + for key in self._value.keys(): if value < key: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 59fc98e142b..0ef3232f1bd 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -46,11 +46,10 @@ def __init__( self._attributes_aggregations = {} self._aggregation = aggregation self._aggregation_config = aggregation_config - aggregation(**aggregation_config) + aggregation(self, **aggregation_config) class _Synchronous(_Instrument): - def add(self, amount, attributes=None): if attributes is None: @@ -59,8 +58,8 @@ def add(self, amount, attributes=None): attributes = frozenset(attributes.items()) if attributes not in self._attributes_aggregations.keys(): - self._attributes_aggregations[attributes] = ( - self._aggregation(**self._aggregation_config) + self._attributes_aggregations[attributes] = self._aggregation( + self, **self._aggregation_config ) self._attributes_aggregations[attributes].aggregate(amount) @@ -79,7 +78,7 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) @@ -97,7 +96,7 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) @@ -116,7 +115,7 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) @@ -135,7 +134,7 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) @@ -153,7 +152,7 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) @@ -172,5 +171,5 @@ def __init__( unit=unit, description=description, aggregation=aggregation, - aggregation_config=aggregation_config + aggregation_config=aggregation_config, ) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 3a092529360..42162bad803 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -13,8 +13,10 @@ # limitations under the License. +from logging import WARNING from math import inf from unittest import TestCase +from unittest.mock import Mock from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, @@ -30,7 +32,7 @@ def test_aggregate(self): `NoneAggregation` drops all measurements. """ - none_aggregation = NoneAggregation() + none_aggregation = NoneAggregation(Mock()) none_aggregation.aggregate(1) none_aggregation.aggregate(2) @@ -45,7 +47,7 @@ def test_aggregate(self): `SumAggregation` collects data for sum metric points """ - sum_aggregation = SumAggregation() + sum_aggregation = SumAggregation(Mock()) sum_aggregation.aggregate(1) sum_aggregation.aggregate(2) @@ -61,7 +63,7 @@ def test_aggregate(self): temporality """ - last_value_aggregation = LastValueAggregation() + last_value_aggregation = LastValueAggregation(Mock()) last_value_aggregation.aggregate(1) self.assertEqual(last_value_aggregation.value, 1) @@ -80,7 +82,7 @@ def test_aggregate(self): """ explicit_bucket_histogram_aggregation = ( - ExplicitBucketHistogramAggregation() + ExplicitBucketHistogramAggregation(Mock()) ) explicit_bucket_histogram_aggregation.aggregate(-1) @@ -95,3 +97,38 @@ def test_aggregate(self): self.assertEqual( explicit_bucket_histogram_aggregation.value[inf], 9999 ) + + def test_min_max(self): + """ + `record_min_max` indicates the aggregator to record the minimum and + maximum value in the population + """ + + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation(Mock()) + ) + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + self.assertEqual(explicit_bucket_histogram_aggregation.min, -1) + self.assertEqual(explicit_bucket_histogram_aggregation.max, 9999) + + explicit_bucket_histogram_aggregation = ( + ExplicitBucketHistogramAggregation(Mock(), record_min_max=False) + ) + + explicit_bucket_histogram_aggregation.aggregate(-1) + explicit_bucket_histogram_aggregation.aggregate(2) + explicit_bucket_histogram_aggregation.aggregate(7) + explicit_bucket_histogram_aggregation.aggregate(8) + explicit_bucket_histogram_aggregation.aggregate(9999) + + with self.assertLogs(level=WARNING): + self.assertEqual(explicit_bucket_histogram_aggregation.min, inf) + + with self.assertLogs(level=WARNING): + self.assertEqual(explicit_bucket_histogram_aggregation.max, -inf) diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py index 19a163c5739..3cbd564e6db 100644 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -15,12 +15,8 @@ from unittest import TestCase -from opentelemetry.sdk._metrics.instrument import ( - _Synchronous -) -from opentelemetry.sdk._metrics.aggregation import ( - SumAggregation -) +from opentelemetry.sdk._metrics.aggregation import SumAggregation +from opentelemetry.sdk._metrics.instrument import _Synchronous class Test_Synchronous(TestCase): @@ -38,11 +34,11 @@ def test_add(self): synchronous._attributes_aggregations[ frozenset({("name0", "value0")}) ], - SumAggregation + SumAggregation, ) self.assertIsInstance( synchronous._attributes_aggregations[ frozenset({("name1", "value1")}) ], - SumAggregation + SumAggregation, ) diff --git a/tox.ini b/tox.ini index 6e27ff132c0..2999b7fe434 100644 --- a/tox.ini +++ b/tox.ini @@ -99,7 +99,7 @@ changedir = exporter-zipkin-combined: exporter/opentelemetry-exporter-zipkin/tests exporter-zipkin-proto-http: exporter/opentelemetry-exporter-zipkin-proto-http/tests exporter-zipkin-json: exporter/opentelemetry-exporter-zipkin-json/tests - + propagator-b3: propagator/opentelemetry-propagator-b3/tests propagator-jaeger: propagator/opentelemetry-propagator-jaeger/tests @@ -111,7 +111,6 @@ commands_pre = opentelemetry: pip install {toxinidir}/opentelemetry-api {toxinidir}/opentelemetry-semantic-conventions {toxinidir}/opentelemetry-sdk {toxinidir}/tests/opentelemetry-test-utils protobuf: pip install {toxinidir}/opentelemetry-proto - sdk: pip install {toxinidir}/opentelemetry-proto getting-started: pip install requests==2.26.0 flask==2.0.1 getting-started: pip install -e "{env:CONTRIB_REPO}#egg=opentelemetry-util-http&subdirectory=util/opentelemetry-util-http" From 2f084f12514aa89afb8320ba0dc777970b686b95 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Sun, 14 Nov 2021 21:47:35 -0600 Subject: [PATCH 36/48] Add test for negative values --- opentelemetry-sdk/tests/metrics/test_aggregation.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 42162bad803..6a2814edaee 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -55,6 +55,14 @@ def test_aggregate(self): self.assertEqual(sum_aggregation.value, 6) + sum_aggregation = SumAggregation(Mock()) + + sum_aggregation.aggregate(1) + sum_aggregation.aggregate(-2) + sum_aggregation.aggregate(3) + + self.assertEqual(sum_aggregation.value, 2) + class TestLastValueAggregation(TestCase): def test_aggregate(self): From 641823c3f2a17e385e0dd8b87061c87d89c455a6 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 16 Nov 2021 15:15:35 -0600 Subject: [PATCH 37/48] Remove collect method from aggregations --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 100734b224b..e7374a57884 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -32,9 +32,6 @@ def value(self): def aggregate(self, value): pass - def collect(self): - return self._value # pylint: disable=no-member - class NoneAggregation(Aggregation): """ From ed502c50ef841172b63dd12a34c105c5b5ac8369 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 16 Nov 2021 15:49:50 -0600 Subject: [PATCH 38/48] Add make_point_and_reset --- .../opentelemetry/sdk/_metrics/aggregation.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index e7374a57884..7061b36a7c8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -32,6 +32,10 @@ def value(self): def aggregate(self, value): pass + @abstractmethod + def make_point_and_reset(self): + pass + class NoneAggregation(Aggregation): """ @@ -44,6 +48,9 @@ def __init__(self, instrument): def aggregate(self, value): pass + def make_point_and_reset(self): + pass + class SumAggregation(Aggregation): """ @@ -56,6 +63,9 @@ def __init__(self, instrument): def aggregate(self, value): self._value = self._value + value + def make_point_and_reset(self): + pass + class LastValueAggregation(Aggregation): @@ -71,6 +81,9 @@ def aggregate(self, value): self._value = value self._timestamp = _time_ns() + def make_point_and_reset(self): + pass + class ExplicitBucketHistogramAggregation(Aggregation): @@ -132,3 +145,6 @@ def aggregate(self, value): self._value[key] = self._value[key] + value break + + def make_point_and_reset(self): + pass From 5f50e74cc47b5fff4665a4516d663e172a924e8a Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 16 Nov 2021 15:51:36 -0600 Subject: [PATCH 39/48] Remove add implementation --- .../opentelemetry/sdk/_metrics/instrument.py | 12 +---- .../tests/metrics/test_instrument.py | 44 ------------------- 2 files changed, 1 insertion(+), 55 deletions(-) delete mode 100644 opentelemetry-sdk/tests/metrics/test_instrument.py diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 0ef3232f1bd..f99fda6c2e4 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -51,17 +51,7 @@ def __init__( class _Synchronous(_Instrument): def add(self, amount, attributes=None): - - if attributes is None: - raise Exception("Missing attributes") - - attributes = frozenset(attributes.items()) - if attributes not in self._attributes_aggregations.keys(): - - self._attributes_aggregations[attributes] = self._aggregation( - self, **self._aggregation_config - ) - self._attributes_aggregations[attributes].aggregate(amount) + pass class Counter(_Synchronous, Counter): diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py deleted file mode 100644 index 3cbd564e6db..00000000000 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright The OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# 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. - - -from unittest import TestCase - -from opentelemetry.sdk._metrics.aggregation import SumAggregation -from opentelemetry.sdk._metrics.instrument import _Synchronous - - -class Test_Synchronous(TestCase): - def test_add(self): - """ - Test that `_Synchronous.add` can handle multiple aggregations - """ - - synchronous = _Synchronous("synchronous", aggregation=SumAggregation) - - synchronous.add(1, {"name0": "value0"}) - synchronous.add(1, {"name1": "value1"}) - - self.assertIsInstance( - synchronous._attributes_aggregations[ - frozenset({("name0", "value0")}) - ], - SumAggregation, - ) - self.assertIsInstance( - synchronous._attributes_aggregations[ - frozenset({("name1", "value1")}) - ], - SumAggregation, - ) From f51cb8edc4d6d4b6ce4647909e2a2a2394d01ced Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 16 Nov 2021 15:55:36 -0600 Subject: [PATCH 40/48] Remove _Synchronous --- .../src/opentelemetry/sdk/_metrics/instrument.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index f99fda6c2e4..380063d25df 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -49,12 +49,7 @@ def __init__( aggregation(self, **aggregation_config) -class _Synchronous(_Instrument): - def add(self, amount, attributes=None): - pass - - -class Counter(_Synchronous, Counter): +class Counter(Counter): def __init__( self, name, @@ -72,7 +67,7 @@ def __init__( ) -class UpDownCounter(_Synchronous, UpDownCounter): +class UpDownCounter(UpDownCounter): def __init__( self, name, @@ -128,7 +123,7 @@ def __init__( ) -class Histogram(_Synchronous, Histogram): +class Histogram(Histogram): def __init__( self, name, From c4196f7ca62db36fcdfb03cfac74d286736651a6 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 10:58:23 -0600 Subject: [PATCH 41/48] Update opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py Co-authored-by: Aaron Abbott --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 7061b36a7c8..6b0012aa5e5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -34,6 +34,9 @@ def aggregate(self, value): @abstractmethod def make_point_and_reset(self): + """ + Atomically return a point for the current value of the metric and reset the internal state. + """ pass From 4c8454c55a463b50a632303e0e7cdecd3ad9689b Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 14:21:16 -0600 Subject: [PATCH 42/48] Requested fixes --- opentelemetry-sdk/setup.cfg | 1 - opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/opentelemetry-sdk/setup.cfg b/opentelemetry-sdk/setup.cfg index fdbd0202e92..92c3ddbcef5 100644 --- a/opentelemetry-sdk/setup.cfg +++ b/opentelemetry-sdk/setup.cfg @@ -45,7 +45,6 @@ include_package_data = True install_requires = opentelemetry-api == 1.7.1 opentelemetry-semantic-conventions == 0.26b1 - opentelemetry-proto == 1.7.1 [options.packages.find] where = src diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 6b0012aa5e5..f121c17b979 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -98,7 +98,7 @@ def __init__( self, instrument, *args, - boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000, inf), + boundaries=(0, 5, 10, 25, 50, 75, 100, 250, 500, 1000), record_min_max=True, ): super().__init__() From d8797bc4fcb6889ce17bd218e0016cb89be38c2d Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 14:22:05 -0600 Subject: [PATCH 43/48] Remove NoneAggregation --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index f121c17b979..261826c055f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -40,21 +40,6 @@ def make_point_and_reset(self): pass -class NoneAggregation(Aggregation): - """ - This aggregation drops all instrument measurements. - """ - - def __init__(self, instrument): - self._value = None - - def aggregate(self, value): - pass - - def make_point_and_reset(self): - pass - - class SumAggregation(Aggregation): """ This aggregation collects data for the SDK sum metric point. From 90f0ef8e510e36e91d787ddeb659d78e8ecf6fc8 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 14:24:00 -0600 Subject: [PATCH 44/48] Add changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53f742dce3d..d8ae12684e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.7.0-0.26b0...HEAD) +- Add support for Python 3.10 + ([#2234](https://github.com/open-telemetry/opentelemetry-python/pull/2234)) + ## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11 From dd685f5d1ef09c25c1f67e916cae4764b6944850 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 14:45:33 -0600 Subject: [PATCH 45/48] Fix tests --- .../tests/metrics/test_aggregation.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 6a2814edaee..1c4fa1420e6 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -21,26 +21,10 @@ from opentelemetry.sdk._metrics.aggregation import ( ExplicitBucketHistogramAggregation, LastValueAggregation, - NoneAggregation, SumAggregation, ) -class TestNoneAggregation(TestCase): - def test_aggregate(self): - """ - `NoneAggregation` drops all measurements. - """ - - none_aggregation = NoneAggregation(Mock()) - - none_aggregation.aggregate(1) - none_aggregation.aggregate(2) - none_aggregation.aggregate(3) - - self.assertIs(none_aggregation.value, None) - - class TestSumAggregation(TestCase): def test_aggregate(self): """ From b7f05b32e1da7e444055d965ce7ba5af3769d785 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 14:54:23 -0600 Subject: [PATCH 46/48] Fix boundaries --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 261826c055f..7165dcd3c8f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -87,8 +87,7 @@ def __init__( record_min_max=True, ): super().__init__() - self._boundaries = boundaries - self._value = OrderedDict([(key, 0) for key in boundaries]) + self._value = OrderedDict([(key, 0) for key in (*boundaries, inf)]) self._min = inf self._max = -inf self._sum = 0 From 8c1a8aa64d55080a10c2162a199eb0f2594cc275 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 15:01:13 -0600 Subject: [PATCH 47/48] More fixes --- .../src/opentelemetry/sdk/_metrics/aggregation.py | 2 +- .../src/opentelemetry/sdk/_metrics/instrument.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 7165dcd3c8f..456e4471621 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -37,7 +37,6 @@ def make_point_and_reset(self): """ Atomically return a point for the current value of the metric and reset the internal state. """ - pass class SumAggregation(Aggregation): @@ -117,6 +116,7 @@ def sum(self): "Sum is not filled out when the associated " "instrument is not monotonic" ) + return None def aggregate(self, value): if self._record_min_max: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py index 380063d25df..fc63311ce7b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/instrument.py @@ -49,7 +49,7 @@ def __init__( aggregation(self, **aggregation_config) -class Counter(Counter): +class Counter(_Instrument, Counter): def __init__( self, name, @@ -67,7 +67,7 @@ def __init__( ) -class UpDownCounter(UpDownCounter): +class UpDownCounter(_Instrument, UpDownCounter): def __init__( self, name, @@ -123,7 +123,7 @@ def __init__( ) -class Histogram(Histogram): +class Histogram(_Instrument, Histogram): def __init__( self, name, From ec591ed4c909ab5d4dfd24839015dcf5f6a2eff7 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Wed, 17 Nov 2021 23:58:46 -0600 Subject: [PATCH 48/48] Update CHANGELOG.md Co-authored-by: Srikanth Chekuri --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ae12684e5..c7e3c2fce16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.7.0-0.26b0...HEAD) -- Add support for Python 3.10 +- Adds Aggregation and instruments as part of Metrics SDK ([#2234](https://github.com/open-telemetry/opentelemetry-python/pull/2234)) ## [1.7.1-0.26b1](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.7.0-0.26b0) - 2021-11-11