diff --git a/CHANGELOG.md b/CHANGELOG.md index ad876d36bf0..127343dec2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044)) - `opentelemetry-sdk` Add support for `OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT` env var ([#2056](https://github.com/open-telemetry/opentelemetry-python/pull/2056)) +- `opentelemetry-api` Attribute keys must be non-empty strings. + ([#2057](https://github.com/open-telemetry/opentelemetry-python/pull/2057)) ## [0.23.1](https://github.com/open-telemetry/opentelemetry-python/pull/1987) - 2021-07-26 diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 877f98e8be4..a266839326a 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -47,8 +47,8 @@ def _clean_attribute( - It needs to be encoded/decoded e.g, bytes to strings. """ - if key is None or key == "": - _logger.warning("invalid key `%s` (empty or null)", key) + if not (key and isinstance(key, str)): + _logger.warning("invalid key `%s`. must be non-empty string.", key) return None if isinstance(value, _VALID_ATTR_VALUE_TYPES): @@ -118,7 +118,7 @@ def _clean_attribute_value( if isinstance(value, bytes): try: value = value.decode() - except ValueError: + except UnicodeDecodeError: _logger.warning("Byte attribute could not be decoded.") return None diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index 045c1f7125c..fa0606b347c 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -31,6 +31,16 @@ def assertValid(self, value, key="k"): def assertInvalid(self, value, key="k"): self.assertIsNone(_clean_attribute(key, value, None)) + def test_attribute_key_validation(self): + # only non-empty strings are valid keys + self.assertInvalid(1, "") + self.assertInvalid(1, 1) + self.assertInvalid(1, {}) + self.assertInvalid(1, []) + self.assertInvalid(1, b"1") + self.assertValid(1, "k") + self.assertValid(1, "1") + def test_clean_attribute(self): self.assertInvalid([1, 2, 3.4, "ss", 4]) self.assertInvalid([dict(), 1, 2, 3.4, 4]) @@ -142,10 +152,10 @@ def test_bounded_dict(self): def test_no_limit_code(self): bdict = BoundedAttributes(maxlen=None, immutable=False) for num in range(100): - bdict[num] = num + bdict[str(num)] = num for num in range(100): - self.assertEqual(bdict[num], num) + self.assertEqual(bdict[str(num)], num) def test_immutable(self): bdict = BoundedAttributes() diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 7acffecd85f..249057aef78 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -536,6 +536,12 @@ class SpanLimits: environment variable. - If the environment variable is not set, the default value for the limit is used. + Limit precedence: + + - If a model specific limit is set, it will be used. + - Else if the model specific limit has a default value, the default value will be used. + - Else if model specific limit has a corresponding global limit, the global limit will be used. + Args: max_attributes: Maximum number of attributes that can be added to a Span. Environment variable: OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT @@ -593,10 +599,6 @@ def __init__( OTEL_LINK_ATTRIBUTE_COUNT_LIMIT, _DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT, ) - self.max_attribute_length = self._from_env_if_absent( - max_attribute_length, - OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT, - ) self.max_attribute_length = self._from_env_if_absent( max_attribute_length,