From 11a3b3663f1c8d7c879592a8c36e19395e1fa9bb Mon Sep 17 00:00:00 2001 From: Soumyajit Ghosh Date: Sun, 6 Sep 2026 02:47:20 +0530 Subject: [PATCH 1/2] sdk: check default-view fallback stream for conflicting metric identities Fixes #5629 MetricReaderStorage creates metric streams for instruments either via matching user-configured views in _handle_view_instrument_match or via the _DEFAULT_VIEW fallback in _get_or_init_view_instrument_match when no views match. Previously, only _handle_view_instrument_match executed conflict checking against existing matches. As a result, when an explicit view renamed an instrument to a name matching an instrument that uses the default-view fallback, the conflict warning was only emitted if the renamed instrument arrived second. If the renamed instrument arrived first, the default-view fallback stream was appended without checking for conflicts, silently exporting duplicate metric identities in violation of the OpenTelemetry specification. Extract _check_conflicts_and_add_match to route both stream-creation paths through conflict scanning, ensuring conflicting metric identities are reliably warned regardless of instrument registration order. Signed-off-by: Soumyajit Ghosh --- .../_internal/metric_reader_storage.py | 37 ++++++----- .../metrics/test_metric_reader_storage.py | 64 ++++++++++++++++++- 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py index d0ef125d797..0d07d8dfea0 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/metric_reader_storage.py @@ -81,13 +81,13 @@ def _get_or_init_view_instrument_match(self, instrument: _Instrument) -> list[_V # if no view targeted the instrument, use the default if not view_instrument_matches: - view_instrument_matches.append( - _ViewInstrumentMatch( - view=_DEFAULT_VIEW, - instrument=instrument, - instrument_class_aggregation=(self._instrument_class_aggregation), - ) + default_view_instrument_match = _ViewInstrumentMatch( + view=_DEFAULT_VIEW, + instrument=instrument, + instrument_class_aggregation=(self._instrument_class_aggregation), ) + self._check_conflicts_and_add_match(default_view_instrument_match, view_instrument_matches) + self._instrument_view_instrument_matches[instrument] = view_instrument_matches return view_instrument_matches @@ -225,16 +225,23 @@ def _handle_view_instrument_match( instrument_class_aggregation=(self._instrument_class_aggregation), ) - for existing_view_instrument_matches in self._instrument_view_instrument_matches.values(): - for existing_view_instrument_match in existing_view_instrument_matches: - if existing_view_instrument_match.conflicts(new_view_instrument_match): - _logger.warning( - "Views %s and %s will cause conflicting metrics identities", - existing_view_instrument_match._view, - new_view_instrument_match._view, - ) + self._check_conflicts_and_add_match(new_view_instrument_match, view_instrument_matches) + + def _check_conflicts_and_add_match( + self, + new_view_instrument_match: "_ViewInstrumentMatch", + view_instrument_matches: list["_ViewInstrumentMatch"], + ) -> None: + for existing_view_instrument_matches in self._instrument_view_instrument_matches.values(): + for existing_view_instrument_match in existing_view_instrument_matches: + if existing_view_instrument_match.conflicts(new_view_instrument_match): + _logger.warning( + "Views %s and %s will cause conflicting metrics identities", + existing_view_instrument_match._view, + new_view_instrument_match._view, + ) - view_instrument_matches.append(new_view_instrument_match) + view_instrument_matches.append(new_view_instrument_match) @staticmethod def _check_view_instrument_compatibility(view: View, instrument: _Instrument) -> bool: diff --git a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py index 581e066216b..41f0ed99a4d 100644 --- a/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py +++ b/opentelemetry-sdk/tests/metrics/test_metric_reader_storage.py @@ -247,7 +247,8 @@ def test_default_view_enabled(self, MockViewInstrumentMatch: Mock): self.assertEqual(len(MockViewInstrumentMatch.call_args_list), 1) MockViewInstrumentMatch.call_args_list.clear() - storage.consume_measurement(Measurement(1, time_ns(), instrument2, Context())) + with self.assertLogs(level=WARNING): + storage.consume_measurement(Measurement(1, time_ns(), instrument2, Context())) self.assertEqual(len(MockViewInstrumentMatch.call_args_list), 1) def test_drop_aggregation(self): @@ -753,3 +754,64 @@ def test_view_instrument_match_conflict_8(self): "will cause conflicting metrics", log.records[0].message, ) + + def test_view_instrument_match_conflict_default_view_order_independence(self): + # A conflict between a view-renamed instrument and a default-view + # fallback instrument is reported regardless of arrival order. + + observable_counter_bar = _ObservableCounter( + "bar", + Mock(), + [Mock()], + unit="unit", + description="description", + ) + observable_counter_foo = _ObservableCounter( + "foo", + Mock(), + [Mock()], + unit="unit", + description="description", + ) + + def make_storage(): + return MetricReaderStorage( + SdkConfiguration( + exemplar_filter=Mock(), + resource=Mock(), + views=( + View(instrument_name="bar", name="foo"), + ), + ), + MagicMock(**{"__getitem__.return_value": AggregationTemporality.CUMULATIVE}), + MagicMock(**{"__getitem__.return_value": DefaultAggregation()}), + ) + + # Order 1: default-view instrument arrives first, view-renamed arrives second + storage_1 = make_storage() + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + storage_1.consume_measurement(Measurement(1, time_ns(), observable_counter_foo, Context())) + + with self.assertLogs(level=WARNING) as log_1: + storage_1.consume_measurement(Measurement(1, time_ns(), observable_counter_bar, Context())) + + self.assertIn( + "will cause conflicting metrics", + log_1.records[0].message, + ) + + # Order 2: view-renamed arrives first, default-view instrument arrives second + storage_2 = make_storage() + with self.assertRaises(AssertionError): + with self.assertLogs(level=WARNING): + storage_2.consume_measurement(Measurement(1, time_ns(), observable_counter_bar, Context())) + + with self.assertLogs(level=WARNING) as log_2: + storage_2.consume_measurement(Measurement(1, time_ns(), observable_counter_foo, Context())) + + self.assertIn( + "will cause conflicting metrics", + log_2.records[0].message, + ) + From f645ba344ad48a9f28fd157e84452f865c7bef7d Mon Sep 17 00:00:00 2001 From: Soumyajit Ghosh Date: Sun, 6 Sep 2026 02:47:25 +0530 Subject: [PATCH 2/2] changelog: add entry for PR #5632 Signed-off-by: Soumyajit Ghosh --- .changelog/5632.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/5632.fixed diff --git a/.changelog/5632.fixed b/.changelog/5632.fixed new file mode 100644 index 00000000000..4c928ac0b7d --- /dev/null +++ b/.changelog/5632.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: check default-view fallback stream for conflicting metric identities to ensure order-independent warnings