Skip to content

Commit ddc0697

Browse files
authored
Improve FunctionInvokingChatClient's awareness of agents (#7030)
This addresses several issues: 1. When FunctionInvokingChatClient is used from an agent, and the agent creates an invoke_agent span, that effectively serves the same purpose as the orchestrate_tools span the FICC would have otherwise created, yielding duplication in the telemetry. If we detect an "invoke_agent" span is the parent, we can simply not create "orchestrate_tools". 2. When FunctionInvokingChatClient is used from an agent, its operation is much more closely tied to the agent than to the inner chat client it's orchestrating. So whereas by default it gets its ActivitySource from the inner client, if it detects an "invoke_agent" span as current, it will instead use that activity's source. This means that we can still get "execute_tool" spans created even if telemetry isn't enabled for the inner chat client but is for the parent agent. 3. "execute_tool" optionally has sensitive data to emit. FICC currently determines whether to emit sensitive data based on the inner OpenTelemetryChatClient, since it's also using its ActivitySource. Now when it uses the "invoke_agent" span, it also picks up the sensitivity setting from a custom property on that span.
1 parent dda277d commit ddc0697

File tree

4 files changed

+249
-6
lines changed

4 files changed

+249
-6
lines changed

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/FunctionInvokingChatClient.cs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using Microsoft.Shared.Diagnostics;
1616

1717
#pragma warning disable CA2213 // Disposable fields should be disposed
18+
#pragma warning disable S2219 // Runtime type checking should be simplified
1819
#pragma warning disable S3353 // Unchanged local variables should be "const"
1920

2021
namespace Microsoft.Extensions.AI;
@@ -268,8 +269,9 @@ public override async Task<ChatResponse> GetResponseAsync(
268269
_ = Throw.IfNull(messages);
269270

270271
// A single request into this GetResponseAsync may result in multiple requests to the inner client.
271-
// Create an activity to group them together for better observability.
272-
using Activity? activity = _activitySource?.StartActivity(OpenTelemetryConsts.GenAI.OrchestrateToolsName);
272+
// Create an activity to group them together for better observability. If there's already a genai "invoke_agent"
273+
// span that's current, however, we just consider that the group and don't add a new one.
274+
using Activity? activity = CurrentActivityIsInvokeAgent ? null : _activitySource?.StartActivity(OpenTelemetryConsts.GenAI.OrchestrateToolsName);
273275

274276
// Copy the original messages in order to avoid enumerating the original messages multiple times.
275277
// The IEnumerable can represent an arbitrary amount of work.
@@ -407,8 +409,9 @@ public override async IAsyncEnumerable<ChatResponseUpdate> GetStreamingResponseA
407409
_ = Throw.IfNull(messages);
408410

409411
// A single request into this GetStreamingResponseAsync may result in multiple requests to the inner client.
410-
// Create an activity to group them together for better observability.
411-
using Activity? activity = _activitySource?.StartActivity(OpenTelemetryConsts.GenAI.OrchestrateToolsName);
412+
// Create an activity to group them together for better observability. If there's already a genai "invoke_agent"
413+
// span that's current, however, we just consider that the group and don't add a new one.
414+
using Activity? activity = CurrentActivityIsInvokeAgent ? null : _activitySource?.StartActivity(OpenTelemetryConsts.GenAI.OrchestrateToolsName);
412415
UsageDetails? totalUsage = activity is { IsAllDataRequested: true } ? new() : null; // tracked usage across all turns, to be used for activity purposes
413416

414417
// Copy the original messages in order to avoid enumerating the original messages multiple times.
@@ -1108,6 +1111,10 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
11081111
}
11091112
}
11101113

