Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove enable_default_view option from sdk MeterProvider #2547

Merged
merged 8 commits into from
Mar 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2540](https://github.com/open-telemetry/opentelemetry-python/pull/2540))
- Drop the usage of name field from log model in OTLP
([#2565](https://github.com/open-telemetry/opentelemetry-python/pull/2565))
- 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

Expand Down
7 changes: 7 additions & 0 deletions docs/sdk/metrics.aggregation.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
opentelemetry.sdk._metrics.aggregation
==========================================

.. automodule:: opentelemetry.sdk._metrics.aggregation
:members:
:undoc-members:
:show-inheritance:
7 changes: 7 additions & 0 deletions docs/sdk/metrics.metric_reader.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
opentelemetry.sdk._metrics.metric_reader
==========================================

.. automodule:: opentelemetry.sdk._metrics.metric_reader
:members:
:undoc-members:
:show-inheritance:
6 changes: 6 additions & 0 deletions docs/sdk/metrics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ opentelemetry.sdk._metrics package
Submodules
----------

.. toctree::

metrics.view
metrics.aggregation
metrics.metric_reader

.. automodule:: opentelemetry.sdk._metrics
:members:
:undoc-members:
Expand Down
7 changes: 7 additions & 0 deletions docs/sdk/metrics.view.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
opentelemetry.sdk._metrics.view
==========================================

.. automodule:: opentelemetry.sdk._metrics.view
:members:
:undoc-members:
:show-inheritance:
31 changes: 28 additions & 3 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,41 @@ def create_observable_up_down_counter(


class MeterProvider(APIMeterProvider):
"""See `opentelemetry._metrics.MeterProvider`."""
r"""See `opentelemetry._metrics.MeterProvider`.

Args:
aabmass marked this conversation as resolved.
Show resolved Hide resolved
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

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()
Expand All @@ -165,7 +191,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""
.. Explicitly document private _AggregationFactory
.. autoclass:: _AggregationFactory
aabmass marked this conversation as resolved.
Show resolved Hide resolved
"""

from abc import ABC, abstractmethod
from bisect import bisect_left
from dataclasses import replace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
"""
Original file line number Diff line number Diff line change
Expand Up @@ -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:
aabmass marked this conversation as resolved.
Show resolved Hide resolved
view_instrument_matches.append(
_ViewInstrumentMatch(
view=_DEFAULT_VIEW,
Expand Down Expand Up @@ -104,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()
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ class SdkConfiguration:
resource: Resource
metric_readers: Sequence[MetricReader]
views: Sequence[View]
enable_default_view: bool
24 changes: 11 additions & 13 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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):
Expand All @@ -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)]
Expand Down
34 changes: 0 additions & 34 deletions opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ def test_creates_view_instrument_matches(
resource=Mock(),
metric_readers=(),
views=(view1, view2),
enable_default_view=True,
)
)

Expand Down Expand Up @@ -101,7 +100,6 @@ def test_forwards_calls_to_view_instrument_match(
resource=Mock(),
metric_readers=(),
views=(view1, view2),
enable_default_view=True,
)
)

Expand Down Expand Up @@ -149,7 +147,6 @@ def test_race_concurrent_measurements(self, MockViewInstrumentMatch: Mock):
resource=Mock(),
metric_readers=(),
views=(view1,),
enable_default_view=True,
)
)

Expand All @@ -175,7 +172,6 @@ def test_default_view_enabled(self, MockViewInstrumentMatch: Mock):
resource=Mock(),
metric_readers=(),
views=(),
enable_default_view=True,
)
)

Expand All @@ -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())
Expand All @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down