.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.
This commit is contained in:
Roger Barreto
2026-05-13 12:49:37 +01:00
Unverified
parent 3d4a8c4151
commit 2848b66ebf
2 changed files with 80 additions and 7 deletions
@@ -54,7 +54,7 @@ public sealed class OpenTelemetryAgent : DelegatingAIAgent, IDisposable
/// telemetry collection according to OpenTelemetry semantic conventions for AI systems.
/// </remarks>
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
/// <summary>
/// If auto-wiring is enabled and the inner agent is a <see cref="ChatClientAgent"/> whose underlying
/// <see cref="IChatClient"/> is not already instrumented with <see cref="OpenTelemetryChatClient"/>, returns a
/// new <see cref="AgentRunOptions"/> with a <see cref="ChatClientAgentRunOptions.ChatClientFactory"/> that
/// wraps the chat client with <see cref="OpenTelemetryChatClient"/>. Otherwise, returns <paramref name="options"/> unchanged.
/// new <see cref="ChatClientAgentRunOptions"/> with a <see cref="ChatClientAgentRunOptions.ChatClientFactory"/>
/// that wraps the chat client with <see cref="OpenTelemetryChatClient"/>. When <paramref name="options"/> is a
/// plain <see cref="AgentRunOptions"/> (the base type, not <see cref="ChatClientAgentRunOptions"/>), the base
/// properties are copied onto the new <see cref="ChatClientAgentRunOptions"/> so high-level callers that pass
/// the abstract <see cref="AgentRunOptions"/> still benefit from auto-wiring and propagate their settings to
/// the inner agent. Otherwise, returns <paramref name="options"/> unchanged.
/// </summary>
private AgentRunOptions? GetRunOptionsWithChatClientWiring(AgentRunOptions? options)
{
@@ -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<Activity>();
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<Activity>();
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<Activity>();
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<IEnumerable<ChatMessage>, ChatOptions?>? OnGetResponseAsync { get; set; }