Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
bba1fea
Named meters
lzchen Feb 19, 2020
712ccd3
Remove test
lzchen Feb 19, 2020
b50c269
fix comment
lzchen Feb 19, 2020
fd3d175
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Feb 19, 2020
08b1117
Fix tests
lzchen Feb 19, 2020
c92f837
address comments + fix lint
lzchen Feb 19, 2020
7b8b67d
Rename MeterSource to MeterProvider
lzchen Feb 19, 2020
63824f3
black
lzchen Feb 19, 2020
16e728b
black
lzchen Feb 19, 2020
7733a87
remove black
lzchen Feb 20, 2020
b436c62
fix lint
lzchen Feb 20, 2020
d6c97f0
fix lint
lzchen Feb 20, 2020
a2f2e0f
fix lint
lzchen Feb 20, 2020
32cf2c1
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
lzchen Feb 25, 2020
ec9c673
Merge
lzchen Feb 25, 2020
7ebd438
Rename to MeterProvider, remove Batcher from constructor
lzchen Feb 25, 2020
27d75ba
fix mypy
lzchen Feb 25, 2020
08095b6
black
lzchen Feb 25, 2020
6a743c7
Black
lzchen Feb 25, 2020
e226eda
address comments
lzchen Feb 27, 2020
c7432ef
update tests
lzchen Feb 27, 2020
60b2f38
merge
lzchen Feb 27, 2020
bd53f84
fix tests
lzchen Feb 27, 2020
6f6a37d
Fix tests
lzchen Feb 27, 2020
4770ec4
black
lzchen Feb 27, 2020
0f36b31
remove black
lzchen Feb 27, 2020
4f4632b
ADDRESS COMMENTS
lzchen Feb 27, 2020
e591a39
black
lzchen Feb 27, 2020
0878f5d
fix lint
lzchen Feb 27, 2020
584d996
fix example
lzchen Feb 27, 2020
f854923
Fix lint
lzchen Feb 27, 2020
4df2553
merge
lzchen Feb 28, 2020
40ee67b
fix labelset
lzchen Feb 28, 2020
0b93285
fix lint
lzchen Feb 28, 2020
3735d88
set to none
lzchen Feb 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions examples/metrics/record.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
import time

from opentelemetry import metrics
from opentelemetry.sdk.metrics import Counter, Meter
from opentelemetry.sdk.metrics import Counter, MeterSource
from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter
from opentelemetry.sdk.metrics.export.controller import PushController

# The preferred tracer implementation must be set, as the opentelemetry-api
# defines the interface with a no-op implementation.
metrics.set_preferred_meter_source_implementation(lambda _: MeterSource())
# Meter is responsible for creating and recording metrics
metrics.set_preferred_meter_implementation(lambda _: Meter())
meter = metrics.meter()
meter = metrics.meter_source().get_meter(__name__)
Copy link
Member

Choose a reason for hiding this comment

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

In the same vein as #430, can we add metrics.get_meter()? Would save a lot of boilerplate and an abstraction many app developers will not have to deal with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meter_source().get_meter() takes in an optional Batcher type in the constructor. How do I do the typing for this in the API without needing a dependency on the SDK where the Batcher class exists?

# exporter to export metrics to the console
exporter = ConsoleMetricsExporter()
# controller collects metrics created from meter and exports it via the
Expand Down

This file was deleted.

84 changes: 64 additions & 20 deletions opentelemetry-api/src/opentelemetry/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,9 +223,49 @@ def record(self, value: ValueT, label_set: LabelSet) -> None:
label_set: `LabelSet` to associate with the returned handle.
"""

class MeterSource(abc.ABC):

MetricT = TypeVar("MetricT", Counter, Gauge, Measure)
@abc.abstractmethod
def get_meter(
self,
instrumenting_module_name: str,
instrumenting_library_version: str = "",
) -> "Meter":
"""Returns a `Meter` for use by the given instrumentation library.

This function may return different `Meter` types (e.g. a no-op meter
vs. a functional meter).

Args:
instrumenting_module_name: The name of the instrumenting module
(usually just ``__name__``).

This should *not* be the name of the module that is
instrumented but the name of the module doing the instrumentation.
E.g., instead of ``"requests"``, use
``"opentelemetry.ext.http_requests"``.

