Address PR feedback

Author: stephentoub

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

@@ -178,6 +178,15 @@ public int? MaximumIterationsPerRequest
         }
     }
 
+    /// <summary>
+    /// Gets or sets a value indicating whether <see cref="Activity"/>s should be used to
+    /// provide telemetry for function invocation.
+    /// </summary>
+    /// <remarks>
+    /// The default value is <see langword="true"/>.
+    /// </remarks>
+    public bool EnableTelemetry { get; set; } = true;
+
     /// <inheritdoc/>
     public override async Task<ChatCompletion> CompleteAsync(IList<ChatMessage> chatMessages, ChatOptions? options = null, CancellationToken cancellationToken = default)
     {
@@ -576,7 +585,7 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
     {
         _ = Throw.IfNull(context);
 
-        using Activity? activity = _activitySource.StartActivity(context.Function.Metadata.Name);
+        using Activity? activity = EnableTelemetry ? _activitySource.StartActivity(context.Function.Metadata.Name) : null;
 
         long startingTimestamp = 0;
         if (_logger.IsEnabled(LogLevel.Debug))
@@ -601,8 +610,8 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
         {
             if (activity is not null)
             {
-                activity.SetTag("error.type", e.GetType().FullName);
-                activity.SetStatus(ActivityStatusCode.Error, e.Message);
+                _ = activity.SetTag("error.type", e.GetType().FullName)
+                            .SetStatus(ActivityStatusCode.Error, e.Message);
             }
 
             if (e is OperationCanceledException)
@@ -620,12 +629,7 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
         {
             if (_logger.IsEnabled(LogLevel.Debug))
             {
-                TimeSpan elapsed =
-#if NET
-                    Stopwatch.GetElapsedTime(startingTimestamp);
-#else
-                    new TimeSpan((long)((Stopwatch.GetTimestamp() - startingTimestamp) * ((double)TimeSpan.TicksPerSecond / Stopwatch.Frequency)));
-#endif
+                TimeSpan elapsed = GetElapsedTime(startingTimestamp);
 
                 if (result is not null && _logger.IsEnabled(LogLevel.Trace))
                 {
@@ -641,6 +645,13 @@ FunctionResultContent CreateFunctionResultContent(FunctionInvocationResult resul
         return result;
     }
 
+    private static TimeSpan GetElapsedTime(long startingTimestamp) =>
+#if NET
+        Stopwatch.GetElapsedTime(startingTimestamp);
+#else
+        new((long)((Stopwatch.GetTimestamp() - startingTimestamp) * ((double)TimeSpan.TicksPerSecond / Stopwatch.Frequency)));
+#endif
+
     [LoggerMessage(LogLevel.Debug, "Invoking {MethodName}.", SkipEnabledCheck = true)]
     private partial void LogInvoking(string methodName);
 

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

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Shared.Diagnostics;
 
@@ -29,6 +30,8 @@ public static ChatClientBuilder UseFunctionInvocation(
 
         return builder.Use((services, innerClient) =>
         {
+            loggerFactory ??= services.GetService<ILoggerFactory>();
+
             var chatClient = new FunctionInvokingChatClient(innerClient, loggerFactory?.CreateLogger(typeof(FunctionInvokingChatClient)));
             configure?.Invoke(chatClient);
             return chatClient;

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

@@ -1,6 +1,10 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+#pragma warning disable CA1031 // Do not catch general exception types
+#pragma warning disable S108 // Nested blocks of code should not be left empty
+#pragma warning disable S2486 // Generic exceptions should not be ignored
+
 using System.Text.Json;
 
 namespace Microsoft.Extensions.AI;
@@ -9,16 +13,22 @@ namespace Microsoft.Extensions.AI;
 internal static class LoggingHelpers
 {
     /// <summary>Serializes <paramref name="value"/> as JSON for logging purposes.</summary>
-    public static string AsJson<T>(T value, JsonSerializerOptions? options = null)
+    public static string AsJson<T>(T value, JsonSerializerOptions? options)
     {
         if (options?.TryGetTypeInfo(typeof(T), out var typeInfo) is true ||
             AIJsonUtilities.DefaultOptions.TryGetTypeInfo(typeof(T), out typeInfo))
         {
-            return JsonSerializer.Serialize(value, typeInfo);
+            try
+            {
+                return JsonSerializer.Serialize(value, typeInfo);
+            }
+            catch
+            {
+            }
         }
 
-        // Unable to get a type info for the value, so return an empty JSON object.
-        // We do not want lack of type info to disrupt application behavior with exceptions.
+        // If we're unable to get a type info for the value, or if we fail to serialize,
+        // return an empty JSON object. We do not want lack of type info to disrupt application behavior with exceptions.
         return "{}";
     }
 }

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

@@ -3,11 +3,13 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using OpenTelemetry.Trace;
 using Xunit;
 
 namespace Microsoft.Extensions.AI;
@@ -345,6 +347,45 @@ await InvokeAndAssertAsync(options, [
         }
     }
 
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task FunctionInvocationTrackedWithActivity(bool enableTelemetry)
+    {
+        var activities = new List<Activity>();
+        using var tracerProvider = OpenTelemetry.Sdk.CreateTracerProviderBuilder()
+            .AddSource("Microsoft.Extensions.AI.*")
+            .AddInMemoryExporter(activities)
+            .Build();
+
+        var options = new ChatOptions
+        {
+            Tools = [AIFunctionFactory.Create(() => "Result 1", "Func1")]
+        };
+
+        await InvokeAndAssertAsync(options, [
+            new ChatMessage(ChatRole.User, "hello"),
+            new ChatMessage(ChatRole.Assistant, [new FunctionCallContent("callId1", "Func1", new Dictionary<string, object?> { ["arg1"] = "value1" })]),
+            new ChatMessage(ChatRole.Tool, [new FunctionResultContent("callId1", "Func1", result: "Result 1")]),
+            new ChatMessage(ChatRole.Assistant, "world"),
+        ], configurePipeline: b => b.Use(c =>
+            new FunctionInvokingChatClient(c) { EnableTelemetry = enableTelemetry }));
+
+        if (enableTelemetry)
+        {
+            var activity = Assert.Single(activities);
+
+            Assert.NotNull(activity.Id);
+            Assert.NotEmpty(activity.Id);
+
+            Assert.Equal("Func1", activity.DisplayName);
+        }
+        else
+        {
+            Assert.Empty(activities);
+        }
+    }
+
     private static async Task<List<ChatMessage>> InvokeAndAssertAsync(
         ChatOptions options,
         List<ChatMessage> plan,