diff --git a/.changelog/5855.fixed b/.changelog/5855.fixed new file mode 100644 index 00000000000..7277f0b97a0 --- /dev/null +++ b/.changelog/5855.fixed @@ -0,0 +1 @@ +`opentelemetry-api`: truncate byte array attribute values when attribute value length limit is set diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index f589b6b982e..a40d8302dc3 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -27,24 +27,24 @@ def _is_non_custom_str(key: Any) -> bool: @overload def _clean_attribute_value( value: Mapping[str, types.AnyValue], - max_string_value_length: int | None, + max_value_length: int | None, ) -> Mapping[str, types.AnyValue]: ... @overload def _clean_attribute_value( value: types.AnyValue, - max_string_value_length: int | None, + max_value_length: int | None, ) -> types.AnyValue: ... def _clean_attribute_value( value: types.AnyValue, - max_string_value_length: int | None, + max_value_length: int | None, ) -> types.AnyValue: """Recursively checks if an attribute value is valid and cleans it if required. - String values are truncated to max_string_value_length if provided. + String and byte array values are truncated to max_value_length if provided. Anything that isn't of `types.AnyValue`, we attempt to cast to `str`. If this fails, the value is replaced with None. Sequence's are converted to tuples and mappings are copied into new dicts. @@ -52,18 +52,19 @@ def _clean_attribute_value( Returns: The recursively cleaned AnyValue. """ - if isinstance(value, (NoneType, bool, int, float, bytes)): + if isinstance(value, (NoneType, bool, int, float)): return value - if isinstance(value, str): - if max_string_value_length is not None and len(value) > max_string_value_length: + if isinstance(value, (str, bytes, bytearray)): + if max_value_length is not None and len(value) > max_value_length: _logger.warning( - "String attribute value exceeds max length of %d, truncating.", - max_string_value_length, + "%s attribute value exceeds max length of %d, truncating.", + "Byte array" if isinstance(value, (bytes, bytearray)) else "String", + max_value_length, ) - value = value[:max_string_value_length] - return value + value = value[:max_value_length] + return bytes(value) if isinstance(value, bytearray) else value if isinstance(value, Sequence): - return tuple(_clean_attribute_value(v, max_string_value_length) for v in value) + return tuple(_clean_attribute_value(v, max_value_length) for v in value) if isinstance(value, Mapping): cleaned_mapping: dict[str, types.AnyValue] = {} for key, val in value.items(): @@ -84,7 +85,7 @@ def _clean_attribute_value( key = str(key) else: continue - cleaned_mapping[key] = _clean_attribute_value(val, max_string_value_length) + cleaned_mapping[key] = _clean_attribute_value(val, max_value_length) return cleaned_mapping if TYPE_CHECKING: assert_never(value) @@ -105,7 +106,7 @@ class BoundedAttributes(MutableMapping[str, types.AnyValue]): maxlen: The maximum number of attributes to store, use None for no limit. attributes: The initial attributes to store. immutable: Defaults to true. Whether to allow adding/removing of attributes after the initialisation of the instance. - max_value_len: The maximum length of string values, use None for no limit. + max_value_len: The maximum length of string and byte array values, use None for no limit. extended_attributes: Deprecated. Kept for backwards compatibility. Extended attributes are now always used for attributes everywhere. When the dict is full, a new key is dropped; existing keys can still be updated. Attributes are made to be immutable when set in this container. diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index e9fd681bcbf..95a90fa8e03 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -47,7 +47,23 @@ def test_clean_attribute_value_with_various_params(self): for valid_primitive in valid_primitive_sequence: self.assertEqual(_clean_attribute_value(valid_primitive, None), valid_primitive) - self.assertEqual(_clean_attribute_value(b"hello", 4), b"hello") + # Byte array is not truncated when length <= limit. + self.assertEqual(_clean_attribute_value(b"hello", 5), b"hello") + self.assertEqual(_clean_attribute_value(b"hello", None), b"hello") + + with self.assertLogs("opentelemetry", level="WARNING") as cm: + # Byte array is truncated. + self.assertEqual(_clean_attribute_value(b"hello", 4), b"hell") + self.assertEqual(len(cm.output), 1) + self.assertIn("Byte array attribute value exceeds max length", cm.output[0]) + + with self.assertLogs("opentelemetry", level="WARNING") as cm: + # Mutable bytearray is truncated and converted to immutable bytes. + cleaned_bytearray = _clean_attribute_value(bytearray(b"hello"), 4) + self.assertEqual(cleaned_bytearray, b"hell") + self.assertIsInstance(cleaned_bytearray, bytes) + self.assertEqual(len(cm.output), 1) + self.assertIn("Byte array attribute value exceeds max length", cm.output[0]) with self.assertLogs("opentelemetry", level="WARNING") as cm: # String is truncated. @@ -285,3 +301,30 @@ def test_deepcopy_preserves_immutability(self): with self.assertRaises(TypeError): bdict_copy["invalid"] = "invalid" + + def test_byte_array_truncation_in_bounded_attributes(self): + bdict = BoundedAttributes(immutable=False, max_value_len=4) + with self.assertLogs("opentelemetry", level="WARNING") as cm: + bdict["bytes_val"] = b"1234567" + self.assertEqual(bdict["bytes_val"], b"1234") + self.assertEqual(len(cm.output), 1) + self.assertIn("Byte array attribute value exceeds max length of 4, truncating.", cm.output[0]) + + with self.assertLogs("opentelemetry", level="WARNING") as cm: + bdict["bytearray_val"] = bytearray(b"abcdefg") + self.assertEqual(bdict["bytearray_val"], b"abcd") + self.assertIsInstance(bdict["bytearray_val"], bytes) + + def test_byte_array_in_nested_structures_truncated(self): + # In sequences + with self.assertLogs("opentelemetry", level="WARNING") as cm: + cleaned_seq = _clean_attribute_value([b"abcdef", b"xy", bytearray(b"123456")], 3) + self.assertEqual(cleaned_seq, (b"abc", b"xy", b"123")) + self.assertIsInstance(cleaned_seq[2], bytes) + self.assertEqual(len(cm.output), 2) + + # In mappings + with self.assertLogs("opentelemetry", level="WARNING") as cm: + cleaned_map = _clean_attribute_value({"payload": b"abcdef", "ok": b"12"}, 3) + self.assertEqual(cleaned_map, {"payload": b"abc", "ok": b"12"}) + self.assertEqual(len(cm.output), 1)