instrumenting_library_version: Optional. The version string of the
instrumenting library. Usually this should be the same as
``pkg_resources.get_distribution(instrumenting_library_name).version``.
"""

class DefaultMeterSource(MeterSource):
"""The default MeterSource, used when no implementation is available.

All operations are no-op.
"""

def get_meter(
self,
instrumenting_module_name: str,
instrumenting_library_version: str = "",
) -> "Meter":
# pylint:disable=no-self-use,unused-argument
return DefaultMeter()


MetricT = TypeVar("MetricT", Counter, Gauge, Measure)

# pylint: disable=unused-argument
class Meter(abc.ABC):
Expand Down Expand Up @@ -322,45 +362,49 @@ def get_label_set(self, labels: Dict[str, str]) -> "LabelSet":
# Once https://github.com/python/mypy/issues/7092 is resolved,
# the following type definition should be replaced with
# from opentelemetry.util.loader import ImplementationFactory
ImplementationFactory = Callable[[Type[Meter]], Optional[Meter]]
ImplementationFactory = Callable[[Type[MeterSource]], Optional[MeterSource]]

_METER = None
_METER_FACTORY = None
_METER_SOURCE = None
_METER_SOURCE_FACTORY = None


def meter() -> Meter:
"""Gets the current global :class:`~.Meter` object.
def meter_source() -> MeterSource:
"""Gets the current global :class:`~.MeterSource` object.

If there isn't one set yet, a default will be loaded.
"""
global _METER, _METER_FACTORY # pylint:disable=global-statement
global _METER_SOURCE, _METER_SOURCE_FACTORY # pylint:disable=global-statement

if _METER is None:
if _METER_SOURCE is None:
# pylint:disable=protected-access
try:
_METER = loader._load_impl(Meter, _METER_FACTORY) # type: ignore
_METER_SOURCE = loader._load_impl(
MeterSource, _METER_SOURCE_FACTORY # type: ignore
)
except TypeError:
# if we raised an exception trying to instantiate an
# abstract class, default to no-op tracer impl
_METER = DefaultMeter()
del _METER_FACTORY
# abstract class, default to no-op meter impl
_METER_SOURCE = DefaultMeterSource()
del _METER_SOURCE_FACTORY
Copy link
Member

Choose a reason for hiding this comment

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

Should this variable be deleted, or just set to None? It's a lot easier to see if a variable is None that go to locals() and check if the _METER_SOURCE_FACTORY key is in there as a guard condition to check on the _METER_SOURCE_FACTORY.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have a strong opinion on this. I'm taking what is being done in the tracing API to be consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on setting it to None, stays consistent with the _TRACER_PROVIDER_FACTORY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

my bad, i missed that.


return _METER
return _METER_SOURCE


def set_preferred_meter_implementation(factory: ImplementationFactory) -> None:
"""Set the factory to be used to create the meter.
def set_preferred_meter_source_implementation(
factory: ImplementationFactory
) -> None:
"""Set the factory to be used to create the meter source.

See :mod:`opentelemetry.util.loader` for details.

This function may not be called after a meter is already loaded.

Args:
factory: Callback that should create a new :class:`Meter` instance.
factory: Callback that should create a new :class:`MeterSource` instance.
"""
global _METER, _METER_FACTORY # pylint:disable=global-statement
global _METER_SOURCE_FACTORY # pylint:disable=global-statement

if _METER:
raise RuntimeError("Meter already loaded.")
if _METER_SOURCE:
raise RuntimeError("MeterSource already loaded.")

_METER_FACTORY = factory
_METER_SOURCE_FACTORY = factory
49 changes: 0 additions & 49 deletions opentelemetry-api/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,6 @@


# pylint: disable=no-self-use
class TestMeter(unittest.TestCase):
def setUp(self):
self.meter = metrics.DefaultMeter()

def test_record_batch(self):
counter = metrics.Counter()
label_set = metrics.LabelSet()
self.meter.record_batch(label_set, ((counter, 1),))

def test_create_metric(self):
metric = self.meter.create_metric("", "", "", float, metrics.Counter)
self.assertIsInstance(metric, metrics.DefaultMetric)

def test_get_label_set(self):
metric = self.meter.get_label_set({})
self.assertIsInstance(metric, metrics.DefaultLabelSet)


class TestMetrics(unittest.TestCase):
def test_default(self):
default = metrics.DefaultMetric()
Expand Down Expand Up @@ -92,34 +74,3 @@ def test_gauge_handle(self):
def test_measure_handle(self):
handle = metrics.MeasureHandle()
handle.record(1)


@contextmanager
# type: ignore
def patch_metrics_globals(meter=None, meter_factory=None):
"""Mock metrics._METER and metrics._METER_FACTORY.

