Skip to content

Commit 5d2f274

Browse files
fix: use get_span_context() to handle NonRecordingSpan
When the token resolver returns None on the first turn, the TracerProvider produces a NonRecordingSpan. The .context attribute only exists on concrete ReadableSpan implementations, causing AttributeError crashes on the first agent message. Replace all span.context accesses with the public get_span_context() interface method, which is available on all Span types including NonRecordingSpan. Also guard the .name access in _end() with getattr() since NonRecordingSpan lacks that attribute as well. Changes: - opentelemetry_scope.py: use get_span_context() in span start/end logging - enriched_span.py: delegate context property via get_span_context() - agent365_exporter.py: use get_span_context() in _map_span() - Update test mocks to set get_span_context() return value - Add test_nonrecording_span.py with 6 targeted tests Ported from: microsoft/opentelemetry-distro-python#168 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 41775eb commit 5d2f274

8 files changed

Lines changed: 207 additions & 5 deletions

File tree

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/agent365_exporter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def _build_envelope(
330330
}
331331

332332
def _map_span(self, sp: ReadableSpan) -> dict[str, Any]:
333-
ctx = sp.context
333+
ctx = sp.get_span_context()
334334

335335
parent_span_id = None
336336
if sp.parent is not None and sp.parent.span_id != 0:

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/enriched_span.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ def name(self):
5454
@property
5555
def context(self):
5656
"""Return the span context."""
57-
return self._span.context
57+
return self._span.get_span_context()
5858

