From f571df9c077ceecd53e1bbf3b52d8a4c39292040 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 5 Sep 2025 20:33:41 -0400 Subject: [PATCH] Change OpenTelemetryChatClient/EmbeddingGenerator to log raw additional properties For historical reasons (based on older versions of the genai convention), we were mangling the key names. Now just use the key names as sourced from the dictionary, enabling a developer to more easily augment the spans with data of their choice. --- .../ChatCompletion/OpenTelemetryChatClient.cs | 34 ++++++------------- .../OpenTelemetryEmbeddingGenerator.cs | 26 ++++---------- .../OpenTelemetryConsts.cs | 4 --- .../OpenTelemetryChatClientTests.cs | 8 ++--- .../OpenTelemetryEmbeddingGeneratorTests.cs | 8 ++--- 5 files changed, 25 insertions(+), 55 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs index 234e56a68bc..f9215060dea 100644 --- a/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs +++ b/src/Libraries/Microsoft.Extensions.AI/ChatCompletion/OpenTelemetryChatClient.cs @@ -364,20 +364,13 @@ private static string SerializeChatMessages(IEnumerable messages, C } } - if (_providerName is not null) + // Log all additional request options as raw values on the span. + // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data. + if (EnableSensitiveData && options.AdditionalProperties is { } props) { - // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data - if (EnableSensitiveData && options.AdditionalProperties is { } props) + foreach (KeyValuePair prop in props) { - // Log all additional request options as per-provider tags. This is non-normative, but it covers cases where - // there's a per-provider specification in a best-effort manner (e.g. gen_ai.openai.request.service_tier), - // and more generally cases where there's additional useful information to be logged. - foreach (KeyValuePair prop in props) - { - _ = activity.AddTag( - OpenTelemetryConsts.GenAI.Request.PerProvider(_providerName, JsonNamingPolicy.SnakeCaseLower.ConvertName(prop.Key)), - prop.Value); - } + _ = activity.AddTag(prop.Key, prop.Value); } } } @@ -467,20 +460,13 @@ private void TraceResponse( _ = activity.AddTag(OpenTelemetryConsts.GenAI.Usage.OutputTokens, (int)outputTokens); } - if (_providerName is not null) + // Log all additional response properties as raw values on the span. + // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data. + if (EnableSensitiveData && response.AdditionalProperties is { } props) { - // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data - if (EnableSensitiveData && response.AdditionalProperties is { } props) + foreach (KeyValuePair prop in props) { - // Log all additional response properties as per-provider tags. This is non-normative, but it covers cases where - // there's a per-provider specification in a best-effort manner (e.g. gen_ai.openai.response.system_fingerprint), - // and more generally cases where there's additional useful information to be logged. - foreach (KeyValuePair prop in props) - { - _ = activity.AddTag( - OpenTelemetryConsts.GenAI.Response.PerProvider(_providerName, JsonNamingPolicy.SnakeCaseLower.ConvertName(prop.Key)), - prop.Value); - } + _ = activity.AddTag(prop.Key, prop.Value); } } } diff --git a/src/Libraries/Microsoft.Extensions.AI/Embeddings/OpenTelemetryEmbeddingGenerator.cs b/src/Libraries/Microsoft.Extensions.AI/Embeddings/OpenTelemetryEmbeddingGenerator.cs index 6db30409b26..402222feaa7 100644 --- a/src/Libraries/Microsoft.Extensions.AI/Embeddings/OpenTelemetryEmbeddingGenerator.cs +++ b/src/Libraries/Microsoft.Extensions.AI/Embeddings/OpenTelemetryEmbeddingGenerator.cs @@ -6,7 +6,6 @@ using System.Diagnostics; using System.Diagnostics.Metrics; using System.Linq; -using System.Text.Json; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -175,19 +174,13 @@ protected override void Dispose(bool disposing) _ = activity.AddTag(OpenTelemetryConsts.GenAI.Request.EmbeddingDimensions, dimensionsValue); } - // Log all additional request options as per-provider tags. This is non-normative, but it covers cases where - // there's a per-provider specification in a best-effort manner (e.g. gen_ai.openai.request.service_tier), - // and more generally cases where there's additional useful information to be logged. + // Log all additional request options as raw values on the span. // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data. - if (EnableSensitiveData && - _providerName is not null && - options?.AdditionalProperties is { } props) + if (EnableSensitiveData && options?.AdditionalProperties is { } props) { foreach (KeyValuePair prop in props) { - _ = activity.AddTag( - OpenTelemetryConsts.GenAI.Request.PerProvider(_providerName, JsonNamingPolicy.SnakeCaseLower.ConvertName(prop.Key)), - prop.Value); + _ = activity.AddTag(prop.Key, prop.Value); } } } @@ -255,18 +248,13 @@ private void TraceResponse( _ = activity.AddTag(OpenTelemetryConsts.GenAI.Response.Model, responseModelId); } - // Log all additional response properties as per-provider tags. This is non-normative, but it covers cases where - // there's a per-provider specification in a best-effort manner (e.g. gen_ai.openai.response.system_fingerprint), - // and more generally cases where there's additional useful information to be logged. - if (EnableSensitiveData && - _providerName is not null && - embeddings?.AdditionalProperties is { } props) + // Log all additional response properties as raw values on the span. + // Since AdditionalProperties has undefined meaning, we treat it as potentially sensitive data. + if (EnableSensitiveData && embeddings?.AdditionalProperties is { } props) { foreach (KeyValuePair prop in props) { - _ = activity.AddTag( - OpenTelemetryConsts.GenAI.Response.PerProvider(_providerName, JsonNamingPolicy.SnakeCaseLower.ConvertName(prop.Key)), - prop.Value); + _ = activity.AddTag(prop.Key, prop.Value); } } } diff --git a/src/Libraries/Microsoft.Extensions.AI/OpenTelemetryConsts.cs b/src/Libraries/Microsoft.Extensions.AI/OpenTelemetryConsts.cs index a1934faa526..faf7c32e4cb 100644 --- a/src/Libraries/Microsoft.Extensions.AI/OpenTelemetryConsts.cs +++ b/src/Libraries/Microsoft.Extensions.AI/OpenTelemetryConsts.cs @@ -90,8 +90,6 @@ public static class Request public const string Temperature = "gen_ai.request.temperature"; public const string TopK = "gen_ai.request.top_k"; public const string TopP = "gen_ai.request.top_p"; - - public static string PerProvider(string providerName, string parameterName) => $"{providerName}.request.{parameterName}"; } public static class Response @@ -99,8 +97,6 @@ public static class Response public const string FinishReasons = "gen_ai.response.finish_reasons"; public const string Id = "gen_ai.response.id"; public const string Model = "gen_ai.response.model"; - - public static string PerProvider(string providerName, string parameterName) => $"{providerName}.response.{parameterName}"; } public static class Token diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/OpenTelemetryChatClientTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/OpenTelemetryChatClientTests.cs index fcee53a68ee..4640b78f75d 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/OpenTelemetryChatClientTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/ChatCompletion/OpenTelemetryChatClientTests.cs @@ -163,16 +163,16 @@ async static IAsyncEnumerable CallbackAsync( Assert.Equal(7, activity.GetTagItem("gen_ai.request.top_k")); Assert.Equal(123, activity.GetTagItem("gen_ai.request.max_tokens")); Assert.Equal("""["hello", "world"]""", activity.GetTagItem("gen_ai.request.stop_sequences")); - Assert.Equal(enableSensitiveData ? "value1" : null, activity.GetTagItem("testservice.request.service_tier")); - Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("testservice.request.something_else")); + Assert.Equal(enableSensitiveData ? "value1" : null, activity.GetTagItem("service_tier")); + Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("SomethingElse")); Assert.Equal(42L, activity.GetTagItem("gen_ai.request.seed")); Assert.Equal("id123", activity.GetTagItem("gen_ai.response.id")); Assert.Equal("""["stop"]""", activity.GetTagItem("gen_ai.response.finish_reasons")); Assert.Equal(10, activity.GetTagItem("gen_ai.usage.input_tokens")); Assert.Equal(20, activity.GetTagItem("gen_ai.usage.output_tokens")); - Assert.Equal(enableSensitiveData ? "abcdefgh" : null, activity.GetTagItem("testservice.response.system_fingerprint")); - Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("testservice.response.and_something_else")); + Assert.Equal(enableSensitiveData ? "abcdefgh" : null, activity.GetTagItem("system_fingerprint")); + Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("AndSomethingElse")); Assert.True(activity.Duration.TotalMilliseconds > 0); diff --git a/test/Libraries/Microsoft.Extensions.AI.Tests/Embeddings/OpenTelemetryEmbeddingGeneratorTests.cs b/test/Libraries/Microsoft.Extensions.AI.Tests/Embeddings/OpenTelemetryEmbeddingGeneratorTests.cs index c3661f2c62b..c8419338e44 100644 --- a/test/Libraries/Microsoft.Extensions.AI.Tests/Embeddings/OpenTelemetryEmbeddingGeneratorTests.cs +++ b/test/Libraries/Microsoft.Extensions.AI.Tests/Embeddings/OpenTelemetryEmbeddingGeneratorTests.cs @@ -86,12 +86,12 @@ public async Task ExpectedInformationLogged_Async(string? perRequestModelId, boo Assert.Equal(expectedModelName, activity.GetTagItem("gen_ai.request.model")); Assert.Equal(1234, activity.GetTagItem("gen_ai.request.embedding.dimensions")); - Assert.Equal(enableSensitiveData ? "value1" : null, activity.GetTagItem("testservice.request.service_tier")); - Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("testservice.request.something_else")); + Assert.Equal(enableSensitiveData ? "value1" : null, activity.GetTagItem("service_tier")); + Assert.Equal(enableSensitiveData ? "value2" : null, activity.GetTagItem("SomethingElse")); Assert.Equal(10, activity.GetTagItem("gen_ai.usage.input_tokens")); - Assert.Equal(enableSensitiveData ? "abcdefgh" : null, activity.GetTagItem("testservice.response.system_fingerprint")); - Assert.Equal(enableSensitiveData ? "value3" : null, activity.GetTagItem("testservice.response.and_something_else")); + Assert.Equal(enableSensitiveData ? "abcdefgh" : null, activity.GetTagItem("system_fingerprint")); + Assert.Equal(enableSensitiveData ? "value3" : null, activity.GetTagItem("AndSomethingElse")); Assert.True(activity.Duration.TotalMilliseconds > 0); }