From 551dd16eb66dafab198c78068424c2d03d856b97 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Sat, 7 Nov 2026 21:58:32 +0530 Subject: [PATCH 1/2] fix(sdk): wire MeterProvider in add_metric_reader and allocate reader component ID once --- .../sdk/metrics/_internal/__init__.py | 2 + .../sdk/metrics/_internal/export/__init__.py | 19 ++++--- .../export/_metric_reader_metrics.py | 22 +++++--- .../metrics/test_in_memory_metric_reader.py | 52 ++++++++++++++++++- .../test_periodic_exporting_metric_reader.py | 51 +++++++++++++++++- 5 files changed, 131 insertions(+), 15 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py index 2a43dc299c7..4780c806836 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py @@ -642,6 +642,8 @@ def add_metric_reader(self, metric_reader: "opentelemetry.sdk.metrics.export.Met self._measurement_consumer.add_metric_reader(metric_reader) # pylint: disable-next=protected-access metric_reader._set_collect_callback(self._measurement_consumer.collect) + # pylint: disable-next=protected-access + metric_reader._set_meter_provider(self) self._all_metric_readers.add(metric_reader) def remove_metric_reader( 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 63656979972..3601ec1a18a 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py @@ -25,7 +25,7 @@ detach, set_value, ) -from opentelemetry.metrics import MeterProvider, NoOpMeterProvider +from opentelemetry.metrics import MeterProvider from opentelemetry.sdk.environment_variables import ( OTEL_METRIC_EXPORT_INTERVAL, OTEL_METRIC_EXPORT_TIMEOUT, @@ -67,7 +67,12 @@ ) from opentelemetry.util._once import Once -from ._metric_reader_metrics import create_metric_reader_metrics +from ._metric_reader_metrics import ( + MetricReaderMetrics, + MetricReaderMetricsT, + NoOpMetricReaderMetrics, + create_metric_reader_metrics, +) _logger = getLogger(__name__) @@ -379,11 +384,8 @@ def __init__( self._instrument_class_cardinality_limit[_validate_cardinality_limit(typ, limit)] = limit self._otel_component_type = otel_component_type.value if otel_component_type else type(self).__qualname__ - self._metrics = create_metric_reader_metrics( - self._otel_component_type, - NoOpMeterProvider(), - parse_boolean_environment_variable(OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED), - ) + self._metrics: MetricReaderMetricsT = NoOpMetricReaderMetrics() + self._otel_component_id: int | None = None @final def collect(self, timeout_millis: float = 10_000) -> None: @@ -443,7 +445,10 @@ def _set_meter_provider(self, meter_provider: MeterProvider) -> None: self._otel_component_type, meter_provider, parse_boolean_environment_variable(OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED), + component_id=self._otel_component_id, ) + if isinstance(self._metrics, MetricReaderMetrics): + self._otel_component_id = self._metrics._component_id def force_flush(self, timeout_millis: float = 10_000) -> bool: self.collect(timeout_millis=timeout_millis) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/_metric_reader_metrics.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/_metric_reader_metrics.py index 019f1cc1725..88f8b87721d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/_metric_reader_metrics.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/_metric_reader_metrics.py @@ -4,7 +4,7 @@ from collections import Counter from typing import Protocol -from opentelemetry.metrics import MeterProvider +from opentelemetry.metrics import MeterProvider, NoOpMeterProvider from opentelemetry.semconv._incubating.attributes.otel_attributes import ( OTEL_COMPONENT_NAME, OTEL_COMPONENT_TYPE, @@ -26,12 +26,21 @@ def record_collection(self, duration: float) -> None: class MetricReaderMetrics: - def __init__(self, component_type: str, meter_provider: MeterProvider) -> None: + def __init__( + self, + component_type: str, + meter_provider: MeterProvider, + component_id: int | None = None, + ) -> None: meter = meter_provider.get_meter("opentelemetry-sdk") - count = _component_counter[component_type] - _component_counter[component_type] = count + 1 + if component_id is None: + count = _component_counter[component_type] + _component_counter[component_type] = count + 1 + else: + count = component_id + self._component_id = count self._standard_attrs = { OTEL_COMPONENT_TYPE: component_type, OTEL_COMPONENT_NAME: f"{component_type}/{count}", @@ -47,8 +56,9 @@ def create_metric_reader_metrics( component_type: str, meter_provider: MeterProvider, enabled: bool, + component_id: int | None = None, ) -> MetricReaderMetricsT: - if not enabled: + if not enabled or isinstance(meter_provider, NoOpMeterProvider): return NoOpMetricReaderMetrics() - return MetricReaderMetrics(component_type, meter_provider) + return MetricReaderMetrics(component_type, meter_provider, component_id=component_id) diff --git a/opentelemetry-sdk/tests/metrics/test_in_memory_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_in_memory_metric_reader.py index 47aea81a424..5cbda5d4b8a 100644 --- a/opentelemetry-sdk/tests/metrics/test_in_memory_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_in_memory_metric_reader.py @@ -5,10 +5,17 @@ from time import sleep from unittest import TestCase -from unittest.mock import Mock +from unittest.mock import Mock, patch from opentelemetry.metrics import Observation +from opentelemetry.sdk.environment_variables import ( + OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED, +) from opentelemetry.sdk.metrics import Counter, MeterProvider +from opentelemetry.sdk.metrics._internal.export._metric_reader_metrics import ( + MetricReaderMetrics, + NoOpMetricReaderMetrics, +) from opentelemetry.sdk.metrics._internal.point import ( MetricsData, ResourceMetrics, @@ -166,6 +173,49 @@ def test_synchronous_gauge_cumulative_multiple_collect(self): [({"key": "value"}, 5)], ) + @patch.dict("os.environ", {OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED: "true"}) + def test_add_metric_reader_wires_meter_provider(self): + reader = InMemoryMetricReader() + meter_provider = MeterProvider() + self.addCleanup(meter_provider.shutdown) + self.addCleanup(reader.shutdown) + + # Before registration, reader metrics is a no-op and component ID is not allocated + self.assertIsInstance(reader._metrics, NoOpMetricReaderMetrics) + self.assertIsNone(reader._otel_component_id) + + meter_provider.add_metric_reader(reader) + + # After registration via add_metric_reader, reader metrics is wired and component ID is allocated once + self.assertIsInstance(reader._metrics, MetricReaderMetrics) + self.assertEqual(reader._otel_component_id, 0) + self.assertEqual( + reader._metrics._standard_attrs["otel.component.name"], + "InMemoryMetricReader/0", + ) + + # First collection records duration + reader.collect() + # Second collection captures the recorded duration metric + metrics_data = reader.get_metrics_data() + self.assertIsNotNone(metrics_data) + found_duration_metric = False + for rm in metrics_data.resource_metrics: + for sm in rm.scope_metrics: + for m in sm.metrics: + if m.name == "otel.sdk.metric_reader.collection.duration": + found_duration_metric = True + data_point = m.data.data_points[0] + self.assertEqual( + data_point.attributes["otel.component.name"], + "InMemoryMetricReader/0", + ) + self.assertEqual( + data_point.attributes["otel.component.type"], + "InMemoryMetricReader", + ) + self.assertTrue(found_duration_metric) + class TestInMemoryMetricReaderCardinalityLimits(TestCase): def test_default_limit(self): diff --git a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py index 3157c6dc7e4..7de32f1bc64 100644 --- a/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py +++ b/opentelemetry-sdk/tests/metrics/test_periodic_exporting_metric_reader.py @@ -24,6 +24,9 @@ MetricsTimeoutError, ) from opentelemetry.sdk.metrics._internal import _Counter +from opentelemetry.sdk.metrics._internal.export._metric_reader_metrics import ( + _component_counter, +) from opentelemetry.sdk.metrics._internal.point import ( HistogramDataPoint, MetricsData, @@ -381,7 +384,9 @@ def test_garbage_collected_processor_does_not_crash_on_fork(self): def test_metric_reader_metrics(self): exporter = FakeMetricsExporter() pmr = PeriodicExportingMetricReader(exporter, export_interval_millis=100000) + self.addCleanup(pmr.shutdown) mp = MeterProvider(metric_readers=[pmr]) + self.addCleanup(mp.shutdown) counter = mp.get_meter("test").create_counter("test_counter") counter.add(1) @@ -412,4 +417,48 @@ def test_metric_reader_metrics(self): assert isinstance(name, str) self.assertTrue(name.startswith("periodic_metric_reader/")) - mp.shutdown() + @patch.dict("os.environ", {OTEL_PYTHON_SDK_INTERNAL_METRICS_ENABLED: "true"}) + def test_metric_reader_component_id_allocation(self): + initial_count = _component_counter["periodic_metric_reader"] + + pmr1 = PeriodicExportingMetricReader(FakeMetricsExporter(), export_interval_millis=100000) + self.addCleanup(pmr1.shutdown) + self.assertIsNone(pmr1._otel_component_id) + + mp1 = MeterProvider(metric_readers=[pmr1]) + self.addCleanup(mp1.shutdown) + self.assertEqual(pmr1._otel_component_id, initial_count) + self.assertEqual( + pmr1._metrics._standard_attrs["otel.component.name"], + f"periodic_metric_reader/{initial_count}", + ) + self.assertEqual( + _component_counter["periodic_metric_reader"], + initial_count + 1, + ) + + # Re-binding does not increment component ID + pmr1._set_meter_provider(mp1) + self.assertEqual(pmr1._otel_component_id, initial_count) + self.assertEqual( + pmr1._metrics._standard_attrs["otel.component.name"], + f"periodic_metric_reader/{initial_count}", + ) + self.assertEqual( + _component_counter["periodic_metric_reader"], + initial_count + 1, + ) + + pmr2 = PeriodicExportingMetricReader(FakeMetricsExporter(), export_interval_millis=100000) + self.addCleanup(pmr2.shutdown) + mp2 = MeterProvider(metric_readers=[pmr2]) + self.addCleanup(mp2.shutdown) + self.assertEqual(pmr2._otel_component_id, initial_count + 1) + self.assertEqual( + pmr2._metrics._standard_attrs["otel.component.name"], + f"periodic_metric_reader/{initial_count + 1}", + ) + self.assertEqual( + _component_counter["periodic_metric_reader"], + initial_count + 2, + ) From b37259d5f9c884d6c113de9e6cf546eb230500f7 Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Sat, 7 Nov 2026 21:59:39 +0530 Subject: [PATCH 2/2] docs: add changelog fragment for PR 5897 --- .changelog/5897.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/5897.fixed diff --git a/.changelog/5897.fixed b/.changelog/5897.fixed new file mode 100644 index 00000000000..80774addc8f --- /dev/null +++ b/.changelog/5897.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: wire `MeterProvider` in `add_metric_reader` and allocate `MetricReader` component ID once