From 0ea0301a3ff244dc1b8b4403811188fe31e96808 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 22:08:50 -0700 Subject: [PATCH 01/10] init --- docs/examples/basic_meter/basic_metrics.py | 20 +++++++------ .../basic_meter/calling_conventions.py | 11 ++++--- docs/examples/basic_meter/observer.py | 10 +++++-- .../src/opentelemetry/sdk/metrics/__init__.py | 30 +++++++++++++++---- .../sdk/metrics/export/controller.py | 25 +++++++++++----- 5 files changed, 66 insertions(+), 30 deletions(-) diff --git a/docs/examples/basic_meter/basic_metrics.py b/docs/examples/basic_meter/basic_metrics.py index b9ff8d87417..07b72e299ad 100644 --- a/docs/examples/basic_meter/basic_metrics.py +++ b/docs/examples/basic_meter/basic_metrics.py @@ -28,8 +28,6 @@ from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter from opentelemetry.sdk.metrics.export.controller import PushController -stateful = True - print( "Starting example, values will be printed to the console every 5 seconds." ) @@ -37,17 +35,21 @@ # Stateful determines whether how metrics are collected: if true, metrics # accumulate over the process lifetime. If false, metrics are reset at the # beginning of each collection interval. -metrics.set_meter_provider(MeterProvider(stateful)) -# The Meter is responsible for creating and recording metrics. Each meter has a -# unique name, which we set as the module's name here. -meter = metrics.get_meter(__name__) +stateful = True # Exporter to export metrics to the console exporter = ConsoleMetricsExporter() -# A PushController collects metrics created from meter and exports it via the -# exporter every interval -controller = PushController(meter=meter, exporter=exporter, interval=5) +metrics.set_meter_provider( + MeterProvider( + exporter=exporter, + interval=5, + stateful=stateful + ) +) +# The Meter is responsible for creating and recording metrics. Each meter has a +# unique name, which we set as the module's name here. +meter = metrics.get_meter(__name__) # Metric instruments allow to capture measurements requests_counter = meter.create_metric( diff --git a/docs/examples/basic_meter/calling_conventions.py b/docs/examples/basic_meter/calling_conventions.py index f8cc3dddbb1..b1b88bae991 100644 --- a/docs/examples/basic_meter/calling_conventions.py +++ b/docs/examples/basic_meter/calling_conventions.py @@ -24,10 +24,13 @@ from opentelemetry.sdk.metrics.export.controller import PushController # Use the meter type provided by the SDK package -metrics.set_meter_provider(MeterProvider()) +metrics.set_meter_provider( + MeterProvider( + exporter=ConsoleMetricsExporter(), + interval=5 + ) +) meter = metrics.get_meter(__name__) -exporter = ConsoleMetricsExporter() -controller = PushController(meter=meter, exporter=exporter, interval=5) requests_counter = meter.create_metric( name="requests", @@ -62,7 +65,7 @@ # You can record metrics directly using the metric instrument. You pass in # labels that you would like to record for. requests_counter.add(25, labels) -time.sleep(5) +time.sleep(10) print("Updating using a bound instrument...") # You can record metrics with bound metric instruments. Bound metric diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index b61b9e4db80..54380647ee9 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -24,10 +24,13 @@ from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher from opentelemetry.sdk.metrics.export.controller import PushController -metrics.set_meter_provider(MeterProvider()) +metrics.set_meter_provider( + MeterProvider( + exporter=ConsoleMetricsExporter(), + interval=5, + ) +) meter = metrics.get_meter(__name__) -exporter = ConsoleMetricsExporter() -controller = PushController(meter=meter, exporter=exporter, interval=2) # Callback to gather cpu usage @@ -60,6 +63,7 @@ def get_ram_usage_callback(observer): description="RAM memory usage", unit="1", value_type=float, + observer_type=ValueObserver, label_keys=(), ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 507e00d8ead..6a9644b6ed0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -17,8 +17,10 @@ from typing import Dict, Sequence, Tuple, Type from opentelemetry import metrics as metrics_api +from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter from opentelemetry.sdk.metrics.export.aggregate import Aggregator from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher +from opentelemetry.sdk.metrics.export.controller import PushController from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationInfo @@ -406,10 +408,23 @@ class MeterProvider(metrics_api.MeterProvider): """ def __init__( - self, stateful=True, resource: Resource = Resource.create_empty(), + self, + exporter=None, + interval=15.0, + stateful=True, + resource: Resource = Resource.create_empty(), ): + if not exporter: + self.exporter = ConsoleMetricsExporter() self.stateful = stateful self.resource = resource + # InstrumentationInfo to Meter + self.meter_registry = {} + self.controller = PushController( + self.meter_registry, + exporter, + interval + ) def get_meter( self, @@ -418,9 +433,12 @@ def get_meter( ) -> "metrics_api.Meter": if not instrumenting_module_name: # Reject empty strings too. raise ValueError("get_meter called with missing module name.") - return Meter( - self, - InstrumentationInfo( - instrumenting_module_name, instrumenting_library_version - ), + info = InstrumentationInfo( + instrumenting_module_name, + instrumenting_library_version, ) + meter = self.meter_registry.get(info) + if not meter: + meter = Meter(self, info) + self.meter_registry[info] = meter + return meter diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py index 88abed410a1..bddd01eaa99 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py @@ -27,9 +27,15 @@ class PushController(threading.Thread): daemon = True - def __init__(self, meter, exporter, interval, shutdown_on_exit=True): + def __init__( + self, + meter_registry, + exporter, + interval, + shutdown_on_exit=True, + ): super().__init__() - self.meter = meter + self.meter_registry = meter_registry self.exporter = exporter self.interval = interval self.finished = threading.Event() @@ -40,7 +46,9 @@ def __init__(self, meter, exporter, interval, shutdown_on_exit=True): def run(self): while not self.finished.wait(self.interval): - self.tick() + # Only run if meters exist + if self.meter_registry: + self.tick() def shutdown(self): self.finished.set() @@ -53,10 +61,11 @@ def shutdown(self): def tick(self): # Collect all of the meter's metrics to be exported - self.meter.collect() token = attach(set_value("suppress_instrumentation", True)) - # Export the given metrics in the batcher - self.exporter.export(self.meter.batcher.checkpoint_set()) + for meter in self.meter_registry.values(): + meter.collect() + # Export the collected metrics + self.exporter.export(meter.batcher.checkpoint_set()) + # Perform post-exporting logic + meter.batcher.finished_collection() detach(token) - # Perform post-exporting logic based on batcher configuration - self.meter.batcher.finished_collection() From 8e8893514e56ef3f81d9ee4641c205f70daa1794 Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 22:30:38 -0700 Subject: [PATCH 02/10] changelog --- opentelemetry-api/CHANGELOG.md | 2 ++ opentelemetry-sdk/CHANGELOG.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 4503489fa85..071e7603d3e 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -8,6 +8,8 @@ ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) +- Move exporter and controller as part of MeterProvider + ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) ## 0.8b0 diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index d8eb23ab525..2c319372db8 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -10,6 +10,8 @@ ([#775](https://github.com/open-telemetry/opentelemetry-python/pull/775)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) +- Move exporter and controller as part of MeterProvider + ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) ## 0.8b0 From 207a658cc96b597e33b996aef3b98cfc2311981d Mon Sep 17 00:00:00 2001 From: Leighton Date: Mon, 8 Jun 2020 23:54:02 -0700 Subject: [PATCH 03/10] pipeline --- docs/examples/basic_meter/basic_metrics.py | 19 ++--- .../basic_meter/calling_conventions.py | 9 +-- docs/examples/basic_meter/observer.py | 11 +-- opentelemetry-api/CHANGELOG.md | 2 - opentelemetry-sdk/CHANGELOG.md | 2 +- .../src/opentelemetry/sdk/metrics/__init__.py | 74 +++++++++++++------ .../sdk/metrics/export/controller.py | 35 +++------ 7 files changed, 75 insertions(+), 77 deletions(-) diff --git a/docs/examples/basic_meter/basic_metrics.py b/docs/examples/basic_meter/basic_metrics.py index 07b72e299ad..1f1165b6277 100644 --- a/docs/examples/basic_meter/basic_metrics.py +++ b/docs/examples/basic_meter/basic_metrics.py @@ -26,7 +26,6 @@ from opentelemetry import metrics from opentelemetry.sdk.metrics import Counter, MeterProvider, ValueRecorder from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter -from opentelemetry.sdk.metrics.export.controller import PushController print( "Starting example, values will be printed to the console every 5 seconds." @@ -37,20 +36,18 @@ # beginning of each collection interval. stateful = True -# Exporter to export metrics to the console -exporter = ConsoleMetricsExporter() +# Sets the global MeterProvider instance +metrics.set_meter_provider(MeterProvider()) -metrics.set_meter_provider( - MeterProvider( - exporter=exporter, - interval=5, - stateful=stateful - ) -) # The Meter is responsible for creating and recording metrics. Each meter has a # unique name, which we set as the module's name here. meter = metrics.get_meter(__name__) +# Exporter to export metrics to the console +exporter = ConsoleMetricsExporter() + +metrics.get_meter_provider().start_pipeline(meter, exporter, 5) + # Metric instruments allow to capture measurements requests_counter = meter.create_metric( name="requests", @@ -79,7 +76,7 @@ # Update the metric instruments using the direct calling convention requests_counter.add(25, staging_labels) requests_size.record(100, staging_labels) -time.sleep(5) +time.sleep(10) requests_counter.add(50, staging_labels) requests_size.record(5000, staging_labels) diff --git a/docs/examples/basic_meter/calling_conventions.py b/docs/examples/basic_meter/calling_conventions.py index b1b88bae991..3615f60d7d4 100644 --- a/docs/examples/basic_meter/calling_conventions.py +++ b/docs/examples/basic_meter/calling_conventions.py @@ -21,16 +21,11 @@ from opentelemetry import metrics from opentelemetry.sdk.metrics import Counter, MeterProvider, ValueRecorder from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter -from opentelemetry.sdk.metrics.export.controller import PushController # Use the meter type provided by the SDK package -metrics.set_meter_provider( - MeterProvider( - exporter=ConsoleMetricsExporter(), - interval=5 - ) -) +metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) +metrics.get_meter_provider().start_pipeline(meter, ConsoleMetricsExporter(), 5) requests_counter = meter.create_metric( name="requests", diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index 54380647ee9..56ebfd9ed68 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -21,17 +21,10 @@ from opentelemetry import metrics 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 -metrics.set_meter_provider( - MeterProvider( - exporter=ConsoleMetricsExporter(), - interval=5, - ) -) +metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) - +metrics.get_meter_provider().start_pipeline(meter, ConsoleMetricsExporter(), 5) # Callback to gather cpu usage def get_cpu_usage_callback(observer): diff --git a/opentelemetry-api/CHANGELOG.md b/opentelemetry-api/CHANGELOG.md index 071e7603d3e..4503489fa85 100644 --- a/opentelemetry-api/CHANGELOG.md +++ b/opentelemetry-api/CHANGELOG.md @@ -8,8 +8,6 @@ ([#761](https://github.com/open-telemetry/opentelemetry-python/pull/761)) - Rename Observer to ValueObserver ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) -- Move exporter and controller as part of MeterProvider - ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) ## 0.8b0 diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 2c319372db8..cb2ad8bf670 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)) -- Move exporter and controller as part of MeterProvider +- Add start_pipeline to MeterProvider ([#764](https://github.com/open-telemetry/opentelemetry-python/pull/764)) ## 0.8b0 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 6a9644b6ed0..23719a13880 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -12,12 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. +import atexit import logging import threading from typing import Dict, Sequence, Tuple, Type from opentelemetry import metrics as metrics_api -from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter +from opentelemetry.sdk.metrics.export import ( + ConsoleMetricsExporter, + MetricsExporter, +) from opentelemetry.sdk.metrics.export.aggregate import Aggregator from opentelemetry.sdk.metrics.export.batcher import UngroupedBatcher from opentelemetry.sdk.metrics.export.controller import PushController @@ -405,40 +409,68 @@ class MeterProvider(metrics_api.MeterProvider): Args: stateful: Indicates whether meters created are going to be stateful resource: Resource for this MeterProvider + shutdown_on_exit: Register an atexit hook to shut down when the + application exists """ def __init__( self, - exporter=None, - interval=15.0, stateful=True, resource: Resource = Resource.create_empty(), + shutdown_on_exit: bool = True, ): - if not exporter: - self.exporter = ConsoleMetricsExporter() self.stateful = stateful self.resource = resource - # InstrumentationInfo to Meter - self.meter_registry = {} - self.controller = PushController( - self.meter_registry, - exporter, - interval - ) + self._controllers = [] + self._exporters = set() + self._atexit_handler = None + if shutdown_on_exit: + self._atexit_handler = atexit.register(self.shutdown) def get_meter( self, instrumenting_module_name: str, instrumenting_library_version: str = "", ) -> "metrics_api.Meter": + """See `opentelemetry.metrics.MeterProvider`.get_meter.""" if not instrumenting_module_name: # Reject empty strings too. - raise ValueError("get_meter called with missing module name.") - info = InstrumentationInfo( - instrumenting_module_name, - instrumenting_library_version, + instrumenting_module_name = "ERROR:MISSING MODULE NAME" + logger.error("get_meter called with missing module name.") + return Meter( + self, + InstrumentationInfo( + instrumenting_module_name, + instrumenting_library_version, + ) + ) + + def start_pipeline( + self, + meter: metrics_api.Meter, + exporter: MetricsExporter = None, + interval: float = 15.0 + ) -> None: + """Method to begin the collect/export pipeline. + + Args: + meter: The meter to collect metrics from. + exporter: The exporter to export metrics to. + interval: The collect/export interval in seconds. + """ + if not exporter: + exporter = ConsoleMetricsExporter() + self._exporters.add(exporter) + # TODO: Controller type configurable? + self._controllers.append( + PushController( + meter, + exporter, + interval + ) ) - meter = self.meter_registry.get(info) - if not meter: - meter = Meter(self, info) - self.meter_registry[info] = meter - return meter + + def shutdown(self) -> None: + for controller in self._controllers: + controller.shutdown() + for exporter in self._exporters: + exporter.shutdown() diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py index bddd01eaa99..4a5eb1b40c8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py @@ -12,14 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import atexit import threading from opentelemetry.context import attach, detach, set_value class PushController(threading.Thread): - """A push based controller, used for exporting. + """A push based controller, used for collecting and exporting. Uses a worker thread that periodically collects metrics for exporting, exports them and performs some post-processing. @@ -27,45 +26,29 @@ class PushController(threading.Thread): daemon = True - def __init__( - self, - meter_registry, - exporter, - interval, - shutdown_on_exit=True, - ): + def __init__(self, meter, exporter, interval): super().__init__() - self.meter_registry = meter_registry + self.meter = meter self.exporter = exporter self.interval = interval self.finished = threading.Event() - self._atexit_handler = None - if shutdown_on_exit: - self._atexit_handler = atexit.register(self.shutdown) self.start() def run(self): while not self.finished.wait(self.interval): - # Only run if meters exist - if self.meter_registry: - self.tick() + self.tick() def shutdown(self): self.finished.set() # Run one more collection pass to flush metrics batched in the meter self.tick() - self.exporter.shutdown() - if self._atexit_handler is not None: - atexit.unregister(self._atexit_handler) - self._atexit_handler = None def tick(self): # Collect all of the meter's metrics to be exported + self.meter.collect() + # Export the collected metrics token = attach(set_value("suppress_instrumentation", True)) - for meter in self.meter_registry.values(): - meter.collect() - # Export the collected metrics - self.exporter.export(meter.batcher.checkpoint_set()) - # Perform post-exporting logic - meter.batcher.finished_collection() + self.exporter.export(self.meter.batcher.checkpoint_set()) detach(token) + # Perform post-exporting logic + self.meter.batcher.finished_collection() From 5a68ecf8fb840510cb1e757bbae695592ec65ac5 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 00:39:12 -0700 Subject: [PATCH 04/10] add tests --- docs/examples/basic_meter/basic_metrics.py | 2 ++ .../tests/metrics/export/test_export.py | 1 - .../tests/metrics/test_implementation.py | 1 - .../tests/metrics/test_metrics.py | 20 +++++++++++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/examples/basic_meter/basic_metrics.py b/docs/examples/basic_meter/basic_metrics.py index 1f1165b6277..e65aa788b83 100644 --- a/docs/examples/basic_meter/basic_metrics.py +++ b/docs/examples/basic_meter/basic_metrics.py @@ -46,6 +46,8 @@ # Exporter to export metrics to the console exporter = ConsoleMetricsExporter() +# start_pipeline will notify the MeterProvider to begin collecting/exporting +# metrics with the given meter, exporter and interval in seconds metrics.get_meter_provider().start_pipeline(meter, exporter, 5) # Metric instruments allow to capture measurements diff --git a/opentelemetry-sdk/tests/metrics/export/test_export.py b/opentelemetry-sdk/tests/metrics/export/test_export.py index 178e41b2134..901d5a94046 100644 --- a/opentelemetry-sdk/tests/metrics/export/test_export.py +++ b/opentelemetry-sdk/tests/metrics/export/test_export.py @@ -614,7 +614,6 @@ def test_push_controller(self): controller.shutdown() self.assertTrue(controller.finished.isSet()) - exporter.shutdown.assert_any_call() # shutdown should flush the meter self.assertEqual(meter.collect.call_count, 1) diff --git a/opentelemetry-sdk/tests/metrics/test_implementation.py b/opentelemetry-sdk/tests/metrics/test_implementation.py index 1679f618341..d57e6d39783 100644 --- a/opentelemetry-sdk/tests/metrics/test_implementation.py +++ b/opentelemetry-sdk/tests/metrics/test_implementation.py @@ -25,7 +25,6 @@ class TestMeterImplementation(unittest.TestCase): to the API with different expected results. See issue for more details: https://github.com/open-telemetry/opentelemetry-python/issues/142 """ - def test_meter(self): meter = metrics.MeterProvider().get_meter(__name__) metric = meter.create_metric("", "", "", float, metrics.Counter) diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 4c2d691549d..f360bd887d6 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -38,6 +38,26 @@ def test_resource_empty(self): # pylint: disable=protected-access self.assertIs(meter.resource, resources._EMPTY_RESOURCE) + def test_start_pipeline(self): + exporter = mock.Mock() + meter_provider = metrics.MeterProvider() + meter = meter_provider.get_meter(__name__) + # pylint: disable=protected-access + meter_provider.start_pipeline(meter, exporter, 6) + self.assertEqual(len(meter_provider._exporters), 1) + self.assertEqual(len(meter_provider._controllers), 1) + + def test_shutdown(self): + controller = mock.Mock() + exporter = mock.Mock() + meter_provider = metrics.MeterProvider() + # pylint: disable=protected-access + meter_provider._controllers = [controller] + meter_provider._exporters = [exporter] + meter_provider.shutdown() + self.assertEqual(controller.shutdown.call_count, 1) + self.assertEqual(exporter.shutdown.call_count, 1) + class TestMeter(unittest.TestCase): def test_extends_api(self): From ec22325ad6ffa3c3470a8db95eb20d13267a613a Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 09:37:48 -0700 Subject: [PATCH 05/10] lint --- .../src/opentelemetry/sdk/metrics/__init__.py | 15 ++++----------- .../tests/metrics/test_implementation.py | 1 + 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 23719a13880..0ebb9d397ba 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -439,16 +439,15 @@ def get_meter( return Meter( self, InstrumentationInfo( - instrumenting_module_name, - instrumenting_library_version, - ) + instrumenting_module_name, instrumenting_library_version, + ), ) def start_pipeline( self, meter: metrics_api.Meter, exporter: MetricsExporter = None, - interval: float = 15.0 + interval: float = 15.0, ) -> None: """Method to begin the collect/export pipeline. @@ -461,13 +460,7 @@ def start_pipeline( exporter = ConsoleMetricsExporter() self._exporters.add(exporter) # TODO: Controller type configurable? - self._controllers.append( - PushController( - meter, - exporter, - interval - ) - ) + self._controllers.append(PushController(meter, exporter, interval)) def shutdown(self) -> None: for controller in self._controllers: diff --git a/opentelemetry-sdk/tests/metrics/test_implementation.py b/opentelemetry-sdk/tests/metrics/test_implementation.py index d57e6d39783..1679f618341 100644 --- a/opentelemetry-sdk/tests/metrics/test_implementation.py +++ b/opentelemetry-sdk/tests/metrics/test_implementation.py @@ -25,6 +25,7 @@ class TestMeterImplementation(unittest.TestCase): to the API with different expected results. See issue for more details: https://github.com/open-telemetry/opentelemetry-python/issues/142 """ + def test_meter(self): meter = metrics.MeterProvider().get_meter(__name__) metric = meter.create_metric("", "", "", float, metrics.Counter) From 4592653885449323013e92688e287e4b05969452 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 14:31:34 -0700 Subject: [PATCH 06/10] address comments --- docs/examples/basic_meter/observer.py | 1 + docs/examples/cloud_monitoring/basic_metrics.py | 10 +++------- .../opencensus-exporter-metrics/collector.py | 3 +-- .../src/opentelemetry/ext/prometheus/__init__.py | 8 +++----- .../opentelemetry/ext/system_metrics/__init__.py | 7 +++---- .../src/opentelemetry/sdk/metrics/__init__.py | 5 ++++- .../opentelemetry/sdk/metrics/export/controller.py | 14 +++++++++++++- opentelemetry-sdk/tests/metrics/test_metrics.py | 2 ++ 8 files changed, 30 insertions(+), 20 deletions(-) diff --git a/docs/examples/basic_meter/observer.py b/docs/examples/basic_meter/observer.py index 56ebfd9ed68..076c416c0ab 100644 --- a/docs/examples/basic_meter/observer.py +++ b/docs/examples/basic_meter/observer.py @@ -26,6 +26,7 @@ meter = metrics.get_meter(__name__) metrics.get_meter_provider().start_pipeline(meter, ConsoleMetricsExporter(), 5) + # Callback to gather cpu usage def get_cpu_usage_callback(observer): for (number, percent) in enumerate(psutil.cpu_percent(percpu=True)): diff --git a/docs/examples/cloud_monitoring/basic_metrics.py b/docs/examples/cloud_monitoring/basic_metrics.py index e0ceb420b62..1a8e8eb0151 100644 --- a/docs/examples/cloud_monitoring/basic_metrics.py +++ b/docs/examples/cloud_monitoring/basic_metrics.py @@ -20,14 +20,10 @@ CloudMonitoringMetricsExporter, ) from opentelemetry.sdk.metrics import Counter, MeterProvider -from opentelemetry.sdk.metrics.export.controller import PushController -meter = metrics.get_meter(__name__, True) - -# Gather and export metrics every 5 seconds -controller = PushController( - meter=meter, exporter=CloudMonitoringMetricsExporter(), interval=5 -) +metrics.set_meter_provider(MeterProvider()) +meter = metrics.get_meter(__name__) +metrics.get_meter_provider().start_pipeline(meter, CloudMonitoringMetricsExporter(), 5) requests_counter = meter.create_metric( name="request_counter", diff --git a/docs/examples/opencensus-exporter-metrics/collector.py b/docs/examples/opencensus-exporter-metrics/collector.py index 89dabd12eab..725f07b77ab 100644 --- a/docs/examples/opencensus-exporter-metrics/collector.py +++ b/docs/examples/opencensus-exporter-metrics/collector.py @@ -21,7 +21,6 @@ OpenCensusMetricsExporter, ) from opentelemetry.sdk.metrics import Counter, MeterProvider -from opentelemetry.sdk.metrics.export.controller import PushController exporter = OpenCensusMetricsExporter( service_name="basic-service", endpoint="localhost:55678" @@ -29,7 +28,7 @@ metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) -controller = PushController(meter, exporter, 5) +metrics.get_meter_provider().start_pipeline(meter, exporter, 5) requests_counter = meter.create_metric( name="requests", 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 59ef3f1708a..da22042dcc5 100644 --- a/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py +++ b/ext/opentelemetry-ext-prometheus/src/opentelemetry/ext/prometheus/__init__.py @@ -29,7 +29,6 @@ from opentelemetry import metrics from opentelemetry.ext.prometheus import PrometheusMetricsExporter from opentelemetry.sdk.metrics import Counter, Meter - from opentelemetry.sdk.metrics.export.controller import PushController from prometheus_client import start_http_server # Start Prometheus client @@ -37,13 +36,12 @@ # Meter is responsible for creating and recording metrics metrics.set_meter_provider(MeterProvider()) - meter = metrics.meter() + meter = metrics.get_meter(__name__) # exporter to export metrics to Prometheus prefix = "MyAppPrefix" exporter = PrometheusMetricsExporter(prefix) - # controller collects metrics created from meter and exports it via the - # exporter every interval - controller = PushController(meter, exporter, 5) + # Starts the collect/export pipeline for metrics + metrics.get_meter_provider().start_pipeline(meter, exporter, 5) counter = meter.create_metric( "requests", 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 09c633f14b4..cd9ccd8bdcb 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 @@ -28,9 +28,11 @@ .. code:: python + from opentelemetry import metrics from opentelemetry.ext.system_metrics import SystemMetrics from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter + metrics.set_meter_provider(MeterProvider()) exporter = ConsoleMetricsExporter() SystemMetrics(exporter) @@ -60,7 +62,6 @@ 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 class SystemMetrics: @@ -73,9 +74,7 @@ def __init__( ): self._labels = {} if labels is None else labels self.meter = metrics.get_meter(__name__) - self.controller = PushController( - meter=self.meter, exporter=exporter, interval=interval - ) + metrics.get_meter_provider().start_pipeline(meter, exporter, interval) if config is None: self._config = { "system_memory": ["total", "available", "used", "free"], diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py index 0ebb9d397ba..f126ca47cce 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/__init__.py @@ -450,7 +450,7 @@ def start_pipeline( interval: float = 15.0, ) -> None: """Method to begin the collect/export pipeline. - + Args: meter: The meter to collect metrics from. exporter: The exporter to export metrics to. @@ -467,3 +467,6 @@ def shutdown(self) -> None: controller.shutdown() for exporter in self._exporters: exporter.shutdown() + if self._atexit_handler is not None: + atexit.unregister(self._atexit_handler) + self._atexit_handler = None diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py index 4a5eb1b40c8..88a7e950730 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py @@ -15,6 +15,8 @@ import threading from opentelemetry.context import attach, detach, set_value +from opentelemetry.metrics import Meter +from opentelemetry.sdk.metrics.export import MetricsExporter class PushController(threading.Thread): @@ -22,11 +24,21 @@ class PushController(threading.Thread): Uses a worker thread that periodically collects metrics for exporting, exports them and performs some post-processing. + + Args: + meter: The meter used to collect metrics. + exporter: The exporter used to export metrics. + interval: The collect/export interval in seconds. """ daemon = True - def __init__(self, meter, exporter, interval): + def __init__( + self, + meter: Meter, + exporter: MetricsExporter, + interval: float + ): super().__init__() self.meter = meter self.exporter = exporter diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index f360bd887d6..a6d59ffc28c 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -46,6 +46,7 @@ def test_start_pipeline(self): meter_provider.start_pipeline(meter, exporter, 6) self.assertEqual(len(meter_provider._exporters), 1) self.assertEqual(len(meter_provider._controllers), 1) + meter_provider.shutdown() def test_shutdown(self): controller = mock.Mock() @@ -57,6 +58,7 @@ def test_shutdown(self): meter_provider.shutdown() self.assertEqual(controller.shutdown.call_count, 1) self.assertEqual(exporter.shutdown.call_count, 1) + self.assertIsNone(meter_provider._atexit_handler) class TestMeter(unittest.TestCase): From 1203111824e3b926fe5a4ae102573fe1ee5b3740 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 15:00:21 -0700 Subject: [PATCH 07/10] address comment --- .../src/opentelemetry/ext/system_metrics/__init__.py | 2 +- opentelemetry-sdk/tests/metrics/test_metrics.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) 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 cd9ccd8bdcb..2bb57ad6a3f 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 @@ -74,7 +74,7 @@ def __init__( ): self._labels = {} if labels is None else labels self.meter = metrics.get_meter(__name__) - metrics.get_meter_provider().start_pipeline(meter, exporter, interval) + metrics.get_meter_provider().start_pipeline(self.meter, exporter, interval) if config is None: self._config = { "system_memory": ["total", "available", "used", "free"], diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index a6d59ffc28c..4eb8a0d9fc1 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -44,9 +44,11 @@ def test_start_pipeline(self): meter = meter_provider.get_meter(__name__) # pylint: disable=protected-access meter_provider.start_pipeline(meter, exporter, 6) - self.assertEqual(len(meter_provider._exporters), 1) - self.assertEqual(len(meter_provider._controllers), 1) - meter_provider.shutdown() + try: + self.assertEqual(len(meter_provider._exporters), 1) + self.assertEqual(len(meter_provider._controllers), 1) + finally: + meter_provider.shutdown() def test_shutdown(self): controller = mock.Mock() From 449e8a9b9d3782845973e78e291fe5f76aba0439 Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 15:27:26 -0700 Subject: [PATCH 08/10] fix tests --- .../src/opentelemetry/ext/system_metrics/__init__.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 2bb57ad6a3f..10b09795571 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 @@ -30,6 +30,7 @@ from opentelemetry import metrics from opentelemetry.ext.system_metrics import SystemMetrics + from opentelemetry.sdk.metrics import MeterProvider, from opentelemetry.sdk.metrics.export import ConsoleMetricsExporter metrics.set_meter_provider(MeterProvider()) @@ -62,6 +63,7 @@ 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 class SystemMetrics: @@ -74,7 +76,9 @@ def __init__( ): self._labels = {} if labels is None else labels self.meter = metrics.get_meter(__name__) - metrics.get_meter_provider().start_pipeline(self.meter, exporter, interval) + self.controller = PushController( + meter=self.meter, exporter=exporter, interval=interval + ) if config is None: self._config = { "system_memory": ["total", "available", "used", "free"], From 5a45c836a0217dd139a966633442ef9fbc46abbd Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 15:28:24 -0700 Subject: [PATCH 09/10] lint --- .../src/opentelemetry/sdk/metrics/export/controller.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py index 88a7e950730..7448f353c45 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/export/controller.py @@ -34,10 +34,7 @@ class PushController(threading.Thread): daemon = True def __init__( - self, - meter: Meter, - exporter: MetricsExporter, - interval: float + self, meter: Meter, exporter: MetricsExporter, interval: float ): super().__init__() self.meter = meter From 0c5a051ad7a95d70ddaaa092fd216b12d470a93b Mon Sep 17 00:00:00 2001 From: Leighton Date: Tue, 9 Jun 2020 15:55:59 -0700 Subject: [PATCH 10/10] lint --- docs/examples/cloud_monitoring/basic_metrics.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/examples/cloud_monitoring/basic_metrics.py b/docs/examples/cloud_monitoring/basic_metrics.py index 1a8e8eb0151..fa00fc068b6 100644 --- a/docs/examples/cloud_monitoring/basic_metrics.py +++ b/docs/examples/cloud_monitoring/basic_metrics.py @@ -23,7 +23,9 @@ metrics.set_meter_provider(MeterProvider()) meter = metrics.get_meter(__name__) -metrics.get_meter_provider().start_pipeline(meter, CloudMonitoringMetricsExporter(), 5) +metrics.get_meter_provider().start_pipeline( + meter, CloudMonitoringMetricsExporter(), 5 +) requests_counter = meter.create_metric( name="request_counter",