Skip to content

Commit 4547c02

Browse files
XinRanZhAWSADOT Patch workflow
andauthored
Fix deep copy related issue (#46)
*Description of changes:* Remove deepcopy for span to avoid issue By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --------- Co-authored-by: ADOT Patch workflow <[email protected]>
1 parent 8b32ac8 commit 4547c02

File tree

2 files changed

+11
-78
lines changed

2 files changed

+11
-78
lines changed

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/aws_metric_attributes_span_exporter.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
# SPDX-License-Identifier: Apache-2.0
3-
import copy
43
from typing import List, Sequence, TypeVar
54

65
from typing_extensions import override
@@ -103,11 +102,14 @@ def copy_attributes_with_local_root(attributes: BoundedAttributes) -> BoundedAtt
103102
)
104103

105104

105+
# TODO: AwsMetricAttributesSpanExporter depends on internal ReadableSpan method _attributes.
106+
# This is a bit risky but is required for our implementation.
107+
# The risk is that the implementation of _attributes changes in the future.
108+
# We need tests that thoroughly test this behaviour to make sure it does not change upstream.
106109
def wrap_span_with_attributes(span: ReadableSpan, attributes: BoundedAttributes) -> ReadableSpan:
107110
# To make sure we create a new span without influence original span's Attributes
108111
# We have to create a deepcopy for it
109-
new_span = copy.deepcopy(span)
110-
original_attributes: AttributesT = new_span.attributes
112+
original_attributes: AttributesT = span.attributes
111113
update_attributes: types.Attributes = {}
112114
# Copy all attribute in span into update_attributes
113115
for key, value in original_attributes.items():
@@ -117,12 +119,12 @@ def wrap_span_with_attributes(span: ReadableSpan, attributes: BoundedAttributes)
117119
update_attributes[key] = value
118120

