Skip to content

Commit 533fafd

Browse files
authored
support code attributes for custom span (aws-observability#518)
*Description of changes:* Support code attributes for spans manually created, such as ``` @tracer.start_as_current_span("create-otel-span-by-decorator") def _otel_span_decorator(): pass def _otel_span_api_with(): """A demo function to illustrate OpenTelemetry span context manager.""" with tracer.start_as_current_span("create-otel-span-by-api"): pass def _otel_span_api_start_span(): span = tracer.start_span("create-otel-span-by-api") span.end() ``` By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
1 parent bc2d933 commit 533fafd

File tree

4 files changed

+173
-40
lines changed

4 files changed

+173
-40
lines changed

aws-opentelemetry-distro/src/amazon/opentelemetry/distro/code_correlation/code_attributes_span_processor.py

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from .utils import add_code_attributes_to_span_from_frame
2323

2424

25+
# pylint: disable=no-self-use
2526
class CodeAttributesSpanProcessor(SpanProcessor):
2627
"""
2728
A SpanProcessor that captures and attaches code attributes to spans.
@@ -68,26 +69,47 @@ def on_start(
6869
# Capture code attributes from stack trace
6970
self._capture_code_attributes(span)
7071

71-
@staticmethod
72-
def _should_process_span(span: Span) -> bool:
72+
def _should_process_span(self, span: Span) -> bool:
7373
"""
7474
Determine if span should be processed for code attributes.
7575
7676
Returns False if:
77+
- Span is library instrumentation SERVER or INTERNAL span
7778
- Span already has code attributes
78-
- Span is SERVER or INTERNAL span
79+
80+
Note: Library instrumentation CLIENT/PRODUCER/CONSUMER spans are still processed
81+
as they provide valuable context for tracing call chains.
7982
"""
80-
# Skip if span already has code attributes
83+
84+
if span.kind in (SpanKind.SERVER, SpanKind.INTERNAL) and self._is_library_instrumentation_span(span):
85+
return False
86+
8187
if span.attributes is not None and CODE_FUNCTION_NAME in span.attributes:
8288
return False
8389

84-
# Process spans except SERVER and INTERNAL spans
85-
return span.kind not in (SpanKind.SERVER, SpanKind.INTERNAL)
90+
return True
91+
92+
def _is_library_instrumentation_span(self, span: Span) -> bool:
93+
"""
94+
Check if span is created by library instrumentation.
95+
96+
Args:
97+
span: The span to check
98+
99+
Returns:
100+
True if span is from library instrumentation, False otherwise
101+
"""
102+
scope = span.instrumentation_scope
103+
104+
if scope is None or scope.name is None:
105+
return False # No scope info, assume user-created
106+
107+
return scope.name.startswith("opentelemetry.instrumentation")
86108

87109
def _capture_code_attributes(self, span: Span) -> None:
88110
"""Capture and attach code attributes from current stack trace."""
89111
try:
90-
current_frame = sys._getframe(1)
112+
current_frame = sys._getframe(1) # pylint: disable=protected-access
91113

92114
for frame_index, frame in enumerate(self._iter_stack_frames(current_frame)):
93115
if frame_index >= self.MAX_STACK_FRAMES:
@@ -112,6 +134,6 @@ def shutdown(self) -> None:
112134
"""Called when the processor is shutdown. No cleanup needed."""
113135
# No cleanup needed for code attributes processor
114136

115-
def force_flush(self, timeout_millis: int = 30000) -> bool: # pylint: disable=no-self-use,unused-argument
137+
def force_flush(self, timeout_millis: int = 30000) -> bool: # pylint: disable=unused-argument
116138
"""Force flush any pending spans. Always returns True as no pending work."""
117139
return True

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/code_correlation/test_code_attributes_span_processor.py

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from amazon.opentelemetry.distro.code_correlation.code_attributes_span_processor import CodeAttributesSpanProcessor
99
from opentelemetry.context import Context
1010
from opentelemetry.sdk.trace import ReadableSpan, Span
11+
from opentelemetry.sdk.util.instrumentation import InstrumentationScope
1112
from opentelemetry.semconv.attributes.code_attributes import CODE_FUNCTION_NAME
1213
from opentelemetry.trace import SpanKind
1314

@@ -82,57 +83,85 @@ def test_max_stack_frames_constant(self):
8283
"""Test that MAX_STACK_FRAMES is set to expected value."""
8384
self.assertEqual(CodeAttributesSpanProcessor.MAX_STACK_FRAMES, 50)
8485

85-
def create_mock_span(self, span_kind=SpanKind.CLIENT, attributes=None):
86+
def create_mock_span(self, span_kind=SpanKind.CLIENT, attributes=None, instrumentation_scope_name=None):
8687
"""Helper to create a mock span with specified properties."""
8788
mock_span = MagicMock(spec=Span)
8889
mock_span.kind = span_kind
8990
mock_span.attributes = attributes
91+
92+
# Set up instrumentation scope
93+
if instrumentation_scope_name is not None:
94+
mock_scope = MagicMock(spec=InstrumentationScope)
95+
mock_scope.name = instrumentation_scope_name
96+
mock_span.instrumentation_scope = mock_scope
97+
else:
98+
mock_span.instrumentation_scope = None
99+
90100
return mock_span
91101

92-
def test_should_process_span_client_span_without_attributes(self):
93-
"""Test that CLIENT spans without code attributes should be processed."""
94-
span = self.create_mock_span(SpanKind.CLIENT, attributes=None)
95-
result = CodeAttributesSpanProcessor._should_process_span(span)
102+
def test_should_process_span_user_client_span_without_attributes(self):
103+
"""Test that user CLIENT spans without code attributes should be processed."""
104+
span = self.create_mock_span(SpanKind.CLIENT, attributes=None, instrumentation_scope_name="my-app")
105+
result = self.processor._should_process_span(span)
96106
self.assertTrue(result)
97107

98-
def test_should_process_span_client_span_with_empty_attributes(self):
99-
"""Test that CLIENT spans with empty attributes should be processed."""
100-
span = self.create_mock_span(SpanKind.CLIENT, attributes={})
101-
result = CodeAttributesSpanProcessor._should_process_span(span)
108+
def test_should_process_span_user_client_span_with_empty_attributes(self):
109+
"""Test that user CLIENT spans with empty attributes should be processed."""
110+
span = self.create_mock_span(SpanKind.CLIENT, attributes={}, instrumentation_scope_name="my-app")
111+
result = self.processor._should_process_span(span)
102112
self.assertTrue(result)
103113

104114
def test_should_process_span_client_span_with_existing_code_attributes(self):
105-
"""Test that CLIENT spans with existing code attributes should not be processed."""
115+
"""Test that spans with existing code attributes should not be processed."""
106116
attributes = {CODE_FUNCTION_NAME: "existing.function"}
107-
span = self.create_mock_span(SpanKind.CLIENT, attributes=attributes)
108-
result = CodeAttributesSpanProcessor._should_process_span(span)
117+
span = self.create_mock_span(SpanKind.CLIENT, attributes=attributes, instrumentation_scope_name="my-app")
118+
result = self.processor._should_process_span(span)
109119
self.assertFalse(result)
110120

111-
def test_should_process_span_server_and_internal_spans(self):
112-
"""Test that SERVER and INTERNAL spans should not be processed."""
113-
test_cases = [
114-
SpanKind.SERVER,
115-
SpanKind.INTERNAL,
116-
]
121+
def test_should_process_span_user_server_spans(self):
122+
"""Test that user SERVER spans should be processed (new logic)."""
123+
span = self.create_mock_span(SpanKind.SERVER, attributes=None, instrumentation_scope_name="my-app")
124+
result = self.processor._should_process_span(span)
125+
self.assertTrue(result)
117126

118-
for span_kind in test_cases:
119-
with self.subTest(span_kind=span_kind):
120-
span = self.create_mock_span(span_kind, attributes=None)
121-
result = CodeAttributesSpanProcessor._should_process_span(span)
122-
self.assertFalse(result)
127+
def test_should_process_span_library_server_spans_not_processed(self):
128+
"""Test that library instrumentation SERVER spans should not be processed."""
129+
span = self.create_mock_span(
130+
SpanKind.SERVER, attributes=None, instrumentation_scope_name="opentelemetry.instrumentation.flask"
131+
)
132+
result = self.processor._should_process_span(span)
133+
self.assertFalse(result)
123134

124-
def test_should_process_span_client_producer_consumer_spans(self):
125-
"""Test that CLIENT, PRODUCER, and CONSUMER spans should be processed."""
135+
def test_should_process_span_library_internal_spans_not_processed(self):
136+
"""Test that library instrumentation INTERNAL spans should not be processed."""
137+
span = self.create_mock_span(
138+
SpanKind.INTERNAL, attributes=None, instrumentation_scope_name="opentelemetry.instrumentation.botocore"
139+
)
140+
result = self.processor._should_process_span(span)
141+
self.assertFalse(result)
142+
143+
def test_should_process_span_library_client_spans_processed(self):
144+
"""Test that library instrumentation CLIENT spans should be processed."""
145+
span = self.create_mock_span(
146+
SpanKind.CLIENT, attributes=None, instrumentation_scope_name="opentelemetry.instrumentation.requests"
147+
)
148+
result = self.processor._should_process_span(span)
149+
self.assertTrue(result)
150+
151+
def test_should_process_span_user_spans_all_kinds(self):
152+
"""Test that user spans of all kinds should be processed."""
126153
test_cases = [
127154
SpanKind.CLIENT,
155+
SpanKind.SERVER,
128156
SpanKind.PRODUCER,
129157
SpanKind.CONSUMER,
158+
SpanKind.INTERNAL,
130159
]
131160

132161
for span_kind in test_cases:
133162
with self.subTest(span_kind=span_kind):
134-
span = self.create_mock_span(span_kind, attributes=None)
135-
result = CodeAttributesSpanProcessor._should_process_span(span)
163+
span = self.create_mock_span(span_kind, attributes=None, instrumentation_scope_name="my-app")
164+
result = self.processor._should_process_span(span)
136165
self.assertTrue(result)
137166

138167
@patch("amazon.opentelemetry.distro.code_correlation.code_attributes_span_processor.sys._getframe")
@@ -300,7 +329,8 @@ def is_user_code_side_effect(filename):
300329

301330
def test_on_start_should_not_process_span(self):
302331
"""Test on_start when span should not be processed."""
303-
span = self.create_mock_span(SpanKind.SERVER) # Non-client span
332+
# Library instrumentation SERVER span should not be processed
333+
span = self.create_mock_span(SpanKind.SERVER, instrumentation_scope_name="opentelemetry.instrumentation.flask")
304334

305335
with patch.object(self.processor, "_capture_code_attributes") as mock_capture:
306336
self.processor.on_start(span)

aws-opentelemetry-distro/tests/amazon/opentelemetry/distro/patches/test_aio_pika_patches.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def test_patch_callback_decorator_decorate(self):
4646
# Verify we got a function back (the enhanced decorated callback)
4747
self.assertTrue(callable(result))
4848

49-
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches." "get_code_correlation_enabled_status")
49+
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches.get_code_correlation_enabled_status")
5050
def test_apply_aio_pika_instrumentation_patches_disabled(self, mock_get_status):
5151
"""Test patches are not applied when code correlation is disabled."""
5252
mock_get_status.return_value = False
@@ -61,7 +61,7 @@ def test_apply_aio_pika_instrumentation_patches_disabled(self, mock_get_status):
6161
# Should not raise exception when code correlation is disabled
6262
_apply_aio_pika_instrumentation_patches()
6363

64-
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches." "get_code_correlation_enabled_status")
64+
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches.get_code_correlation_enabled_status")
6565
def test_apply_aio_pika_instrumentation_patches_enabled(self, mock_get_status):
6666
"""Test patches are applied when code correlation is enabled."""
6767
mock_get_status.return_value = True
@@ -83,7 +83,7 @@ def test_apply_aio_pika_instrumentation_patches_enabled(self, mock_get_status):
8383
# Verify the decorate method was patched
8484
self.assertTrue(hasattr(mock_callback_decorator, "decorate"))
8585

86-
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches." "get_code_correlation_enabled_status")
86+
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches.get_code_correlation_enabled_status")
8787
def test_apply_aio_pika_instrumentation_patches_import_error(self, mock_get_status):
8888
"""Test patches handle import errors gracefully."""
8989
mock_get_status.return_value = True
@@ -99,7 +99,7 @@ def test_apply_aio_pika_instrumentation_patches_import_error(self, mock_get_stat
9999
_apply_aio_pika_instrumentation_patches()
100100

101101
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches.logger")
102-
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches." "get_code_correlation_enabled_status")
102+
@patch("amazon.opentelemetry.distro.patches._aio_pika_patches.get_code_correlation_enabled_status")
103103
def test_apply_aio_pika_instrumentation_patches_exception_handling(self, mock_get_status, mock_logger):
104104
"""Test patches handle general exceptions gracefully."""
105105
mock_get_status.side_effect = Exception("Test exception")
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
from typing import Dict, List
4+
5+
from mock_collector_client import ResourceScopeSpan
6+
from requests import Response
7+
from typing_extensions import override
8+
9+
from amazon.base.contract_test_base import ContractTestBase
10+
from opentelemetry.proto.common.v1.common_pb2 import AnyValue, KeyValue
11+
from opentelemetry.proto.trace.v1.trace_pb2 import Span
12+
from opentelemetry.semconv.attributes.code_attributes import CODE_LINE_NUMBER
13+
14+
15+
class CodeAttributesTest(ContractTestBase):
16+
@override
17+
@staticmethod
18+
def get_application_image_name() -> str:
19+
return "aws-application-signals-tests-requests-app"
20+
21+
@override
22+
def get_application_network_aliases(self) -> List[str]:
23+
"""
24+
This will be the target hostname of the clients making http requests in the application image, so that they
25+
don't use localhost.
26+
"""
27+
return ["backend"]
28+
29+
@override
30+
def get_application_extra_environment_variables(self) -> Dict[str, str]:
31+
"""
32+
This does not appear to do anything, as it does not seem that OTEL supports peer service for Python. Keeping
33+
for consistency with Java contract tests at this time.
34+
"""
35+
return {
36+
"OTEL_INSTRUMENTATION_COMMON_PEER_SERVICE_MAPPING": "backend=backend:8080",
37+
"OTEL_AWS_CODE_CORRELATION_ENABLED": "true",
38+
}
39+
40+
def test_success(self) -> None:
41+
self.do_test_requests("success", "GET", 200, 0, 0, request_method="GET")
42+
43+
def test_error(self) -> None:
44+
self.do_test_requests("error", "GET", 400, 1, 0, request_method="GET")
45+
46+
def test_fault(self) -> None:
47+
self.do_test_requests("fault", "GET", 500, 0, 1, request_method="GET")
48+
49+
def test_success_post(self) -> None:
50+
self.do_test_requests("success/postmethod", "POST", 200, 0, 0, request_method="POST")
51+
52+
def test_error_post(self) -> None:
53+
self.do_test_requests("error/postmethod", "POST", 400, 1, 0, request_method="POST")
54+
55+
def test_fault_post(self) -> None:
56+
self.do_test_requests("fault/postmethod", "POST", 500, 0, 1, request_method="POST")
57+
58+
def do_test_requests(
59+
self, path: str, method: str, status_code: int, expected_error: int, expected_fault: int, **kwargs
60+
) -> None:
61+
response: Response = self.send_request(method, path)
62+
self.assertEqual(status_code, response.status_code)
63+
64+
resource_scope_spans: List[ResourceScopeSpan] = self.mock_collector_client.get_traces()
65+
self._assert_span_code_attributes(resource_scope_spans, path, **kwargs)
66+
67+
@override
68+
def _assert_span_code_attributes(self, resource_scope_spans: List[ResourceScopeSpan], path: str, **kwargs) -> None:
69+
target_spans: List[Span] = []
70+
for resource_scope_span in resource_scope_spans:
71+
# pylint: disable=no-member
72+
if resource_scope_span.span.kind == Span.SPAN_KIND_CLIENT:
73+
target_spans.append(resource_scope_span.span)
74+
75+
self.assertEqual(len(target_spans), 1)
76+
self._assert_code_attribute(target_spans[0].attributes)
77+
78+
def _assert_code_attribute(self, attributes_list: List[KeyValue]):
79+
attributes_dict: Dict[str, AnyValue] = self._get_attributes_dict(attributes_list)
80+
# The line number of calling requests.request in requests_server.py
81+
self._assert_int_attribute(attributes_dict, CODE_LINE_NUMBER, 41)

0 commit comments

Comments
 (0)