This prevents previous changes to these values from affecting the code in
this scope, and prevents changes in this scope from leaking out and
affecting other tests.
"""
with mock.patch("opentelemetry.metrics._METER", meter):
with mock.patch("opentelemetry.metrics._METER_FACTORY", meter_factory):
yield


class TestGlobals(unittest.TestCase):
def test_meter_default_factory(self):
"""Check that the default meter is a DefaultMeter."""
with patch_metrics_globals():
meter = metrics.meter()
self.assertIsInstance(meter, metrics.DefaultMeter)
# Check that we don't create a new instance on each call
self.assertIs(meter, metrics.meter())

def test_meter_custom_factory(self):
"""Check that we use the provided factory for custom global meters."""
mock_meter = mock.Mock(metrics.Meter)
with patch_metrics_globals(meter_factory=lambda _: mock_meter):
meter = metrics.meter()
self.assertIs(meter, mock_meter)
20 changes: 20 additions & 0 deletions opentelemetry-api/tests/test_implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class TestAPIOnlyImplementation(unittest.TestCase):
https://github.com/open-telemetry/opentelemetry-python/issues/142
"""

# TRACER

def test_tracer(self):
with self.assertRaises(TypeError):
# pylint: disable=abstract-class-instantiated
Expand Down Expand Up @@ -54,12 +56,30 @@ def test_default_span(self):
self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT)
self.assertIs(span.is_recording_events(), False)

# METER

def test_meter(self):
with self.assertRaises(TypeError):
# pylint: disable=abstract-class-instantiated
metrics.Meter() # type:ignore

def test_default_meter(self):
meter_source = metrics.DefaultMeterSource()
meter = meter_source.get_meter(__name__)
self.assertIsInstance(meter, metrics.DefaultMeter)

def test_record_batch(self):
meter = metrics.DefaultMeter()
counter = metrics.Counter()
label_set = metrics.LabelSet()
meter.record_batch(label_set, ((counter, 1),))

def test_create_metric(self):
meter = metrics.DefaultMeter()
metric = meter.create_metric("", "", "", float, metrics.Counter)
self.assertIsInstance(metric, metrics.DefaultMetric)

def test_get_label_set(self):
meter = metrics.DefaultMeter()
metric = meter.get_label_set({})
self.assertIsInstance(metric, metrics.DefaultLabelSet)
27 changes: 26 additions & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from opentelemetry import metrics as metrics_api
from opentelemetry.sdk.metrics.export.aggregate import Aggregator
from opentelemetry.sdk.metrics.export.batcher import Batcher, UngroupedBatcher
from opentelemetry.sdk.util import InstrumentationInfo
from opentelemetry.util import time_ns

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -261,11 +262,17 @@ class Meter(metrics_api.Meter):
"""See `opentelemetry.metrics.Meter`.

Args:
instrumentation_info: The `InstrumentationInfo` for this meter.
batcher: The `Batcher` used for this meter.
"""

def __init__(self, batcher: Batcher = UngroupedBatcher(True)):
def __init__(
self,
instrumentation_info: InstrumentationInfo,
batcher: Batcher
):
self.batcher = batcher
self.instrumentation_info = instrumentation_info
self.metrics = set()

def collect(self) -> None:
Expand Down Expand Up @@ -328,3 +335,21 @@ def get_label_set(self, labels: Dict[str, str]):
if len(labels) == 0:
return EMPTY_LABEL_SET
return LabelSet(labels=labels)

class MeterSource(metrics_api.MeterSource):

def get_meter(
self,
instrumenting_module_name: str,
instrumenting_library_version: str = "",
batcher: Batcher = UngroupedBatcher(True),
) -> "metrics_api.Meter":
if not instrumenting_module_name: # Reject empty strings too.
instrumenting_module_name = "ERROR:MISSING MODULE NAME"
logger.error("get_meter called with missing module name.")
return Meter(
InstrumentationInfo(
instrumenting_module_name, instrumenting_library_version
),
batcher,
)
Loading