diff --git a/.changelog/5303.fixed b/.changelog/5303.fixed new file mode 100644 index 0000000000..537f490321 --- /dev/null +++ b/.changelog/5303.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: fix `TypeError` when metric attribute values are lists or other non-hashable sequences diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py index d9ba05363b..e413522122 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/_view_instrument_match.py @@ -19,6 +19,7 @@ from opentelemetry.sdk.metrics._internal.measurement import Measurement from opentelemetry.sdk.metrics._internal.point import DataPointT from opentelemetry.sdk.metrics._internal.view import View +from opentelemetry.util.types import AttributeValue _logger = getLogger(__name__) @@ -98,7 +99,15 @@ def consume_measurement( else: attributes = {} - aggr_key = frozenset(attributes.items()) + try: + aggr_key = frozenset(attributes.items()) + except TypeError: + for attr_key, value in attributes.items(): + if isinstance(value, Sequence) and not isinstance( + value, (str, bytes) + ): + attributes[attr_key] = cast(AttributeValue, tuple(value)) + aggr_key = frozenset(attributes.items()) if aggr_key not in self._attributes_aggregation: with self._lock: diff --git a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py index 73b0e6e24a..2ee0ff8baa 100644 --- a/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py +++ b/opentelemetry-sdk/tests/metrics/test_view_instrument_match.py @@ -483,6 +483,136 @@ def test_data_point_check(self): self.assertEqual(len(list(result)), 3) + def test_consume_measurement_with_list_attributes(self): + instrument1 = Mock(name="instrument1") + instrument1.instrumentation_scope = self.mock_instrumentation_scope + view_instrument_match = _ViewInstrumentMatch( + view=View( + instrument_name="instrument1", + name="name", + aggregation=self.mock_aggregation_factory, + ), + instrument=instrument1, + instrument_class_aggregation=MagicMock( + **{"__getitem__.return_value": DefaultAggregation()} + ), + ) + + view_instrument_match.consume_measurement( + Measurement( + value=1, + time_unix_nano=time_ns(), + instrument=instrument1, + context=Context(), + attributes={ + "key": "value", + "tags": [1, 2, 3], + }, + ) + ) + + expected_key = frozenset([("key", "value"), ("tags", (1, 2, 3))]) + self.assertIn( + expected_key, view_instrument_match._attributes_aggregation + ) + + def test_consume_measurement_list_and_tuple_produce_same_key(self): + instrument1 = Mock(name="instrument1") + instrument1.instrumentation_scope = self.mock_instrumentation_scope + view_instrument_match = _ViewInstrumentMatch( + view=View( + instrument_name="instrument1", + name="name", + aggregation=self.mock_aggregation_factory, + ), + instrument=instrument1, + instrument_class_aggregation=MagicMock( + **{"__getitem__.return_value": DefaultAggregation()} + ), + ) + + view_instrument_match.consume_measurement( + Measurement( + value=1, + time_unix_nano=time_ns(), + instrument=instrument1, + context=Context(), + attributes={"tags": [1, 2]}, + ) + ) + view_instrument_match.consume_measurement( + Measurement( + value=2, + time_unix_nano=time_ns(), + instrument=instrument1, + context=Context(), + attributes={"tags": (1, 2)}, + ) + ) + + self.assertEqual(len(view_instrument_match._attributes_aggregation), 1) + + def test_consume_measurement_string_attributes_not_converted(self): + instrument1 = Mock(name="instrument1") + instrument1.instrumentation_scope = self.mock_instrumentation_scope + view_instrument_match = _ViewInstrumentMatch( + view=View( + instrument_name="instrument1", + name="name", + aggregation=self.mock_aggregation_factory, + ), + instrument=instrument1, + instrument_class_aggregation=MagicMock( + **{"__getitem__.return_value": DefaultAggregation()} + ), + ) + + view_instrument_match.consume_measurement( + Measurement( + value=1, + time_unix_nano=time_ns(), + instrument=instrument1, + context=Context(), + attributes={"name": "hello"}, + ) + ) + + expected_key = frozenset([("name", "hello")]) + self.assertIn( + expected_key, view_instrument_match._attributes_aggregation + ) + + def test_consume_measurement_with_list_attributes_and_view_filter(self): + instrument1 = Mock(name="instrument1") + instrument1.instrumentation_scope = self.mock_instrumentation_scope + view_instrument_match = _ViewInstrumentMatch( + view=View( + instrument_name="instrument1", + name="name", + aggregation=self.mock_aggregation_factory, + attribute_keys={"tags"}, + ), + instrument=instrument1, + instrument_class_aggregation=MagicMock( + **{"__getitem__.return_value": DefaultAggregation()} + ), + ) + + view_instrument_match.consume_measurement( + Measurement( + value=1, + time_unix_nano=time_ns(), + instrument=instrument1, + context=Context(), + attributes={"tags": ["a", "b"], "extra": "dropped"}, + ) + ) + + expected_key = frozenset([("tags", ("a", "b"))]) + self.assertIn( + expected_key, view_instrument_match._attributes_aggregation + ) + def test_setting_aggregation(self): instrument1 = _Counter( name="instrument1",