From 2001eeb5e6352996fd0d53a90b92629a56d7cb54 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 1 Jun 2020 19:29:22 -0700 Subject: [PATCH 01/29] value observer --- docs/examples/basic_meter/observer.py | 3 +- .../ext/system_metrics/__init__.py | 19 +++++++--- opentelemetry-api/CHANGELOG.md | 3 ++ .../src/opentelemetry/metrics/__init__.py | 20 +++++++++- opentelemetry-sdk/CHANGELOG.md | 3 ++ .../src/opentelemetry/sdk/metrics/__init__.py | 11 +++--- .../sdk/metrics/export/aggregate.py | 2 +- .../sdk/metrics/export/batcher.py | 8 ++-- .../tests/metrics/export/test_export.py | 38 +++++++++---------- .../tests/metrics/test_metrics.py | 18 ++++----- 10 files changed, 78 insertions(+), 47 deletions(-) diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index 0490fbe8efb..409402fa1cd 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -19,7 +19,7 @@ import psutil from opentelemetry import metrics -from opentelemetry.sdk.metrics import MeterProvider +from opentelemetry.sdk.metrics import MeterProvider, ValueObserver from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher from opentelemetry.sdk.metrics.export.controller import PushController @@ -45,6 +45,7 @@ def get_cpu_usage_callback(observer): description="per-cpu usage", unit="1", value_type=float, + observer_type=ValueObserver, label_keys=("cpu_number",), ) diff --git a/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py b/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py index 88f36f4ac48..09c633f14b4 100644 --- a/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py +++ b/ext/opentelemetry-ext-system-metrics/src/opentelemetry/ext/system_metrics/__init__.py @@ -58,6 +58,7 @@ import psutil from opentelemetry import metrics +from opentelemetry.sdk.metrics import ValueObserver from opentelemetry.sdk.metrics.export import MetricsExporter from opentelemetry.sdk.metrics.export.controller import PushController @@ -106,6 +107,7 @@ def __init__( description="System memory", unit="bytes", value_type=int, + observer_type=ValueObserver, ) self.meter.register_observer( @@ -114,6 +116,7 @@ def __init__( description="System CPU", unit="seconds", value_type=float, + observer_type=ValueObserver, ) self.meter.register_observer( @@ -122,6 +125,7 @@ def __init__( description="System network bytes", unit="bytes", value_type=int, + observer_type=ValueObserver, ) self.meter.register_observer( @@ -130,6 +134,7 @@ def __init__( description="Runtime memory", unit="bytes", value_type=int, + observer_type=ValueObserver, ) self.meter.register_observer( @@ -138,6 +143,7 @@ def __init__( description="Runtime CPU", unit="seconds", value_type=float, + observer_type=ValueObserver, ) self.meter.register_observer( @@ -146,9 +152,10 @@ def __init__( description="Runtime: gc objects", unit="objects", value_type=int, + observer_type=ValueObserver, ) - def _get_system_memory(self, observer: metrics.Observer) -> None: + def _get_system_memory(self, observer: metrics.ValueObserver) -> None: """Observer callback for memory available Args: @@ -161,7 +168,7 @@ def _get_system_memory(self, observer: metrics.Observer) -> None: getattr(system_memory, metric), self._system_memory_labels ) - def _get_system_cpu(self, observer: metrics.Observer) -> None: + def _get_system_cpu(self, observer: metrics.ValueObserver) -> None: """Observer callback for system cpu Args: @@ -174,7 +181,7 @@ def _get_system_cpu(self, observer: metrics.Observer) -> None: getattr(cpu_times, _type), self._system_cpu_labels ) - def _get_network_bytes(self, observer: metrics.Observer) -> None: + def _get_network_bytes(self, observer: metrics.ValueObserver) -> None: """Observer callback for network bytes Args: @@ -187,7 +194,7 @@ def _get_network_bytes(self, observer: metrics.Observer) -> None: getattr(net_io, _type), self._network_bytes_labels ) - def _get_runtime_memory(self, observer: metrics.Observer) -> None: + def _get_runtime_memory(self, observer: metrics.ValueObserver) -> None: """Observer callback for runtime memory Args: @@ -200,7 +207,7 @@ def _get_runtime_memory(self, observer: metrics.Observer) -> None: getattr(proc_memory, _type), self._runtime_memory_labels ) - def _get_runtime_cpu(self, observer: metrics.Observer) -> None: + def _get_runtime_cpu(self, observer: metrics.ValueObserver) -> None: """Observer callback for runtime CPU Args: @@ -213,7 +220,7 @@ def _get_runtime_cpu(self, observer: metrics.Observer) -> None: getattr(proc_cpu, _type), self._runtime_cpu_labels ) - def _get_runtime_gc_count(self, observer: metrics.Observer) -> None: + def _get_runtime_gc_count(self, observer: metrics.ValueObserver) -> None: """Observer callback for garbage collection Args: diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 386bd75d470..dc9247305f9 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Rename Observer to ValueObserver + ([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751)) + ## 0.8b0 Released 2020-05-27 diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index b7ad62adb2e..1fefed31061 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -173,7 +173,6 @@ class Observer(abc.ABC): """An observer type metric instrument used to capture a current set of values. - Observer instruments are asynchronous, a callback is invoked with the observer instrument as argument allowing the user to capture multiple values per collection interval. @@ -201,6 +200,18 @@ def observe(self, value: ValueT, labels: Dict[str, str]) -> None: """ +class ValueObserver(Observer): + """No-op implementation of ``ValueObserver``.""" + + def observe(self, value: ValueT, labels: Dict[str, str]) -> None: + """Captures ``value`` to the valueobserver. + + Args: + value: The value to capture to this valueobserver metric. + labels: Labels associated to ``value``. + """ + + class MeterProvider(abc.ABC): @abc.abstractmethod def get_meter( @@ -251,7 +262,10 @@ def get_meter( return DefaultMeter() -MetricT = TypeVar("MetricT", Counter, Measure, Observer) +MetricT = TypeVar("MetricT", Counter, Measure) +InstrumentT = TypeVar("InstrumentT", Counter, Measure, Observer) +# TODO: Will populate with other observers when implemented +ObserverT = TypeVar("ObserverT", DefaultObserver, ValueObserver) ObserverCallbackT = Callable[[Observer], None] @@ -302,6 +316,7 @@ def create_metric( unit: Unit of the metric values following the UCUM convention (https://unitsofmeasure.org/ucum.html). value_type: The type of values being recorded by the metric. + observer_type: The type of observer being registered. metric_type: The type of metric being created. label_keys: The keys for the labels with dynamic values. enabled: Whether to report the metric by default. @@ -316,6 +331,7 @@ def register_observer( description: str, unit: str, value_type: Type[ValueT], + observer_type: Type[ObserverT], label_keys: Sequence[str] = (), enabled: bool = True, ) -> "Observer": diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 50ea751e15d..b4b8d3d4736 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Rename Observer to ValueObserver + ([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751)) + ## 0.8b0 Released 2020-05-27 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 1d35648fd35..3bc05e3659c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -190,8 +190,8 @@ def record( UPDATE_FUNCTION = record -class Observer(metrics_api.Observer): - """See `opentelemetry.metrics.Observer`.""" +class ValueObserver(metrics_api.ValueObserver): + """See `opentelemetry.metrics.ValueObserver`.""" def __init__( self, @@ -257,7 +257,7 @@ class Record: def __init__( self, - metric: metrics_api.MetricT, + instrument: metrics_api.InstrumentT, labels: Dict[str, str], aggregator: Aggregator, ): @@ -375,10 +375,11 @@ def register_observer( description: str, unit: str, value_type: Type[metrics_api.ValueT], + observer_type = Type[metrics_api.ObserverT], label_keys: Sequence[str] = (), enabled: bool = True, - ) -> metrics_api.Observer: - ob = Observer( + ) -> metrics_api.ObserverT: + ob = observer_type( callback, name, description, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py index ea8c40a7e72..d628f0f7147 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py @@ -125,7 +125,7 @@ def merge(self, other): ) -class ObserverAggregator(Aggregator): +class ValueObserverAggregator(Aggregator): """Same as MinMaxSumCount but also with last value.""" _TYPE = namedtuple("minmaxsumcountlast", "min max sum count last") diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index 7b599f4c7da..9e1f40b0a80 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -15,13 +15,13 @@ import abc from typing import Sequence, Type -from opentelemetry.metrics import Counter, Measure, MetricT, Observer +from opentelemetry.metrics import Counter, Measure, MetricT, ValueObserver from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, CounterAggregator, MinMaxSumCountAggregator, - ObserverAggregator, + ValueObserverAggregator, ) @@ -51,8 +51,8 @@ def aggregator_for(self, metric_type: Type[MetricT]) -> Aggregator: return CounterAggregator() if issubclass(metric_type, Measure): return MinMaxSumCountAggregator() - if issubclass(metric_type, Observer): - return ObserverAggregator() + if issubclass(metric_type, ValueObserver): + return ValueObserverAggregator() # TODO: Add other aggregators return CounterAggregator() diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index c77a132459c..4e94b249e3f 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -26,7 +26,7 @@ from opentelemetry.sdk.metrics.export.aggregate import ( CounterAggregator, MinMaxSumCountAggregator, - ObserverAggregator, + ValueObserverAggregator, ) from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher from opentelemetry.sdk.metrics.export.controller import PushController @@ -432,11 +432,11 @@ def test_concurrent_update_and_checkpoint(self): self.assertEqual(checkpoint_total, fut.result()) -class TestObserverAggregator(unittest.TestCase): +class TestValueObserverAggregator(unittest.TestCase): @mock.patch("opentelemetry.sdk.metrics.export.aggregate.time_ns") def test_update(self, time_mock): time_mock.return_value = 123 - observer = ObserverAggregator() + observer = ValueObserverAggregator() # test current values without any update self.assertEqual(observer.mmsc.current, (None, None, None, 0)) self.assertIsNone(observer.current) @@ -455,7 +455,7 @@ def test_update(self, time_mock): self.assertEqual(observer.current, values[-1]) def test_checkpoint(self): - observer = ObserverAggregator() + observer = ValueObserverAggregator() # take checkpoint wihtout any update observer.take_checkpoint() @@ -473,15 +473,15 @@ def test_checkpoint(self): ) def test_merge(self): - observer1 = ObserverAggregator() - observer2 = ObserverAggregator() + observer1 = ValueObserverAggregator() + observer2 = ValueObserverAggregator() mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) - checkpoint2 = ObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -507,15 +507,15 @@ def test_merge(self): self.assertEqual(observer1.last_update_timestamp, 123) def test_merge_last_updated(self): - observer1 = ObserverAggregator() - observer2 = ObserverAggregator() + observer1 = ValueObserverAggregator() + observer2 = ValueObserverAggregator() mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) - checkpoint2 = ObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -541,15 +541,15 @@ def test_merge_last_updated(self): self.assertEqual(observer1.last_update_timestamp, 123) def test_merge_last_updated_none(self): - observer1 = ObserverAggregator() - observer2 = ObserverAggregator() + observer1 = ValueObserverAggregator() + observer2 = ValueObserverAggregator() mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) - checkpoint2 = ObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -575,11 +575,11 @@ def test_merge_last_updated_none(self): self.assertEqual(observer1.last_update_timestamp, 100) def test_merge_with_empty(self): - observer1 = ObserverAggregator() - observer2 = ObserverAggregator() + observer1 = ValueObserverAggregator() + observer2 = ValueObserverAggregator() mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) - checkpoint1 = ObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer1.checkpoint = checkpoint1 diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 32980647055..35540ac57f8 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -83,7 +83,7 @@ def callback(observer): self.assertIsInstance(observer, metrics_api.Observer) observer.observe(45, {}) - observer = metrics.Observer( + observer = metrics.ValueObserver( callback, "name", "desc", "unit", int, meter, (), True ) @@ -160,7 +160,7 @@ def test_register_observer(self): callback = mock.Mock() observer = meter.register_observer( - callback, "name", "desc", "unit", int, (), True + callback, "name", "desc", "unit", int, metrics.ValueObserver ) self.assertIsInstance(observer, metrics_api.Observer) @@ -180,7 +180,7 @@ def test_unregister_observer(self): callback = mock.Mock() observer = meter.register_observer( - callback, "name", "desc", "unit", int, (), True + callback, "name", "desc", "unit", int, metrics.ValueObserver ) meter.unregister_observer(observer) @@ -283,10 +283,10 @@ def test_record(self): ) -class TestObserver(unittest.TestCase): +class TestValueObserver(unittest.TestCase): def test_observe(self): meter = metrics.MeterProvider().get_meter(__name__) - observer = metrics.Observer( + observer = metrics.ValueObserver( None, "name", "desc", "unit", int, meter, ("key",), True ) labels = {"key": "value"} @@ -303,7 +303,7 @@ def test_observe(self): def test_observe_disabled(self): meter = metrics.MeterProvider().get_meter(__name__) - observer = metrics.Observer( + observer = metrics.ValueObserver( None, "name", "desc", "unit", int, meter, ("key",), False ) labels = {"key": "value"} @@ -313,7 +313,7 @@ def test_observe_disabled(self): @mock.patch("opentelemetry.sdk.metrics.logger") def test_observe_incorrect_type(self, logger_mock): meter = metrics.MeterProvider().get_meter(__name__) - observer = metrics.Observer( + observer = metrics.ValueObserver( None, "name", "desc", "unit", int, meter, ("key",), True ) labels = {"key": "value"} @@ -325,7 +325,7 @@ def test_run(self): meter = metrics.MeterProvider().get_meter(__name__) callback = mock.Mock() - observer = metrics.Observer( + observer = metrics.ValueObserver( callback, "name", "desc", "unit", int, meter, (), True ) @@ -339,7 +339,7 @@ def test_run_exception(self, logger_mock): callback = mock.Mock() callback.side_effect = Exception("We have a problem!") - observer = metrics.Observer( + observer = metrics.ValueObserver( callback, "name", "desc", "unit", int, meter, (), True ) From 429339817cb71c2cfef9a72e6bcc1c02753e430a Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 1 Jun 2020 22:17:43 -0700 Subject: [PATCH 02/29] lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 4 +-- .../tests/metrics/export/test_export.py | 28 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 3bc05e3659c..8d36be3949e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -261,7 +261,7 @@ def __init__( labels: Dict[str, str], aggregator: Aggregator, ): - self.metric = metric + self.instrument = instrument self.labels = labels self.aggregator = aggregator @@ -375,7 +375,7 @@ def register_observer( description: str, unit: str, value_type: Type[metrics_api.ValueT], - observer_type = Type[metrics_api.ObserverT], + observer_type=Type[metrics_api.ObserverT], label_keys: Sequence[str] = (), enabled: bool = True, ) -> metrics_api.ObserverT: diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index 4e94b249e3f..5b8c2230bd8 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -479,9 +479,13 @@ def test_merge(self): mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint1 + (23,)) + ) - checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint2 + (27,)) + ) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -513,9 +517,13 @@ def test_merge_last_updated(self): mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint1 + (23,)) + ) - checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint2 + (27,)) + ) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -547,9 +555,13 @@ def test_merge_last_updated_none(self): mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) mmsc_checkpoint2 = MinMaxSumCountAggregator._TYPE(1, 33, 44, 2) - checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint1 + (23,)) + ) - checkpoint2 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint2 + (27,))) + checkpoint2 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint2 + (27,)) + ) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer2.mmsc.checkpoint = mmsc_checkpoint2 @@ -579,7 +591,9 @@ def test_merge_with_empty(self): observer2 = ValueObserverAggregator() mmsc_checkpoint1 = MinMaxSumCountAggregator._TYPE(3, 150, 101, 3) - checkpoint1 = ValueObserverAggregator._TYPE(*(mmsc_checkpoint1 + (23,))) + checkpoint1 = ValueObserverAggregator._TYPE( + *(mmsc_checkpoint1 + (23,)) + ) observer1.mmsc.checkpoint = mmsc_checkpoint1 observer1.checkpoint = checkpoint1 From e65de5c30c25dab249bf9d098a568ade8bd8b646 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 1 Jun 2020 22:33:03 -0700 Subject: [PATCH 03/29] instrument --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 2 +- .../src/opentelemetry/sdk/metrics/export/__init__.py | 2 +- .../src/opentelemetry/sdk/metrics/export/batcher.py | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 8d36be3949e..734aeb27592 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -393,7 +393,7 @@ def register_observer( self.observers.add(ob) return ob - def unregister_observer(self, observer: "Observer") -> None: + def unregister_observer(self, observer: metrics_api.ObserverT) -> None: with self.observers_lock: self.observers.remove(observer) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py index f5a8693268e..cc74df63c74 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py @@ -79,7 +79,7 @@ def export( print( '{}(data="{}", labels="{}", value={})'.format( type(self).__name__, - record.metric, + record.instrument, record.labels, record.aggregator.checkpoint, ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index 9e1f40b0a80..c03d72db31a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -63,8 +63,8 @@ def checkpoint_set(self) -> Sequence[MetricRecord]: data in all of the aggregators in this batcher. """ metric_records = [] - for (metric, labels), aggregator in self._batch_map.items(): - metric_records.append(MetricRecord(aggregator, labels, metric)) + for (instrument, labels), aggregator in self._batch_map.items(): + metric_records.append(MetricRecord(aggregator, labels, instrument)) return metric_records def finished_collection(self): @@ -90,7 +90,7 @@ class UngroupedBatcher(Batcher): def process(self, record): # Checkpoints the current aggregator value to be collected for export record.aggregator.take_checkpoint() - batch_key = (record.metric, record.labels) + batch_key = (record.instrument, record.labels) batch_value = self._batch_map.get(batch_key) aggregator = record.aggregator if batch_value: From 38504d40f9bb2c7febe6ab7583982f416b14cbdc Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 00:19:13 -0700 Subject: [PATCH 04/29] instrument --- .../src/opentelemetry/sdk/metrics/export/batcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index c03d72db31a..f9c837a5687 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -101,6 +101,6 @@ def process(self, record): if self.stateful: # if stateful batcher, create a copy of the aggregator and update # it with the current checkpointed value for long-term storage - aggregator = self.aggregator_for(record.metric.__class__) + aggregator = self.aggregator_for(record.instrument.__class__) aggregator.merge(record.aggregator) self._batch_map[batch_key] = aggregator From efd685e09cf9e7a4770279337a7427b4f38bc93c Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 00:38:02 -0700 Subject: [PATCH 05/29] batcher --- .../src/opentelemetry/sdk/metrics/export/batcher.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index f9c837a5687..ad79006acb5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -41,17 +41,17 @@ def __init__(self, stateful: bool): # (deltas) self.stateful = stateful - def aggregator_for(self, metric_type: Type[MetricT]) -> Aggregator: - """Returns an aggregator based on metric type. + def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: + """Returns an aggregator based on metric instrument type. Aggregators keep track of and updates values when metrics get updated. """ # pylint:disable=R0201 - if issubclass(metric_type, Counter): + if issubclass(instrument_type, Counter): return CounterAggregator() - if issubclass(metric_type, Measure): + if issubclass(instrument_type, Measure): return MinMaxSumCountAggregator() - if issubclass(metric_type, ValueObserver): + if issubclass(instrument_type, ValueObserver): return ValueObserverAggregator() # TODO: Add other aggregators return CounterAggregator() From e72f06a1f88756531fdf795a661dc981fca4b0ad Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 00:51:14 -0700 Subject: [PATCH 06/29] InstrumentT --- .../src/opentelemetry/sdk/metrics/export/batcher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index ad79006acb5..b3ea4335068 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -15,7 +15,7 @@ import abc from typing import Sequence, Type -from opentelemetry.metrics import Counter, Measure, MetricT, ValueObserver +from opentelemetry.metrics import Counter, Measure, InstrumentT, ValueObserver from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, From d0505d18f39f4be15c46abda2b47057e2ba0ea86 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 01:07:34 -0700 Subject: [PATCH 07/29] mypy --- opentelemetry-api/src/opentelemetry/metrics/__init__.py | 1 + .../src/opentelemetry/sdk/metrics/export/batcher.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 1fefed31061..21f245e0eeb 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -389,6 +389,7 @@ def register_observer( description: str, unit: str, value_type: Type[ValueT], + observer_type: Type[ObserverT], label_keys: Sequence[str] = (), enabled: bool = True, ) -> "Observer": diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index b3ea4335068..977a8c72655 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -15,7 +15,7 @@ import abc from typing import Sequence, Type -from opentelemetry.metrics import Counter, Measure, InstrumentT, ValueObserver +from opentelemetry.metrics import Counter, InstrumentT, Measure, ValueObserver from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, From f2e446237f96520f30f6ef53ff24e65b2d4dc9f8 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 01:29:41 -0700 Subject: [PATCH 08/29] reorder metricrecord --- .../tests/test_otcollector_metrics_exporter.py | 10 +++++----- .../tests/test_prometheus_exporter.py | 6 +++--- .../src/opentelemetry/sdk/metrics/export/__init__.py | 8 ++++---- .../src/opentelemetry/sdk/metrics/export/batcher.py | 2 +- opentelemetry-sdk/tests/metrics/export/test_export.py | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py index 63ea28cd935..ca904fe7376 100644 --- a/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py +++ b/ext/opentelemetry-ext-opencensusexporter/tests/test_otcollector_metrics_exporter.py @@ -92,7 +92,7 @@ def test_get_collector_point(self): "testName", "testDescription", "unit", float, Measure ) result = metrics_exporter.get_collector_point( - MetricRecord(aggregator, self._key_labels, int_counter) + MetricRecord(int_counter, self._key_labels, aggregator) ) self.assertIsInstance(result, metrics_pb2.Point) self.assertIsInstance(result.timestamp, Timestamp) @@ -100,13 +100,13 @@ def test_get_collector_point(self): aggregator.update(123.5) aggregator.take_checkpoint() result = metrics_exporter.get_collector_point( - MetricRecord(aggregator, self._key_labels, float_counter) + MetricRecord(float_counter, self._key_labels, aggregator) ) self.assertEqual(result.double_value, 123.5) self.assertRaises( TypeError, metrics_exporter.get_collector_point( - MetricRecord(aggregator, self._key_labels, measure) + MetricRecord(measure, self._key_labels, aggregator) ), ) @@ -122,7 +122,7 @@ def test_export(self): "testname", "testdesc", "unit", int, Counter, ["environment"] ) record = MetricRecord( - aggregate.CounterAggregator(), self._key_labels, test_metric + test_metric, self._key_labels, aggregate.CounterAggregator(), ) result = collector_exporter.export([record]) @@ -147,7 +147,7 @@ def test_translate_to_collector(self): aggregator = aggregate.CounterAggregator() aggregator.update(123) aggregator.take_checkpoint() - record = MetricRecord(aggregator, self._key_labels, test_metric) + record = MetricRecord(test_metric, self._key_labels, aggregator,) output_metrics = metrics_exporter.translate_to_collector([record]) self.assertEqual(len(output_metrics), 1) self.assertIsInstance(output_metrics[0], metrics_pb2.Metric) diff --git a/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py b/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py index f986e0c4f5f..e97f8f4b459 100644 --- a/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py +++ b/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py @@ -67,7 +67,7 @@ def test_shutdown(self): def test_export(self): with self._registry_register_patch: record = MetricRecord( - CounterAggregator(), self._labels_key, self._test_metric + self._test_metric, self._labels_key, CounterAggregator(), ) exporter = PrometheusMetricsExporter() result = exporter.export([record]) @@ -90,7 +90,7 @@ def test_counter_to_prometheus(self): aggregator = CounterAggregator() aggregator.update(123) aggregator.take_checkpoint() - record = MetricRecord(aggregator, key_labels, metric) + record = MetricRecord(metric, key_labels, aggregator, key_labels) collector = CustomCollector("testprefix") collector.add_metrics_data([record]) @@ -118,7 +118,7 @@ def test_invalid_metric(self): ) labels = {"environment": "staging"} key_labels = metrics.get_labels_as_key(labels) - record = MetricRecord(None, key_labels, metric) + record = MetricRecord(metric, key_labels, None) collector = CustomCollector("testprefix") collector.add_metrics_data([record]) collector.collect() diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py index cc74df63c74..16911f94efb 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/__init__.py @@ -27,13 +27,13 @@ class MetricsExportResult(Enum): class MetricRecord: def __init__( self, - aggregator: Aggregator, + instrument: metrics_api.InstrumentT, labels: Tuple[Tuple[str, str]], - metric: metrics_api.MetricT, + aggregator: Aggregator, ): - self.aggregator = aggregator + self.instrument = instrument self.labels = labels - self.metric = metric + self.aggregator = aggregator class MetricsExporter: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index 977a8c72655..d48fdbfeff6 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -64,7 +64,7 @@ def checkpoint_set(self) -> Sequence[MetricRecord]: """ metric_records = [] for (instrument, labels), aggregator in self._batch_map.items(): - metric_records.append(MetricRecord(aggregator, labels, instrument)) + metric_records.append(MetricRecord(instrument, labels, aggregator)) return metric_records def finished_collection(self): diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index 5b8c2230bd8..af63fdff10e 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -48,7 +48,7 @@ def test_export(self): ) labels = {"environment": "staging"} aggregator = CounterAggregator() - record = MetricRecord(aggregator, labels, metric) + record = MetricRecord(metric, labels, aggregator) result = '{}(data="{}", labels="{}", value={})'.format( ConsoleMetricsExporter.__name__, metric, From 0d26ec6c119fe0fb89d043ae181aacc682878b94 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 11:02:47 -0700 Subject: [PATCH 09/29] fix tests --- .../tests/test_prometheus_exporter.py | 2 +- .../tests/test_system_metrics.py | 2 +- opentelemetry-sdk/tests/metrics/export/test_export.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py b/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py index e97f8f4b459..1862f789c0c 100644 --- a/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py +++ b/ext/opentelemetry-ext-prometheus/tests/test_prometheus_exporter.py @@ -90,7 +90,7 @@ def test_counter_to_prometheus(self): aggregator = CounterAggregator() aggregator.update(123) aggregator.take_checkpoint() - record = MetricRecord(metric, key_labels, aggregator, key_labels) + record = MetricRecord(metric, key_labels, aggregator) collector = CustomCollector("testprefix") collector.add_metrics_data([record]) diff --git a/ext/opentelemetry-ext-system-metrics/tests/test_system_metrics.py b/ext/opentelemetry-ext-system-metrics/tests/test_system_metrics.py index 70ead2c5152..b2d6bab4015 100644 --- a/ext/opentelemetry-ext-system-metrics/tests/test_system_metrics.py +++ b/ext/opentelemetry-ext-system-metrics/tests/test_system_metrics.py @@ -54,7 +54,7 @@ def _assert_metrics(self, observer_name, system_metrics, expected): ): if ( metric.labels in expected - and metric.metric.name == observer_name + and metric.instrument.name == observer_name ): self.assertEqual( metric.aggregator.checkpoint.last, expected[metric.labels], diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index af63fdff10e..178e41b2134 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -90,7 +90,7 @@ def test_checkpoint_set(self): batcher._batch_map = _batch_map records = batcher.checkpoint_set() self.assertEqual(len(records), 1) - self.assertEqual(records[0].metric, metric) + self.assertEqual(records[0].instrument, metric) self.assertEqual(records[0].labels, labels) self.assertEqual(records[0].aggregator, aggregator) From 68f980efb0824d45cf073e1670daaee761962bc3 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 11:25:59 -0700 Subject: [PATCH 10/29] fix tests --- .../metrics_exporter/__init__.py | 14 +++++++------- .../src/opentelemetry/ext/prometheus/__init__.py | 12 ++++++------ .../opentelemetry/sdk/metrics/export/batcher.py | 7 ++++++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py index faa0788c7f1..bb1a1ee888c 100644 --- a/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py +++ b/ext/opentelemetry-ext-opencensusexporter/src/opentelemetry/ext/opencensusexporter/metrics_exporter/__init__.py @@ -114,10 +114,10 @@ def translate_to_collector( ) metric_descriptor = metrics_pb2.MetricDescriptor( - name=metric_record.metric.name, - description=metric_record.metric.description, - unit=metric_record.metric.unit, - type=get_collector_metric_type(metric_record.metric), + name=metric_record.instrument.name, + description=metric_record.instrument.description, + unit=metric_record.instrument.unit, + type=get_collector_metric_type(metric_record.instrument), label_keys=label_keys, ) @@ -151,14 +151,14 @@ def get_collector_point(metric_record: MetricRecord) -> metrics_pb2.Point: metric_record.aggregator.last_update_timestamp ) ) - if metric_record.metric.value_type == int: + if metric_record.instrument.value_type == int: point.int64_value = metric_record.aggregator.checkpoint - elif metric_record.metric.value_type == float: + elif metric_record.instrument.value_type == float: point.double_value = metric_record.aggregator.checkpoint else: raise TypeError( "Unsupported metric type: {}".format( - metric_record.metric.value_type + metric_record.instrument.value_type ) ) return point diff --git a/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py b/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py index cc44621ac48..59ef3f1708a 100644 --- a/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py +++ b/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py @@ -152,22 +152,22 @@ def _translate_to_prometheus(self, metric_record: MetricRecord): metric_name = "" if self._prefix != "": metric_name = self._prefix + "_" - metric_name += self._sanitize(metric_record.metric.name) + metric_name += self._sanitize(metric_record.instrument.name) - if isinstance(metric_record.metric, Counter): + if isinstance(metric_record.instrument, Counter): prometheus_metric = CounterMetricFamily( name=metric_name, - documentation=metric_record.metric.description, + documentation=metric_record.instrument.description, labels=label_keys, ) prometheus_metric.add_metric( labels=label_values, value=metric_record.aggregator.checkpoint ) # TODO: Add support for histograms when supported in OT - elif isinstance(metric_record.metric, ValueRecorder): + elif isinstance(metric_record.instrument, ValueRecorder): prometheus_metric = UnknownMetricFamily( name=metric_name, - documentation=metric_record.metric.description, + documentation=metric_record.instrument.description, labels=label_keys, ) prometheus_metric.add_metric( @@ -176,7 +176,7 @@ def _translate_to_prometheus(self, metric_record: MetricRecord): else: logger.warning( - "Unsupported metric type. %s", type(metric_record.metric) + "Unsupported metric type. %s", type(metric_record.instrument) ) return prometheus_metric diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index c1c549b9a15..f5158e1a247 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -15,7 +15,12 @@ import abc from typing import Sequence, Type -from opentelemetry.metrics import Counter, InstrumentT, ValueRecorder, ValueObserver +from opentelemetry.metrics import ( + Counter, + InstrumentT, + ValueRecorder, + ValueObserver, +) from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, From bf17d2d0d6a5b540166152e827eccfa88c17c602 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 11:55:45 -0700 Subject: [PATCH 11/29] mypy --- opentelemetry-api/tests/test_implementation.py | 3 ++- .../src/opentelemetry/sdk/metrics/export/batcher.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/tests/test_implementation.py b/opentelemetry-api/tests/test_implementation.py index 735ac4a683f..0dc37384ee9 100644 --- a/opentelemetry-api/tests/test_implementation.py +++ b/opentelemetry-api/tests/test_implementation.py @@ -83,7 +83,8 @@ def test_create_metric(self): def test_register_observer(self): meter = metrics.DefaultMeter() callback = mock.Mock() - observer = meter.register_observer(callback, "", "", "", int, (), True) + observer = meter.register_observer( + callback, "", "", "", int, metrics.ValueObserver) self.assertIsInstance(observer, metrics.DefaultObserver) def test_unregister_observer(self): diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index f5158e1a247..db3675ecd61 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -18,8 +18,8 @@ from opentelemetry.metrics import ( Counter, InstrumentT, - ValueRecorder, ValueObserver, + ValueRecorder, ) from opentelemetry.sdk.metrics.export import MetricRecord from opentelemetry.sdk.metrics.export.aggregate import ( From 6e34921d265624b77c1234430bf07100dca407ea Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 12:26:57 -0700 Subject: [PATCH 12/29] typing --- 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 45176728f4d..58a43ea1336 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -378,7 +378,7 @@ def register_observer( observer_type=Type[metrics_api.ObserverT], label_keys: Sequence[str] = (), enabled: bool = True, - ) -> metrics_api.ObserverT: + ) -> metrics_api.Observer: ob = observer_type( callback, name, @@ -393,7 +393,7 @@ def register_observer( self.observers.add(ob) return ob - def unregister_observer(self, observer: metrics_api.ObserverT) -> None: + def unregister_observer(self, observer: metrics_api.Observer) -> None: with self.observers_lock: self.observers.remove(observer) From 2ee25855aede71225bc93a2128fe7ae372980061 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 13:16:34 -0700 Subject: [PATCH 13/29] fix lint --- opentelemetry-api/tests/test_implementation.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/tests/test_implementation.py b/opentelemetry-api/tests/test_implementation.py index 0dc37384ee9..d0f9404a911 100644 --- a/opentelemetry-api/tests/test_implementation.py +++ b/opentelemetry-api/tests/test_implementation.py @@ -84,7 +84,8 @@ def test_register_observer(self): meter = metrics.DefaultMeter() callback = mock.Mock() observer = meter.register_observer( - callback, "", "", "", int, metrics.ValueObserver) + callback, "", "", "", int, metrics.ValueObserver + ) self.assertIsInstance(observer, metrics.DefaultObserver) def test_unregister_observer(self): From 677515a8baa0e1f2bec3b6d3a441a126b933bb28 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 2 Jun 2020 15:28:52 -0700 Subject: [PATCH 14/29] comment --- opentelemetry-api/src/opentelemetry/metrics/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 81158f32e16..953a217d461 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -253,8 +253,7 @@ def get_meter( MetricT = TypeVar("MetricT", Counter, ValueRecorder) InstrumentT = TypeVar("InstrumentT", Counter, Observer, ValueRecorder) -# TODO: Will populate with other observers when implemented -ObserverT = TypeVar("ObserverT", DefaultObserver, ValueObserver) +ObserverT = TypeVar("ObserverT", bound=Observer) ObserverCallbackT = Callable[[Observer], None] @@ -305,7 +304,6 @@ def create_metric( unit: Unit of the metric values following the UCUM convention (https://unitsofmeasure.org/ucum.html). value_type: The type of values being recorded by the metric. - observer_type: The type of observer being registered. metric_type: The type of metric being created. label_keys: The keys for the labels with dynamic values. enabled: Whether to report the metric by default. @@ -334,6 +332,7 @@ def register_observer( unit: Unit of the metric values following the UCUM convention (https://unitsofmeasure.org/ucum.html). value_type: The type of values being recorded by the metric. + observer_type: The type of observer being registered. label_keys: The keys for the labels with dynamic values. enabled: Whether to report the metric by default. Returns: A new ``Observer`` metric instrument. From 5061df9206037e9fd4b4f66d224d9d44320cf6f3 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 12:36:24 -0700 Subject: [PATCH 15/29] sumobserver --- opentelemetry-api/CHANGELOG.md | 4 +- .../src/opentelemetry/metrics/__init__.py | 12 +++ .../tests/metrics/test_metrics.py | 10 ++- opentelemetry-sdk/CHANGELOG.md | 2 + .../src/opentelemetry/sdk/metrics/__init__.py | 78 +++++++++++++++---- .../sdk/metrics/export/aggregate.py | 28 +++++++ .../sdk/metrics/export/batcher.py | 4 + .../tests/metrics/test_metrics.py | 72 +++++++++++++++++ 8 files changed, 193 insertions(+), 17 deletions(-) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 3acfb90597e..ada8015a61b 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -2,10 +2,12 @@ ## Unreleased -- Rename Observer to ValueObserver +- Rename Observer to ValueObserver in metrics ([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751)) - Rename Measure to ValueRecorder in metrics ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) +- Add SumObserver in metrics + ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) ## 0.8b0 diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 953a217d461..62bac06f22c 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -189,6 +189,18 @@ def observe(self, value: ValueT, labels: Dict[str, str]) -> None: """ +class SumObserver(Observer): + """No-op implementation of ``SumObserver``.""" + + def observe(self, value: ValueT, labels: Dict[str, str]) -> None: + """Captures ``value`` to the sumobserver. + + Args: + value: The value to capture to this sumobserver metric. + labels: Labels associated to ``value``. + """ + + class ValueObserver(Observer): """No-op implementation of ``ValueObserver``.""" diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 897c7492e42..4d42d361d4b 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -56,6 +56,14 @@ def test_bound_valuerecorder(self): bound_valuerecorder = metrics.BoundValueRecorder() bound_valuerecorder.record(1) - def test_observer(self): + def test_default_observer(self): observer = metrics.DefaultObserver() observer.observe(1, {}) + + def test_sum_observer(self): + observer = metrics.SumObserver() + observer.observe(1, {}) + + def test_value_observer(self): + observer = metrics.ValueObserver() + observer.observe(1, {}) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index d4b92fae1ce..545f443a386 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -6,6 +6,8 @@ ([#751](https://github.com/open-telemetry/opentelemetry-python/pull/751)) - Rename Measure to ValueRecorder in metrics ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) +- Add SumObserver in metrics + ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) ## 0.8b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 58a43ea1336..441a5831cf1 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -105,12 +105,16 @@ def record(self, value: metrics_api.ValueT) -> None: class Metric(metrics_api.Metric): - """Base class for all metric types. - - Also known as metric instrument. This is the class that is used to - represent a metric that is to be continuously recorded and tracked. Each - metric has a set of bound metrics that are created from the metric. See - `BaseBoundInstrument` for information on bound metric instruments. + """Base class for all synchronous metric types. + + This is the class that is used to represent a metric that is to be + synchronously recorded and tracked. Synchronous instruments are called + inside a request, meaning they have an associated distributed context + (i.e. Span context, correlation context). Multiple metric events may occur + for a synchronous instrument within a give collection interval. + + Each metric has a set of bound metrics that are created from the metric. + See `BaseBoundInstrument` for information on bound metric instruments. """ BOUND_INSTR_TYPE = BaseBoundInstrument @@ -190,9 +194,14 @@ def record( UPDATE_FUNCTION = record -class ValueObserver(metrics_api.ValueObserver): - """See `opentelemetry.metrics.ValueObserver`.""" +class Observer(metrics_api.Observer): + """Base class for all asynchronous metric types. + Also known as Observers, observer metric instruments are asynchronous in + that they are reported by a callback, once per collection interval, and + lack context. They are permitted to report only one value per distinct + label set per period. + """ def __init__( self, callback: metrics_api.ObserverCallbackT, @@ -218,15 +227,10 @@ def __init__( def observe( self, value: metrics_api.ValueT, labels: Dict[str, str] ) -> None: - if not self.enabled: - return - if not isinstance(value, self.value_type): - logger.warning( - "Invalid value passed for %s.", self.value_type.__name__ - ) + key = get_labels_as_key(labels) + if not self._validate_observe(value, key): return - key = get_labels_as_key(labels) if key not in self.aggregators: # TODO: how to cleanup aggregators? self.aggregators[key] = self.meter.batcher.aggregator_for( @@ -235,6 +239,20 @@ def observe( aggregator = self.aggregators[key] aggregator.update(value) + def _validate_observe(self, + value: metrics_api.ValueT, + key: Tuple[Tuple[str, str]], + ): + if not self.enabled: + return False + if not isinstance(value, self.value_type): + logger.warning( + "Invalid value passed for %s.", self.value_type.__name__ + ) + return False + + return True + def run(self) -> bool: try: self.callback(self) @@ -252,6 +270,36 @@ def __repr__(self): ) +class SumObserver(Observer, metrics_api.SumObserver): + """See `opentelemetry.metrics.SumObserver`.""" + + def _validate_observe(self, + value: metrics_api.ValueT, + key: Tuple[Tuple[str, str]], + ): + if super()._validate_observe(value, key): + # Must be non-decreasing because monotonic + if key in self.aggregators and \ + self.aggregators[key].current is not None: + if value < self.aggregators[key].current: + logger.warning( + "Value passed must be non-decreasing." + ) + return False + return True + return False + + +class ValueObserver(Observer, metrics_api.ValueObserver): + """See `opentelemetry.metrics.ValueObserver`.""" + + def _validate_observe(self, + value: metrics_api.ValueT, + key: Tuple[Tuple[str, str]], + ): + return super()._validate_observe(value, key) + + class Record: """Container class used for processing in the `Batcher`""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py index 1745d854e9d..ad728d8c502 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/aggregate.py @@ -125,6 +125,34 @@ def merge(self, other): ) +class LastValueAggregator(Aggregator): + """Aggregator that stores last value results.""" + + def __init__(self): + super().__init__() + self._lock = threading.Lock() + self.last_update_timestamp = None + + def update(self, value): + with self._lock: + self.current = value + self.last_update_timestamp = time_ns() + + def take_checkpoint(self): + with self._lock: + self.checkpoint = self.current + self.current = None + + def merge(self, other): + last = self.checkpoint.last + self.last_update_timestamp = get_latest_timestamp( + self.last_update_timestamp, other.last_update_timestamp + ) + if self.last_update_timestamp == other.last_update_timestamp: + last = other.checkpoint.last + self.checkpoint = last + + class ValueObserverAggregator(Aggregator): """Same as MinMaxSumCount but also with last value.""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index db3675ecd61..f3bc91b0602 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -18,6 +18,7 @@ from opentelemetry.metrics import ( Counter, InstrumentT, + SumObserver, ValueObserver, ValueRecorder, ) @@ -25,6 +26,7 @@ from opentelemetry.sdk.metrics.export.aggregate import ( Aggregator, CounterAggregator, + LastValueAggregator, MinMaxSumCountAggregator, ValueObserverAggregator, ) @@ -54,6 +56,8 @@ def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: # pylint:disable=R0201 if issubclass(instrument_type, Counter): return CounterAggregator() + if issubclass(instrument_type, SumObserver): + return LastValueAggregator() if issubclass(instrument_type, ValueRecorder): return MinMaxSumCountAggregator() if issubclass(instrument_type, ValueObserver): diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index d4d48ff06ec..bc9c2918206 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -285,6 +285,78 @@ def test_record(self): ) +class TestSumObserver(unittest.TestCase): + def test_observe(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + key_labels = tuple(sorted(labels.items())) + values = (37, 42, 60, 100) + for val in values: + observer.observe(val, labels) + + self.assertEqual(observer.aggregators[key_labels].current, values[-1]) + + def test_observe_disabled(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), False + ) + labels = {"key": "value"} + observer.observe(37, labels) + self.assertEqual(len(observer.aggregators), 0) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_incorrect_type(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37.0, labels) + self.assertEqual(len(observer.aggregators), 0) + self.assertTrue(logger_mock.warning.called) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_non_decreasing_error(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.SumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37, labels) + observer.observe(14, labels) + self.assertEqual(len(observer.aggregators), 1) + self.assertTrue(logger_mock.warning.called) + + def test_run(self): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + observer = metrics.SumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertTrue(observer.run()) + callback.assert_called_once_with(observer) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_run_exception(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + callback.side_effect = Exception("We have a problem!") + + observer = metrics.SumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertFalse(observer.run()) + self.assertTrue(logger_mock.warning.called) + + class TestValueObserver(unittest.TestCase): def test_observe(self): meter = metrics.MeterProvider().get_meter(__name__) From 9f1081574bcbd20aa16cfc74314c756b18489bbf Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 12:47:27 -0700 Subject: [PATCH 16/29] changelog --- opentelemetry-api/CHANGELOG.md | 2 +- opentelemetry-sdk/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index cbce5f59bc7..88c9c4b9671 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -9,7 +9,7 @@ - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) - Add SumObserver in metrics - ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) + ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 819f7a2caf5..447bce245f5 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -11,7 +11,7 @@ - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) - Add SumObserver in metrics - ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) + ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 From a18ee9589df912ad3537e28347cdfa67d4abc9fe Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 13:36:04 -0700 Subject: [PATCH 17/29] lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index a0949897e7c..f2e1f97c7b5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -202,6 +202,7 @@ class Observer(metrics_api.Observer): lack context. They are permitted to report only one value per distinct label set per period. """ + def __init__( self, callback: metrics_api.ObserverCallbackT, @@ -239,9 +240,8 @@ def observe( aggregator = self.aggregators[key] aggregator.update(value) - def _validate_observe(self, - value: metrics_api.ValueT, - key: Tuple[Tuple[str, str]], + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], ): if not self.enabled: return False @@ -273,18 +273,17 @@ def __repr__(self): class SumObserver(Observer, metrics_api.SumObserver): """See `opentelemetry.metrics.SumObserver`.""" - def _validate_observe(self, - value: metrics_api.ValueT, - key: Tuple[Tuple[str, str]], + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], ): if super()._validate_observe(value, key): # Must be non-decreasing because monotonic - if key in self.aggregators and \ - self.aggregators[key].current is not None: + if ( + key in self.aggregators + and self.aggregators[key].current is not None + ): if value < self.aggregators[key].current: - logger.warning( - "Value passed must be non-decreasing." - ) + logger.warning("Value passed must be non-decreasing.") return False return True return False @@ -293,9 +292,8 @@ def _validate_observe(self, class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" - def _validate_observe(self, - value: metrics_api.ValueT, - key: Tuple[Tuple[str, str]], + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], ): return super()._validate_observe(value, key) From edb310d2910b650c7895cccaa5158266a5e28c45 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 14:15:27 -0700 Subject: [PATCH 18/29] 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 f2e1f97c7b5..d531e79e258 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -112,7 +112,7 @@ class Metric(metrics_api.Metric): inside a request, meaning they have an associated distributed context (i.e. Span context, correlation context). Multiple metric events may occur for a synchronous instrument within a give collection interval. - + Each metric has a set of bound metrics that are created from the metric. See `BaseBoundInstrument` for information on bound metric instruments. """ From 5c67c2ac2af205d53d601b39553ae8923ec55e76 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 14:37:50 -0700 Subject: [PATCH 19/29] lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index d531e79e258..20545dd1384 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -240,8 +240,11 @@ def observe( aggregator = self.aggregators[key] aggregator.update(value) + # pylint: disable=W0613 def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + self, + value: metrics_api.ValueT, + key: Tuple[Tuple[str, str]] = None, ): if not self.enabled: return False @@ -274,7 +277,9 @@ class SumObserver(Observer, metrics_api.SumObserver): """See `opentelemetry.metrics.SumObserver`.""" def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + self, + value: metrics_api.ValueT, + key: Tuple[Tuple[str, str]], ): if super()._validate_observe(value, key): # Must be non-decreasing because monotonic @@ -292,8 +297,9 @@ def _validate_observe( class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" + # pylint: disable=W0235 def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, ): return super()._validate_observe(value, key) From ce333713da9a2eb291d8245ca585b38ed9be50bc Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 15:17:42 -0700 Subject: [PATCH 20/29] black --- .../src/opentelemetry/sdk/metrics/__init__.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 20545dd1384..d21a69e3527 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -242,9 +242,7 @@ def observe( # pylint: disable=W0613 def _validate_observe( - self, - value: metrics_api.ValueT, - key: Tuple[Tuple[str, str]] = None, + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, ): if not self.enabled: return False @@ -277,9 +275,7 @@ class SumObserver(Observer, metrics_api.SumObserver): """See `opentelemetry.metrics.SumObserver`.""" def _validate_observe( - self, - value: metrics_api.ValueT, - key: Tuple[Tuple[str, str]], + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], ): if super()._validate_observe(value, key): # Must be non-decreasing because monotonic From aa0007b6c00e7d17c11d0ad3a3bd62acb32cd709 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 16:09:37 -0700 Subject: [PATCH 21/29] lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index d21a69e3527..3928b8068f6 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -275,12 +275,13 @@ class SumObserver(Observer, metrics_api.SumObserver): """See `opentelemetry.metrics.SumObserver`.""" def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, ): if super()._validate_observe(value, key): # Must be non-decreasing because monotonic if ( - key in self.aggregators + key is not None + and key in self.aggregators and self.aggregators[key].current is not None ): if value < self.aggregators[key].current: From f33e7e4234b20dfd20fa3ef479bbe1da01cf0892 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 22:09:15 -0700 Subject: [PATCH 22/29] fix example --- docs/examples/basic_meter/observer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index b61b9e4db80..d3aa02168d4 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -60,6 +60,7 @@ def get_ram_usage_callback(observer): description="RAM memory usage", unit="1", value_type=float, + observer_type=ValueObserver, label_keys=(), ) From 541a11cbdd333aed15b6fe07f6c8d3c345435ef3 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 22:50:44 -0700 Subject: [PATCH 23/29] updownobserver --- .../src/opentelemetry/metrics/__init__.py | 12 ++++ .../tests/metrics/test_metrics.py | 4 ++ opentelemetry-sdk/CHANGELOG.md | 2 +- .../src/opentelemetry/sdk/metrics/__init__.py | 10 ++++ .../sdk/metrics/export/batcher.py | 4 +- .../tests/metrics/test_metrics.py | 60 +++++++++++++++++++ 6 files changed, 90 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/metrics/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/__init__.py index 93289c019b3..569930d6f3b 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/__init__.py @@ -201,6 +201,18 @@ def observe(self, value: ValueT, labels: Dict[str, str]) -> None: """ +class UpDownSumObserver(Observer): + """No-op implementation of ``UpDownSumObserver``.""" + + def observe(self, value: ValueT, labels: Dict[str, str]) -> None: + """Captures ``value`` to the updownsumobserver. + + Args: + value: The value to capture to this updownsumobserver metric. + labels: Labels associated to ``value``. + """ + + class ValueObserver(Observer): """No-op implementation of ``ValueObserver``.""" diff --git a/opentelemetry-api/tests/metrics/test_metrics.py b/opentelemetry-api/tests/metrics/test_metrics.py index 4d42d361d4b..b3cbdefd15a 100644 --- a/opentelemetry-api/tests/metrics/test_metrics.py +++ b/opentelemetry-api/tests/metrics/test_metrics.py @@ -64,6 +64,10 @@ def test_sum_observer(self): observer = metrics.SumObserver() observer.observe(1, {}) + def test_updown_sum_observer(self): + observer = metrics.UpDownSumObserver() + observer.observe(1, {}) + def test_value_observer(self): observer = metrics.ValueObserver() observer.observe(1, {}) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 447bce245f5..6213e31ae61 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -10,7 +10,7 @@ ([#775](https://github.com/open-telemetry/opentelemetry-python/pull/775)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) -- Add SumObserver in metrics +- Add SumObserver and UpDownSumObserver in metrics ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 3928b8068f6..909b16b1698 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -291,6 +291,16 @@ def _validate_observe( return False +class UpDownSumObserver(Observer, metrics_api.UpDownSumObserver): + """See `opentelemetry.metrics.UpDownSumObserver`.""" + + # pylint: disable=W0235 + def _validate_observe( + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, + ): + return super()._validate_observe(value, key) + + class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index f3bc91b0602..a3cb5ef65b0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -19,6 +19,7 @@ Counter, InstrumentT, SumObserver, + UpDownSumObserver, ValueObserver, ValueRecorder, ) @@ -56,7 +57,8 @@ def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: # pylint:disable=R0201 if issubclass(instrument_type, Counter): return CounterAggregator() - if issubclass(instrument_type, SumObserver): + if issubclass(instrument_type, SumObserver) or \ + issubclass(instrument_type, UpDownSumObserver): return LastValueAggregator() if issubclass(instrument_type, ValueRecorder): return MinMaxSumCountAggregator() diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 8a616e1da97..02cf2c93548 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -362,6 +362,66 @@ def test_run_exception(self, logger_mock): self.assertTrue(logger_mock.warning.called) +class TestUpDownSumObserver(unittest.TestCase): + def test_observe(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + key_labels = tuple(sorted(labels.items())) + values = (37, 42, 14, 30) + for val in values: + observer.observe(val, labels) + + self.assertEqual(observer.aggregators[key_labels].current, values[-1]) + + def test_observe_disabled(self): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), False + ) + labels = {"key": "value"} + observer.observe(37, labels) + self.assertEqual(len(observer.aggregators), 0) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_observe_incorrect_type(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + observer = metrics.UpDownSumObserver( + None, "name", "desc", "unit", int, meter, ("key",), True + ) + labels = {"key": "value"} + observer.observe(37.0, labels) + self.assertEqual(len(observer.aggregators), 0) + self.assertTrue(logger_mock.warning.called) + + def test_run(self): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + observer = metrics.UpDownSumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertTrue(observer.run()) + callback.assert_called_once_with(observer) + + @mock.patch("opentelemetry.sdk.metrics.logger") + def test_run_exception(self, logger_mock): + meter = metrics.MeterProvider().get_meter(__name__) + + callback = mock.Mock() + callback.side_effect = Exception("We have a problem!") + + observer = metrics.UpDownSumObserver( + callback, "name", "desc", "unit", int, meter, (), True + ) + + self.assertFalse(observer.run()) + self.assertTrue(logger_mock.warning.called) + + class TestValueObserver(unittest.TestCase): def test_observe(self): meter = metrics.MeterProvider().get_meter(__name__) From e9489c7dcd27de28b2ba70daea17c209947b9e8c Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 23:19:30 -0700 Subject: [PATCH 24/29] lint --- .../src/opentelemetry/sdk/metrics/export/batcher.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index a3cb5ef65b0..e4b5b42697d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -57,8 +57,9 @@ def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: # pylint:disable=R0201 if issubclass(instrument_type, Counter): return CounterAggregator() - if issubclass(instrument_type, SumObserver) or \ - issubclass(instrument_type, UpDownSumObserver): + if issubclass(instrument_type, SumObserver) or issubclass( + instrument_type, UpDownSumObserver + ): return LastValueAggregator() if issubclass(instrument_type, ValueRecorder): return MinMaxSumCountAggregator() From f1a276703a44d04cc086bcfd8436d2db83c0e761 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 14:11:29 -0700 Subject: [PATCH 25/29] address comments --- .../src/opentelemetry/sdk/metrics/__init__.py | 46 ++++++++----------- .../sdk/metrics/export/batcher.py | 4 +- 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 909b16b1698..92a68caf664 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -242,8 +242,8 @@ def observe( # pylint: disable=W0613 def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, - ): + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + ) -> bool: if not self.enabled: return False if not isinstance(value, self.value_type): @@ -275,40 +275,30 @@ class SumObserver(Observer, metrics_api.SumObserver): """See `opentelemetry.metrics.SumObserver`.""" def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, - ): - if super()._validate_observe(value, key): - # Must be non-decreasing because monotonic - if ( - key is not None - and key in self.aggregators - and self.aggregators[key].current is not None - ): - if value < self.aggregators[key].current: - logger.warning("Value passed must be non-decreasing.") - return False - return True - return False + self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]], + ) -> bool: + if not super()._validate_observe(value, key): + return False + # Must be non-decreasing because monotonic + if ( + key in self.aggregators + and self.aggregators[key].current is not None + ): + if value < self.aggregators[key].current: + logger.warning("Value passed must be non-decreasing.") + return False + return True + class UpDownSumObserver(Observer, metrics_api.UpDownSumObserver): """See `opentelemetry.metrics.UpDownSumObserver`.""" - - # pylint: disable=W0235 - def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, - ): - return super()._validate_observe(value, key) + pass class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" - - # pylint: disable=W0235 - def _validate_observe( - self, value: metrics_api.ValueT, key: Tuple[Tuple[str, str]] = None, - ): - return super()._validate_observe(value, key) + pass class Record: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py index e4b5b42697d..c0405d1ffb8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/batcher.py @@ -57,9 +57,7 @@ def aggregator_for(self, instrument_type: Type[InstrumentT]) -> Aggregator: # pylint:disable=R0201 if issubclass(instrument_type, Counter): return CounterAggregator() - if issubclass(instrument_type, SumObserver) or issubclass( - instrument_type, UpDownSumObserver - ): + if issubclass(instrument_type, (SumObserver, UpDownSumObserver)): return LastValueAggregator() if issubclass(instrument_type, ValueRecorder): return MinMaxSumCountAggregator() From 168c874f552107933c38485ff7e5ec8bc5ba8a35 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 14:33:41 -0700 Subject: [PATCH 26/29] black --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 92a68caf664..cab6d8ae607 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -290,14 +290,15 @@ def _validate_observe( return True - class UpDownSumObserver(Observer, metrics_api.UpDownSumObserver): """See `opentelemetry.metrics.UpDownSumObserver`.""" + pass class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" + pass From 5a37c0ceacf08c454d71b479612a57b8008046aa Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 9 Jun 2020 15:09:17 -0700 Subject: [PATCH 27/29] Update opentelemetry-api/CHANGELOG.md Co-authored-by: alrex --- opentelemetry-api/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 1862c23093d..670e3d7a7f5 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -10,7 +10,7 @@ ([#552](https://github.com/open-telemetry/opentelemetry-python/pull/552)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) -- Add SumObserver in metrics +- Add SumObserver and UpDownSumObserver in metrics ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 From 5463add632d5667dd640833ce81b3ea32dc42012 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 9 Jun 2020 15:09:26 -0700 Subject: [PATCH 28/29] Update opentelemetry-sdk/CHANGELOG.md Co-authored-by: alrex --- opentelemetry-sdk/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index fdb7bc6810e..23007dde5a8 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -12,7 +12,7 @@ ([#775](https://github.com/open-telemetry/opentelemetry-python/pull/775)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) -- Add SumObserver and UpDownSumObserver in metrics +- Add SumObserver, UpDownSumObserver and LastValueAggregator in metrics ([#789](https://github.com/open-telemetry/opentelemetry-python/pull/789)) ## 0.8b0 From 9de59a9a164777a28db56dc593190350d7c8673b Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 15:09:45 -0700 Subject: [PATCH 29/29] lint --- opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index cab6d8ae607..7156f68c165 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -293,14 +293,10 @@ def _validate_observe( class UpDownSumObserver(Observer, metrics_api.UpDownSumObserver): """See `opentelemetry.metrics.UpDownSumObserver`.""" - pass - class ValueObserver(Observer, metrics_api.ValueObserver): """See `opentelemetry.metrics.ValueObserver`.""" - pass - class Record: """Container class used for processing in the `Batcher`"""