From 43288ca9a36144668797c11ca2654836ec8b5e99 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 1 Aug 2022 23:25:05 +0530 Subject: [PATCH] Instrument instances are always created through a Meter (#2844) * Instrument instances are always created through a Meter * Fix lint and add changelog * fix lint --- CHANGELOG.md | 2 + .../sdk/metrics/_internal/__init__.py | 38 +++--- .../sdk/metrics/_internal/export/__init__.py | 98 ++++++++++++--- .../sdk/metrics/_internal/instrument.py | 59 ++++++++- .../tests/metrics/test_aggregation.py | 38 +++--- .../tests/metrics/test_instrument.py | 74 ++++++++--- .../tests/metrics/test_metric_reader.py | 117 +++++++----------- .../metrics/test_metric_reader_storage.py | 58 ++++----- .../metrics/test_view_instrument_match.py | 10 +- 9 files changed, 309 insertions(+), 185 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97869c80d5..641d382961 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2754](https://github.com/open-telemetry/opentelemetry-python/pull/2754)) - Fix --insecure of CLI argument ([#2696](https://github.com/open-telemetry/opentelemetry-python/pull/2696)) +- Instrument instances are always created through a Meter + ([#2844](https://github.com/open-telemetry/opentelemetry-python/pull/2844)) ## [1.12.0rc2-0.32b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0rc2) - 2022-07-04 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py index c75a175007..97d910a67b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py @@ -31,12 +31,12 @@ ) from opentelemetry.metrics import UpDownCounter as APIUpDownCounter from opentelemetry.sdk.metrics._internal.instrument import ( - Counter, - Histogram, - ObservableCounter, - ObservableGauge, - ObservableUpDownCounter, - UpDownCounter, + _Counter, + _Histogram, + _ObservableCounter, + _ObservableGauge, + _ObservableUpDownCounter, + _UpDownCounter, ) from opentelemetry.sdk.metrics._internal.measurement_consumer import ( MeasurementConsumer, @@ -72,7 +72,7 @@ def create_counter(self, name, unit="", description="") -> APICounter: ( is_instrument_registered, instrument_id, - ) = self._is_instrument_registered(name, Counter, unit, description) + ) = self._is_instrument_registered(name, _Counter, unit, description) if is_instrument_registered: # FIXME #2558 go through all views here and check if this @@ -89,7 +89,7 @@ def create_counter(self, name, unit="", description="") -> APICounter: with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = Counter( + instrument = _Counter( name, self._instrumentation_scope, self._measurement_consumer, @@ -109,7 +109,7 @@ def create_up_down_counter( is_instrument_registered, instrument_id, ) = self._is_instrument_registered( - name, UpDownCounter, unit, description + name, _UpDownCounter, unit, description ) if is_instrument_registered: @@ -127,7 +127,7 @@ def create_up_down_counter( with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = UpDownCounter( + instrument = _UpDownCounter( name, self._instrumentation_scope, self._measurement_consumer, @@ -147,7 +147,7 @@ def create_observable_counter( is_instrument_registered, instrument_id, ) = self._is_instrument_registered( - name, ObservableCounter, unit, description + name, _ObservableCounter, unit, description ) if is_instrument_registered: @@ -165,7 +165,7 @@ def create_observable_counter( with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = ObservableCounter( + instrument = _ObservableCounter( name, self._instrumentation_scope, self._measurement_consumer, @@ -185,7 +185,7 @@ def create_histogram(self, name, unit="", description="") -> APIHistogram: ( is_instrument_registered, instrument_id, - ) = self._is_instrument_registered(name, Histogram, unit, description) + ) = self._is_instrument_registered(name, _Histogram, unit, description) if is_instrument_registered: # FIXME #2558 go through all views here and check if this @@ -202,7 +202,7 @@ def create_histogram(self, name, unit="", description="") -> APIHistogram: with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = Histogram( + instrument = _Histogram( name, self._instrumentation_scope, self._measurement_consumer, @@ -221,7 +221,7 @@ def create_observable_gauge( is_instrument_registered, instrument_id, ) = self._is_instrument_registered( - name, ObservableGauge, unit, description + name, _ObservableGauge, unit, description ) if is_instrument_registered: @@ -239,7 +239,7 @@ def create_observable_gauge( with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = ObservableGauge( + instrument = _ObservableGauge( name, self._instrumentation_scope, self._measurement_consumer, @@ -261,7 +261,9 @@ def create_observable_up_down_counter( ( is_instrument_registered, instrument_id, - ) = self._is_instrument_registered(name, Counter, unit, description) + ) = self._is_instrument_registered( + name, _ObservableUpDownCounter, unit, description + ) if is_instrument_registered: # FIXME #2558 go through all views here and check if this @@ -278,7 +280,7 @@ def create_observable_up_down_counter( with self._instrument_id_instrument_lock: return self._instrument_id_instrument[instrument_id] - instrument = ObservableUpDownCounter( + instrument = _ObservableUpDownCounter( name, self._instrumentation_scope, self._measurement_consumer, diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py index 8ed5596c81..ed5db17d97 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -45,6 +45,12 @@ ObservableGauge, ObservableUpDownCounter, UpDownCounter, + _Counter, + _Histogram, + _ObservableCounter, + _ObservableGauge, + _ObservableUpDownCounter, + _UpDownCounter, ) from opentelemetry.sdk.metrics._internal.point import MetricsData from opentelemetry.util._once import Once @@ -164,6 +170,7 @@ class MetricReader(ABC): # FIXME add :std:envvar:`OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE` # to the end of the documentation paragraph above. + # pylint: disable=too-many-branches def __init__( self, preferred_temporality: Dict[type, AggregationTemporality] = None, @@ -189,22 +196,22 @@ def __init__( == "DELTA" ): self._instrument_class_temporality = { - Counter: AggregationTemporality.DELTA, - UpDownCounter: AggregationTemporality.CUMULATIVE, - Histogram: AggregationTemporality.DELTA, - ObservableCounter: AggregationTemporality.DELTA, - ObservableUpDownCounter: AggregationTemporality.CUMULATIVE, - ObservableGauge: AggregationTemporality.CUMULATIVE, + _Counter: AggregationTemporality.DELTA, + _UpDownCounter: AggregationTemporality.CUMULATIVE, + _Histogram: AggregationTemporality.DELTA, + _ObservableCounter: AggregationTemporality.DELTA, + _ObservableUpDownCounter: AggregationTemporality.CUMULATIVE, + _ObservableGauge: AggregationTemporality.CUMULATIVE, } else: self._instrument_class_temporality = { - Counter: AggregationTemporality.CUMULATIVE, - UpDownCounter: AggregationTemporality.CUMULATIVE, - Histogram: AggregationTemporality.CUMULATIVE, - ObservableCounter: AggregationTemporality.CUMULATIVE, - ObservableUpDownCounter: AggregationTemporality.CUMULATIVE, - ObservableGauge: AggregationTemporality.CUMULATIVE, + _Counter: AggregationTemporality.CUMULATIVE, + _UpDownCounter: AggregationTemporality.CUMULATIVE, + _Histogram: AggregationTemporality.CUMULATIVE, + _ObservableCounter: AggregationTemporality.CUMULATIVE, + _ObservableUpDownCounter: AggregationTemporality.CUMULATIVE, + _ObservableGauge: AggregationTemporality.CUMULATIVE, } if preferred_temporality is not None: @@ -217,18 +224,69 @@ def __init__( f"Invalid temporality value found {temporality}" ) - self._instrument_class_temporality.update(preferred_temporality or {}) + if preferred_temporality is not None: + for typ, temporality in preferred_temporality.items(): + if typ is Counter: + self._instrument_class_temporality[_Counter] = temporality + elif typ is UpDownCounter: + self._instrument_class_temporality[ + _UpDownCounter + ] = temporality + elif typ is Histogram: + self._instrument_class_temporality[ + _Histogram + ] = temporality + elif typ is ObservableCounter: + self._instrument_class_temporality[ + _ObservableCounter + ] = temporality + elif typ is ObservableUpDownCounter: + self._instrument_class_temporality[ + _ObservableUpDownCounter + ] = temporality + elif typ is ObservableGauge: + self._instrument_class_temporality[ + _ObservableGauge + ] = temporality + else: + raise Exception(f"Invalid instrument class found {typ}") + self._preferred_temporality = preferred_temporality self._instrument_class_aggregation = { - Counter: DefaultAggregation(), - UpDownCounter: DefaultAggregation(), - Histogram: DefaultAggregation(), - ObservableCounter: DefaultAggregation(), - ObservableUpDownCounter: DefaultAggregation(), - ObservableGauge: DefaultAggregation(), + _Counter: DefaultAggregation(), + _UpDownCounter: DefaultAggregation(), + _Histogram: DefaultAggregation(), + _ObservableCounter: DefaultAggregation(), + _ObservableUpDownCounter: DefaultAggregation(), + _ObservableGauge: DefaultAggregation(), } - self._instrument_class_aggregation.update(preferred_aggregation or {}) + if preferred_aggregation is not None: + for typ, aggregation in preferred_aggregation.items(): + if typ is Counter: + self._instrument_class_aggregation[_Counter] = aggregation + elif typ is UpDownCounter: + self._instrument_class_aggregation[ + _UpDownCounter + ] = aggregation + elif typ is Histogram: + self._instrument_class_aggregation[ + _Histogram + ] = aggregation + elif typ is ObservableCounter: + self._instrument_class_aggregation[ + _ObservableCounter + ] = aggregation + elif typ is ObservableUpDownCounter: + self._instrument_class_aggregation[ + _ObservableUpDownCounter + ] = aggregation + elif typ is ObservableGauge: + self._instrument_class_aggregation[ + _ObservableGauge + ] = aggregation + else: + raise Exception(f"Invalid instrument class found {typ}") @final def collect(self, timeout_millis: float = 10_000) -> None: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py index efef08c67f..a0dc9c8da9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/instrument.py @@ -132,6 +132,11 @@ def callback( class Counter(_Synchronous, APICounter): + def __new__(cls, *args, **kwargs): + if cls is Counter: + raise TypeError("Counter must be instantiated via a meter.") + return super().__new__(cls) + def add( self, amount: Union[int, float], attributes: Dict[str, str] = None ): @@ -146,6 +151,11 @@ def add( class UpDownCounter(_Synchronous, APIUpDownCounter): + def __new__(cls, *args, **kwargs): + if cls is UpDownCounter: + raise TypeError("UpDownCounter must be instantiated via a meter.") + return super().__new__(cls) + def add( self, amount: Union[int, float], attributes: Dict[str, str] = None ): @@ -155,14 +165,29 @@ def add( class ObservableCounter(_Asynchronous, APIObservableCounter): - pass + def __new__(cls, *args, **kwargs): + if cls is ObservableCounter: + raise TypeError( + "ObservableCounter must be instantiated via a meter." + ) + return super().__new__(cls) class ObservableUpDownCounter(_Asynchronous, APIObservableUpDownCounter): - pass + def __new__(cls, *args, **kwargs): + if cls is ObservableUpDownCounter: + raise TypeError( + "ObservableUpDownCounter must be instantiated via a meter." + ) + return super().__new__(cls) class Histogram(_Synchronous, APIHistogram): + def __new__(cls, *args, **kwargs): + if cls is Histogram: + raise TypeError("Histogram must be instantiated via a meter.") + return super().__new__(cls) + def record( self, amount: Union[int, float], attributes: Dict[str, str] = None ): @@ -178,4 +203,34 @@ def record( class ObservableGauge(_Asynchronous, APIObservableGauge): + def __new__(cls, *args, **kwargs): + if cls is ObservableGauge: + raise TypeError( + "ObservableGauge must be instantiated via a meter." + ) + return super().__new__(cls) + + +# Below classes exist to prevent the direct instantiation +class _Counter(Counter): + pass + + +class _UpDownCounter(UpDownCounter): + pass + + +class _ObservableCounter(ObservableCounter): + pass + + +class _ObservableUpDownCounter(ObservableUpDownCounter): + pass + + +class _Histogram(Histogram): + pass + + +class _ObservableGauge(ObservableGauge): pass diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index b2245c4136..5acf701b18 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -18,19 +18,19 @@ from unittest import TestCase from unittest.mock import Mock -from opentelemetry.sdk.metrics import ( - Counter, - Histogram, - ObservableCounter, - ObservableGauge, - ObservableUpDownCounter, - UpDownCounter, -) from opentelemetry.sdk.metrics._internal.aggregation import ( _ExplicitBucketHistogramAggregation, _LastValueAggregation, _SumAggregation, ) +from opentelemetry.sdk.metrics._internal.instrument import ( + _Counter, + _Histogram, + _ObservableCounter, + _ObservableGauge, + _ObservableUpDownCounter, + _UpDownCounter, +) from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.metrics.export import ( AggregationTemporality, @@ -381,7 +381,7 @@ def test_collect(self): class TestAggregationFactory(TestCase): def test_sum_factory(self): - counter = Counter("name", Mock(), Mock()) + counter = _Counter("name", Mock(), Mock()) factory = SumAggregation() aggregation = factory._create_aggregation(counter, Mock(), 0) self.assertIsInstance(aggregation, _SumAggregation) @@ -392,7 +392,7 @@ def test_sum_factory(self): aggregation2 = factory._create_aggregation(counter, Mock(), 0) self.assertNotEqual(aggregation, aggregation2) - counter = UpDownCounter("name", Mock(), Mock()) + counter = _UpDownCounter("name", Mock(), Mock()) factory = SumAggregation() aggregation = factory._create_aggregation(counter, Mock(), 0) self.assertIsInstance(aggregation, _SumAggregation) @@ -401,7 +401,7 @@ def test_sum_factory(self): aggregation._instrument_temporality, AggregationTemporality.DELTA ) - counter = ObservableCounter("name", Mock(), Mock(), None) + counter = _ObservableCounter("name", Mock(), Mock(), None) factory = SumAggregation() aggregation = factory._create_aggregation(counter, Mock(), 0) self.assertIsInstance(aggregation, _SumAggregation) @@ -412,7 +412,7 @@ def test_sum_factory(self): ) def test_explicit_bucket_histogram_factory(self): - histo = Histogram("name", Mock(), Mock()) + histo = _Histogram("name", Mock(), Mock()) factory = ExplicitBucketHistogramAggregation( boundaries=( 0.0, @@ -428,7 +428,7 @@ def test_explicit_bucket_histogram_factory(self): self.assertNotEqual(aggregation, aggregation2) def test_last_value_factory(self): - counter = Counter("name", Mock(), Mock()) + counter = _Counter("name", Mock(), Mock()) factory = LastValueAggregation() aggregation = factory._create_aggregation(counter, Mock(), 0) self.assertIsInstance(aggregation, _LastValueAggregation) @@ -444,7 +444,7 @@ def setUpClass(cls): def test_counter(self): aggregation = self.default_aggregation._create_aggregation( - Counter("name", Mock(), Mock()), Mock(), 0 + _Counter("name", Mock(), Mock()), Mock(), 0 ) self.assertIsInstance(aggregation, _SumAggregation) self.assertTrue(aggregation._instrument_is_monotonic) @@ -455,7 +455,7 @@ def test_counter(self): def test_up_down_counter(self): aggregation = self.default_aggregation._create_aggregation( - UpDownCounter("name", Mock(), Mock()), Mock(), 0 + _UpDownCounter("name", Mock(), Mock()), Mock(), 0 ) self.assertIsInstance(aggregation, _SumAggregation) self.assertFalse(aggregation._instrument_is_monotonic) @@ -466,7 +466,7 @@ def test_up_down_counter(self): def test_observable_counter(self): aggregation = self.default_aggregation._create_aggregation( - ObservableCounter("name", Mock(), Mock(), callbacks=[Mock()]), + _ObservableCounter("name", Mock(), Mock(), callbacks=[Mock()]), Mock(), 0, ) @@ -480,7 +480,7 @@ def test_observable_counter(self): def test_observable_up_down_counter(self): aggregation = self.default_aggregation._create_aggregation( - ObservableUpDownCounter( + _ObservableUpDownCounter( "name", Mock(), Mock(), callbacks=[Mock()] ), Mock(), @@ -496,7 +496,7 @@ def test_observable_up_down_counter(self): def test_histogram(self): aggregation = self.default_aggregation._create_aggregation( - Histogram( + _Histogram( "name", Mock(), Mock(), @@ -509,7 +509,7 @@ def test_histogram(self): def test_observable_gauge(self): aggregation = self.default_aggregation._create_aggregation( - ObservableGauge( + _ObservableGauge( "name", Mock(), Mock(), diff --git a/opentelemetry-sdk/tests/metrics/test_instrument.py b/opentelemetry-sdk/tests/metrics/test_instrument.py index 7a7bef4efc..8add3e69a0 100644 --- a/opentelemetry-sdk/tests/metrics/test_instrument.py +++ b/opentelemetry-sdk/tests/metrics/test_instrument.py @@ -25,40 +25,58 @@ ObservableUpDownCounter, UpDownCounter, ) +from opentelemetry.sdk.metrics._internal.instrument import ( + _Counter, + _Histogram, + _ObservableCounter, + _ObservableGauge, + _ObservableUpDownCounter, + _UpDownCounter, +) from opentelemetry.sdk.metrics._internal.measurement import Measurement class TestCounter(TestCase): def testname(self): - self.assertEqual(Counter("name", Mock(), Mock()).name, "name") - self.assertEqual(Counter("Name", Mock(), Mock()).name, "name") + self.assertEqual(_Counter("name", Mock(), Mock()).name, "name") + self.assertEqual(_Counter("Name", Mock(), Mock()).name, "name") def test_add(self): mc = Mock() - counter = Counter("name", Mock(), mc) + counter = _Counter("name", Mock(), mc) counter.add(1.0) mc.consume_measurement.assert_called_once() def test_add_non_monotonic(self): mc = Mock() - counter = Counter("name", Mock(), mc) + counter = _Counter("name", Mock(), mc) counter.add(-1.0) mc.consume_measurement.assert_not_called() + def test_disallow_direct_counter_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + Counter("name", Mock(), Mock()) + class TestUpDownCounter(TestCase): def test_add(self): mc = Mock() - counter = UpDownCounter("name", Mock(), mc) + counter = _UpDownCounter("name", Mock(), mc) counter.add(1.0) mc.consume_measurement.assert_called_once() def test_add_non_monotonic(self): mc = Mock() - counter = UpDownCounter("name", Mock(), mc) + counter = _UpDownCounter("name", Mock(), mc) counter.add(-1.0) mc.consume_measurement.assert_called_once() + def test_disallow_direct_up_down_counter_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + UpDownCounter("name", Mock(), Mock()) + TEST_ATTRIBUTES = {"foo": "bar"} @@ -103,11 +121,11 @@ def generator_callback_1(): class TestObservableGauge(TestCase): def testname(self): - self.assertEqual(ObservableGauge("name", Mock(), Mock()).name, "name") - self.assertEqual(ObservableGauge("Name", Mock(), Mock()).name, "name") + self.assertEqual(_ObservableGauge("name", Mock(), Mock()).name, "name") + self.assertEqual(_ObservableGauge("Name", Mock(), Mock()).name, "name") def test_callable_callback_0(self): - observable_gauge = ObservableGauge( + observable_gauge = _ObservableGauge( "name", Mock(), Mock(), [callable_callback_0] ) @@ -127,7 +145,7 @@ def test_callable_callback_0(self): ) def test_callable_multiple_callable_callback(self): - observable_gauge = ObservableGauge( + observable_gauge = _ObservableGauge( "name", Mock(), Mock(), [callable_callback_0, callable_callback_1] ) @@ -156,7 +174,7 @@ def test_callable_multiple_callable_callback(self): ) def test_generator_callback_0(self): - observable_gauge = ObservableGauge( + observable_gauge = _ObservableGauge( "name", Mock(), Mock(), [generator_callback_0()] ) @@ -177,7 +195,7 @@ def test_generator_callback_0(self): def test_generator_multiple_generator_callback(self): self.maxDiff = None - observable_gauge = ObservableGauge( + observable_gauge = _ObservableGauge( "name", Mock(), Mock(), @@ -208,10 +226,15 @@ def test_generator_multiple_generator_callback(self): ], ) + def test_disallow_direct_observable_gauge_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + ObservableGauge("name", Mock(), Mock()) + class TestObservableCounter(TestCase): def test_callable_callback_0(self): - observable_counter = ObservableCounter( + observable_counter = _ObservableCounter( "name", Mock(), Mock(), [callable_callback_0] ) @@ -237,7 +260,7 @@ def test_callable_callback_0(self): ) def test_generator_callback_0(self): - observable_counter = ObservableCounter( + observable_counter = _ObservableCounter( "name", Mock(), Mock(), [generator_callback_0()] ) @@ -262,10 +285,15 @@ def test_generator_callback_0(self): ], ) + def test_disallow_direct_observable_counter_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + ObservableCounter("name", Mock(), Mock()) + class TestObservableUpDownCounter(TestCase): def test_callable_callback_0(self): - observable_up_down_counter = ObservableUpDownCounter( + observable_up_down_counter = _ObservableUpDownCounter( "name", Mock(), Mock(), [callable_callback_0] ) @@ -291,7 +319,7 @@ def test_callable_callback_0(self): ) def test_generator_callback_0(self): - observable_up_down_counter = ObservableUpDownCounter( + observable_up_down_counter = _ObservableUpDownCounter( "name", Mock(), Mock(), [generator_callback_0()] ) @@ -316,16 +344,26 @@ def test_generator_callback_0(self): ], ) + def test_disallow_direct_observable_up_down_counter_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + ObservableUpDownCounter("name", Mock(), Mock()) + class TestHistogram(TestCase): def test_record(self): mc = Mock() - hist = Histogram("name", Mock(), mc) + hist = _Histogram("name", Mock(), mc) hist.record(1.0) mc.consume_measurement.assert_called_once() def test_record_non_monotonic(self): mc = Mock() - hist = Histogram("name", Mock(), mc) + hist = _Histogram("name", Mock(), mc) hist.record(-1.0) mc.consume_measurement.assert_not_called() + + def test_disallow_direct_histogram_creation(self): + with self.assertRaises(TypeError): + # pylint: disable=abstract-class-instantiated + Histogram("name", Mock(), Mock()) diff --git a/opentelemetry-sdk/tests/metrics/test_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_metric_reader.py index bc72e8bf98..e0d2813a5f 100644 --- a/opentelemetry-sdk/tests/metrics/test_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_metric_reader.py @@ -20,13 +20,14 @@ from opentelemetry.sdk.environment_variables import ( OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE, ) -from opentelemetry.sdk.metrics import ( - Counter, - Histogram, - ObservableCounter, - ObservableGauge, - ObservableUpDownCounter, - UpDownCounter, +from opentelemetry.sdk.metrics import Counter, Histogram, ObservableGauge +from opentelemetry.sdk.metrics._internal.instrument import ( + _Counter, + _Histogram, + _ObservableCounter, + _ObservableGauge, + _ObservableUpDownCounter, + _UpDownCounter, ) from opentelemetry.sdk.metrics.export import ( AggregationTemporality, @@ -39,6 +40,15 @@ LastValueAggregation, ) +_expected_keys = [ + _Counter, + _UpDownCounter, + _Histogram, + _ObservableCounter, + _ObservableUpDownCounter, + _ObservableGauge, +] + class DummyMetricReader(MetricReader): def __init__( @@ -74,16 +84,7 @@ def test_configure_temporality_cumulative(self): self.assertEqual( dummy_metric_reader._instrument_class_temporality.keys(), - set( - [ - Counter, - UpDownCounter, - Histogram, - ObservableCounter, - ObservableUpDownCounter, - ObservableGauge, - ] - ), + set(_expected_keys), ) for ( value @@ -99,43 +100,36 @@ def test_configure_temporality_delta(self): self.assertEqual( dummy_metric_reader._instrument_class_temporality.keys(), - set( - [ - Counter, - UpDownCounter, - Histogram, - ObservableCounter, - ObservableUpDownCounter, - ObservableGauge, - ] - ), - ) - self.assertEqual( - dummy_metric_reader._instrument_class_temporality[Counter], + set(_expected_keys), + ) + self.assertEqual( + dummy_metric_reader._instrument_class_temporality[_Counter], AggregationTemporality.DELTA, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[UpDownCounter], + dummy_metric_reader._instrument_class_temporality[_UpDownCounter], AggregationTemporality.CUMULATIVE, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[Histogram], + dummy_metric_reader._instrument_class_temporality[_Histogram], AggregationTemporality.DELTA, ) self.assertEqual( dummy_metric_reader._instrument_class_temporality[ - ObservableCounter + _ObservableCounter ], AggregationTemporality.DELTA, ) self.assertEqual( dummy_metric_reader._instrument_class_temporality[ - ObservableUpDownCounter + _ObservableUpDownCounter ], AggregationTemporality.CUMULATIVE, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[ObservableGauge], + dummy_metric_reader._instrument_class_temporality[ + _ObservableGauge + ], AggregationTemporality.CUMULATIVE, ) @@ -150,43 +144,36 @@ def test_configure_temporality_parameter(self): self.assertEqual( dummy_metric_reader._instrument_class_temporality.keys(), - set( - [ - Counter, - UpDownCounter, - Histogram, - ObservableCounter, - ObservableUpDownCounter, - ObservableGauge, - ] - ), - ) - self.assertEqual( - dummy_metric_reader._instrument_class_temporality[Counter], + set(_expected_keys), + ) + self.assertEqual( + dummy_metric_reader._instrument_class_temporality[_Counter], AggregationTemporality.CUMULATIVE, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[UpDownCounter], + dummy_metric_reader._instrument_class_temporality[_UpDownCounter], AggregationTemporality.CUMULATIVE, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[Histogram], + dummy_metric_reader._instrument_class_temporality[_Histogram], AggregationTemporality.DELTA, ) self.assertEqual( dummy_metric_reader._instrument_class_temporality[ - ObservableCounter + _ObservableCounter ], AggregationTemporality.CUMULATIVE, ) self.assertEqual( dummy_metric_reader._instrument_class_temporality[ - ObservableUpDownCounter + _ObservableUpDownCounter ], AggregationTemporality.CUMULATIVE, ) self.assertEqual( - dummy_metric_reader._instrument_class_temporality[ObservableGauge], + dummy_metric_reader._instrument_class_temporality[ + _ObservableGauge + ], AggregationTemporality.DELTA, ) @@ -194,16 +181,7 @@ def test_default_temporality(self): dummy_metric_reader = DummyMetricReader() self.assertEqual( dummy_metric_reader._instrument_class_aggregation.keys(), - set( - [ - Counter, - UpDownCounter, - Histogram, - ObservableCounter, - ObservableUpDownCounter, - ObservableGauge, - ] - ), + set(_expected_keys), ) for ( value @@ -215,18 +193,9 @@ def test_default_temporality(self): ) self.assertEqual( dummy_metric_reader._instrument_class_aggregation.keys(), - set( - [ - Counter, - UpDownCounter, - Histogram, - ObservableCounter, - ObservableUpDownCounter, - ObservableGauge, - ] - ), + set(_expected_keys), ) self.assertIsInstance( - dummy_metric_reader._instrument_class_aggregation[Counter], + dummy_metric_reader._instrument_class_aggregation[_Counter], LastValueAggregation, ) diff --git a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py index 1ab0af6ad6..7cea60d393 100644 --- a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py +++ b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py @@ -15,15 +15,15 @@ from logging import WARNING from unittest.mock import MagicMock, Mock, patch -from opentelemetry.sdk.metrics import ( - Counter, - Histogram, - ObservableCounter, - UpDownCounter, -) from opentelemetry.sdk.metrics._internal.aggregation import ( _LastValueAggregation, ) +from opentelemetry.sdk.metrics._internal.instrument import ( + _Counter, + _Histogram, + _ObservableCounter, + _UpDownCounter, +) from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.metrics._internal.metric_reader_storage import ( _DEFAULT_VIEW, @@ -292,7 +292,7 @@ def test_default_view_enabled(self, MockViewInstrumentMatch: Mock): def test_drop_aggregation(self): - counter = Counter("name", Mock(), Mock()) + counter = _Counter("name", Mock(), Mock()) metric_reader_storage = MetricReaderStorage( SdkConfiguration( resource=Mock(), @@ -324,8 +324,8 @@ def test_drop_aggregation(self): def test_same_collection_start(self): - counter = Counter("name", Mock(), Mock()) - up_down_counter = UpDownCounter("name", Mock(), Mock()) + counter = _Counter("name", Mock(), Mock()) + up_down_counter = _UpDownCounter("name", Mock(), Mock()) metric_reader_storage = MetricReaderStorage( SdkConfiguration( @@ -365,7 +365,7 @@ def test_same_collection_start(self): def test_conflicting_view_configuration(self): - observable_counter = ObservableCounter( + observable_counter = _ObservableCounter( "observable_counter", Mock(), [Mock()], @@ -406,14 +406,14 @@ def test_conflicting_view_configuration(self): def test_view_instrument_match_conflict_0(self): # There is a conflict between views and instruments. - observable_counter_0 = ObservableCounter( + observable_counter_0 = _ObservableCounter( "observable_counter_0", Mock(), [Mock()], unit="unit", description="description", ) - observable_counter_1 = ObservableCounter( + observable_counter_1 = _ObservableCounter( "observable_counter_1", Mock(), [Mock()], @@ -456,21 +456,21 @@ def test_view_instrument_match_conflict_0(self): def test_view_instrument_match_conflict_1(self): # There is a conflict between views and instruments. - observable_counter_foo = ObservableCounter( + observable_counter_foo = _ObservableCounter( "foo", Mock(), [Mock()], unit="unit", description="description", ) - observable_counter_bar = ObservableCounter( + observable_counter_bar = _ObservableCounter( "bar", Mock(), [Mock()], unit="unit", description="description", ) - observable_counter_baz = ObservableCounter( + observable_counter_baz = _ObservableCounter( "baz", Mock(), [Mock()], @@ -530,14 +530,14 @@ def test_view_instrument_match_conflict_1(self): def test_view_instrument_match_conflict_2(self): # There is no conflict because the metric streams names are different. - observable_counter_foo = ObservableCounter( + observable_counter_foo = _ObservableCounter( "foo", Mock(), [Mock()], unit="unit", description="description", ) - observable_counter_bar = ObservableCounter( + observable_counter_bar = _ObservableCounter( "bar", Mock(), [Mock()], @@ -578,14 +578,14 @@ def test_view_instrument_match_conflict_3(self): # There is no conflict because the aggregation temporality of the # instruments is different. - counter_bar = Counter( + counter_bar = _Counter( "bar", Mock(), [Mock()], unit="unit", description="description", ) - observable_counter_baz = ObservableCounter( + observable_counter_baz = _ObservableCounter( "baz", Mock(), [Mock()], @@ -626,14 +626,14 @@ def test_view_instrument_match_conflict_4(self): # There is no conflict because the monotonicity of the instruments is # different. - counter_bar = Counter( + counter_bar = _Counter( "bar", Mock(), [Mock()], unit="unit", description="description", ) - up_down_counter_baz = UpDownCounter( + up_down_counter_baz = _UpDownCounter( "baz", Mock(), [Mock()], @@ -673,14 +673,14 @@ def test_view_instrument_match_conflict_4(self): def test_view_instrument_match_conflict_5(self): # There is no conflict because the instrument units are different. - observable_counter_0 = ObservableCounter( + observable_counter_0 = _ObservableCounter( "observable_counter_0", Mock(), [Mock()], unit="unit_0", description="description", ) - observable_counter_1 = ObservableCounter( + observable_counter_1 = _ObservableCounter( "observable_counter_1", Mock(), [Mock()], @@ -720,14 +720,14 @@ def test_view_instrument_match_conflict_6(self): # There is no conflict because the instrument data points are # different. - observable_counter = ObservableCounter( + observable_counter = _ObservableCounter( "observable_counter", Mock(), [Mock()], unit="unit", description="description", ) - histogram = Histogram( + histogram = _Histogram( "histogram", Mock(), [Mock()], @@ -767,14 +767,14 @@ def test_view_instrument_match_conflict_7(self): # There is a conflict between views and instruments because the # description being different does not avoid a conflict. - observable_counter_0 = ObservableCounter( + observable_counter_0 = _ObservableCounter( "observable_counter_0", Mock(), [Mock()], unit="unit", description="description_0", ) - observable_counter_1 = ObservableCounter( + observable_counter_1 = _ObservableCounter( "observable_counter_1", Mock(), [Mock()], @@ -821,14 +821,14 @@ def test_view_instrument_match_conflict_8(self): # and also the temporality and monotonicity of the up down counter and # the histogram are the same. - observable_counter = UpDownCounter( + observable_counter = _UpDownCounter( "up_down_counter", Mock(), [Mock()], unit="unit", description="description", ) - histogram = Histogram( + histogram = _Histogram( "histogram", Mock(), [Mock()], diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index d2730086a4..c22c2d7a96 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -15,7 +15,6 @@ from unittest import TestCase from unittest.mock import MagicMock, Mock -from opentelemetry.sdk.metrics import Counter from opentelemetry.sdk.metrics._internal._view_instrument_match import ( _ViewInstrumentMatch, ) @@ -23,6 +22,7 @@ _DropAggregation, _LastValueAggregation, ) +from opentelemetry.sdk.metrics._internal.instrument import _Counter from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.metrics._internal.sdk_configuration import ( SdkConfiguration, @@ -172,7 +172,7 @@ def test_consume_measurement(self): ) def test_collect(self): - instrument1 = Counter( + instrument1 = _Counter( "instrument1", Mock(), Mock(), @@ -213,7 +213,7 @@ def test_collect(self): self.assertEqual(number_data_point.value, 0) def test_data_point_check(self): - instrument1 = Counter( + instrument1 = _Counter( "instrument1", Mock(), Mock(), @@ -285,7 +285,7 @@ def test_data_point_check(self): self.assertEqual(len(list(result)), 3) def test_setting_aggregation(self): - instrument1 = Counter( + instrument1 = _Counter( name="instrument1", instrumentation_scope=Mock(), measurement_consumer=Mock(), @@ -301,7 +301,7 @@ def test_setting_aggregation(self): attribute_keys={"a", "c"}, ), instrument=instrument1, - instrument_class_aggregation={Counter: LastValueAggregation()}, + instrument_class_aggregation={_Counter: LastValueAggregation()}, ) view_instrument_match.consume_measurement(