Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ def _build_envelope(
}

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

parent_span_id = None
if sp.parent is not None and sp.parent.span_id != 0:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def name(self):
@property
def context(self):
"""Return the span context."""
return self._span.context
return self._span.get_span_context()

@property
def parent(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,8 @@ def __init__(

# Log span creation
if self._span:
span_id = f"{self._span.context.span_id:016x}" if self._span.context else "unknown"
sc = self._span.get_span_context()
span_id = f"{sc.span_id:016x}" if sc else "unknown"
logger.info(f"Span started: '{activity_name}' ({span_id})")
else:
logger.error(f"Failed to create span: '{activity_name}' - tracer returned None")
Expand Down Expand Up @@ -267,8 +268,10 @@ def _end(self) -> None:
"""End the span and record metrics."""
if self._span and self._is_telemetry_enabled() and not self._has_ended:
self._has_ended = True
span_id = f"{self._span.context.span_id:016x}" if self._span.context else "unknown"
logger.info(f"Span ended: '{self._span.name}' ({span_id})")
sc = self._span.get_span_context()
span_id = f"{sc.span_id:016x}" if sc else "unknown"
span_name = getattr(self._span, "name", "unknown")
logger.info(f"Span ended: '{span_name}' ({span_id})")

# Convert custom end time to OTel-compatible format (nanoseconds since epoch)
otel_end_time = self._datetime_to_ns(self._custom_end_time)
Expand Down
1 change: 1 addition & 0 deletions tests/observability/core/exporters/test_enriched_span.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def test_delegates_all_properties_to_wrapped_span(self):
mock_span = Mock()
mock_span.name = "test-span"
mock_span.context = Mock(trace_id=123, span_id=456)
mock_span.get_span_context.return_value = mock_span.context
mock_span.parent = Mock(span_id=789)
mock_span.start_time = 1000000000
mock_span.end_time = 2000000000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def enricher(span):
original_span.context = Mock()
original_span.context.trace_id = 123
original_span.context.span_id = 456
original_span.get_span_context.return_value = original_span.context

# Call on_end
processor.on_end(original_span)
Expand All @@ -125,6 +126,7 @@ def failing_enricher(span):
original_span.context = Mock()
original_span.context.trace_id = 123
original_span.context.span_id = 456
original_span.get_span_context.return_value = original_span.context

# Should not raise despite failing enricher
processor.on_end(original_span)
Expand All @@ -142,6 +144,7 @@ def test_on_end_works_without_enricher(self):
original_span.context = Mock()
original_span.context.trace_id = 123
original_span.context.span_id = 456
original_span.get_span_context.return_value = original_span.context

# Should not raise
processor.on_end(original_span)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def _make_span(self, span_id: int, attribute_size: int) -> ReadableSpan:
ctx.trace_id = 0x1
ctx.span_id = span_id
mock_span.context = ctx
mock_span.get_span_context.return_value = ctx
mock_span.parent = None
mock_span.start_time = 1640995200000000000
mock_span.end_time = 1640995260000000000
Expand Down
1 change: 1 addition & 0 deletions tests/observability/core/test_agent365_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def _create_mock_span(
mock_context.trace_id = trace_id
mock_context.span_id = span_id
mock_span.context = mock_context
mock_span.get_span_context.return_value = mock_context

mock_span.parent = Mock() if parent_id else None
if parent_id:
Expand Down
192 changes: 192 additions & 0 deletions tests/observability/core/test_nonrecording_span.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT License.

"""Tests that observability code handles NonRecordingSpan gracefully.

When the TracerProvider has no valid exporter (e.g. token resolver returns
None on the first turn), it produces NonRecordingSpan instances. These only
expose get_span_context() — the .context attribute does not exist. The code
must use get_span_context() everywhere to avoid AttributeError crashes.
"""

import os
import unittest
from unittest.mock import MagicMock, patch
Comment thread
nikhilNava marked this conversation as resolved.

from microsoft_agents_a365.observability.core.exporters.enriched_span import (
EnrichedReadableSpan,
)
from microsoft_agents_a365.observability.core.opentelemetry_scope import (
OpenTelemetryScope,
)
from opentelemetry.trace import NonRecordingSpan, SpanContext, TraceFlags


def _make_non_recording_span(trace_id: int = 0, span_id: int = 0) -> NonRecordingSpan:
"""Create a NonRecordingSpan with the given trace/span IDs."""
return NonRecordingSpan(
SpanContext(
trace_id=trace_id,
span_id=span_id,
is_remote=False,
trace_flags=TraceFlags(0),
)
)


class TestOpenTelemetryScopeNonRecordingSpan(unittest.TestCase):
"""Verify OpenTelemetryScope works when the tracer returns a NonRecordingSpan."""

@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
@patch.object(OpenTelemetryScope, "_get_tracer")
def test_init_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
"""__init__ must not crash when tracer.start_span returns NonRecordingSpan."""
mock_tracer = MagicMock()
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
mock_tracer.start_span.return_value = nr_span
mock_get_tracer.return_value = mock_tracer

# Should not raise AttributeError
scope = OpenTelemetryScope(
operation_name="invoke_agent",
activity_name="test_activity",
)

self.assertIs(scope._span, nr_span)

@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
@patch.object(OpenTelemetryScope, "_get_tracer")
def test_end_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
"""_end() must not crash when the span is a NonRecordingSpan."""
mock_tracer = MagicMock()
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
mock_tracer.start_span.return_value = nr_span
mock_get_tracer.return_value = mock_tracer

scope = OpenTelemetryScope(
operation_name="invoke_agent",
activity_name="test_activity",
)

# Should not raise AttributeError
scope._end()

@patch.dict(os.environ, {"ENABLE_OBSERVABILITY": "true"})
@patch.object(OpenTelemetryScope, "_get_tracer")
def test_dispose_with_non_recording_span(self, mock_get_tracer: MagicMock) -> None:
"""Full dispose lifecycle must not crash with NonRecordingSpan."""
mock_tracer = MagicMock()
nr_span = _make_non_recording_span(trace_id=0xABCD, span_id=0x1234)
mock_tracer.start_span.return_value = nr_span
mock_get_tracer.return_value = mock_tracer

scope = OpenTelemetryScope(
operation_name="invoke_agent",
activity_name="test_activity",
)

# Context manager exit should not raise
scope.__exit__(None, None, None)


class TestEnrichedReadableSpanNonRecordingSpan(unittest.TestCase):
"""Verify EnrichedReadableSpan delegates context via get_span_context()."""

def test_context_property_uses_get_span_context(self) -> None:
"""EnrichedReadableSpan.context must call get_span_context(), not .context."""
mock_span = MagicMock()
expected_ctx = SpanContext(
trace_id=0xFF,
span_id=0xAA,
is_remote=False,
trace_flags=TraceFlags(0),
)
mock_span.get_span_context.return_value = expected_ctx
# Ensure .context attribute is NOT used
del mock_span.context

enriched = EnrichedReadableSpan(mock_span, extra_attributes={"key": "val"})
Comment thread
nikhilNava marked this conversation as resolved.

result = enriched.context
self.assertIs(result, expected_ctx)
mock_span.get_span_context.assert_called_once()

def test_to_json_with_non_recording_span_context(self) -> None:
"""to_json() must not crash when wrapped span uses get_span_context()."""
mock_span = MagicMock()
mock_span.name = "test"
mock_span.get_span_context.return_value = SpanContext(
trace_id=0x1234,
span_id=0x5678,
is_remote=False,
trace_flags=TraceFlags(0),
)
del mock_span.context
mock_span.attributes = {"key": "value"}
mock_span.kind = "INTERNAL"
Comment thread
nikhilNava marked this conversation as resolved.
mock_span.parent = None
mock_span.start_time = 1000000000
mock_span.end_time = 2000000000
mock_span.status = None
mock_span.events = []
mock_span.links = []
mock_span.resource = None

enriched = EnrichedReadableSpan(mock_span, extra_attributes={})

# Should not raise
json_str = enriched.to_json()
self.assertIn("test", json_str)
self.assertIn("0x00000000000000000000000000001234", json_str)


class TestAgent365ExporterMapSpanGetSpanContext(unittest.TestCase):
"""Verify _map_span uses get_span_context() instead of .context."""

@patch.dict(os.environ, {}, clear=True)
def test_map_span_calls_get_span_context(self) -> None:
"""_map_span must use get_span_context(), not .context attribute."""
from microsoft_agents_a365.observability.core.exporters.agent365_exporter import (
_Agent365Exporter,
)

exporter = _Agent365Exporter(token_resolver=lambda a, t: "token")

span = MagicMock()
span.name = "test_span"
span.attributes = {"gen_ai.operation.name": "invoke_agent"}

expected_ctx = SpanContext(
trace_id=0xABCD,
span_id=0x1234,
is_remote=False,
trace_flags=TraceFlags(0),
)
span.get_span_context.return_value = expected_ctx
# Remove .context so it would fail if accessed
del span.context
Comment thread
nikhilNava marked this conversation as resolved.

span.parent = None
span.kind = MagicMock()
span.start_time = 1000000000
span.end_time = 2000000000
span.status = MagicMock()
span.status.status_code = MagicMock()
span.status.description = ""
span.events = []
span.links = []
span.instrumentation_scope = MagicMock()
span.instrumentation_scope.name = "test"
span.instrumentation_scope.version = "1.0"

# Should not raise AttributeError
mapped = exporter._map_span(span)

span.get_span_context.assert_called_once()
self.assertIn("traceId", mapped)
self.assertIn("spanId", mapped)
exporter.shutdown()


if __name__ == "__main__":
unittest.main()
Loading