1114+
/// <summary>Gets a value indicating whether <see cref="Activity.Current"/> represents an "invoke_agent" span.</summary>
1115+
private static bool CurrentActivityIsInvokeAgent =>
1116+
Activity.Current?.DisplayName == OpenTelemetryConsts.GenAI.InvokeAgentName;
1117+
11111118
/// <summary>Invokes the function asynchronously.</summary>
11121119
/// <param name="context">
11131120
/// The function invocation context detailing the function to be invoked and its arguments along with additional request information.
@@ -1119,7 +1126,12 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
11191126
{
11201127
_ = Throw.IfNull(context);
11211128

1122-
using Activity? activity = _activitySource?.StartActivity(
1129+
// We have multiple possible ActivitySource's we could use. In a chat scenario, we ask the inner client whether it has an ActivitySource.
1130+
// In an agent scenario, we use the ActivitySource from the surrounding "invoke_agent" activity.
1131+
Activity? invokeAgentActivity = CurrentActivityIsInvokeAgent ? Activity.Current : null;
1132+
ActivitySource? source = invokeAgentActivity?.Source ?? _activitySource;
1133+
1134+
using Activity? activity = source?.StartActivity(
11231135
$"{OpenTelemetryConsts.GenAI.ExecuteToolName} {context.Function.Name}",
11241136
ActivityKind.Internal,
11251137
default(ActivityContext),
@@ -1133,7 +1145,14 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
11331145

11341146
long startingTimestamp = Stopwatch.GetTimestamp();
11351147

1136-
bool enableSensitiveData = activity is { IsAllDataRequested: true } && InnerClient.GetService<OpenTelemetryChatClient>()?.EnableSensitiveData is true;
1148+
// If we're in the chat scenario, we determine whether sensitive data is enabled by querying the inner chat client.
1149+
// If we're in the agent scenario, we determine whether sensitive data is enabled by checking for the relevant custom property on the activity.
1150+
bool enableSensitiveData =
1151+
activity is { IsAllDataRequested: true } &&
1152+
(invokeAgentActivity is not null ?
1153+
invokeAgentActivity.GetCustomProperty(OpenTelemetryChatClient.SensitiveDataEnabledCustomKey) as string is OpenTelemetryChatClient.SensitiveDataEnabledTrueValue :
1154+
InnerClient.GetService<OpenTelemetryChatClient>()?.EnableSensitiveData is true);
1155+
11371156
bool traceLoggingEnabled = _logger.IsEnabled(LogLevel.Trace);
11381157
bool loggedInvoke = false;
11391158
if (enableSensitiveData || traceLoggingEnabled)

src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ namespace Microsoft.Extensions.AI;
3030
/// </remarks>
3131
public sealed partial class OpenTelemetryChatClient : DelegatingChatClient
3232
{
33+
internal const string SensitiveDataEnabledCustomKey = "__EnableSensitiveData__";
34+
internal const string SensitiveDataEnabledTrueValue = "true";
35+
3336
private readonly ActivitySource _activitySource;
3437
private readonly Meter _meter;
3538

@@ -373,6 +376,11 @@ internal static string SerializeChatMessages(
373376
string.IsNullOrWhiteSpace(modelId) ? OpenTelemetryConsts.GenAI.ChatName : $"{OpenTelemetryConsts.GenAI.ChatName} {modelId}",
374377
ActivityKind.Client);
375378

379+
if (EnableSensitiveData)
380+
{
381+
activity?.SetCustomProperty(SensitiveDataEnabledCustomKey, SensitiveDataEnabledTrueValue);
382+
}
383+
376384
if (activity is { IsAllDataRequested: true })
377385
{
378386
_ = activity

src/Libraries/Microsoft.Extensions.AI/OpenTelemetryConsts.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ public static class GenAI
3535
public const string ChatName = "chat";
3636
public const string EmbeddingsName = "embeddings";
3737
public const string ExecuteToolName = "execute_tool";
38+
public const string InvokeAgentName = "invoke_agent";
3839
public const string OrchestrateToolsName = "orchestrate_tools"; // Non-standard
3940
public const string GenerateContentName = "generate_content";
4041

test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/FunctionInvokingChatClientTests.cs

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using Xunit;
1616

1717
#pragma warning disable SA1118 // Parameter should not span multiple lines
18+
#pragma warning disable SA1204 // Static elements should appear before instance elements
1819

1920
namespace Microsoft.Extensions.AI;
2021

@@ -1232,6 +1233,220 @@ public async Task ClonesChatOptionsAndResetContinuationTokenForBackgroundRespons
12321233
Assert.Null(actualChatOptions!.ContinuationToken);
12331234
}
12341235

1236+
[Fact]
1237+
public async Task DoesNotCreateOrchestrateToolsSpanWhenInvokeAgentIsParent()
1238+
{
1239+
string agentSourceName = Guid.NewGuid().ToString();
1240+
string clientSourceName = Guid.NewGuid().ToString();
1241+
1242+
List<ChatMessage> plan =
1243+
[
1244+
new ChatMessage(ChatRole.User, "hello"),
1245+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]),
1246+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1")]),
1247+
new ChatMessage(ChatRole.Assistant, "world"),
1248+
];
1249+
1250+
ChatOptions options = new()
1251+
{
1252+
Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
1253+
};
1254+
1255+
Func<ChatClientBuilder, ChatClientBuilder> configure = b => b.Use(c =>
1256+
new FunctionInvokingChatClient(new OpenTelemetryChatClient(c, sourceName: clientSourceName)));
1257+
1258+
var activities = new List<Activity>();
1259+
1260+
using TracerProvider tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
1261+
.AddSource(agentSourceName)
1262+
.AddSource(clientSourceName)
1263+
.AddInMemoryExporter(activities)
1264+
.Build();
1265+
1266+
using (var agentSource = new ActivitySource(agentSourceName))
1267+
using (var invokeAgentActivity = agentSource.StartActivity("invoke_agent"))
1268+
{
1269+
Assert.NotNull(invokeAgentActivity);
1270+
await InvokeAndAssertAsync(options, plan, configurePipeline: configure);
1271+
}
1272+
1273+
Assert.DoesNotContain(activities, a => a.DisplayName == "orchestrate_tools");
1274+
Assert.Contains(activities, a => a.DisplayName == "chat");
1275+
Assert.Contains(activities, a => a.DisplayName == "execute_tool Func1");
1276+
1277+
var invokeAgent = Assert.Single(activities, a => a.DisplayName == "invoke_agent");
1278+
var childActivities = activities.Where(a => a != invokeAgent).ToList();
1279+
Assert.All(childActivities, activity => Assert.Same(invokeAgent, activity.Parent));
1280+
}
1281+
1282+
[Fact]
1283+
public async Task UsesAgentActivitySourceWhenInvokeAgentIsParent()
1284+
{
1285+
string agentSourceName = Guid.NewGuid().ToString();
1286+
string clientSourceName = Guid.NewGuid().ToString();
1287+
1288+
List<ChatMessage> plan =
1289+
[
1290+
new ChatMessage(ChatRole.User, "hello"),
1291+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]),
1292+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1")]),
1293+
new ChatMessage(ChatRole.Assistant, "world"),
1294+
];
1295+
1296+
ChatOptions options = new()
1297+
{
1298+
Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
1299+
};
1300+
1301+
Func<ChatClientBuilder, ChatClientBuilder> configure = b => b.Use(c =>
1302+
new FunctionInvokingChatClient(new OpenTelemetryChatClient(c, sourceName: clientSourceName)));
1303+
1304+
var activities = new List<Activity>();
1305+
1306+
using TracerProvider tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
1307+
.AddSource(agentSourceName)
1308+
.AddSource(clientSourceName)
1309+
.AddInMemoryExporter(activities)
1310+
.Build();
1311+
1312+
using (var agentSource = new ActivitySource(agentSourceName))
1313+
using (var invokeAgentActivity = agentSource.StartActivity("invoke_agent"))
1314+
{
1315+
Assert.NotNull(invokeAgentActivity);
1316+
await InvokeAndAssertAsync(options, plan, configurePipeline: configure);
1317+
}
1318+
1319+
var executeToolActivities = activities.Where(a => a.DisplayName == "execute_tool Func1").ToList();
1320+
Assert.NotEmpty(executeToolActivities);
1321+
Assert.All(executeToolActivities, executeTool => Assert.Equal(agentSourceName, executeTool.Source.Name));
1322+
}
1323+
1324+
public static IEnumerable<object[]> SensitiveDataPropagatesFromAgentActivityWhenInvokeAgentIsParent_MemberData() =>
1325+
from invokeAgentSensitiveData in new bool?[] { null, false, true }
1326+
from innerOpenTelemetryChatClient in new bool?[] { null, false, true }
1327+
select new object?[] { invokeAgentSensitiveData, innerOpenTelemetryChatClient };
1328+
1329+
[Theory]
1330+
[MemberData(nameof(SensitiveDataPropagatesFromAgentActivityWhenInvokeAgentIsParent_MemberData))]
1331+
public async Task SensitiveDataPropagatesFromAgentActivityWhenInvokeAgentIsParent(
1332+
bool? invokeAgentSensitiveData, bool? innerOpenTelemetryChatClient)
1333+
{
1334+
string agentSourceName = Guid.NewGuid().ToString();
1335+
string clientSourceName = Guid.NewGuid().ToString();
1336+
1337+
List<ChatMessage> plan =
1338+
[
1339+
new ChatMessage(ChatRole.User, "hello"),
1340+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1", new Dictionary<string, object?> { ["arg1"] = "secret" })]),
1341+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1")]),
1342+
new ChatMessage(ChatRole.Assistant, "world"),
1343+
];
1344+
1345+
ChatOptions options = new()
1346+
{
1347+
Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
1348+
};
1349+
1350+
var activities = new List<Activity>();
1351+
1352+
using TracerProvider tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
1353+
.AddSource(agentSourceName)
1354+
.AddSource(clientSourceName)
1355+
.AddInMemoryExporter(activities)
1356+
.Build();
1357+
1358+
using (var agentSource = new ActivitySource(agentSourceName))
1359+
using (var invokeAgentActivity = agentSource.StartActivity("invoke_agent"))
1360+
{
1361+
if (invokeAgentSensitiveData is not null)
1362+
{
1363+
invokeAgentActivity?.SetCustomProperty("__EnableSensitiveData__", invokeAgentSensitiveData is true ? "true" : "false");
1364+
}
1365+
1366+
await InvokeAndAssertAsync(options, plan, configurePipeline: b =>
1367+
{
1368+
b.UseFunctionInvocation();
1369+
1370+
if (innerOpenTelemetryChatClient is not null)
1371+
{
1372+
b.UseOpenTelemetry(sourceName: clientSourceName, configure: c =>
1373+
{
1374+
c.EnableSensitiveData = innerOpenTelemetryChatClient.Value;
1375+
});
1376+
}
1377+
1378+
return b;
1379+
});
1380+
}
1381+
1382+
var executeToolActivity = Assert.Single(activities, a => a.DisplayName == "execute_tool Func1");
1383+
1384+
var hasArguments = executeToolActivity.Tags.Any(t => t.Key == "gen_ai.tool.call.arguments");
1385+
var hasResult = executeToolActivity.Tags.Any(t => t.Key == "gen_ai.tool.call.result");
1386+
1387+
if (invokeAgentSensitiveData is true)
1388+
{
1389+
Assert.True(hasArguments, "Expected arguments to be logged when agent EnableSensitiveData is true");
1390+
Assert.True(hasResult, "Expected result to be logged when agent EnableSensitiveData is true");
1391+
1392+
var argsTag = Assert.Single(executeToolActivity.Tags, t => t.Key == "gen_ai.tool.call.arguments");
1393+
Assert.Contains("arg1", argsTag.Value);
1394+
}
1395+
else
1396+
{
1397+
Assert.False(hasArguments, "Expected arguments NOT to be logged when agent EnableSensitiveData is false");
1398+
Assert.False(hasResult, "Expected result NOT to be logged when agent EnableSensitiveData is false");
1399+
}
1400+
}
1401+
1402+
[Theory]
1403+
[InlineData(false)]
1404+
[InlineData(true)]
1405+
public async Task CreatesOrchestrateToolsSpanWhenNoInvokeAgentParent(bool streaming)
1406+
{
1407+
string clientSourceName = Guid.NewGuid().ToString();
1408+
1409+
List<ChatMessage> plan =
1410+
[
1411+
new ChatMessage(ChatRole.User, "hello"),
1412+
new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1")]),
1413+
new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", result: "Result 1")]),
1414+
new ChatMessage(ChatRole.Assistant, "world"),
1415+
];
1416+
1417+
ChatOptions options = new()
1418+
{
1419+
Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
1420+
};
1421+
1422+
Func<ChatClientBuilder, ChatClientBuilder> configure = b => b.Use(c =>
1423+
new FunctionInvokingChatClient(new OpenTelemetryChatClient(c, sourceName: clientSourceName)));
1424+
1425+
var activities = new List<Activity>();
1426+
using TracerProvider tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
1427+
.AddSource(clientSourceName)
1428+
.AddInMemoryExporter(activities)
1429+
.Build();
1430+
1431+
if (streaming)
1432+
{
1433+
await InvokeAndAssertStreamingAsync(options, plan, configurePipeline: configure);
1434+
}
1435+
else
1436+
{
1437+
await InvokeAndAssertAsync(options, plan, configurePipeline: configure);
1438+
}
1439+
1440+
var orchestrateTools = Assert.Single(activities, a => a.DisplayName == "orchestrate_tools");
1441+
1442+
var executeTools = activities.Where(a => a.DisplayName.StartsWith("execute_tool")).ToList();
1443+
Assert.NotEmpty(executeTools);
1444+
foreach (var executeTool in executeTools)
1445+
{
1446+
Assert.Same(orchestrateTools, executeTool.Parent);
1447+
}
1448+
}
1449+
12351450
private sealed class CustomSynchronizationContext : SynchronizationContext
12361451
{
12371452
public override void Post(SendOrPostCallback d, object? state)

0 commit comments

Comments
 (0)