Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -580,3 +580,31 @@ def get_meter(
self._measurement_consumer,
)
return self._meters[info]

def add_metric_reader(
self, metric_reader: "opentelemetry.sdk.metrics.export.MetricReader"
) -> None:
with self._all_metric_readers_lock:
if metric_reader in self._all_metric_readers:
raise ValueError(
f"MetricReader {metric_reader} has been registered already!"
)
self._measurement_consumer.add_metric_reader(metric_reader)
metric_reader._set_collect_callback(
self._measurement_consumer.collect
)
self._all_metric_readers.add(metric_reader)

def remove_metric_reader(
self,
metric_reader: "opentelemetry.sdk.metrics.export.MetricReader",
) -> None:
with self._all_metric_readers_lock:
if metric_reader not in self._all_metric_readers:
raise ValueError(
f"MetricReader {metric_reader} has not been registered!"
)
self._measurement_consumer.remove_metric_reader(metric_reader)
metric_reader._set_collect_callback(None)
metric_reader.shutdown()
self._all_metric_readers.remove(metric_reader)
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,33 @@ def collect(
result = self._reader_storages[metric_reader].collect()

return result

def add_metric_reader(
self, metric_reader: "opentelemetry.sdk.metrics.MetricReader"
) -> None:
"""Registers a new metric reader."""
with self._lock:
self._sdk_config.metric_readers += type(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really isn't a good idea, we don't know that the type of self._sdk_config.metric_readers will even have a constructor that accepts an iterable.

Copy link
Author

@JP-MY JP-MY Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestions for a better alternative? given the Sequence type, this is the best I could come up with. It'd be better if the type is explicitly defined as something like Tuple | List but that would be a breaking API change

Copy link

@herin049 herin049 Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend we remove the metric readers attribute entirely from that configuration object and let the measurement consumer accept a sequence of metric readers, allowing it to store them in whichever data structure it wants.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will defer to other contributors/maintainers on this one.

self._sdk_config.metric_readers
)((metric_reader,))
self._reader_storages[metric_reader] = MetricReaderStorage(
self._sdk_config,
metric_reader._instrument_class_temporality,
metric_reader._instrument_class_aggregation,
)

def remove_metric_reader(
self, metric_reader: "opentelemetry.sdk.metrics.MetricReader"
) -> None:
"""Unregisters the given metric reader."""
with self._lock:
self._reader_storages.pop(metric_reader, None)
self._sdk_config.metric_readers = type(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same deal here with the typing, we should not be doing this.

self._sdk_config.metric_readers
)(
(
reader
for reader in self._sdk_config.metric_readers
if reader is not metric_reader
)
)
34 changes: 32 additions & 2 deletions opentelemetry-sdk/tests/metrics/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
# limitations under the License.

# pylint: disable=protected-access,no-self-use

import weakref
from logging import WARNING
from logging import DEBUG, WARNING
from time import sleep
from typing import Iterable, Sequence
from unittest.mock import MagicMock, Mock, patch
Expand All @@ -36,6 +35,7 @@
)
from opentelemetry.sdk.metrics._internal import SynchronousMeasurementConsumer
from opentelemetry.sdk.metrics.export import (
InMemoryMetricReader,
Metric,
MetricExporter,
MetricExportResult,
Expand Down Expand Up @@ -426,6 +426,36 @@ def test_consume_measurement_gauge(self, mock_sync_measurement_consumer):

sync_consumer_instance.consume_measurement.assert_called()

def test_addition_of_metric_reader(self):
# Suppress warnings for calling collect on an unregistered metric reader
with self.assertLogs(
"opentelemetry.sdk.metrics._internal.export", DEBUG
):
reader = InMemoryMetricReader()
meter_provider = MeterProvider()
meter = meter_provider.get_meter(__name__)
counter = meter.create_counter("counter")
counter.add(1)
self.assertIsNone(reader.get_metrics_data())

meter_provider.add_metric_reader(reader)
counter.add(1)
self.assertIsNotNone(reader.get_metrics_data())

with self.assertRaises(ValueError) as cm:
meter_provider.add_metric_reader(reader)
self.assertIn(
"has been registered already!", str(cm.exception)
)

meter_provider.remove_metric_reader(reader)
counter.add(1)
self.assertIsNone(reader.get_metrics_data())

with self.assertRaises(ValueError) as cm:
meter_provider.remove_metric_reader(reader)
self.assertIn("has not been registered!", str(cm.exception))


class TestMeter(TestCase):
def setUp(self):
Expand Down