fix(sdk): wire MeterProvider in add_metric_reader and allocate reader component ID once

.changelog/5897.fixed

@@ -0,0 +1 @@
+`opentelemetry-sdk`: wire `MeterProvider` in `add_metric_reader` and allocate `MetricReader` component ID once

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(

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)

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)

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):

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,
+        )