Skip to content

Commit 5c8ef2e

Browse files
committed
Added support for OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT
Fixes #2045 Fixes #2043 Fixes #2042 Fixes #2041
1 parent 4e0642b commit 5c8ef2e

File tree

7 files changed

+280
-174
lines changed

7 files changed

+280
-174
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
- Add Trace ID validation to meet [TraceID spec](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/overview.md#spancontext) ([#1992](https://github.com/open-telemetry/opentelemetry-python/pull/1992))
1717
- Fixed Python 3.10 incompatibility in `opentelemetry-opentracing-shim` tests
1818
([#2018](https://github.com/open-telemetry/opentelemetry-python/pull/2018))
19+
- `opentelemetry-sdk` added support for `OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT`
20+
([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
21+
- `opentelemetry-sdk` Fixed bugs (#2041 & #2042) in Span Limits
22+
([#2044](https://github.com/open-telemetry/opentelemetry-python/pull/2044))
1923

2024
## [0.23.1](https://github.com/open-telemetry/opentelemetry-python/pull/1987) - 2021-07-26
2125

opentelemetry-api/src/opentelemetry/attributes/__init__.py

+62-41
Original file line numberDiff line numberDiff line change
@@ -17,36 +17,57 @@
1717
import threading
1818
from collections import OrderedDict
1919
from collections.abc import MutableMapping
20-
from typing import MutableSequence, Optional, Sequence
20+
from typing import Optional, Sequence, Union
2121

2222
from opentelemetry.util import types
2323

24-
_VALID_ATTR_VALUE_TYPES = (bool, str, int, float)
24+
# bytes are accepted as a user supplied value for attributes but
25+
# decoded to strings internally.
26+
_VALID_ATTR_VALUE_TYPES = (bool, str, bytes, int, float)
2527

2628

2729
_logger = logging.getLogger(__name__)
2830

2931

30-
def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
31-
"""Checks if attribute value is valid.
32+
def _clean_attribute(
33+
key: str, value: types.AttributeValue, max_len: Optional[int]
34+
) -> Optional[types.AttributeValue]:
35+
"""Checks if attribute value is valid and cleans it if required.
36+
37+
The function returns the cleaned value or None if the value is not valid.
3238
3339
An attribute value is valid if it is either:
3440
- A primitive type: string, boolean, double precision floating
3541
point (IEEE 754-1985) or integer.
3642
- An array of primitive type values. The array MUST be homogeneous,
3743
i.e. it MUST NOT contain values of different types.
38-
"""
3944
40-
if isinstance(value, _VALID_ATTR_VALUE_TYPES):
41-
return True
45+
An attribute needs cleansing if:
46+
- Its length is greater than the maximum allowed length.
47+
- It needs to be encoded/decoded e.g, bytes to strings.
48+
"""
4249

43-
if isinstance(value, Sequence):
50+
if key is None or key == "":
51+
_logger.warning("invalid key `%s` (empty or null)", key)
52+
return None
4453

54+
if isinstance(value, _VALID_ATTR_VALUE_TYPES):
55+
return _clean_attribute_value(value, max_len)
56+
elif isinstance(value, Sequence):
4557
sequence_first_valid_type = None
58+
cleaned_seq = []
59+
4660
for element in value:
61+
# None is considered valid in any sequence
62+
if element is None:
63+
cleaned_seq.append(element)
64+
65+
element = _clean_attribute_value(element, max_len)
4766
if element is None:
4867
continue
68+
4969
element_type = type(element)
70+
# Reject attribute value if sequence contains a value with an incompatible type.
5071
if element_type not in _VALID_ATTR_VALUE_TYPES:
5172
_logger.warning(
5273
"Invalid type %s in attribute value sequence. Expected one of "
@@ -57,56 +78,52 @@ def _is_valid_attribute_value(value: types.AttributeValue) -> bool:
5778
for valid_type in _VALID_ATTR_VALUE_TYPES
5879
],
5980
)
60-
return False
81+
return None
82+
6183
# The type of the sequence must be homogeneous. The first non-None
6284
# element determines the type of the sequence
6385
if sequence_first_valid_type is None:
6486
sequence_first_valid_type = element_type
65-
elif not isinstance(element, sequence_first_valid_type):
87+
# use equality instead of isinstance as isinstance(True, int) evaluates to True
88+
elif element_type != sequence_first_valid_type:
6689
_logger.warning(
6790
"Mixed types %s and %s in attribute value sequence",
6891
sequence_first_valid_type.__name__,
6992
type(element).__name__,
7093
)
71-
return False
72-
return True
94+
return None
95+
96+
# reject invalid elements
97+
if element is not None:
98+
cleaned_seq.append(element)
99+
100+
# Freeze mutable sequences defensively
101+
return tuple(cleaned_seq)
73102

74103
_logger.warning(
75104
"Invalid type %s for attribute value. Expected one of %s or a "
76105
"sequence of those types",
77106
type(value).__name__,
78107
[valid_type.__name__ for valid_type in _VALID_ATTR_VALUE_TYPES],
79108
)
80-
return False
81-
82109

83-
def _filter_attributes(attributes: types.Attributes) -> None:
84-
"""Applies attribute validation rules and drops (key, value) pairs
85-
that doesn't adhere to attributes specification.
86110

87-
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/common.md#attributes.
88-
"""
89-
if attributes:
90-
for attr_key, attr_value in list(attributes.items()):
91-
if not attr_key:
92-
_logger.warning("invalid key `%s` (empty or null)", attr_key)
93-
attributes.pop(attr_key)
94-
continue
111+
def _clean_attribute_value(
112+
value: types.AttributeValue, limit: Optional[int]
113+
) -> Union[types.AttributeValue, None]:
114+
if value is None:
115+
return
95116

96-
if _is_valid_attribute_value(attr_value):
97-
if isinstance(attr_value, MutableSequence):
98-
attributes[attr_key] = tuple(attr_value)
99-
if isinstance(attr_value, bytes):
100-
try:
101-
attributes[attr_key] = attr_value.decode()
102-
except ValueError:
103-
attributes.pop(attr_key)
104-
_logger.warning("Byte attribute could not be decoded.")
105-
else:
106-
attributes.pop(attr_key)
117+
if isinstance(value, bytes):
118+
try:
119+
value = value.decode()
120+
except ValueError:
121+
_logger.warning("Byte attribute could not be decoded.")
122+
return None
107123

108-
109-
_DEFAULT_LIMIT = 128
124+
if limit is not None and isinstance(value, str):
125+
value = value[:limit]
126+
return value
110127

111128

112129
class BoundedAttributes(MutableMapping):
@@ -118,9 +135,10 @@ class BoundedAttributes(MutableMapping):
118135

119136
def __init__(
120137
self,
121-
maxlen: Optional[int] = _DEFAULT_LIMIT,
138+
maxlen: Optional[int] = None,
122139
attributes: types.Attributes = None,
123140
immutable: bool = True,
141+
max_value_len: Optional[int] = None,
124142
):
125143
if maxlen is not None:
126144
if not isinstance(maxlen, int) or maxlen < 0:
@@ -129,10 +147,10 @@ def __init__(
129147
)
130148
self.maxlen = maxlen
131149
self.dropped = 0
150+
self.max_value_len = max_value_len
132151
self._dict = OrderedDict() # type: OrderedDict
133152
self._lock = threading.Lock() # type: threading.Lock
134153
if attributes:
135-
_filter_attributes(attributes)
136154
for key, value in attributes.items():
137155
self[key] = value
138156
self._immutable = immutable
@@ -158,7 +176,10 @@ def __setitem__(self, key, value):
158176
elif self.maxlen is not None and len(self._dict) == self.maxlen:
159177
del self._dict[next(iter(self._dict.keys()))]
160178
self.dropped += 1
161-
self._dict[key] = value
179+
180+
value = _clean_attribute(key, value, self.max_value_len)
181+
if value is not None:
182+
self._dict[key] = value
162183

163184
def __delitem__(self, key):
164185
if getattr(self, "_immutable", False):

opentelemetry-api/tests/attributes/test_attributes.py

+37-58
Original file line numberDiff line numberDiff line change
@@ -16,69 +16,48 @@
1616

1717
import collections
1818
import unittest
19+
from typing import MutableSequence
1920

20-
from opentelemetry.attributes import (
21-
BoundedAttributes,
22-
_filter_attributes,
23-
_is_valid_attribute_value,
24-
)
21+
from opentelemetry.attributes import BoundedAttributes, _clean_attribute
2522

2623

2724
class TestAttributes(unittest.TestCase):
28-
def test_is_valid_attribute_value(self):
29-
self.assertFalse(_is_valid_attribute_value([1, 2, 3.4, "ss", 4]))
30-
self.assertFalse(_is_valid_attribute_value([dict(), 1, 2, 3.4, 4]))
31-
self.assertFalse(_is_valid_attribute_value(["sw", "lf", 3.4, "ss"]))
32-
self.assertFalse(_is_valid_attribute_value([1, 2, 3.4, 5]))
33-
self.assertFalse(_is_valid_attribute_value(dict()))
34-
self.assertTrue(_is_valid_attribute_value(True))
35-
self.assertTrue(_is_valid_attribute_value("hi"))
36-
self.assertTrue(_is_valid_attribute_value(3.4))
37-
self.assertTrue(_is_valid_attribute_value(15))
38-
self.assertTrue(_is_valid_attribute_value([1, 2, 3, 5]))
39-
self.assertTrue(_is_valid_attribute_value([1.2, 2.3, 3.4, 4.5]))
40-
self.assertTrue(_is_valid_attribute_value([True, False]))
41-
self.assertTrue(_is_valid_attribute_value(["ss", "dw", "fw"]))
42-
self.assertTrue(_is_valid_attribute_value([]))
25+
def assertValid(self, value, key="k"):
26+
expected = value
27+
if isinstance(value, MutableSequence):
28+
expected = tuple(value)
29+
self.assertEqual(_clean_attribute(key, value, None), expected)
30+
31+
def assertInvalid(self, value, key="k"):
32+
self.assertIsNone(_clean_attribute(key, value, None))
33+
34+
def test_clean_attribute(self):
35+
self.assertInvalid([1, 2, 3.4, "ss", 4])
36+
self.assertInvalid([dict(), 1, 2, 3.4, 4])
37+
self.assertInvalid(["sw", "lf", 3.4, "ss"])
38+
self.assertInvalid([1, 2, 3.4, 5])
39+
self.assertInvalid(dict())
40+
self.assertInvalid([1, True])
41+
self.assertValid(True)
42+
self.assertValid("hi")
43+
self.assertValid(3.4)
44+
self.assertValid(15)
45+
self.assertValid([1, 2, 3, 5])
46+
self.assertValid([1.2, 2.3, 3.4, 4.5])
47+
self.assertValid([True, False])
48+
self.assertValid(["ss", "dw", "fw"])
49+
self.assertValid([])
4350
# None in sequences are valid
44-
self.assertTrue(_is_valid_attribute_value(["A", None, None]))
45-
self.assertTrue(_is_valid_attribute_value(["A", None, None, "B"]))
46-
self.assertTrue(_is_valid_attribute_value([None, None]))
47-
self.assertFalse(_is_valid_attribute_value(["A", None, 1]))
48-
self.assertFalse(_is_valid_attribute_value([None, "A", None, 1]))
49-
50-
def test_filter_attributes(self):
51-
attrs_with_invalid_keys = {
52-
"": "empty-key",
53-
None: "None-value",
54-
"attr-key": "attr-value",
55-
}
56-
_filter_attributes(attrs_with_invalid_keys)
57-
self.assertTrue(len(attrs_with_invalid_keys), 1)
58-
self.assertEqual(attrs_with_invalid_keys, {"attr-key": "attr-value"})
59-
60-
attrs_with_invalid_values = {
61-
"nonhomogeneous": [1, 2, 3.4, "ss", 4],
62-
"nonprimitive": dict(),
63-
"mixed": [1, 2.4, "st", dict()],
64-
"validkey1": "validvalue1",
65-
"intkey": 5,
66-
"floatkey": 3.14,
67-
"boolkey": True,
68-
"valid-byte-string": b"hello-otel",
69-
}
70-
_filter_attributes(attrs_with_invalid_values)
71-
self.assertEqual(len(attrs_with_invalid_values), 5)
72-
self.assertEqual(
73-
attrs_with_invalid_values,
74-
{
75-
"validkey1": "validvalue1",
76-
"intkey": 5,
77-
"floatkey": 3.14,
78-
"boolkey": True,
79-
"valid-byte-string": "hello-otel",
80-
},
81-
)
51+
self.assertValid(["A", None, None])
52+
self.assertValid(["A", None, None, "B"])
53+
self.assertValid([None, None])
54+
self.assertInvalid(["A", None, 1])
55+
self.assertInvalid([None, "A", None, 1])
56+
57+
# test keys
58+
self.assertValid("value", "key")
59+
self.assertInvalid("value", "")
60+
self.assertInvalid("value", None)
8261

8362

8463
class TestBoundedAttributes(unittest.TestCase):

opentelemetry-sdk/src/opentelemetry/sdk/environment_variables/__init__.py

+25
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,22 @@
9898
Default: 512
9999
"""
100100

101+
OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT = "OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT"
102+
"""
103+
.. envvar:: OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT
104+
105+
The :envvar:`OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT` represents the maximum allowed event attribute count.
106+
Default: 128
107+
"""
108+
109+
OTEL_LINK_ATTRIBUTE_COUNT_LIMIT = "OTEL_LINK_ATTRIBUTE_COUNT_LIMIT"
110+
"""
111+
.. envvar:: OTEL_LINK_ATTRIBUTE_COUNT_LIMIT
112+
113+
The :envvar:`OTEL_LINK_ATTRIBUTE_COUNT_LIMIT` represents the maximum allowed link attribute count.
114+
Default: 128
115+
"""
116+
101117
OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = "OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT"
102118
"""
103119
.. envvar:: OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT
@@ -106,6 +122,15 @@
106122
Default: 128
107123
"""
108124

125+
OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = (
126+
"OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT"
127+
)
128+
"""
129+
.. envvar:: OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT
130+
131+
The :envvar:`OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT` represents the maximum allowed length attribute values can have.
132+
"""
133+
109134
OTEL_SPAN_EVENT_COUNT_LIMIT = "OTEL_SPAN_EVENT_COUNT_LIMIT"
110135
"""
111136
.. envvar:: OTEL_SPAN_EVENT_COUNT_LIMIT

0 commit comments

Comments
 (0)