From 2848b66ebf46dd6386047ab221346d43d15ad73d Mon Sep 17 00:00:00 2001 From: Roger Barreto <19890735+rogerbarreto@users.noreply.github.com> Date: Wed, 13 May 2026 12:49:37 +0100 Subject: [PATCH] .NET: OpenTelemetryAgent address westey-m PR review - Use string.IsNullOrWhiteSpace (not IsNullOrEmpty) when normalizing the constructor sourceName, so callers passing whitespace-only strings still land on OpenTelemetryConsts.DefaultSourceName instead of an unsubscribed ActivitySource. - Fix the misleading pragma comment on the 2-arg ctor delegating call: auto-wiring is the new default, it does not preserve the original (pre-PR) behavior. - Expand the GetRunOptionsWithChatClientWiring XML doc to spell out that a base AgentRunOptions (not ChatClientAgentRunOptions) is also accepted: it is converted to ChatClientAgentRunOptions with the auto-wire factory installed and base properties copied. - Tests: extend the source-name normalization Theory with whitespace cases (' ' and '\t'); add end-to-end coverage for plain AgentRunOptions over a real ChatClientAgent (sync + streaming) asserting the inner chat client is invoked and both invoke_agent + chat spans are emitted. --- .../Microsoft.Agents.AI/OpenTelemetryAgent.cs | 14 ++-- .../OpenTelemetryAgentTests.cs | 73 ++++++++++++++++++- 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI/OpenTelemetryAgent.cs b/dotnet/src/Microsoft.Agents.AI/OpenTelemetryAgent.cs index fb407789a3..cffde717e4 100644 --- a/dotnet/src/Microsoft.Agents.AI/OpenTelemetryAgent.cs +++ b/dotnet/src/Microsoft.Agents.AI/OpenTelemetryAgent.cs @@ -54,7 +54,7 @@ public sealed class OpenTelemetryAgent : DelegatingAIAgent, IDisposable /// telemetry collection according to OpenTelemetry semantic conventions for AI systems. /// public OpenTelemetryAgent(AIAgent innerAgent, string? sourceName = null) -#pragma warning disable MAAI001 // Delegating to the experimental autoWireChatClient overload with the default-on value preserves the original behavior. +#pragma warning disable MAAI001 // Auto-wiring is the new default; the experimental opt-out lives on the 3-arg overload. : this(innerAgent, sourceName, autoWireChatClient: true) #pragma warning restore MAAI001 { @@ -84,8 +84,8 @@ public sealed class OpenTelemetryAgent : DelegatingAIAgent, IDisposable // Resolve once so the outer OpenTelemetryChatClient and the auto-wired inner // OpenTelemetryChatClient always emit spans under the same ActivitySource, even when - // the caller passes "" (which neither the outer nor inner client should treat as a real source). - this._sourceName = string.IsNullOrEmpty(sourceName) ? OpenTelemetryConsts.DefaultSourceName : sourceName!; + // the caller passes "" or whitespace (which neither client should treat as a real source). + this._sourceName = string.IsNullOrWhiteSpace(sourceName) ? OpenTelemetryConsts.DefaultSourceName : sourceName!; this._autoWireChatClient = autoWireChatClient; this._otelClient = new OpenTelemetryChatClient( @@ -206,8 +206,12 @@ public sealed class OpenTelemetryAgent : DelegatingAIAgent, IDisposable /// /// If auto-wiring is enabled and the inner agent is a whose underlying /// is not already instrumented with , returns a - /// new with a that - /// wraps the chat client with . Otherwise, returns unchanged. + /// new with a + /// that wraps the chat client with . When is a + /// plain (the base type, not ), the base + /// properties are copied onto the new so high-level callers that pass + /// the abstract still benefit from auto-wiring and propagate their settings to + /// the inner agent. Otherwise, returns unchanged. /// private AgentRunOptions? GetRunOptionsWithChatClientWiring(AgentRunOptions? options) { diff --git a/dotnet/tests/Microsoft.Agents.AI.UnitTests/OpenTelemetryAgentTests.cs b/dotnet/tests/Microsoft.Agents.AI.UnitTests/OpenTelemetryAgentTests.cs index d65406b305..1f04d09ba3 100644 --- a/dotnet/tests/Microsoft.Agents.AI.UnitTests/OpenTelemetryAgentTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.UnitTests/OpenTelemetryAgentTests.cs @@ -864,10 +864,12 @@ public class OpenTelemetryAgentTests [Theory] [InlineData(null)] [InlineData("")] - public async Task Ctor_NullOrEmptySourceName_AutoWiredChatClientUsesDefaultSource_Async(string? sourceName) + [InlineData(" ")] + [InlineData("\t")] + public async Task Ctor_NullOrWhitespaceSourceName_AutoWiredChatClientUsesDefaultSource_Async(string? sourceName) { // Both the agent-level invoke_agent span and the auto-wired chat span must be emitted under - // OpenTelemetryConsts.DefaultSourceName when the caller passes null or "" so they reach + // OpenTelemetryConsts.DefaultSourceName when the caller passes null, "", or whitespace, so they reach // the same ActivitySource and are not silently dropped by the exporter. var activities = new List(); using var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder() @@ -985,6 +987,73 @@ public class OpenTelemetryAgentTests Assert.StartsWith("invoke_agent", activity.DisplayName); } + [Fact] + public async Task AutoWireChatClient_PlainAgentRunOptions_RealChatClientAgent_EmitsChatSpan_Async() + { + // High-level callers may pass the abstract base AgentRunOptions (not ChatClientAgentRunOptions) when + // wiring a ChatClientAgent. Auto-wiring must still kick in: convert to ChatClientAgentRunOptions, + // install the OTel-wrapping factory, and produce both the invoke_agent and chat spans end-to-end. + var sourceName = Guid.NewGuid().ToString(); + var activities = new List(); + using var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder() + .AddSource(sourceName) + .AddInMemoryExporter(activities) + .Build(); + + ChatOptions? observedChatOptions = null; + var fakeChatClient = new AutoWireTestChatClient + { + OnGetResponseAsync = (_, opts) => observedChatOptions = opts, + }; + var inner = new ChatClientAgent(fakeChatClient); + using var agent = new OpenTelemetryAgent(inner, sourceName); + + // Pass the base AgentRunOptions, not ChatClientAgentRunOptions. + var inputOptions = new AgentRunOptions { AllowBackgroundResponses = false }; + + _ = await agent.RunAsync("hi", options: inputOptions); + + // Inner chat client was actually invoked (auto-wired factory ran without breaking the pipeline). + Assert.NotNull(observedChatOptions); + + Assert.Equal(2, activities.Count); + Assert.Contains(activities, a => a.DisplayName.StartsWith("invoke_agent", StringComparison.Ordinal)); + Assert.Contains(activities, a => string.Equals(a.GetTagItem("gen_ai.operation.name") as string, "chat", StringComparison.Ordinal)); + } + + [Fact] + public async Task AutoWireChatClient_PlainAgentRunOptions_RealChatClientAgent_StreamingEmitsChatSpan_Async() + { + // Same as the sync test above but for the streaming path so both invocation paths + // are covered when callers pass a base AgentRunOptions. + var sourceName = Guid.NewGuid().ToString(); + var activities = new List(); + using var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder() + .AddSource(sourceName) + .AddInMemoryExporter(activities) + .Build(); + + ChatOptions? observedChatOptions = null; + var fakeChatClient = new AutoWireTestChatClient + { + OnGetResponseAsync = (_, opts) => observedChatOptions = opts, + }; + var inner = new ChatClientAgent(fakeChatClient); + using var agent = new OpenTelemetryAgent(inner, sourceName); + + var inputOptions = new AgentRunOptions { AllowBackgroundResponses = false }; + + await foreach (var _ in agent.RunStreamingAsync("hi", options: inputOptions)) + { + } + + Assert.NotNull(observedChatOptions); + + Assert.Equal(2, activities.Count); + Assert.Contains(activities, a => a.DisplayName.StartsWith("invoke_agent", StringComparison.Ordinal)); + Assert.Contains(activities, a => string.Equals(a.GetTagItem("gen_ai.operation.name") as string, "chat", StringComparison.Ordinal)); + } + private sealed class AutoWireTestChatClient : IChatClient { public Action, ChatOptions?>? OnGetResponseAsync { get; set; }