From e7550e74b9bbbbaee396adfed91c73c597c6b65f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 16 Jun 2021 14:18:49 -0700 Subject: [PATCH 01/11] use BoundedDict for attributes in link, event, resource, spans --- .../src/opentelemetry/attributes/__init__.py | 75 ++++++++++++++- .../src/opentelemetry/trace/__init__.py | 8 +- .../tests/attributes/test_attributes.py | 95 +++++++++++++++++++ .../opentelemetry/sdk/resources/__init__.py | 8 +- .../src/opentelemetry/sdk/trace/__init__.py | 56 ++++++----- .../src/opentelemetry/sdk/util/__init__.py | 3 + opentelemetry-sdk/tests/test_util.py | 91 +----------------- 7 files changed, 209 insertions(+), 127 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 6875f56631f..0be3b8cad24 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 @@ -108,3 +111,73 @@ def _create_immutable_attributes( attributes: types.Attributes, ) -> types.Attributes: return MappingProxyType(attributes.copy() if attributes else {}) + + +_DEFAULT_LIMIT = 128 + + +class BoundedDict(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): + raise ValueError + if maxlen < 0: + raise ValueError + 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 + 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..1d4a86475d3 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 BoundedDict 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 = BoundedDict( + 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..fb68a450264 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -14,9 +14,11 @@ # type: ignore +import collections import unittest from opentelemetry.attributes import ( + BoundedDict, _create_immutable_attributes, _filter_attributes, _is_valid_attribute_value, @@ -85,3 +87,96 @@ def test_create_immutable_attributes(self): # TypeError: 'mappingproxy' object does not support item assignment with self.assertRaises(TypeError): immutable["pi"] = 1.34 + + +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(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(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, 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 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, 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 = BoundedDict() + with self.assertRaises(TypeError): + 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..ae66b2daf35 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 BoundedDict 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 = BoundedDict(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..8d58d162222 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -39,11 +39,7 @@ from opentelemetry import context as context_api from opentelemetry import trace as trace_api -from opentelemetry.attributes import ( - _create_immutable_attributes, - _filter_attributes, - _is_valid_attribute_value, -) +from opentelemetry.attributes import BoundedDict, _is_valid_attribute_value from opentelemetry.sdk import util from opentelemetry.sdk.environment_variables import ( OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, @@ -53,7 +49,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 +61,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" @@ -526,6 +524,10 @@ 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 @@ -533,12 +535,16 @@ class SpanLimits: max_attributes: int max_events: int max_links: int + max_event_attributes: int + max_link_attributes: 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,6 +561,16 @@ 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( @@ -591,6 +607,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 +679,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 = BoundedDict( + 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 = BoundedDict( + self._limits.max_event_attributes, event.attributes ) self._events.append(event) @@ -690,9 +700,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 +752,12 @@ def add_event( attributes: types.Attributes = None, timestamp: Optional[int] = None, ) -> None: - _filter_attributes(attributes) - attributes = _create_immutable_attributes(attributes) + attributes = BoundedDict(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) From f237baea52ef5f00913fe312820a7b6c29f5ccdd Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 16 Jun 2021 14:23:16 -0700 Subject: [PATCH 02/11] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a3872dcf1d..1ff5ad76fc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,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 `BoundedDict` 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 From c00d04a4053e4a4b94be0a48cd963ddc64fb6f3f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 08:39:07 -0700 Subject: [PATCH 03/11] fix mypy --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 1d4a86475d3..0762e04b335 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -82,7 +82,7 @@ from typing import Iterator, Optional, Sequence, cast from opentelemetry import context as context_api -from opentelemetry.attributes import BoundedDict +from opentelemetry.attributes import BoundedDict # type: ignore from opentelemetry.context.context import Context from opentelemetry.environment_variables import OTEL_PYTHON_TRACER_PROVIDER from opentelemetry.trace.propagation import ( From 9bd9596ef2da60fa846b09d37e26bdd2dc3a3e9a Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 08:57:11 -0700 Subject: [PATCH 04/11] fix lint --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 0762e04b335..06260467902 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -82,7 +82,7 @@ from typing import Iterator, Optional, Sequence, cast from opentelemetry import context as context_api -from opentelemetry.attributes import BoundedDict # type: ignore +from opentelemetry.attributes import BoundedDict # type: ignore from opentelemetry.context.context import Context from opentelemetry.environment_variables import OTEL_PYTHON_TRACER_PROVIDER from opentelemetry.trace.propagation import ( From 736500da3d5f38e4868b30b73b59c7c6f36f2209 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 09:39:16 -0700 Subject: [PATCH 05/11] removing unused vars --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 8d58d162222..5b845f10fd3 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -532,12 +532,6 @@ class SpanLimits: UNSET = -1 - max_attributes: int - max_events: int - max_links: int - max_event_attributes: int - max_link_attributes: int - def __init__( self, max_attributes: Optional[int] = None, From c9662ed9984bd935e4999dfb61866b054f9fa004 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 09:44:10 -0700 Subject: [PATCH 06/11] removing _create_immutable_attributes --- .../src/opentelemetry/attributes/__init__.py | 6 ------ opentelemetry-api/tests/attributes/test_attributes.py | 8 -------- 2 files changed, 14 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 0be3b8cad24..8875fcafdbb 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -107,12 +107,6 @@ 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 diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index fb68a450264..a026bd4d82b 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -19,7 +19,6 @@ from opentelemetry.attributes import ( BoundedDict, - _create_immutable_attributes, _filter_attributes, _is_valid_attribute_value, ) @@ -81,13 +80,6 @@ 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 - with self.assertRaises(TypeError): - immutable["pi"] = 1.34 - class TestBoundedDict(unittest.TestCase): base = collections.OrderedDict( From 943b0786a44597bcba23c5e013ec19c5e8dde979 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 13:11:22 -0700 Subject: [PATCH 07/11] add lock to delete --- opentelemetry-api/src/opentelemetry/attributes/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 8875fcafdbb..70a09adc774 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -164,7 +164,8 @@ def __setitem__(self, key, value): def __delitem__(self, key): if getattr(self, "_immutable", False): raise TypeError - del self._dict[key] + with self._lock: + del self._dict[key] def __iter__(self): with self._lock: From f05094d2e1e5e1b8ba4f511677c52d9176dd4805 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 13:23:19 -0700 Subject: [PATCH 08/11] add value error message --- .../src/opentelemetry/attributes/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 70a09adc774..4fee012b771 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -124,10 +124,10 @@ def __init__( immutable: bool = True, ): if maxlen is not None: - if not isinstance(maxlen, int): - raise ValueError - if maxlen < 0: - raise ValueError + 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 From f29c775c3e60f79d4d9659ee88ca5b30f40a2f66 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 21 Jun 2021 13:23:28 -0700 Subject: [PATCH 09/11] fix __repr__ --- .../src/opentelemetry/sdk/trace/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 5b845f10fd3..f89280c0bf9 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -567,8 +567,13 @@ def __init__( ) 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 From 8a84a0aacc70ab0c4ed6576dd4347142e7d43a83 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 24 Jun 2021 12:41:00 -0700 Subject: [PATCH 10/11] rename BoundedDict to BoundedAttributes --- CHANGELOG.md | 2 +- .../src/opentelemetry/attributes/__init__.py | 2 +- .../src/opentelemetry/trace/__init__.py | 4 ++-- .../tests/attributes/test_attributes.py | 18 +++++++++--------- .../opentelemetry/sdk/resources/__init__.py | 4 ++-- .../src/opentelemetry/sdk/trace/__init__.py | 10 +++++----- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55f58e6406b..306004537d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ 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 `BoundedDict` to the API to make it available for `Link` which is defined in the +- 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)) diff --git a/opentelemetry-api/src/opentelemetry/attributes/__init__.py b/opentelemetry-api/src/opentelemetry/attributes/__init__.py index 4fee012b771..5236627d030 100644 --- a/opentelemetry-api/src/opentelemetry/attributes/__init__.py +++ b/opentelemetry-api/src/opentelemetry/attributes/__init__.py @@ -110,7 +110,7 @@ def _filter_attributes(attributes: types.Attributes) -> None: _DEFAULT_LIMIT = 128 -class BoundedDict(MutableMapping): +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 diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 06260467902..487592f60d1 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -82,7 +82,7 @@ from typing import Iterator, Optional, Sequence, cast from opentelemetry import context as context_api -from opentelemetry.attributes import BoundedDict # type: ignore +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 ( @@ -140,7 +140,7 @@ def __init__( attributes: types.Attributes = None, ) -> None: super().__init__(context) - self._attributes = BoundedDict( + self._attributes = BoundedAttributes( attributes=attributes ) # type: types.Attributes diff --git a/opentelemetry-api/tests/attributes/test_attributes.py b/opentelemetry-api/tests/attributes/test_attributes.py index a026bd4d82b..c1151bf4d41 100644 --- a/opentelemetry-api/tests/attributes/test_attributes.py +++ b/opentelemetry-api/tests/attributes/test_attributes.py @@ -18,7 +18,7 @@ import unittest from opentelemetry.attributes import ( - BoundedDict, + BoundedAttributes, _filter_attributes, _is_valid_attribute_value, ) @@ -81,7 +81,7 @@ def test_filter_attributes(self): ) -class TestBoundedDict(unittest.TestCase): +class TestBoundedAttributes(unittest.TestCase): base = collections.OrderedDict( [ ("name", "Firulais"), @@ -93,12 +93,12 @@ class TestBoundedDict(unittest.TestCase): def test_negative_maxlen(self): with self.assertRaises(ValueError): - BoundedDict(-1) + BoundedAttributes(-1) def test_from_map(self): dic_len = len(self.base) base_copy = collections.OrderedDict(self.base) - bdict = BoundedDict(dic_len, base_copy) + bdict = BoundedAttributes(dic_len, base_copy) self.assertEqual(len(bdict), dic_len) @@ -114,14 +114,14 @@ def test_from_map(self): # map too big half_len = dic_len // 2 - bdict = BoundedDict(half_len, self.base) + 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 = BoundedDict(dic_len, immutable=False) + bdict = BoundedAttributes(dic_len, immutable=False) self.assertEqual(len(bdict), 0) # fill dict @@ -134,7 +134,7 @@ def test_bounded_dict(self): for key in self.base: self.assertEqual(bdict[key], self.base[key]) - # test __iter__ in BoundedDict + # test __iter__ in BoundedAttributes for key in bdict: self.assertEqual(bdict[key], self.base[key]) @@ -161,7 +161,7 @@ def test_bounded_dict(self): _ = bdict["new-name"] def test_no_limit_code(self): - bdict = BoundedDict(maxlen=None, immutable=False) + bdict = BoundedAttributes(maxlen=None, immutable=False) for num in range(100): bdict[num] = num @@ -169,6 +169,6 @@ def test_no_limit_code(self): self.assertEqual(bdict[num], num) def test_immutable(self): - bdict = BoundedDict() + bdict = BoundedAttributes() with self.assertRaises(TypeError): 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 ae66b2daf35..3ad01b0aec2 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -64,7 +64,7 @@ import pkg_resources -from opentelemetry.attributes import BoundedDict +from opentelemetry.attributes import BoundedAttributes from opentelemetry.sdk.environment_variables import ( OTEL_RESOURCE_ATTRIBUTES, OTEL_SERVICE_NAME, @@ -144,7 +144,7 @@ class Resource: def __init__( self, attributes: Attributes, schema_url: typing.Optional[str] = None ): - self._attributes = BoundedDict(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 f89280c0bf9..0ab55b1540e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -39,7 +39,7 @@ from opentelemetry import context as context_api from opentelemetry import trace as trace_api -from opentelemetry.attributes import BoundedDict, _is_valid_attribute_value +from opentelemetry.attributes import BoundedAttributes, _is_valid_attribute_value from opentelemetry.sdk import util from opentelemetry.sdk.environment_variables import ( OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, @@ -473,7 +473,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() @@ -678,13 +678,13 @@ def __init__( self._span_processor = span_processor self._limits = limits self._lock = threading.Lock() - self._attributes = BoundedDict( + self._attributes = BoundedAttributes( self._limits.max_attributes, attributes, immutable=False ) self._events = self._new_events() if events: for event in events: - event._attributes = BoundedDict( + event._attributes = BoundedAttributes( self._limits.max_event_attributes, event.attributes ) self._events.append(event) @@ -751,7 +751,7 @@ def add_event( attributes: types.Attributes = None, timestamp: Optional[int] = None, ) -> None: - attributes = BoundedDict(self._limits.max_event_attributes, attributes) + attributes = BoundedAttributes(self._limits.max_event_attributes, attributes) self._add_event( Event( name=name, From 1b98aa9329bf5f8687ab15c0d3c68ce388ba9624 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 24 Jun 2021 12:42:28 -0700 Subject: [PATCH 11/11] fix lint --- .../src/opentelemetry/sdk/trace/__init__.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 0ab55b1540e..26e12c12b90 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -39,7 +39,10 @@ from opentelemetry import context as context_api from opentelemetry import trace as trace_api -from opentelemetry.attributes import BoundedAttributes, _is_valid_attribute_value +from opentelemetry.attributes import ( + BoundedAttributes, + _is_valid_attribute_value, +) from opentelemetry.sdk import util from opentelemetry.sdk.environment_variables import ( OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT, @@ -751,7 +754,9 @@ def add_event( attributes: types.Attributes = None, timestamp: Optional[int] = None, ) -> None: - attributes = BoundedAttributes(self._limits.max_event_attributes, attributes) + attributes = BoundedAttributes( + self._limits.max_event_attributes, attributes + ) self._add_event( Event( name=name,