From a493ab7ef2eddba9f3771386c508b4fd0961ae16 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 24 Mar 2022 12:28:46 -0600 Subject: [PATCH 1/4] Remove enable_default_view option from sdk MeterProvider --- CHANGELOG.md | 2 + docs/sdk/metrics.aggregation.rst | 7 +++ docs/sdk/metrics.rst | 5 ++ docs/sdk/metrics.view.rst | 7 +++ .../opentelemetry/sdk/_metrics/__init__.py | 26 ++++++++- .../sdk/_metrics/metric_reader_storage.py | 5 +- .../sdk/_metrics/sdk_configuration.py | 1 - .../test_disable_default_views.py | 57 +++++++++++++++++++ .../metrics/test_measurement_consumer.py | 4 -- .../metrics/test_metric_reader_storage.py | 34 ----------- .../metrics/test_view_instrument_match.py | 2 - 11 files changed, 102 insertions(+), 48 deletions(-) create mode 100644 docs/sdk/metrics.aggregation.rst create mode 100644 docs/sdk/metrics.view.rst create mode 100644 opentelemetry-sdk/tests/metrics/integration_test/test_disable_default_views.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e2017006f9..f24f8e15b46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2533](https://github.com/open-telemetry/opentelemetry-python/pull/2533)) - Add InMemoryMetricReader to metrics SDK ([#2540](https://github.com/open-telemetry/opentelemetry-python/pull/2540)) +- Remove `enable_default_view` option from sdk MeterProvider + ([#2547](https://github.com/open-telemetry/opentelemetry-python/pull/2547)) ## [1.10.0-0.29b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.10.0-0.29b0) - 2022-03-10 diff --git a/docs/sdk/metrics.aggregation.rst b/docs/sdk/metrics.aggregation.rst new file mode 100644 index 00000000000..34155a6e44e --- /dev/null +++ b/docs/sdk/metrics.aggregation.rst @@ -0,0 +1,7 @@ +opentelemetry.sdk._metrics.aggregation +========================================== + +.. automodule:: opentelemetry.sdk._metrics.aggregation + :members: + :undoc-members: + :no-show-inheritance: diff --git a/docs/sdk/metrics.rst b/docs/sdk/metrics.rst index 39041ea27d7..bf3a46c5510 100644 --- a/docs/sdk/metrics.rst +++ b/docs/sdk/metrics.rst @@ -11,6 +11,11 @@ opentelemetry.sdk._metrics package Submodules ---------- +.. toctree:: + + metrics.view + metrics.aggregation + .. automodule:: opentelemetry.sdk._metrics :members: :undoc-members: diff --git a/docs/sdk/metrics.view.rst b/docs/sdk/metrics.view.rst new file mode 100644 index 00000000000..b2f78b9bf2d --- /dev/null +++ b/docs/sdk/metrics.view.rst @@ -0,0 +1,7 @@ +opentelemetry.sdk._metrics.view +========================================== + +.. automodule:: opentelemetry.sdk._metrics.view + :members: + :undoc-members: + :show-inheritance: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py index 869f85d62e8..7ed01b41f60 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py @@ -148,7 +148,29 @@ def create_observable_up_down_counter( class MeterProvider(APIMeterProvider): - """See `opentelemetry._metrics.MeterProvider`.""" + r"""See `opentelemetry._metrics.MeterProvider`. + + Args: + shutdown_on_exit: If true, registers an `atexit` handler to call + `MeterProvider.shutdown` + views: The views to configure the metric output the SDK + + By default, instruments which do not match any `View` (or if no `View`\ s are provided) + will report metrics with the default aggregation for the instrument's kind. To disable + instruments by default, configure a match-all `View` with `DropAggregation` and then create + `View`\ s to re-enable individual instruments: + + .. code-block:: python + :caption: Disable default views + + MeterProvider( + views=[ + View(instrument_name="*", aggregation=DropAggregation()), + View(instrument_name="mycounter"), + ], + # ... + ) + """ def __init__( self, @@ -156,7 +178,6 @@ def __init__( resource: Resource = Resource.create({}), shutdown_on_exit: bool = True, views: Sequence[View] = (), - enable_default_view: bool = True, ): self._lock = Lock() self._meter_lock = Lock() @@ -165,7 +186,6 @@ def __init__( resource=resource, metric_readers=metric_readers, views=views, - enable_default_view=enable_default_view, ) self._measurement_consumer = SynchronousMeasurementConsumer( sdk_config=self._sdk_config diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py index f7a24100244..59a958be33f 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py @@ -66,10 +66,7 @@ def _get_or_init_view_instrument_match( ) # if no view targeted the instrument, use the default - if ( - not view_instrument_matches - and self._sdk_config.enable_default_view - ): + if not view_instrument_matches: view_instrument_matches.append( _ViewInstrumentMatch( view=_DEFAULT_VIEW, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/sdk_configuration.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/sdk_configuration.py index c6176967171..2c603b5e4da 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/sdk_configuration.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/sdk_configuration.py @@ -11,4 +11,3 @@ class SdkConfiguration: resource: Resource metric_readers: Sequence[MetricReader] views: Sequence[View] - enable_default_view: bool diff --git a/opentelemetry-sdk/tests/metrics/integration_test/test_disable_default_views.py b/opentelemetry-sdk/tests/metrics/integration_test/test_disable_default_views.py new file mode 100644 index 00000000000..cb2ab8b54c6 --- /dev/null +++ b/opentelemetry-sdk/tests/metrics/integration_test/test_disable_default_views.py @@ -0,0 +1,57 @@ +# Copyright The OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import TestCase + +from opentelemetry.sdk._metrics import MeterProvider +from opentelemetry.sdk._metrics.aggregation import DropAggregation +from opentelemetry.sdk._metrics.export import InMemoryMetricReader +from opentelemetry.sdk._metrics.view import View + + +class TestDisableDefaultViews(TestCase): + def test_disable_default_views(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[View(instrument_name="*", aggregation=DropAggregation())], + ) + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + counter.add(10, {"label": "value1"}) + counter.add(10, {"label": "value2"}) + counter.add(10, {"label": "value3"}) + + self.assertEqual(reader.get_metrics(), []) + + def test_disable_default_views_add_custom(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider( + metric_readers=[reader], + views=[ + View(instrument_name="*", aggregation=DropAggregation()), + View(instrument_name="testhist"), + ], + ) + meter = meter_provider.get_meter("testmeter") + counter = meter.create_counter("testcounter") + histogram = meter.create_histogram("testhist") + counter.add(10, {"label": "value1"}) + counter.add(10, {"label": "value2"}) + counter.add(10, {"label": "value3"}) + histogram.record(12, {"label": "value"}) + + metrics = reader.get_metrics() + self.assertEqual(len(metrics), 1) + self.assertEqual(metrics[0].name, "testhist") diff --git a/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py b/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py index 9e2416a7c1e..3b58ba44889 100644 --- a/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py +++ b/opentelemetry-sdk/tests/metrics/test_measurement_consumer.py @@ -39,7 +39,6 @@ def test_creates_metric_reader_storages(self, MockMetricReaderStorage): resource=Mock(), metric_readers=reader_mocks, views=Mock(), - enable_default_view=Mock(), ) ) self.assertEqual(len(MockMetricReaderStorage.mock_calls), 5) @@ -56,7 +55,6 @@ def test_measurements_passed_to_each_reader_storage( resource=Mock(), metric_readers=reader_mocks, views=Mock(), - enable_default_view=Mock(), ) ) measurement_mock = Mock() @@ -78,7 +76,6 @@ def test_collect_passed_to_reader_stage(self, MockMetricReaderStorage): resource=Mock(), metric_readers=reader_mocks, views=Mock(), - enable_default_view=Mock(), ) ) for r_mock, rs_mock in zip(reader_mocks, reader_storage_mocks): @@ -99,7 +96,6 @@ def test_collect_calls_async_instruments(self, MockMetricReaderStorage): resource=Mock(), metric_readers=[reader_mock], views=Mock(), - enable_default_view=Mock(), ) ) async_instrument_mocks = [MagicMock() for _ in range(5)] diff --git a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py index d73cf893a99..f7ffc6993a2 100644 --- a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py +++ b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py @@ -56,7 +56,6 @@ def test_creates_view_instrument_matches( resource=Mock(), metric_readers=(), views=(view1, view2), - enable_default_view=True, ) ) @@ -101,7 +100,6 @@ def test_forwards_calls_to_view_instrument_match( resource=Mock(), metric_readers=(), views=(view1, view2), - enable_default_view=True, ) ) @@ -149,7 +147,6 @@ def test_race_concurrent_measurements(self, MockViewInstrumentMatch: Mock): resource=Mock(), metric_readers=(), views=(view1,), - enable_default_view=True, ) ) @@ -175,7 +172,6 @@ def test_default_view_enabled(self, MockViewInstrumentMatch: Mock): resource=Mock(), metric_readers=(), views=(), - enable_default_view=True, ) ) @@ -192,35 +188,6 @@ def test_default_view_enabled(self, MockViewInstrumentMatch: Mock): storage.consume_measurement(Measurement(1, instrument2)) self.assertEqual(len(MockViewInstrumentMatch.call_args_list), 1) - @patch( - "opentelemetry.sdk._metrics.metric_reader_storage._ViewInstrumentMatch" - ) - def test_default_view_disabled(self, MockViewInstrumentMatch: Mock): - """Instruments should not be matched with default views when disabled""" - instrument1 = Mock(name="instrument1") - instrument2 = Mock(name="instrument2") - storage = MetricReaderStorage( - SdkConfiguration( - resource=Mock(), - metric_readers=(), - views=(), - enable_default_view=False, - ) - ) - - storage.consume_measurement(Measurement(1, instrument1)) - self.assertEqual( - len(MockViewInstrumentMatch.call_args_list), - 0, - MockViewInstrumentMatch.mock_calls, - ) - storage.consume_measurement(Measurement(1, instrument1)) - self.assertEqual(len(MockViewInstrumentMatch.call_args_list), 0) - - MockViewInstrumentMatch.call_args_list.clear() - storage.consume_measurement(Measurement(1, instrument2)) - self.assertEqual(len(MockViewInstrumentMatch.call_args_list), 0) - def test_drop_aggregation(self): counter = Counter("name", Mock(), Mock()) @@ -233,7 +200,6 @@ def test_drop_aggregation(self): instrument_name="name", aggregation=DropAggregation() ), ), - enable_default_view=False, ) ) metric_reader_storage.consume_measurement(Measurement(1, counter)) diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index 210963d5471..6edba9b4eb4 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -46,7 +46,6 @@ def test_consume_measurement(self): resource=self.mock_resource, metric_readers=[], views=[], - enable_default_view=True, ) view_instrument_match = _ViewInstrumentMatch( view=View( @@ -164,7 +163,6 @@ def test_collect(self): resource=self.mock_resource, metric_readers=[], views=[], - enable_default_view=True, ) view_instrument_match = _ViewInstrumentMatch( view=View( From f55f58f73b10c5a28d82a5fea3f470424e842d14 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 25 Mar 2022 15:33:58 +0000 Subject: [PATCH 2/4] document rest of MeterProvider constructor params --- docs/sdk/metrics.metric_reader.rst | 7 +++++++ docs/sdk/metrics.rst | 1 + .../src/opentelemetry/sdk/_metrics/__init__.py | 5 +++++ .../src/opentelemetry/sdk/_metrics/metric_reader.py | 10 +++++++++- 4 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 docs/sdk/metrics.metric_reader.rst diff --git a/docs/sdk/metrics.metric_reader.rst b/docs/sdk/metrics.metric_reader.rst new file mode 100644 index 00000000000..b2bc9423f10 --- /dev/null +++ b/docs/sdk/metrics.metric_reader.rst @@ -0,0 +1,7 @@ +opentelemetry.sdk._metrics.metric_reader +========================================== + +.. automodule:: opentelemetry.sdk._metrics.metric_reader + :members: + :undoc-members: + :show-inheritance: diff --git a/docs/sdk/metrics.rst b/docs/sdk/metrics.rst index bf3a46c5510..2dbde8f438b 100644 --- a/docs/sdk/metrics.rst +++ b/docs/sdk/metrics.rst @@ -15,6 +15,7 @@ Submodules metrics.view metrics.aggregation + metrics.metric_reader .. automodule:: opentelemetry.sdk._metrics :members: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py index 7ed01b41f60..90ec6772510 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py @@ -151,6 +151,11 @@ class MeterProvider(APIMeterProvider): r"""See `opentelemetry._metrics.MeterProvider`. Args: + metric_readers: Register metric readers to collect metrics from the SDK on demand. Each + `MetricReader` is completely independent and will collect separate streams of + metrics. TODO: reference ``PeriodicExportingMetricReader`` usage with push + exporters here. + resource: The resource representing what the metrics emitted from the SDK pertain to. shutdown_on_exit: If true, registers an `atexit` handler to call `MeterProvider.shutdown` views: The views to configure the metric output the SDK diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader.py index 951f18e09a5..6b345587d68 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader.py @@ -24,6 +24,11 @@ class MetricReader(ABC): + """ + .. document protected _receive_metrics which is a intended to be overriden by subclass + .. automethod:: _receive_metrics + """ + def __init__( self, preferred_temporality: AggregationTemporality = AggregationTemporality.CUMULATIVE, @@ -66,5 +71,8 @@ def shutdown(self) -> bool: only be shutdown once, any subsequent calls are ignored and return failure status. - When a `MetricReader` is registered on a `MeterProvider`, `MeterProvider.shutdown` will invoke this automatically. + When a `MetricReader` is registered on a + :class:`~opentelemetry.sdk._metrics.MeterProvider`, + :meth:`~opentelemetry.sdk._metrics.MeterProvider.shutdown` will invoke this + automatically. """ From 9b7d757e8755f4a7659e36de8f3be43f93b18738 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 25 Mar 2022 15:57:07 +0000 Subject: [PATCH 3/4] fix docs build --- docs/sdk/metrics.aggregation.rst | 2 +- .../opentelemetry/sdk/_metrics/aggregation.py | 5 ++++ .../src/opentelemetry/sdk/_metrics/view.py | 24 +++++++++---------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/docs/sdk/metrics.aggregation.rst b/docs/sdk/metrics.aggregation.rst index 34155a6e44e..add890e11aa 100644 --- a/docs/sdk/metrics.aggregation.rst +++ b/docs/sdk/metrics.aggregation.rst @@ -4,4 +4,4 @@ opentelemetry.sdk._metrics.aggregation .. automodule:: opentelemetry.sdk._metrics.aggregation :members: :undoc-members: - :no-show-inheritance: + :show-inheritance: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 420385c69ca..9771833cf03 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -12,6 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +""" +.. Explicitly document private _AggregationFactory +.. autoclass:: _AggregationFactory +""" + from abc import ABC, abstractmethod from bisect import bisect_left from dataclasses import replace diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py index 7cdcb1ca27d..faecd77d3a3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py @@ -44,35 +44,33 @@ class View: instrument must be to match the view. instrument_name: This is an instrument matching attribute: the name the - instrument must have to match the view. Wild card characters are - supported. Wild card characters should not be used with this - attribute if the view has also a ``name`` defined. + instrument must have to match the view. Wild card characters are supported. Wild + card characters should not be used with this attribute if the view has also a + ``name`` defined. meter_name: This is an instrument matching attribute: the name the instrument meter must have to match the view. - meter_version : This is an instrument matching attribute: the version + meter_version: This is an instrument matching attribute: the version the instrument meter must have to match the view. - meter_schema URL : This is an instrument matching attribute: the schema + meter_schema_url: This is an instrument matching attribute: the schema URL the instrument meter must have to match the view. name: This is a metric stream customizing attribute: the name of the metric stream. If `None`, the name of the instrument will be used. description: This is a metric stream customizing attribute: the - description of the metric stream. If `None`, the description of the - instrument will be used. + description of the metric stream. If `None`, the description of the instrument will + be used. attribute_keys: This is a metric stream customizing attribute: this is - a set of attribute keys. If not `None` then only the measurement - attributes that are in `attribute_keys` will be used to identify the - metric stream. + a set of attribute keys. If not `None` then only the measurement attributes that + are in ``attribute_keys`` will be used to identify the metric stream. aggregation: This is a metric stream customizing attribute: the - aggregatation instance to use when data is aggregated for the - corresponding metrics stream. If `None` the default aggregation of - the instrument will be used. + aggregatation instance to use when data is aggregated for the corresponding metrics + stream. If `None` the default aggregation of the instrument will be used. This class is not intended to be subclassed by the user. """ From 9acbbbfce9f12c0476ad1e5e7203ea0165636c90 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Mon, 28 Mar 2022 15:47:41 +0000 Subject: [PATCH 4/4] remove unused function --- .../src/opentelemetry/sdk/_metrics/metric_reader_storage.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py index 59a958be33f..45c4d4dd793 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/metric_reader_storage.py @@ -101,8 +101,3 @@ def collect(self, temporality: AggregationTemporality) -> Iterable[Metric]: metrics.extend(view_instrument_match.collect(temporality)) return metrics - - -def default_view(instrument: Instrument) -> View: - # TODO: #2247 - return View()