-
Notifications
You must be signed in to change notification settings - Fork 9
Update auto instrumentation logic for Semantic Kernel #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the auto-instrumentation logic for Semantic Kernel by introducing a span enrichment pattern. The changes enable extensions to modify span attributes after spans end but before they are exported, using a registry-based enricher system. This allows for cleaner separation of concerns and more flexible attribute transformation.
Changes:
- Introduced a span enricher registry mechanism with register/unregister functions in the core SDK
- Added EnrichedReadableSpan wrapper to add attributes to immutable ReadableSpan objects
- Implemented Semantic Kernel-specific span enricher to transform SK attributes to standard gen_ai attributes
- Enhanced span processor with proper lifecycle methods (shutdown, force_flush) and execution type tracking
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| utils.py | New utility function to extract content from message JSON, transforming message objects to simple content arrays |
| trace_instrumentor.py | Updated to register/unregister span enricher and improved documentation |
| span_processor.py | Added execution type attribute for invoke_agent spans and implemented proper lifecycle methods |
| span_enricher.py | New enricher that transforms SK-specific attributes to standard gen_ai format for both invoke_agent and execute_tool spans |
| enriched_span.py | New wrapper class that allows adding attributes to immutable ReadableSpan objects |
| config.py | Added enricher registry with thread-safe registration and _EnrichingBatchSpanProcessor that applies enrichers before export |
| init.py | Exported new span enrichment functions and EnrichedReadableSpan class |
...mantickernel/microsoft_agents_a365/observability/extensions/semantickernel/span_processor.py
Outdated
Show resolved
Hide resolved
...emantickernel/microsoft_agents_a365/observability/extensions/semantickernel/span_enricher.py
Outdated
Show resolved
Hide resolved
.../microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/config.py
Outdated
Show resolved
Hide resolved
...emantickernel/microsoft_agents_a365/observability/extensions/semantickernel/span_enricher.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
...nsions-semantickernel/microsoft_agents_a365/observability/extensions/semantickernel/utils.py
Show resolved
Hide resolved
...vability-core/microsoft_agents_a365/observability/core/exporters/enriching_span_processor.py
Outdated
Show resolved
Hide resolved
...vability-core/microsoft_agents_a365/observability/core/exporters/enriching_span_processor.py
Outdated
Show resolved
Hide resolved
...emantickernel/microsoft_agents_a365/observability/extensions/semantickernel/span_enricher.py
Show resolved
Hide resolved
...mantickernel/microsoft_agents_a365/observability/extensions/semantickernel/span_processor.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
| def extract_content_as_string_list(messages_json: str) -> str: | ||
| """Extract content values from messages JSON and return as JSON string list. | ||
| Transforms from: [{"role": "user", "content": "Hello"}] | ||
| To: ["Hello"] | ||
| Args: | ||
| messages_json: JSON string like '[{"role": "user", "content": "Hello"}]' | ||
| Returns: | ||
| JSON string containing only the content values as an array, | ||
| or the original string if parsing fails. | ||
| """ | ||
| try: | ||
| messages = json.loads(messages_json) | ||
| if isinstance(messages, list): | ||
| contents = [] | ||
| for msg in messages: | ||
| if isinstance(msg, dict) and "content" in msg: | ||
| contents.append(msg["content"]) | ||
| elif isinstance(msg, str): | ||
| contents.append(msg) | ||
| return json.dumps(contents) | ||
| return messages_json | ||
| except (json.JSONDecodeError, TypeError): | ||
| # If parsing fails, return as-is | ||
| return messages_json |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the extract_content_as_string_list utility function. This function has multiple code paths (handling lists, dicts with content, plain strings, and error cases), but no tests exist to verify its behavior. Add a dedicated test file (e.g., test_utils.py) that tests all the different input scenarios: valid message objects with content, plain string arrays, malformed JSON, and edge cases like empty arrays or missing content fields.
| def on_end(self, span): | ||
| if span.name.startswith(INVOKE_AGENT_OPERATION_NAME): | ||
| span.set_attribute( | ||
| GEN_AI_EXECUTION_TYPE_KEY, ExecutionType.HUMAN_TO_AGENT.value.lower() |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent casing for gen_ai.execution.type attribute. This code sets the execution type to lowercase ("humantoagent" via ExecutionType.HUMAN_TO_AGENT.value.lower()), but the core SDK in invoke_agent_scope.py:125 sets it using the enum value directly without lowercasing ("HumanToAgent"). This creates an inconsistency where the same attribute has different casings depending on whether spans are created via InvokeAgentScope (manual instrumentation) or SemanticKernelSpanProcessor (auto-instrumentation). Consider either: (1) updating invoke_agent_scope.py to also use .lower(), or (2) documenting why different casings are acceptable in different contexts.
| GEN_AI_EXECUTION_TYPE_KEY, ExecutionType.HUMAN_TO_AGENT.value.lower() | |
| GEN_AI_EXECUTION_TYPE_KEY, ExecutionType.HUMAN_TO_AGENT.value |
| def test_non_matching_span_returns_original(self): | ||
| """Test that spans not matching invoke_agent or execute_tool are returned unchanged.""" | ||
| # Create a mock span with a different operation name | ||
| mock_span = Mock() | ||
| mock_span.name = "some_other_operation" | ||
| mock_span.attributes = { | ||
| "some_key": "some_value", | ||
| } | ||
|
|
||
| # Enrich the span | ||
| result = enrich_semantic_kernel_span(mock_span) | ||
|
|
||
| # Verify it returns the original span unchanged | ||
| self.assertEqual(result, mock_span) |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the execute_tool span enrichment path. The span_enricher.py file includes logic to map SK-specific tool attributes (gen_ai.tool.call.arguments and gen_ai.tool.call.result) to standard attributes (gen_ai.tool.arguments and gen_ai.event.content), but there are no tests verifying this behavior. Add a test case similar to test_invoke_agent_span_extracts_content_from_messages that verifies execute_tool spans are enriched correctly.
Task
Review SK auto instrumentation logic and clean up attributes
Solution
In SK the spans cannot be updated on_end() of span processor. They are readable spans.
Our only option is to update the span before its exported. Here I introduce a new pattern where we wrap the batch processor.
We make updates to the span on the on_end() of class _EnrichingBatchSpanProcessor.
This pattern is extensible and extensions for auto instrumentation need only implement the enrichment. If no enrichment the _EnrichingBatchSpanProcessor is a pass through
Exported spans
Execute tool and Inference span