119121
if isinstance(original_attributes, BoundedAttributes):
120-
new_span._attributes = BoundedAttributes(
122+
span._attributes = BoundedAttributes(
121123
maxlen=original_attributes.maxlen,
122124
attributes=update_attributes,
123125
immutable=original_attributes._immutable,
124126
max_value_len=original_attributes.max_value_len,
125127
)
126128
else:
127-
new_span._attributes = update_attributes
128-
return new_span
129+
span._attributes = update_attributes
130+
return span

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/test_aws_metric_attributes_span_exporter.py

Lines changed: 3 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
# SPDX-License-Identifier: Apache-2.0
33
import copy
4-
from typing import Any
54
from unittest import TestCase
6-
from unittest.mock import MagicMock, Mock, call
5+
from unittest.mock import MagicMock, call
76

87
from amazon.opentelemetry.distro._aws_attribute_keys import AWS_CONSUMER_PARENT_SPAN_KIND, AWS_SPAN_KIND
98
from amazon.opentelemetry.distro._aws_metric_attribute_generator import _AwsMetricAttributeGenerator
@@ -16,11 +15,10 @@
1615
from amazon.opentelemetry.distro.metric_attribute_generator import DEPENDENCY_METRIC, SERVICE_METRIC
1716
from opentelemetry.attributes import BoundedAttributes
1817
from opentelemetry.sdk.resources import Resource
19-
from opentelemetry.sdk.trace import Event, ReadableSpan
18+
from opentelemetry.sdk.trace import ReadableSpan
2019
from opentelemetry.sdk.trace.export import SpanExporter
21-
from opentelemetry.sdk.util.instrumentation import InstrumentationScope
2220
from opentelemetry.semconv.trace import MessagingOperationValues, SpanAttributes
23-
from opentelemetry.trace import Link, SpanContext, SpanKind, Status
21+
from opentelemetry.trace import SpanContext, SpanKind
2422
from opentelemetry.util.types import Attributes
2523

2624
_CONTAINS_ATTRIBUTES: bool = True
@@ -158,54 +156,6 @@ def test_overridden_attributes(self):
158156
self.assertEqual(exported_span._attributes["key2"], "old value2")
159157
self.assertEqual(exported_span._attributes["key3"], "new value3")
160158

161-
def test_export_delegating_span_data_behaviour(self):
162-
span_attributes: Attributes = self._build_span_attributes(_CONTAINS_ATTRIBUTES)
163-
span_data_mock: ReadableSpan = self._build_readable_span_mock_without_deepcopy_support(span_attributes)
164-
metric_attributes: Attributes = self._build_metric_attributes(_CONTAINS_ATTRIBUTES)
165-
self._configure_mock_for_export(span_data_mock, metric_attributes)
166-
167-
self.aws_metric_attributes_span_exporter.export([span_data_mock])
168-
self.delegate_mock.assert_has_calls([call.export([span_data_mock])])
169-
exported_spans: Attributes = self.delegate_mock.export.call_args[0][0]
170-
self.assertEqual(len(exported_spans), 1)
171-
172-
exported_span: ReadableSpan = exported_spans[0]
173-
174-
span_context_mock: SpanContext = MagicMock()
175-
span_data_mock.get_span_context.return_value = span_context_mock
176-
self.assertEqual(exported_span.get_span_context(), span_context_mock)
177-
178-
parent_span_context_mock: SpanContext = MagicMock()
179-
span_data_mock._parent = parent_span_context_mock
180-
self.assertEqual(exported_span._parent, parent_span_context_mock)
181-
182-
span_data_mock.set_attribute("_resource", self.test_resource)
183-
self.assertEqual(exported_span._resource, self.test_resource)
184-
185-
test_instrumentation_scope_info: InstrumentationScope = MagicMock()
186-
span_data_mock.set_attribute("_instrumentation_scope", test_instrumentation_scope_info)
187-
self.assertEqual(exported_span._instrumentation_scope, test_instrumentation_scope_info)
188-
189-
test_name: str = "name"
190-
span_data_mock.set_attribute("_name", test_name)
191-
self.assertEqual(exported_span._name, test_name)
192-
193-
kind_mock: SpanKind = Mock()
194-
span_data_mock.set_attribute("_kind", kind_mock)
195-
self.assertEqual(exported_span._kind, kind_mock)
196-
197-
events_mock: [Event] = [Mock()]
198-
span_data_mock.set_attribute("_events", events_mock)
199-
self.assertEqual(exported_span._events, events_mock)
200-
201-
links_mock: [Link] = [Mock()]
202-
span_data_mock.set_attribute("_links", links_mock)
203-
self.assertEqual(exported_span._links, links_mock)
204-
205-
status_mock: Status = Mock()
206-
span_data_mock.set_attribute("_status", status_mock)
207-
self.assertEqual(exported_span._status, status_mock)
208-
209159
def test_export_delegation_with_two_metrics(self):
210160
span_attributes: Attributes = self._build_span_attributes(_CONTAINS_ATTRIBUTES)
211161

@@ -375,22 +325,3 @@ def _build_readable_span_mock(self, span_attributes: Attributes) -> ReadableSpan
375325
mock_span_data._parent = None
376326
mock_span_data.attributes = mock_span_data._attributes
377327
return mock_span_data
378-
379-
def _build_readable_span_mock_without_deepcopy_support(self, span_attributes: Attributes) -> ReadableSpan:
380-
class NoDeepCopyMock(MagicMock):
381-
def __init__(self, *args: Any, **kw: Any):
382-
super().__init__(*args, **kw)
383-
self._attributes = span_attributes
384-
self._kind = SpanKind.SERVER
385-
self._parent = None
386-
self.attributes = self._attributes
387-
388-
def set_attribute(self, name, value):
389-
setattr(self, name, value)
390-
391-
def __deepcopy__(self, memo):
392-
return self
393-
394-
mock_span_data: ReadableSpan = NoDeepCopyMock()
395-
396-
return mock_span_data

0 commit comments

Comments
 (0)