5959
@property
6060
def parent(self):

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/opentelemetry_scope.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,8 @@ def __init__(
158158

159159
# Log span creation
160160
if self._span:
161-
span_id = f"{self._span.context.span_id:016x}" if self._span.context else "unknown"
161+
sc = self._span.get_span_context()
162+
span_id = f"{sc.span_id:016x}" if sc else "unknown"
162163
logger.info(f"Span started: '{activity_name}' ({span_id})")
163164
else:
164165
logger.error(f"Failed to create span: '{activity_name}' - tracer returned None")
@@ -267,8 +268,10 @@ def _end(self) -> None:
267268
"""End the span and record metrics."""
268269
if self._span and self._is_telemetry_enabled() and not self._has_ended:
269270
self._has_ended = True
270-
span_id = f"{self._span.context.span_id:016x}" if self._span.context else "unknown"
271-
logger.info(f"Span ended: '{self._span.name}' ({span_id})")
271+
sc = self._span.get_span_context()
272+
span_id = f"{sc.span_id:016x}" if sc else "unknown"
273+
span_name = getattr(self._span, "name", "unknown")
274+
logger.info(f"Span ended: '{span_name}' ({span_id})")
272275

273276
# Convert custom end time to OTel-compatible format (nanoseconds since epoch)
274277
otel_end_time = self._datetime_to_ns(self._custom_end_time)

tests/observability/core/exporters/test_enriched_span.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ def test_delegates_all_properties_to_wrapped_span(self):
3434
mock_span = Mock()
3535
mock_span.name = "test-span"
3636
mock_span.context = Mock(trace_id=123, span_id=456)
37+
mock_span.get_span_context.return_value = mock_span.context
3738
mock_span.parent = Mock(span_id=789)
3839
mock_span.start_time = 1000000000
3940
mock_span.end_time = 2000000000

tests/observability/core/exporters/test_enriching_span_processor.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ def enricher(span):
9999
original_span.context = Mock()
100100
original_span.context.trace_id = 123
101101
original_span.context.span_id = 456
102+
original_span.get_span_context.return_value = original_span.context
102103

103104
# Call on_end
104105
processor.on_end(original_span)
@@ -125,6 +126,7 @@ def failing_enricher(span):
125126
original_span.context = Mock()
126127
original_span.context.trace_id = 123
127128
original_span.context.span_id = 456
129+
original_span.get_span_context.return_value = original_span.context
128130

129131
# Should not raise despite failing enricher
130132
processor.on_end(original_span)
@@ -142,6 +144,7 @@ def test_on_end_works_without_enricher(self):
142144
original_span.context = Mock()
143145
original_span.context.trace_id = 123
144146
original_span.context.span_id = 456
147+
original_span.get_span_context.return_value = original_span.context
145148

146149
# Should not raise
147150
processor.on_end(original_span)

tests/observability/core/exporters/test_payload_chunking.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ def _make_span(self, span_id: int, attribute_size: int) -> ReadableSpan:
151151
ctx.trace_id = 0x1
152152
ctx.span_id = span_id
153153
mock_span.context = ctx
154+
mock_span.get_span_context.return_value = ctx
154155
mock_span.parent = None
155156
mock_span.start_time = 1640995200000000000
156157
mock_span.end_time = 1640995260000000000

tests/observability/core/test_agent365_exporter.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ def _create_mock_span(
7070
mock_context.trace_id = trace_id
7171
mock_context.span_id = span_id
7272
mock_span.context = mock_context
73+
mock_span.get_span_context.return_value = mock_context
7374

7475
mock_span.parent = Mock() if parent_id else None
7576
if parent_id:
Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
"""Tests that observability code handles NonRecordingSpan gracefully.
5+
6+
When the TracerProvider has no valid exporter (e.g. token resolver returns
7+
None on the first turn), it produces NonRecordingSpan instances. These only
8+
expose get_span_context() — the .context attribute does not exist. The code
9+
must use get_span_context() everywhere to avoid AttributeError crashes.
10+
"""
11+
12+
import os
13+
import unittest
14+
from unittest.mock import MagicMock, Mock, patch
15+
16+
from opentelemetry.trace import NonRecordingSpan, SpanContext, TraceFlags
17+
18+
from microsoft_agents_a365.observability.core.exporters.enriched_span import (
19+
EnrichedReadableSpan,
20+
)
21+
from microsoft_agents_a365.observability.core.opentelemetry_scope import (
22+
OpenTelemetryScope,
23+
)
24+
25+
26+
def _make_non_recording_span(trace_id: int = 0, span_id: int = 0) -> NonRecordingSpan:
27+
"""Create a NonRecordingSpan with the given trace/span IDs."""
28+
return NonRecordingSpan(
29+
SpanContext(
30+
trace_id=trace_id,
31+
span_id=span_id,
32+
is_remote=False,
33+
trace_flags=TraceFlags(0),
34+
)
35+
)
36+
37+
38+
class TestOpenTelemetryScopeNonRecordingSpan(unittest.TestCase):
39+
"""Verify OpenTelemetryScope works when the tracer returns a NonRecordingSpan."""
40+
41+
@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
42+
@patch.object(OpenTelemetryScope, "_get_tracer")
43+
def test_init_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
44+
"""__init__ must not crash when tracer.start_span returns NonRecordingSpan."""
45+
mock_tracer = MagicMock()
46+
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
47+
mock_tracer.start_span.return_value = nr_span
48+
mock_get_tracer.return_value = mock_tracer
49+
50+
# Should not raise AttributeError
51+
scope = OpenTelemetryScope(
52+
operation_name="invoke_agent",
53+
activity_name="test_activity",
54+
)
55+
56+
self.assertIs(scope._span, nr_span)
57+
58+
@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
59+
@patch.object(OpenTelemetryScope, "_get_tracer")
60+
def test_end_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
61+
"""_end() must not crash when the span is a NonRecordingSpan."""
62+
mock_tracer = MagicMock()
63+
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
64+
mock_tracer.start_span.return_value = nr_span
65+
mock_get_tracer.return_value = mock_tracer
66+
67+
scope = OpenTelemetryScope(
68+
operation_name="invoke_agent",
69+
activity_name="test_activity",
70+
)
71+
72+
# Should not raise AttributeError
73+
scope._end()
74+
75+
@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
76+
@patch.object(OpenTelemetryScope, "_get_tracer")
77+
def test_dispose_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
78+
"""Full dispose lifecycle must not crash with NonRecordingSpan."""
79+
mock_tracer = MagicMock()
80+
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
81+
mock_tracer.start_span.return_value = nr_span
82+
mock_get_tracer.return_value = mock_tracer
83+
84+
scope = OpenTelemetryScope(
85+
operation_name="invoke_agent",
86+
activity_name="test_activity",
87+
)
88+
89+
# Context manager exit should not raise
90+
scope.__exit__(None, None, None)
91+
92+
93+
class TestEnrichedReadableSpanNonRecordingSpan(unittest.TestCase):
94+
"""Verify EnrichedReadableSpan delegates context via get_span_context()."""
95+
96+
def test_context_property_uses_get_span_context(self) -> None:
97+
"""EnrichedReadableSpan.context must call get_span_context(), not .context."""
98+
mock_span = MagicMock()
99+
expected_ctx = SpanContext(
100+
trace_id=0xFF,
101+
span_id=0xAA,
102+
is_remote=False,
103+
trace_flags=TraceFlags(0),
104+
)
105+
mock_span.get_span_context.return_value = expected_ctx
106+
# Ensure .context attribute is NOT used
107+
del mock_span.context
108+
109+
enriched = EnrichedReadableSpan(mock_span, extra_attributes={"key": "val"})
110+
111+
result = enriched.context
112+
self.assertIs(result, expected_ctx)
113+
mock_span.get_span_context.assert_called_once()
114+
115+
def test_to_json_with_non_recording_span_context(self) -> None:
116+
"""to_json() must not crash when wrapped span uses get_span_context()."""
117+
mock_span = MagicMock()
118+
mock_span.name = "test"
119+
mock_span.get_span_context.return_value = SpanContext(
120+
trace_id=0x1234,
121+
span_id=0x5678,
122+
is_remote=False,
123+
trace_flags=TraceFlags(0),
124+
)
125+
del mock_span.context
126+
mock_span.attributes = {"key": "value"}
127+
mock_span.kind = "INTERNAL"
128+
mock_span.parent = None
129+
mock_span.start_time = 1000000000
130+
mock_span.end_time = 2000000000
131+
mock_span.status = None
132+
mock_span.events = []
133+
mock_span.links = []
134+
mock_span.resource = None
135+
136+
enriched = EnrichedReadableSpan(mock_span, extra_attributes={})
137+
138+
# Should not raise
139+
json_str = enriched.to_json()
140+
self.assertIn("test", json_str)
141+
self.assertIn("0x00000000000000000000000000001234", json_str)
142+
143+
144+
class TestAgent365ExporterMapSpanGetSpanContext(unittest.TestCase):
145+
"""Verify _map_span uses get_span_context() instead of .context."""
146+
147+
@patch.dict(os.environ, {}, clear=True)
148+
def test_map_span_calls_get_span_context(self) -> None:
149+
"""_map_span must use get_span_context(), not .context attribute."""
150+
from microsoft_agents_a365.observability.core.exporters.agent365_exporter import (
151+
_Agent365Exporter,
152+
)
153+
154+
exporter = _Agent365Exporter(token_resolver=lambda a, t: "token")
155+
156+
span = MagicMock()
157+
span.name = "test_span"
158+
span.attributes = {"gen_ai.operation.name": "invoke_agent"}
159+
160+
expected_ctx = SpanContext(
161+
trace_id=0xABCD,
162+
span_id=0x1234,
163+
is_remote=False,
164+
trace_flags=TraceFlags(0),
165+
)
166+
span.get_span_context.return_value = expected_ctx
167+
# Remove .context so it would fail if accessed
168+
del span.context
169+
170+
span.parent = None
171+
span.kind = MagicMock()
172+
span.start_time = 1000000000
173+
span.end_time = 2000000000
174+
span.status = MagicMock()
175+
span.status.status_code = MagicMock()
176+
span.status.description = ""
177+
span.events = []
178+
span.links = []
179+
span.instrumentation_scope = MagicMock()
180+
span.instrumentation_scope.name = "test"
181+
span.instrumentation_scope.version = "1.0"
182+
183+
# Should not raise AttributeError
184+
mapped = exporter._map_span(span)
185+
186+
span.get_span_context.assert_called_once()
187+
self.assertIn("traceId", mapped)
188+
self.assertIn("spanId", mapped)
189+
exporter.shutdown()
190+
191+
192+
if __name__ == "__main__":
193+
unittest.main()

0 commit comments

Comments
 (0)