From 8a946b5e272ef803f134067d005556912a6d7d19 Mon Sep 17 00:00:00 2001 From: Andrew Xue Date: Wed, 27 May 2020 12:40:40 -0400 Subject: [PATCH] bugfix: deep copy empty attributes (#714) Addresses #713. Previously it was possible for a user (acting against the api) to mutate a default variable. --- opentelemetry-sdk/CHANGELOG.md | 2 ++ .../src/opentelemetry/sdk/trace/__init__.py | 36 +++++++++---------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index bece33dfca7..c4f09760d94 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -10,6 +10,8 @@ ([#724](https://github.com/open-telemetry/opentelemetry-python/pull/724)) - bugfix: Fix error message ([#729](https://github.com/open-telemetry/opentelemetry-python/pull/729)) +- deep copy empty attributes + ([#714](https://github.com/open-telemetry/opentelemetry-python/pull/714)) ## 0.7b1 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 66e13d9cc8d..20c50a849fd 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -252,11 +252,6 @@ class Span(trace_api.Span): this `Span`. """ - # Initialize these lazily assuming most spans won't have them. - _empty_attributes = BoundedDict(MAX_NUM_ATTRIBUTES) - _empty_events = BoundedList(MAX_NUM_EVENTS) - _empty_links = BoundedList(MAX_NUM_LINKS) - def __init__( self, name: str, @@ -289,22 +284,20 @@ def __init__( self._filter_attribute_values(attributes) if not attributes: - self.attributes = Span._empty_attributes + self.attributes = self._new_attributes() else: self.attributes = BoundedDict.from_map( MAX_NUM_ATTRIBUTES, attributes ) - if events is None: - self.events = Span._empty_events - else: - self.events = BoundedList(MAX_NUM_EVENTS) + self.events = self._new_events() + if events: for event in events: self._filter_attribute_values(event.attributes) self.events.append(event) if links is None: - self.links = Span._empty_links + self.links = self._new_links() else: self.links = BoundedList.from_seq(MAX_NUM_LINKS, links) @@ -325,6 +318,18 @@ def __repr__(self): type(self).__name__, self.name, self.context ) + @staticmethod + def _new_attributes(): + return BoundedDict(MAX_NUM_ATTRIBUTES) + + @staticmethod + def _new_events(): + return BoundedList(MAX_NUM_EVENTS) + + @staticmethod + def _new_links(): + return BoundedList(MAX_NUM_LINKS) + @staticmethod def _format_context(context): x_ctx = OrderedDict() @@ -407,9 +412,6 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None: if not self.is_recording_events(): return has_ended = self.end_time is not None - if not has_ended: - if self.attributes is Span._empty_attributes: - self.attributes = BoundedDict(MAX_NUM_ATTRIBUTES) if has_ended: logger.warning("Setting attribute on ended span.") return @@ -442,9 +444,7 @@ def _add_event(self, event: EventBase) -> None: if not self.is_recording_events(): return has_ended = self.end_time is not None - if not has_ended: - if self.events is Span._empty_events: - self.events = BoundedList(MAX_NUM_EVENTS) + if has_ended: logger.warning("Calling add_event() on an ended span.") return @@ -458,7 +458,7 @@ def add_event( ) -> None: self._filter_attribute_values(attributes) if not attributes: - attributes = Span._empty_attributes + attributes = self._new_attributes() self._add_event( Event( name=name,