diff --git a/CHANGELOG.md b/CHANGELOG.md index acc817a9d59..306004537d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Attributes for `Link` and `Resource` are immutable as they are for `Event`, which means any attempt to modify attributes directly will result in a `TypeError` exception. ([#1909](https://github.com/open-telemetry/opentelemetry-python/pull/1909)) +- Added `BoundedAttributes` to the API to make it available for `Link` which is defined in the + API. Marked `BoundedDict` in the SDK as deprecated as a result. + ([#1915](https://github.com/open-telemetry/opentelemetry-python/pull/1915)) ## [1.3.0-0.22b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.3.0-0.22b0) - 2021-06-01 diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 6875f56631f..5236627d030 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -14,8 +14,11 @@ # type: ignore import logging +import threading +from collections import OrderedDict +from collections.abc import MutableMapping from types import MappingProxyType -from typing import MutableSequence, Sequence +from typing import MutableSequence, Optional, Sequence from opentelemetry.util import types @@ -104,7 +107,72 @@ def _filter_attributes(attributes: types.Attributes) -> None: attributes.pop(attr_key) -def _create_immutable_attributes( - attributes: types.Attributes, -) -> types.Attributes: - return MappingProxyType(attributes.copy() if attributes else {}) +_DEFAULT_LIMIT = 128 + + +class BoundedAttributes(MutableMapping): + """An ordered dict with a fixed max capacity. + + Oldest elements are dropped when the dict is full and a new element is + added. + """ + + def __init__( + self, + maxlen: Optional[int] = _DEFAULT_LIMIT, + attributes: types.Attributes = None, + immutable: bool = True, + ): + if maxlen is not None: + if not isinstance(maxlen, int) or maxlen < 0: + raise ValueError( + "maxlen must be valid int greater or equal to 0" + ) + self.maxlen = maxlen + self.dropped = 0 + self._dict = OrderedDict() # type: OrderedDict + self._lock = threading.Lock() # type: threading.Lock + if attributes: + _filter_attributes(attributes) + for key, value in attributes.items(): + self[key] = value + self._immutable = immutable + + def __repr__(self): + return "{}({}, maxlen={})".format( + type(self).__name__, dict(self._dict), self.maxlen + ) + + def __getitem__(self, key): + return self._dict[key] + + def __setitem__(self, key, value): + if getattr(self, "_immutable", False): + raise TypeError + with self._lock: + if self.maxlen is not None and self.maxlen == 0: + self.dropped += 1 + return + + if key in self._dict: + del self._dict[key] + elif self.maxlen is not None and len(self._dict) == self.maxlen: + del self._dict[next(iter(self._dict.keys()))] + self.dropped += 1 + self._dict[key] = value + + def __delitem__(self, key): + if getattr(self, "_immutable", False): + raise TypeError + with self._lock: + del self._dict[key] + + def __iter__(self): + with self._lock: + return iter(self._dict.copy()) + + def __len__(self): + return len(self._dict) + + def copy(self): + return self._dict.copy() diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 37d19a0950e..487592f60d1 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -82,9 +82,7 @@ from typing import Iterator, Optional, Sequence, cast from opentelemetry import context as context_api -from opentelemetry.attributes import ( # type: ignore - _create_immutable_attributes, -) +from opentelemetry.attributes import BoundedAttributes # type: ignore from opentelemetry.context.context import Context from opentelemetry.environment_variables import OTEL_PYTHON_TRACER_PROVIDER from opentelemetry.trace.propagation import ( @@ -142,8 +140,8 @@ def __init__( attributes: types.Attributes = None, ) -> None: super().__init__(context) - self._attributes = _create_immutable_attributes( - attributes + self._attributes = BoundedAttributes( + attributes=attributes ) # type: types.Attributes @property diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index 2a391f78af7..c1151bf4d41 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -14,10 +14,11 @@ # type: ignore +import collections import unittest from opentelemetry.attributes import ( - _create_immutable_attributes, + BoundedAttributes, _filter_attributes, _is_valid_attribute_value, ) @@ -79,9 +80,95 @@ def test_filter_attributes(self): }, ) - def test_create_immutable_attributes(self): - attrs = {"key": "value", "pi": 3.14} - immutable = _create_immutable_attributes(attrs) - # TypeError: 'mappingproxy' object does not support item assignment + +class TestBoundedAttributes(unittest.TestCase): + base = collections.OrderedDict( + [ + ("name", "Firulais"), + ("age", 7), + ("weight", 13), + ("vaccinated", True), + ] + ) + + def test_negative_maxlen(self): + with self.assertRaises(ValueError): + BoundedAttributes(-1) + + def test_from_map(self): + dic_len = len(self.base) + base_copy = collections.OrderedDict(self.base) + bdict = BoundedAttributes(dic_len, base_copy) + + self.assertEqual(len(bdict), dic_len) + + # modify base_copy and test that bdict is not changed + base_copy["name"] = "Bruno" + base_copy["age"] = 3 + + for key in self.base: + self.assertEqual(bdict[key], self.base[key]) + + # test that iter yields the correct number of elements + self.assertEqual(len(tuple(bdict)), dic_len) + + # map too big + half_len = dic_len // 2 + bdict = BoundedAttributes(half_len, self.base) + self.assertEqual(len(tuple(bdict)), half_len) + self.assertEqual(bdict.dropped, dic_len - half_len) + + def test_bounded_dict(self): + # create empty dict + dic_len = len(self.base) + bdict = BoundedAttributes(dic_len, immutable=False) + self.assertEqual(len(bdict), 0) + + # fill dict + for key in self.base: + bdict[key] = self.base[key] + + self.assertEqual(len(bdict), dic_len) + self.assertEqual(bdict.dropped, 0) + + for key in self.base: + self.assertEqual(bdict[key], self.base[key]) + + # test __iter__ in BoundedAttributes + for key in bdict: + self.assertEqual(bdict[key], self.base[key]) + + # updating an existing element should not drop + bdict["name"] = "Bruno" + self.assertEqual(bdict.dropped, 0) + + # try to append more elements + for key in self.base: + bdict["new-" + key] = self.base[key] + + self.assertEqual(len(bdict), dic_len) + self.assertEqual(bdict.dropped, dic_len) + + # test that elements in the dict are the new ones + for key in self.base: + self.assertEqual(bdict["new-" + key], self.base[key]) + + # delete an element + del bdict["new-name"] + self.assertEqual(len(bdict), dic_len - 1) + + with self.assertRaises(KeyError): + _ = bdict["new-name"] + + def test_no_limit_code(self): + bdict = BoundedAttributes(maxlen=None, immutable=False) + for num in range(100): + bdict[num] = num + + for num in range(100): + self.assertEqual(bdict[num], num) + + def test_immutable(self): + bdict = BoundedAttributes() with self.assertRaises(TypeError): - immutable["pi"] = 1.34 + bdict["should-not-work"] = "dict immutable" diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index a4efe8d7a5b..3ad01b0aec2 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -64,10 +64,7 @@ import pkg_resources -from opentelemetry.attributes import ( - _create_immutable_attributes, - _filter_attributes, -) +from opentelemetry.attributes import BoundedAttributes from opentelemetry.sdk.environment_variables import ( OTEL_RESOURCE_ATTRIBUTES, OTEL_SERVICE_NAME, @@ -147,8 +144,7 @@ class Resource: def __init__( self, attributes: Attributes, schema_url: typing.Optional[str] = None ): - _filter_attributes(attributes) - self._attributes = _create_immutable_attributes(attributes) + self._attributes = BoundedAttributes(attributes=attributes) if schema_url is None: schema_url = "" self._schema_url = schema_url diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index aa172a6a83e..26e12c12b90 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -40,8 +40,7 @@ from opentelemetry import context as context_api from opentelemetry import trace as trace_api from opentelemetry.attributes import ( - _create_immutable_attributes, - _filter_attributes, + BoundedAttributes, _is_valid_attribute_value, ) from opentelemetry.sdk import util @@ -53,7 +52,7 @@ from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.trace import sampling from opentelemetry.sdk.trace.id_generator import IdGenerator, RandomIdGenerator -from opentelemetry.sdk.util import BoundedDict, BoundedList +from opentelemetry.sdk.util import BoundedList from opentelemetry.sdk.util.instrumentation import InstrumentationInfo from opentelemetry.trace import SpanContext from opentelemetry.trace.status import Status, StatusCode @@ -65,6 +64,8 @@ _DEFAULT_OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = 128 _DEFAULT_OTEL_SPAN_EVENT_COUNT_LIMIT = 128 _DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT = 128 +_DEFAULT_OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT = 128 +_DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT = 128 _ENV_VALUE_UNSET = "unset" @@ -475,7 +476,7 @@ def _format_context(context): @staticmethod def _format_attributes(attributes): - if isinstance(attributes, BoundedDict): + if isinstance(attributes, BoundedAttributes): return attributes._dict # pylint: disable=protected-access if isinstance(attributes, MappingProxyType): return attributes.copy() @@ -526,19 +527,21 @@ class SpanLimits: max_links: Maximum number of links that can be added to a Span. Environment variable: OTEL_SPAN_LINK_COUNT_LIMIT Default: {_DEFAULT_SPAN_LINK_COUNT_LIMIT} + max_event_attributes: Maximum number of attributes that can be added to an Event. + Default: {_DEFAULT_OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT} + max_link_attributes: Maximum number of attributes that can be added to a Link. + Default: {_DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT} """ UNSET = -1 - max_attributes: int - max_events: int - max_links: int - def __init__( self, max_attributes: Optional[int] = None, max_events: Optional[int] = None, max_links: Optional[int] = None, + max_event_attributes: Optional[int] = None, + max_link_attributes: Optional[int] = None, ): self.max_attributes = self._from_env_if_absent( max_attributes, @@ -555,10 +558,25 @@ def __init__( OTEL_SPAN_LINK_COUNT_LIMIT, _DEFAULT_OTEL_SPAN_LINK_COUNT_LIMIT, ) + self.max_event_attributes = self._from_env_if_absent( + max_event_attributes, + OTEL_SPAN_LINK_COUNT_LIMIT, + _DEFAULT_OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT, + ) + self.max_link_attributes = self._from_env_if_absent( + max_link_attributes, + OTEL_SPAN_LINK_COUNT_LIMIT, + _DEFAULT_OTEL_LINK_ATTRIBUTE_COUNT_LIMIT, + ) def __repr__(self): - return "max_attributes={}, max_events={}, max_links={}".format( - self.max_attributes, self.max_events, self.max_links + return "{}(max_attributes={}, max_events={}, max_links={}, max_event_attributes={}, max_link_attributes={})".format( + type(self).__name__, + self.max_attributes, + self.max_events, + self.max_links, + self.max_event_attributes, + self.max_link_attributes, ) @classmethod @@ -591,6 +609,8 @@ def _from_env_if_absent( max_attributes=SpanLimits.UNSET, max_events=SpanLimits.UNSET, max_links=SpanLimits.UNSET, + max_event_attributes=SpanLimits.UNSET, + max_link_attributes=SpanLimits.UNSET, ) SPAN_ATTRIBUTE_COUNT_LIMIT = SpanLimits._from_env_if_absent( @@ -661,22 +681,14 @@ def __init__( self._span_processor = span_processor self._limits = limits self._lock = threading.Lock() - - _filter_attributes(attributes) - if not attributes: - self._attributes = self._new_attributes() - else: - self._attributes = BoundedDict.from_map( - self._limits.max_attributes, attributes - ) - + self._attributes = BoundedAttributes( + self._limits.max_attributes, attributes, immutable=False + ) self._events = self._new_events() if events: for event in events: - _filter_attributes(event.attributes) - # pylint: disable=protected-access - event._attributes = _create_immutable_attributes( - event.attributes + event._attributes = BoundedAttributes( + self._limits.max_event_attributes, event.attributes ) self._events.append(event) @@ -690,9 +702,6 @@ def __repr__(self): type(self).__name__, self._name, self._context ) - def _new_attributes(self): - return BoundedDict(self._limits.max_attributes) - def _new_events(self): return BoundedList(self._limits.max_events) @@ -745,13 +754,14 @@ def add_event( attributes: types.Attributes = None, timestamp: Optional[int] = None, ) -> None: - _filter_attributes(attributes) - attributes = _create_immutable_attributes(attributes) + attributes = BoundedAttributes( + self._limits.max_event_attributes, attributes + ) self._add_event( Event( name=name, attributes=attributes, - timestamp=_time_ns() if timestamp is None else timestamp, + timestamp=timestamp, ) ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py index 746f100277d..a94fe647b76 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py @@ -18,6 +18,8 @@ from collections.abc import MutableMapping, Sequence from typing import Optional +from deprecated import deprecated + def ns_to_iso_str(nanoseconds): """Get an ISO 8601 string from time_ns value.""" @@ -91,6 +93,7 @@ def from_seq(cls, maxlen, seq): return bounded_list +@deprecated(version="1.4.0") # type: ignore class BoundedDict(MutableMapping): """An ordered dict with a fixed max capacity. diff --git a/opentelemetry-sdk/tests/test_util.py b/opentelemetry-sdk/tests/test_util.py index f90576afe70..00099090cdc 100644 --- a/opentelemetry-sdk/tests/test_util.py +++ b/opentelemetry-sdk/tests/test_util.py @@ -12,10 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -import collections import unittest -from opentelemetry.sdk.util import BoundedDict, BoundedList +from opentelemetry.sdk.util import BoundedList class TestBoundedList(unittest.TestCase): @@ -142,91 +141,3 @@ def test_no_limit(self): for num in range(100): self.assertEqual(blist[num], num) - - -class TestBoundedDict(unittest.TestCase): - base = collections.OrderedDict( - [ - ("name", "Firulais"), - ("age", 7), - ("weight", 13), - ("vaccinated", True), - ] - ) - - def test_negative_maxlen(self): - with self.assertRaises(ValueError): - BoundedDict(-1) - - def test_from_map(self): - dic_len = len(self.base) - base_copy = collections.OrderedDict(self.base) - bdict = BoundedDict.from_map(dic_len, base_copy) - - self.assertEqual(len(bdict), dic_len) - - # modify base_copy and test that bdict is not changed - base_copy["name"] = "Bruno" - base_copy["age"] = 3 - - for key in self.base: - self.assertEqual(bdict[key], self.base[key]) - - # test that iter yields the correct number of elements - self.assertEqual(len(tuple(bdict)), dic_len) - - # map too big - half_len = dic_len // 2 - bdict = BoundedDict.from_map(half_len, self.base) - self.assertEqual(len(tuple(bdict)), half_len) - self.assertEqual(bdict.dropped, dic_len - half_len) - - def test_bounded_dict(self): - # create empty dict - dic_len = len(self.base) - bdict = BoundedDict(dic_len) - self.assertEqual(len(bdict), 0) - - # fill dict - for key in self.base: - bdict[key] = self.base[key] - - self.assertEqual(len(bdict), dic_len) - self.assertEqual(bdict.dropped, 0) - - for key in self.base: - self.assertEqual(bdict[key], self.base[key]) - - # test __iter__ in BoundedDict - for key in bdict: - self.assertEqual(bdict[key], self.base[key]) - - # updating an existing element should not drop - bdict["name"] = "Bruno" - self.assertEqual(bdict.dropped, 0) - - # try to append more elements - for key in self.base: - bdict["new-" + key] = self.base[key] - - self.assertEqual(len(bdict), dic_len) - self.assertEqual(bdict.dropped, dic_len) - - # test that elements in the dict are the new ones - for key in self.base: - self.assertEqual(bdict["new-" + key], self.base[key]) - - # delete an element - del bdict["new-name"] - self.assertEqual(len(bdict), dic_len - 1) - - with self.assertRaises(KeyError): - _ = bdict["new-name"] - - def test_no_limit_code(self): - bdict = BoundedDict(maxlen=None) - for num in range(100): - bdict[num] = num - - for num in range(100): - self.assertEqual(bdict[num], num)