From f1112e38ad5ba3dcdc8d45af4816f01f57f38212 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Thu, 24 Mar 2022 12:28:46 -0600 Subject: [PATCH] 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(