Skip to content

Commit

Permalink
bugfix: deep copy empty attributes (#714)
Browse files Browse the repository at this point in the history
Addresses #713. Previously it was possible for a user (acting against the api) to mutate a default variable.
  • Loading branch information
Andrew Xue authored May 27, 2020
1 parent 64b3cf2 commit 8a946b5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
2 changes: 2 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 18 additions & 18 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down

0 comments on commit 8a946b5

Please sign in to comment.