Remove enable_default_view option from sdk MeterProvider

open-telemetry · Mar 24, 2022 · a493ab7 · a493ab7
1 parent 0702852
commit a493ab7
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 48 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -148,15 +148,36 @@ 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,
         metric_readers: Sequence[MetricReader] = (),
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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(