diff --git a/sdk/ai/azure-ai-projects/CHANGELOG.md b/sdk/ai/azure-ai-projects/CHANGELOG.md index fdd3ba74147e..a4bb9b35d585 100644 --- a/sdk/ai/azure-ai-projects/CHANGELOG.md +++ b/sdk/ai/azure-ai-projects/CHANGELOG.md @@ -12,6 +12,7 @@ * Agent Endpoint beta operations: Removed required parameters `user_isolation_key` and `chat_isolation_key` from the `HeaderIsolationKeySource` class constructor. ### Bugs Fixed +* Fixed telemetry instrumentor to correctly call is_recording() as a method on spans, ensuring non-recording spans are properly skipped (e.g., when sampling is configured) ([GitHub issue 46544](https://github.com/Azure/azure-sdk-for-python/issues/46544)). ### Sample updates diff --git a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_ai_project_instrumentor.py b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_ai_project_instrumentor.py index 1a22ca314704..c23a994b795e 100644 --- a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_ai_project_instrumentor.py +++ b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_ai_project_instrumentor.py @@ -645,7 +645,7 @@ def _add_message_event( # pylint: disable=too-many-branches,too-many-statements attribute_name = GEN_AI_INPUT_MESSAGES # Set the attribute on the span - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): span.add_attribute(attribute_name, message_json) def _get_field(self, obj: Any, field: str) -> Any: @@ -722,7 +722,7 @@ def _add_instructions_event( # Use attributes for instructions tracing # System instructions format: array of content objects without role/parts wrapper message_json = json.dumps(content_array, ensure_ascii=False) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): span.add_attribute(GEN_AI_SYSTEM_MESSAGE, message_json) def _status_to_string(self, status: Any) -> str: @@ -782,7 +782,7 @@ def start_create_agent_span( # pylint: disable=too-many-locals reasoning_summary=reasoning_summary, structured_inputs=(str(structured_inputs) if structured_inputs is not None else None), ) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): span.add_attribute(GEN_AI_OPERATION_NAME, OperationName.CREATE_AGENT.value) if name: span.add_attribute(GEN_AI_AGENT_NAME, name) @@ -842,7 +842,7 @@ def start_create_thread_span( # _tool_resources: Optional["ToolResources"] = None, ) -> "Optional[AbstractSpan]": span = start_span(OperationName.CREATE_THREAD, server_address=server_address, port=port) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): for message in messages or []: self.add_thread_message_event(span, message) diff --git a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_responses_instrumentor.py b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_responses_instrumentor.py index 95cb28183b35..fdb3cc456214 100644 --- a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_responses_instrumentor.py +++ b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_responses_instrumentor.py @@ -523,7 +523,7 @@ def _set_attributes(self, span: "AbstractSpan", *attrs: Tuple[str, Any]) -> None def _set_span_attribute_safe(self, span: "AbstractSpan", key: str, value: Any) -> None: """Safely set a span attribute only if the value is meaningful.""" - if not span or not span.span_instance.is_recording: + if not span or not span.span_instance.is_recording(): return # Only set attribute if value exists and is meaningful @@ -846,7 +846,7 @@ def _add_workflow_action_events( conversation_id: Optional[str] = None, ) -> None: """Add workflow action events to the span for workflow agents.""" - if not span or not span.span_instance.is_recording: + if not span or not span.span_instance.is_recording(): return # Check if response has output items @@ -1149,7 +1149,7 @@ def _add_tool_call_events( # pylint: disable=too-many-branches conversation_id: Optional[str] = None, ) -> None: """Add tool call events to the span from response output.""" - if not span or not span.span_instance.is_recording: + if not span or not span.span_instance.is_recording(): return # Extract function calls and tool calls from response output @@ -1638,7 +1638,7 @@ def start_responses_span( gen_ai_provider=RESPONSES_PROVIDER, ) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): # Set operation name attribute (start_span doesn't set this automatically) self._set_attributes( span, @@ -2614,7 +2614,7 @@ def cleanup(self): # Join all accumulated output content complete_content = "".join(self.accumulated_output) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): # Add tool call events if we detected any output items (tool calls, etc.) if self.has_output_items: # Create mock response with output items for event generation @@ -2721,7 +2721,7 @@ def __init__( ) # End span with proper status - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): self.span.span_instance.set_status( # pyright: ignore [reportPossiblyUnboundVariable] StatusCode.OK @@ -2764,7 +2764,7 @@ def __next__(self): span_attributes=span_attributes, error_type=str(type(e).__name__), ) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): self.span.span_instance.set_status( # pyright: ignore [reportPossiblyUnboundVariable] StatusCode.ERROR, @@ -2791,7 +2791,7 @@ def _finalize_span(self): span_attributes=span_attributes, ) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): # Note: For streaming responses, response metadata like tokens, finish_reasons # are typically not available in individual chunks, so we focus on content. @@ -3092,7 +3092,7 @@ def cleanup(self): # Join all accumulated output content complete_content = "".join(self.accumulated_output) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): # Add tool call events if we detected any output items (tool calls, etc.) if self.has_output_items: # Create mock response with output items for event generation @@ -3199,7 +3199,7 @@ def __init__( ) # End span with proper status - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): self.span.span_instance.set_status( # pyright: ignore [reportPossiblyUnboundVariable] StatusCode.OK @@ -3241,7 +3241,7 @@ async def __anext__(self): span_attributes=span_attributes, error_type=str(type(e).__name__), ) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): self.span.span_instance.set_status( # pyright: ignore [reportPossiblyUnboundVariable] StatusCode.ERROR, @@ -3268,7 +3268,7 @@ def _finalize_span(self): span_attributes=span_attributes, ) - if self.span.span_instance.is_recording: + if self.span.span_instance.is_recording(): # Note: For streaming responses, response metadata like tokens, finish_reasons # are typically not available in individual chunks, so we focus on content. @@ -3407,7 +3407,7 @@ def start_create_conversation_span( gen_ai_provider=RESPONSES_PROVIDER, ) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): self._set_span_attribute_safe(span, GEN_AI_OPERATION_NAME, OperationName.CREATE_CONVERSATION.value) return span @@ -3605,7 +3605,7 @@ def start_list_conversation_items_span( gen_ai_provider=RESPONSES_PROVIDER, ) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): # Set operation name attribute (start_span doesn't set this automatically) self._set_attributes( span, @@ -3624,7 +3624,7 @@ def _add_conversation_item_event( # pylint: disable=too-many-branches,too-many- item: Any, ) -> None: """Add a conversation item event to the span.""" - if not span or not span.span_instance.is_recording: + if not span or not span.span_instance.is_recording(): return # Extract basic item information diff --git a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_utils.py b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_utils.py index 931c3d2abf7b..47047e2720c3 100644 --- a/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_utils.py +++ b/sdk/ai/azure-ai-projects/azure/ai/projects/telemetry/_utils.py @@ -223,7 +223,7 @@ def start_span( schema_version=GEN_AI_SEMANTIC_CONVENTIONS_SCHEMA_VERSION, ) - if span and span.span_instance.is_recording: + if span and span.span_instance.is_recording(): span.add_attribute(AZ_NAMESPACE, AZ_NAMESPACE_VALUE) span.add_attribute(GEN_AI_PROVIDER_NAME, AGENTS_PROVIDER) diff --git a/sdk/ai/azure-ai-projects/tests/agents/telemetry/test_non_recording_span.py b/sdk/ai/azure-ai-projects/tests/agents/telemetry/test_non_recording_span.py new file mode 100644 index 000000000000..c64b0288bdb1 --- /dev/null +++ b/sdk/ai/azure-ai-projects/tests/agents/telemetry/test_non_recording_span.py @@ -0,0 +1,202 @@ +# ------------------------------------ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# ------------------------------------ +""" +Tests verifying that instrumentors correctly skip non-recording spans. + +When a span is not recording, the instrumentor must not attempt to write +attributes or events to it. These tests use a mock span whose +``is_recording()`` returns False and whose mutation methods raise +``AssertionError`` if called, ensuring the guards work correctly. +""" + +import os +import json +import pytest +from unittest.mock import MagicMock, PropertyMock + +from azure.ai.projects.telemetry._ai_project_instrumentor import ( + _AIAgentsInstrumentorPreview, +) +from azure.ai.projects.telemetry._responses_instrumentor import ( + _ResponsesInstrumentorPreview, +) + + +def _make_non_recording_span(): + """Return a mock AbstractSpan wrapping a non-recording OTel span. + + * ``span_instance.is_recording()`` returns ``False`` + * ``span_instance.is_recording`` (the property/method) also returns ``False`` + so the guard correctly skips writes. + * Any call to ``add_event``, ``set_status``, ``record_exception`` or + ``set_attribute`` raises ``AssertionError``, catching any code path + that fails to check ``is_recording()`` properly. + """ + span_instance = MagicMock() + span_instance.is_recording = MagicMock(return_value=False) + span_instance.add_event = MagicMock(side_effect=AssertionError("add_event called on non-recording span")) + span_instance.set_status = MagicMock(side_effect=AssertionError("set_status called on non-recording span")) + span_instance.record_exception = MagicMock( + side_effect=AssertionError("record_exception called on non-recording span") + ) + + span = MagicMock() + span.span_instance = span_instance + span.add_attribute = MagicMock(side_effect=AssertionError("add_attribute called on non-recording span")) + return span + + +class TestNonRecordingSpanProjectInstrumentor: + """Verify _AIAgentsInstrumentorPreview skips non-recording spans.""" + + def test_add_message_event_skips_non_recording_span(self): + """_add_message_event should not write to a non-recording span.""" + instrumentor = _AIAgentsInstrumentorPreview() + span = _make_non_recording_span() + + # This must not raise; the guard should return early. + instrumentor._add_message_event(span, role="user", content="hello") + + def test_add_instructions_event_skips_non_recording_span(self): + """_add_instructions_event should not write to a non-recording span.""" + instrumentor = _AIAgentsInstrumentorPreview() + span = _make_non_recording_span() + + instrumentor._add_instructions_event(span, instructions="Be helpful", additional_instructions=None) + + def test_start_create_agent_span_skips_non_recording_span(self): + """start_create_agent_span should not write attributes to a non-recording span.""" + instrumentor = _AIAgentsInstrumentorPreview() + + # We need to patch start_span to return our non-recording span + from unittest.mock import patch + + non_recording_span = _make_non_recording_span() + + with patch( + "azure.ai.projects.telemetry._ai_project_instrumentor.start_span", + return_value=non_recording_span, + ): + result = instrumentor.start_create_agent_span( + server_address="test.openai.azure.com", + port=443, + model="gpt-4", + name="test-agent", + instructions="Be helpful", + ) + + # Should return the span but not have written any attributes/events to it + assert result is non_recording_span + non_recording_span.add_attribute.assert_not_called() + non_recording_span.span_instance.add_event.assert_not_called() + + +class TestNonRecordingSpanResponsesInstrumentor: + """Verify _ResponsesInstrumentorPreview skips non-recording spans.""" + + def test_set_span_attribute_safe_skips_non_recording_span(self): + """_set_span_attribute_safe should not write to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + span = _make_non_recording_span() + + # This must not raise; the guard should return early. + instrumentor._set_span_attribute_safe(span, "test.key", "test_value") + + def test_start_responses_span_skips_non_recording_span(self): + """start_responses_span should not write attributes to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + + from unittest.mock import patch + + non_recording_span = _make_non_recording_span() + + with patch( + "azure.ai.projects.telemetry._responses_instrumentor.start_span", + return_value=non_recording_span, + ): + result = instrumentor.start_responses_span( + server_address="test.openai.azure.com", + port=443, + model="gpt-4", + assistant_name="test-agent", + conversation_id="conv-123", + input_text="Hello", + ) + + assert result is non_recording_span + non_recording_span.add_attribute.assert_not_called() + non_recording_span.span_instance.add_event.assert_not_called() + + def test_start_create_conversation_span_skips_non_recording_span(self): + """start_create_conversation_span should not write to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + + from unittest.mock import patch + + non_recording_span = _make_non_recording_span() + + with patch( + "azure.ai.projects.telemetry._responses_instrumentor.start_span", + return_value=non_recording_span, + ): + result = instrumentor.start_create_conversation_span( + server_address="test.openai.azure.com", + port=443, + ) + + assert result is non_recording_span + non_recording_span.add_attribute.assert_not_called() + non_recording_span.span_instance.add_event.assert_not_called() + + def test_start_list_conversation_items_span_skips_non_recording_span(self): + """start_list_conversation_items_span should not write to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + + from unittest.mock import patch + + non_recording_span = _make_non_recording_span() + + with patch( + "azure.ai.projects.telemetry._responses_instrumentor.start_span", + return_value=non_recording_span, + ): + result = instrumentor.start_list_conversation_items_span( + server_address="test.openai.azure.com", + port=443, + conversation_id="conv-123", + ) + + assert result is non_recording_span + non_recording_span.add_attribute.assert_not_called() + non_recording_span.span_instance.add_event.assert_not_called() + + def test_add_tool_call_events_skips_non_recording_span(self): + """_add_tool_call_events should not write to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + span = _make_non_recording_span() + + # Create a mock response with function call output + mock_response = MagicMock() + mock_output_item = MagicMock() + mock_output_item.type = "function_call" + mock_output_item.name = "get_weather" + mock_output_item.call_id = "call_123" + mock_output_item.arguments = '{"city": "Seattle"}' + mock_response.output = [mock_output_item] + + instrumentor._add_tool_call_events(span, mock_response) + + def test_add_conversation_item_event_skips_non_recording_span(self): + """_add_conversation_item_event should not write to a non-recording span.""" + instrumentor = _ResponsesInstrumentorPreview() + span = _make_non_recording_span() + + mock_item = MagicMock() + mock_item.id = "item_123" + mock_item.type = "message" + mock_item.role = "user" + mock_item.content = [] + + instrumentor._add_conversation_item_event(span, mock_item)