Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return proxy instruments from ProxyMeter #2169

Merged
merged 10 commits into from
Oct 14, 2021
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD)

- Return proxy instruments from ProxyMeter
[[#2169](https://github.com/open-telemetry/opentelemetry-python/pull/2169)]
- Make Measurement a concrete class
([#2153](https://github.com/open-telemetry/opentelemetry-python/pull/2153))
- Add metrics API
Expand Down
2 changes: 2 additions & 0 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@
nitpick_ignore = [
("py:class", "ValueT"),
("py:class", "MetricT"),
("py:class", "InstrumentT"),
("py:obj", "opentelemetry.metrics.instrument.InstrumentT"),
# Even if wrapt is added to intersphinx_mapping, sphinx keeps failing
# with "class reference target not found: ObjectProxy".
("py:class", "ObjectProxy"),
Expand Down
194 changes: 147 additions & 47 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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__)
Expand Down Expand Up @@ -71,17 +80,32 @@ def get_meter(


class ProxyMeterProvider(MeterProvider):
aabmass marked this conversation as resolved.
Show resolved Hide resolved
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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if self._real_meter_provider:
if self._real_meter_provider is not None:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be equivalent here right? I'm happy to change it if this is a general thing we enforce throughout our code base.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the time it is not a big deal, but it can be. It is part of PEP8 because of this reason:

Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!

I don't think we have been consistent on enforcing this, though.

return self._real_meter_provider.get_meter(
name, version, schema_url
)

meter = ProxyMeter(name, version=version, schema_url=schema_url)
owais marked this conversation as resolved.
Show resolved Hide resolved
self._meters.append(meter)
return meter

def set_meter_provider(self, meter_provider: MeterProvider) -> None:
aabmass marked this conversation as resolved.
Show resolved Hide resolved
with self._lock:
self._real_meter_provider = meter_provider
for meter in self._meters:
meter.on_set_real_meter_provider(meter_provider)
aabmass marked this conversation as resolved.
Show resolved Hide resolved
owais marked this conversation as resolved.
Show resolved Hide resolved


class Meter(ABC):
Expand Down Expand Up @@ -233,43 +257,103 @@ 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_real_meter_provider(
aabmass marked this conversation as resolved.
Show resolved Hide resolved
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)
aabmass marked this conversation as resolved.
Show resolved Hide resolved

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)
ocelotl marked this conversation as resolved.
Show resolved Hide resolved
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):
Expand Down Expand Up @@ -329,8 +413,19 @@ 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 _reset_globals() -> None:
"""WARNING: only use this for tests."""
global _METER_PROVIDER_SET_ONCE # pylint: disable=global-statement
global _METER_PROVIDER # pylint: disable=global-statement
global _PROXY_METER_PROVIDER # pylint: disable=global-statement
_METER_PROVIDER_SET_ONCE = Once()
_METER_PROVIDER = None
_PROXY_METER_PROVIDER = ProxyMeterProvider()
aabmass marked this conversation as resolved.
Show resolved Hide resolved


def get_meter(
Expand All @@ -350,35 +445,40 @@ def get_meter(
return meter_provider.get_meter(name, version)


def _set_meter_provider(meter_provider: MeterProvider, *, log: bool) -> None:
aabmass marked this conversation as resolved.
Show resolved Hide resolved
aabmass marked this conversation as resolved.
Show resolved Hide resolved
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.set_meter_provider(meter_provider)

did_set = _METER_PROVIDER_SET_ONCE.do_once(set_mp)
aabmass marked this conversation as resolved.
Show resolved Hide resolved

if 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(
meter_provider = cast(
"MeterProvider",
_load_provider(OTEL_PYTHON_METER_PROVIDER, "meter_provider"),
)
_set_meter_provider(meter_provider, log=False)

return _METER_PROVIDER
Loading