Fix Activity.Current being set to null when FunctionInvokingChatClient#7324
Fix Activity.Current being set to null when FunctionInvokingChatClient#7324rogerbarreto wants to merge 1 commit intodotnet:mainfrom
Conversation
…t runs inside invoke_agent span When CurrentActivityIsInvokeAgent is true, the orchestrate_tools activity is correctly suppressed (activity = null). However, the Activity.Current = activity workaround for dotnet/runtime#47802 after each yield return was then setting Activity.Current to null, destroying the ambient trace context. This caused all subsequent spans (tool execution, second LLM call) to become disconnected root traces instead of being children of the invoke_agent span. The fix saves the parent activity (Activity.Current) before the orchestrate_tools decision and uses activity ?? parentActivity as the restoration target. When orchestrate_tools is suppressed inside invoke_agent, Activity.Current is restored to the invoke_agent span instead of null. This preserves both: - The orchestrate_tools suppression fix from PR dotnet#7224 (issue dotnet#7223) - Correct trace context propagation across streaming + tool call cycles Fixes microsoft/agent-framework#4074 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical trace context propagation bug in FunctionInvokingChatClient where Activity.Current was incorrectly set to null after yield returns when the orchestrate_tools span was suppressed inside an invoke_agent span. The fix preserves the parent activity context by using activityToRestore = activity ?? parentActivity, ensuring subsequent spans (tool execution, second LLM call) remain children of the invoke_agent span instead of becoming disconnected root traces.
Changes:
- Fixed Activity.Current restoration in streaming method to preserve parent activity when orchestrate_tools span is suppressed
- Added comprehensive test to verify Activity.Current is preserved across streaming + tool call cycles
- Extended existing test coverage with additional invoke_agent display name format
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs | Captures parent activity before conditional orchestrate_tools creation and restores it (instead of null) after each yield return in streaming method |
| test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs | Adds new streaming test to verify Activity.Current preservation and extends existing test with additional display name format |
| // A single request into this GetResponseAsync may result in multiple requests to the inner client. | ||
| // Create an activity to group them together for better observability. If there's already a genai "invoke_agent" | ||
| // span that's current, however, we just consider that the group and don't add a new one. | ||
| Activity? parentActivity = Activity.Current; |
There was a problem hiding this comment.
The parentActivity variable is captured but never used in the non-streaming GetResponseAsync method. Unlike the streaming method which uses it to restore Activity.Current after each yield, this method doesn't need activity restoration since it doesn't use yield return. Consider removing this line.
| Activity? parentActivity = Activity.Current; |
|
Already addressed. PR #7321 by @flaviocdc |
Motivation and Context
When
CurrentActivityIsInvokeAgentis true, the orchestrate_tools activity is correctly suppressed (activity = null). However, the Activity.Current = activity workaround for dotnet/runtime#47802 after each yield return was then setting Activity.Current to null, destroying the ambient trace context.This caused all subsequent spans (tool execution, second LLM call) to become disconnected root traces instead of being children of the invoke_agent span.
The fix saves the parent activity (Activity.Current) before the orchestrate_tools decision and uses activity ?? parentActivity as the restoration target. When orchestrate_tools is suppressed inside invoke_agent, Activity.Current is restored to the invoke_agent span instead of null.
This preserves both:
The orchestrate_tools suppression fix from PR Fix FunctionInvokingChatClient invoke_agent span detection with exact match or space delimiter #7224 (issue FunctionInvokingChatClient doesn't validate parent span correctly #7223)
Correct trace context propagation across streaming + tool call cycles
Fixes .NET: OpenTelemetry trace nesting broken when using Microsoft.Extensions.AI 10.3.0 (Activity.Current lost after streaming + tool calls) microsoft/agent-framework#4074
Microsoft Reviewers: Open in CodeFlow