From c83125b7dc5d43b58c8f9f003312fe9714aacfe8 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 20 May 2024 13:53:48 +0100 Subject: [PATCH 01/30] Update BaggageActivityProcessor to use baggage key predicate --- src/OpenTelemetry.Extensions/README.md | 12 ++++++++-- .../Trace/BaggageActivityProcessor.cs | 24 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 8e4ca16c64..2dd0df7823 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -45,10 +45,18 @@ and adds the baggage keys and values to the `Activity` as tags (attributes) on s Add this activity processor to a tracer provider. -Example of adding BaggageActivityProcessor to `TracerProvider`: +For example, to add all baggage entries to new activities: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor() + .AddBaggageActivityProcessor(BaggageActivityProcessor.AllowAllBaggageKeys) .Build(); ``` + +Alternatively, you can provide a custom baggage key predicate to select which baggage keys you want to copy. + +For example, to only copy baggage entries that start with `my-key`: + +```cs +new AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key")) +``` diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index 23d08dec76..143c2d8d95 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -1,21 +1,41 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System; using System.Diagnostics; namespace OpenTelemetry.Trace; /// -/// Activity processor that adds fields to every new span. +/// Activity processor that adds fields to every new activity. /// internal sealed class BaggageActivityProcessor : BaseProcessor { + /// + /// A predicate that allows all baggage keys. + /// + public static readonly Predicate AllowAllBaggageKeys = _ => true; + + private readonly Predicate baggageKeyPredicate; + + /// + /// Initializes a new instance of the class. + /// + /// Predicate to determine which baggage keys should be added to the activity. + public BaggageActivityProcessor(Predicate baggageKeyPredicate) + { + this.baggageKeyPredicate = baggageKeyPredicate ?? throw new ArgumentNullException(nameof(baggageKeyPredicate)); + } + /// public override void OnStart(Activity activity) { foreach (var entry in Baggage.Current) { - activity.SetTag(entry.Key, entry.Value); + if (this.baggageKeyPredicate(entry.Key)) + { + activity.SetTag(entry.Key, entry.Value); + } } } } From 5882102eb60affbd138805b3a9f51d7ece0dc874 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 20 May 2024 14:00:39 +0100 Subject: [PATCH 02/30] add changelog entry --- src/OpenTelemetry.Extensions/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Extensions/CHANGELOG.md b/src/OpenTelemetry.Extensions/CHANGELOG.md index c71f4d8029..eb714c2a8c 100644 --- a/src/OpenTelemetry.Extensions/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* Update BaggageActivityProcessor to use baggage key predicate. + ([#1816](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1816)) + ## 1.0.0-beta.5 Released 2024-May-08 From f0197b344b5a3efaf7490a655c4d96e0bf222e48 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 20 May 2024 14:02:59 +0100 Subject: [PATCH 03/30] fix readme lint errors --- src/OpenTelemetry.Extensions/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 2dd0df7823..b897aac411 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -53,7 +53,8 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() .Build(); ``` -Alternatively, you can provide a custom baggage key predicate to select which baggage keys you want to copy. +Alternatively, you can provide a custom baggage key predicate to select which baggage keys +you want to copy. For example, to only copy baggage entries that start with `my-key`: From faf5890cc45489f8a54bf13778ee1c0198a9271f Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 20 May 2024 14:04:54 +0100 Subject: [PATCH 04/30] more linting --- src/OpenTelemetry.Extensions/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index b897aac411..65a019bcd8 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -53,8 +53,8 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() .Build(); ``` -Alternatively, you can provide a custom baggage key predicate to select which baggage keys -you want to copy. +Alternatively, you can provide a custom baggage key predicate to select which +baggage keys you want to copy. For example, to only copy baggage entries that start with `my-key`: From 1d29bdea2c2f27e4bd7ade07d49d510ecce9be96 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 20 May 2024 14:40:56 +0100 Subject: [PATCH 05/30] update extension to take predicate as param --- .../.publicApi/PublicAPI.Unshipped.txt | 2 +- .../TracerProviderBuilderExtensions.cs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt index 4b388d9972..45d27c77d4 100644 --- a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt @@ -11,4 +11,4 @@ OpenTelemetry.Logs.LogToActivityEventConversionOptions.StateConverter.set -> voi OpenTelemetry.Trace.TracerProviderBuilderExtensions static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AttachLogsToActivityEvent(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, System.Action? configure = null) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAutoFlushActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Func! predicate, int timeoutMilliseconds = 10000) -> OpenTelemetry.Trace.TracerProviderBuilder! -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder) -> OpenTelemetry.Trace.TracerProviderBuilder! +static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Predicate! baggageKeyPredicate) -> OpenTelemetry.Trace.TracerProviderBuilder! diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 5034cb11e7..86bffe04fd 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -53,14 +53,16 @@ public static TracerProviderBuilder AddAutoFlushActivityProcessor( /// Adds the to the . /// /// to add the to. + /// Predicate to determine which baggage keys should be added to the activity. /// The instance of to chain the calls. public static TracerProviderBuilder AddBaggageActivityProcessor( - this TracerProviderBuilder builder) + this TracerProviderBuilder builder, + Predicate baggageKeyPredicate) { Guard.ThrowIfNull(builder); #pragma warning disable CA2000 // Dispose objects before losing scope - return builder.AddProcessor(new BaggageActivityProcessor()); + return builder.AddProcessor(new BaggageActivityProcessor(baggageKeyPredicate)); #pragma warning restore CA2000 // Dispose objects before losing scope } } From e1f0a0c44857ecb8bc1c56b0f2f8a34274412cfa Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Tue, 21 May 2024 09:59:02 +0100 Subject: [PATCH 06/30] add regex extension method and example --- .../.publicApi/PublicAPI.Unshipped.txt | 1 + src/OpenTelemetry.Extensions/README.md | 15 ++++++++++++-- .../TracerProviderBuilderExtensions.cs | 20 +++++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt index 45d27c77d4..1e0f80c1fb 100644 --- a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt @@ -11,4 +11,5 @@ OpenTelemetry.Logs.LogToActivityEventConversionOptions.StateConverter.set -> voi OpenTelemetry.Trace.TracerProviderBuilderExtensions static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AttachLogsToActivityEvent(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, System.Action? configure = null) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAutoFlushActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Func! predicate, int timeoutMilliseconds = 10000) -> OpenTelemetry.Trace.TracerProviderBuilder! +static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, string! expr) -> OpenTelemetry.Trace.TracerProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Predicate! baggageKeyPredicate) -> OpenTelemetry.Trace.TracerProviderBuilder! diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 65a019bcd8..57413f2694 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -56,8 +56,19 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() Alternatively, you can provide a custom baggage key predicate to select which baggage keys you want to copy. -For example, to only copy baggage entries that start with `my-key`: +For example, to only copy baggage entries that start with `my-key` using a +regular expression: ```cs -new AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key")) + .AddBaggageActivityProcessor("^my-key") ``` + +For example, to only copy baggage entries that start with `my-key` using a +custom function: + +```cs + .AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key")) +``` + +Warning: The baggage key predicate is executed for every started activity. +Do not use slow or intensive operations. diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 86bffe04fd..7147e545da 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Text.RegularExpressions; using System.Threading; using OpenTelemetry.Internal; @@ -60,9 +61,28 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( Predicate baggageKeyPredicate) { Guard.ThrowIfNull(builder); + Guard.ThrowIfNull(baggageKeyPredicate); #pragma warning disable CA2000 // Dispose objects before losing scope return builder.AddProcessor(new BaggageActivityProcessor(baggageKeyPredicate)); +#pragma warning restore CA2000 // Dispose objects before losing scope + } + + /// + /// Adds the to the . + /// + /// to add the to. + /// Regular expression to determine which baggage keys should be added to the activity. + /// The instance of to chain the calls. + public static TracerProviderBuilder AddBaggageActivityProcessor( + this TracerProviderBuilder builder, string expr) + { + Guard.ThrowIfNull(builder); + Guard.ThrowIfNullOrEmpty(expr); + var regex = new Regex(expr, RegexOptions.Compiled); + +#pragma warning disable CA2000 // Dispose objects before losing scope + return builder.AddProcessor(new BaggageActivityProcessor(regex.IsMatch)); #pragma warning restore CA2000 // Dispose objects before losing scope } } From 7d22d69c0241ce6b6ea845135729d0c5cd071d39 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Wed, 22 May 2024 12:34:39 +0100 Subject: [PATCH 07/30] add tests --- src/OpenTelemetry.Extensions/README.md | 6 +- .../Trace/BaggageActivityProcessor.cs | 7 +- .../TracerProviderBuilderExtensions.cs | 9 +- .../Trace/BaggageSpanProcessorTests.cs | 90 +++++++++++++++++++ 4 files changed, 102 insertions(+), 10 deletions(-) create mode 100644 test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 57413f2694..71ea82c3bd 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -49,12 +49,12 @@ For example, to add all baggage entries to new activities: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor(BaggageActivityProcessor.AllowAllBaggageKeys) + .AddBaggageActivityProcessor("*") .Build(); ``` -Alternatively, you can provide a custom baggage key predicate to select which -baggage keys you want to copy. +Alternatively, you can select which baggage keys you want to copy using +a regular expression or custom predicate function. For example, to only copy baggage entries that start with `my-key` using a regular expression: diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index 143c2d8d95..d8819b1dde 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -11,18 +11,13 @@ namespace OpenTelemetry.Trace; /// internal sealed class BaggageActivityProcessor : BaseProcessor { - /// - /// A predicate that allows all baggage keys. - /// - public static readonly Predicate AllowAllBaggageKeys = _ => true; - private readonly Predicate baggageKeyPredicate; /// /// Initializes a new instance of the class. /// /// Predicate to determine which baggage keys should be added to the activity. - public BaggageActivityProcessor(Predicate baggageKeyPredicate) + internal BaggageActivityProcessor(Predicate baggageKeyPredicate) { this.baggageKeyPredicate = baggageKeyPredicate ?? throw new ArgumentNullException(nameof(baggageKeyPredicate)); } diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 7147e545da..9dceeb16d3 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -79,8 +79,15 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( { Guard.ThrowIfNull(builder); Guard.ThrowIfNullOrEmpty(expr); - var regex = new Regex(expr, RegexOptions.Compiled); + if (expr == "*") + { +#pragma warning disable CA2000 // Dispose objects before losing scope + return builder.AddProcessor(new BaggageActivityProcessor(_ => true)); +#pragma warning restore CA2000 // Dispose objects before losing scope + } + + var regex = new Regex(expr, RegexOptions.Compiled); #pragma warning disable CA2000 // Dispose objects before losing scope return builder.AddProcessor(new BaggageActivityProcessor(regex.IsMatch)); #pragma warning restore CA2000 // Dispose objects before losing scope diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs new file mode 100644 index 0000000000..609e197fc1 --- /dev/null +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -0,0 +1,90 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Diagnostics; +using System.Runtime.CompilerServices; +using OpenTelemetry.Tests; +using OpenTelemetry.Trace; +using Xunit; + +namespace OpenTelemetry.Extensions.Tests.Trace; + +public class BaggageSpanProcessorTests +{ + [Fact] + public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() + { + var activityProcessor = new TestActivityProcessor(); + var sourceName = GetTestMethodName(); + + using var provider = Sdk.CreateTracerProviderBuilder() + .AddProcessor(activityProcessor) + .AddBaggageActivityProcessor("*") + .AddSource(sourceName) + .Build(); + + Baggage.SetBaggage("key", "value"); + Baggage.SetBaggage("other_key", "other_value"); + + using var source = new ActivitySource(sourceName); + using var activity = source.StartActivity("name", ActivityKind.Server); + Assert.NotNull(activity); + activity.Stop(); + + Assert.Contains(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); + Assert.Contains(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); + } + + [Fact] + public void BaggageSpanProcessor_CanUseCustomPredicate() + { + var activityProcessor = new TestActivityProcessor(); + var sourceName = GetTestMethodName(); + + using var provider = Sdk.CreateTracerProviderBuilder() + .AddProcessor(activityProcessor) + .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key")) + .AddSource(sourceName) + .Build(); + + Baggage.SetBaggage("key", "value"); + Baggage.SetBaggage("other_key", "other_value"); + + using var source = new ActivitySource(sourceName); + using var activity = source.StartActivity("name", ActivityKind.Client); + Assert.NotNull(activity); + activity.Stop(); + + Assert.Contains(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); + Assert.DoesNotContain(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); + } + + [Fact] + public void BaggageSpanProcessor_CanUseRegex() + { + var activityProcessor = new TestActivityProcessor(); + var sourceName = GetTestMethodName(); + + using var provider = Sdk.CreateTracerProviderBuilder() + .AddProcessor(activityProcessor) + .AddBaggageActivityProcessor("^other.*") + .AddSource(sourceName) + .Build(); + + Baggage.SetBaggage("key", "value"); + Baggage.SetBaggage("other_key", "other_value"); + + using var source = new ActivitySource(sourceName); + using var activity = source.StartActivity("name", ActivityKind.Client); + Assert.NotNull(activity); + activity.Stop(); + + Assert.DoesNotContain(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); + Assert.Contains(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); + } + + private static string GetTestMethodName([CallerMemberName] string callingMethodName = "") + { + return callingMethodName; + } +} From 65d17e4f480007b0b360413cb8a9b1f39d941d3f Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Wed, 22 May 2024 12:40:23 +0100 Subject: [PATCH 08/30] add stringcompare in tst to resolve warning --- .../Trace/BaggageSpanProcessorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs index 609e197fc1..d98d2edddd 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -43,7 +43,7 @@ public void BaggageSpanProcessor_CanUseCustomPredicate() using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) - .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key")) + .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key", System.StringComparison.Ordinal)) .AddSource(sourceName) .Build(); From 94932d12c5bc1894706040a95b8650136bec2220 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 6 Jun 2024 16:04:50 +0100 Subject: [PATCH 09/30] remove regex extension method, move all keys to processor class --- .../.publicApi/PublicAPI.Unshipped.txt | 1 - .../Trace/BaggageActivityProcessor.cs | 11 +++++--- .../TracerProviderBuilderExtensions.cs | 26 ------------------- .../Trace/BaggageSpanProcessorTests.cs | 12 +++++---- 4 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt index 1e0f80c1fb..45d27c77d4 100644 --- a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt @@ -11,5 +11,4 @@ OpenTelemetry.Logs.LogToActivityEventConversionOptions.StateConverter.set -> voi OpenTelemetry.Trace.TracerProviderBuilderExtensions static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AttachLogsToActivityEvent(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, System.Action? configure = null) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAutoFlushActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Func! predicate, int timeoutMilliseconds = 10000) -> OpenTelemetry.Trace.TracerProviderBuilder! -static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, string! expr) -> OpenTelemetry.Trace.TracerProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Predicate! baggageKeyPredicate) -> OpenTelemetry.Trace.TracerProviderBuilder! diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index d8819b1dde..bd130b23be 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -9,7 +9,7 @@ namespace OpenTelemetry.Trace; /// /// Activity processor that adds fields to every new activity. /// -internal sealed class BaggageActivityProcessor : BaseProcessor +public sealed class BaggageActivityProcessor : BaseProcessor { private readonly Predicate baggageKeyPredicate; @@ -22,14 +22,19 @@ internal BaggageActivityProcessor(Predicate baggageKeyPredicate) this.baggageKeyPredicate = baggageKeyPredicate ?? throw new ArgumentNullException(nameof(baggageKeyPredicate)); } + /// + /// Gets a predicate that returns true for all baggage keys. + /// + public static Predicate AllBaggageKeys => (baggageKey) => true; + /// - public override void OnStart(Activity activity) + public override void OnStart(Activity data) { foreach (var entry in Baggage.Current) { if (this.baggageKeyPredicate(entry.Key)) { - activity.SetTag(entry.Key, entry.Value); + data?.SetTag(entry.Key, entry.Value); } } } diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 9dceeb16d3..36aea231fb 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -3,7 +3,6 @@ using System; using System.Diagnostics; -using System.Text.RegularExpressions; using System.Threading; using OpenTelemetry.Internal; @@ -65,31 +64,6 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( #pragma warning disable CA2000 // Dispose objects before losing scope return builder.AddProcessor(new BaggageActivityProcessor(baggageKeyPredicate)); -#pragma warning restore CA2000 // Dispose objects before losing scope - } - - /// - /// Adds the to the . - /// - /// to add the to. - /// Regular expression to determine which baggage keys should be added to the activity. - /// The instance of to chain the calls. - public static TracerProviderBuilder AddBaggageActivityProcessor( - this TracerProviderBuilder builder, string expr) - { - Guard.ThrowIfNull(builder); - Guard.ThrowIfNullOrEmpty(expr); - - if (expr == "*") - { -#pragma warning disable CA2000 // Dispose objects before losing scope - return builder.AddProcessor(new BaggageActivityProcessor(_ => true)); -#pragma warning restore CA2000 // Dispose objects before losing scope - } - - var regex = new Regex(expr, RegexOptions.Compiled); -#pragma warning disable CA2000 // Dispose objects before losing scope - return builder.AddProcessor(new BaggageActivityProcessor(regex.IsMatch)); #pragma warning restore CA2000 // Dispose objects before losing scope } } diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs index d98d2edddd..2f759bef03 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -3,6 +3,7 @@ using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; @@ -19,7 +20,7 @@ public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) - .AddBaggageActivityProcessor("*") + .AddBaggageActivityProcessor(BaggageActivityProcessor.AllBaggageKeys) .AddSource(sourceName) .Build(); @@ -65,13 +66,14 @@ public void BaggageSpanProcessor_CanUseRegex() var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); + var regex = new Regex("^mykey"); using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) - .AddBaggageActivityProcessor("^other.*") + .AddBaggageActivityProcessor((baggageKey) => regex.IsMatch(baggageKey)) .AddSource(sourceName) .Build(); - Baggage.SetBaggage("key", "value"); + Baggage.SetBaggage("mykey", "value"); Baggage.SetBaggage("other_key", "other_value"); using var source = new ActivitySource(sourceName); @@ -79,8 +81,8 @@ public void BaggageSpanProcessor_CanUseRegex() Assert.NotNull(activity); activity.Stop(); - Assert.DoesNotContain(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); - Assert.Contains(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); + Assert.Contains(activity.Tags, kv => kv.Key == "mykey" && kv.Value == "value"); + Assert.DoesNotContain(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); } private static string GetTestMethodName([CallerMemberName] string callingMethodName = "") From d2d7ab0624b4fff75fd0048ff5ecf2708913429a Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 6 Jun 2024 16:07:01 +0100 Subject: [PATCH 10/30] wrap baggage key invovationto prevent exceptions leaking --- .../TracerProviderBuilderExtensions.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 36aea231fb..67fe05fd8f 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -63,7 +63,18 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( Guard.ThrowIfNull(baggageKeyPredicate); #pragma warning disable CA2000 // Dispose objects before losing scope - return builder.AddProcessor(new BaggageActivityProcessor(baggageKeyPredicate)); + return builder.AddProcessor(new BaggageActivityProcessor(baggageKey => + { + try + { + return baggageKeyPredicate(baggageKey); + } + catch (Exception ex) + { + OpenTelemetrySdkEventSource.Log.BaggageActivityProcessorException(ex); + return false; + } + })); #pragma warning restore CA2000 // Dispose objects before losing scope } } From 603a7a05a46a0d46bcb7a0aa875d5f98545c2711 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Thu, 6 Jun 2024 16:30:15 +0100 Subject: [PATCH 11/30] clean up readme examples --- src/OpenTelemetry.Extensions/README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 71ea82c3bd..2df4098488 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -49,7 +49,7 @@ For example, to add all baggage entries to new activities: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor("*") + .AddBaggageActivityProcessor(BaggageSpanProcessor.AllBaggageKeys) .Build(); ``` @@ -57,17 +57,19 @@ Alternatively, you can select which baggage keys you want to copy using a regular expression or custom predicate function. For example, to only copy baggage entries that start with `my-key` using a -regular expression: +custom function: ```cs - .AddBaggageActivityProcessor("^my-key") + .AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key", System.StringComparison.Ordinal)) ``` For example, to only copy baggage entries that start with `my-key` using a -custom function: +regular expression: ```cs - .AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key")) +var regex = new Regex("^mykey"); +/// ... + .AddBaggageActivityProcessor((baggageKey) => (baggageKey) => regex.IsMatch(baggageKey)) ``` Warning: The baggage key predicate is executed for every started activity. From 798c6dee03201e29c3cb8787a6754a26677d5c6b Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:09:07 +0100 Subject: [PATCH 12/30] use builder pattern in extention method --- .../TracerProviderBuilderExtensions.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 67fe05fd8f..234c81bcf4 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -62,19 +62,17 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( Guard.ThrowIfNull(builder); Guard.ThrowIfNull(baggageKeyPredicate); -#pragma warning disable CA2000 // Dispose objects before losing scope - return builder.AddProcessor(new BaggageActivityProcessor(baggageKey => + return builder.AddProcessor(b => new BaggageActivityProcessor(baggageKey => { try { return baggageKeyPredicate(baggageKey); } - catch (Exception ex) + catch (Exception) { - OpenTelemetrySdkEventSource.Log.BaggageActivityProcessorException(ex); + // OpenTelemetrySdkEventSource.Log.BaggageActivityProcessorException(ex); return false; } })); -#pragma warning restore CA2000 // Dispose objects before losing scope } } From ca4a71b02f79b1f4091b9584ffcee7889be44aad Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:31:19 +0100 Subject: [PATCH 13/30] update all baggage keys name --- src/OpenTelemetry.Extensions/README.md | 15 +++++++++------ .../Trace/BaggageActivityProcessor.cs | 2 +- .../Trace/BaggageSpanProcessorTests.cs | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 2df4098488..9274038cba 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -49,18 +49,20 @@ For example, to add all baggage entries to new activities: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor(BaggageSpanProcessor.AllBaggageKeys) + .AddBaggageActivityProcessor(BaggageSpanProcessor.AllowAllBaggageKeys) .Build(); ``` -Alternatively, you can select which baggage keys you want to copy using -a regular expression or custom predicate function. +Alternatively, you can select which baggage keys you want to copy using a +custom predicate function. For example, to only copy baggage entries that start with `my-key` using a custom function: ```cs - .AddBaggageActivityProcessor(baggageKey => baggageKey.StartWith("my-key", System.StringComparison.Ordinal)) +var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartWith("my-key", System.StringComparison.Ordinal)) + .Build(); ``` For example, to only copy baggage entries that start with `my-key` using a @@ -68,8 +70,9 @@ regular expression: ```cs var regex = new Regex("^mykey"); -/// ... - .AddBaggageActivityProcessor((baggageKey) => (baggageKey) => regex.IsMatch(baggageKey)) +var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddBaggageActivityProcessor((baggageKey) => regex.IsMatch(baggageKey)) + .Build(); ``` Warning: The baggage key predicate is executed for every started activity. diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index bd130b23be..aa6e70e949 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -25,7 +25,7 @@ internal BaggageActivityProcessor(Predicate baggageKeyPredicate) /// /// Gets a predicate that returns true for all baggage keys. /// - public static Predicate AllBaggageKeys => (baggageKey) => true; + public static Predicate AllowAllBaggageKeys => (baggageKey) => true; /// public override void OnStart(Activity data) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs index 2f759bef03..7ca76ab8ba 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -20,7 +20,7 @@ public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) - .AddBaggageActivityProcessor(BaggageActivityProcessor.AllBaggageKeys) + .AddBaggageActivityProcessor(BaggageActivityProcessor.AllowAllBaggageKeys) .AddSource(sourceName) .Build(); From 20dce8315010fb2015ba4dad9214bb38ddb848f7 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:31:38 +0100 Subject: [PATCH 14/30] add baggage activity error to event source --- .../OpenTelemetryExtensionsEventSource.cs | 6 +++++ .../TracerProviderBuilderExtensions.cs | 4 ++-- .../Trace/BaggageSpanProcessorTests.cs | 23 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs index 0e56043645..d65b2cdd8e 100644 --- a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs @@ -44,4 +44,10 @@ public void LogRecordFilterException(string? categoryName, string exception) { this.WriteEvent(2, categoryName, exception); } + + [Event(1, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Error)] + public void BaggageKeyPredicateException(string exception) + { + this.WriteEvent(1, exception); + } } diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index 234c81bcf4..ca8fb50b87 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -68,9 +68,9 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( { return baggageKeyPredicate(baggageKey); } - catch (Exception) + catch (Exception exception) { - // OpenTelemetrySdkEventSource.Log.BaggageActivityProcessorException(ex); + OpenTelemetryExtensionsEventSource.Log.BaggageKeyPredicateException(exception.Message); return false; } })); diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs index 7ca76ab8ba..f673b6bed1 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -1,6 +1,7 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +using System; using System.Diagnostics; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; @@ -85,6 +86,28 @@ public void BaggageSpanProcessor_CanUseRegex() Assert.DoesNotContain(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); } + [Fact] + public void BaggageSpanProcessor_PredicateThrows_DoesNothing() + { + var activityProcessor = new TestActivityProcessor(); + var sourceName = GetTestMethodName(); + + using var provider = Sdk.CreateTracerProviderBuilder() + .AddProcessor(activityProcessor) + .AddBaggageActivityProcessor(_ => throw new Exception("Predicate throws an exception.")) + .AddSource(sourceName) + .Build(); + + Baggage.SetBaggage("key", "value"); + + using var source = new ActivitySource(sourceName); + using var activity = source.StartActivity("name", ActivityKind.Server); + Assert.NotNull(activity); + activity.Stop(); + + Assert.DoesNotContain(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); + } + private static string GetTestMethodName([CallerMemberName] string callingMethodName = "") { return callingMethodName; From aa70c90863ec3bb88baddf109242b8497b8d7227 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:47:59 +0100 Subject: [PATCH 15/30] update public api --- .../.publicApi/PublicAPI.Unshipped.txt | 3 +++ src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt index 45d27c77d4..76f352124e 100644 --- a/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Extensions/.publicApi/PublicAPI.Unshipped.txt @@ -8,7 +8,10 @@ OpenTelemetry.Logs.LogToActivityEventConversionOptions.ScopeConverter.get -> Sys OpenTelemetry.Logs.LogToActivityEventConversionOptions.ScopeConverter.set -> void OpenTelemetry.Logs.LogToActivityEventConversionOptions.StateConverter.get -> System.Action>!>! OpenTelemetry.Logs.LogToActivityEventConversionOptions.StateConverter.set -> void +OpenTelemetry.Trace.BaggageActivityProcessor OpenTelemetry.Trace.TracerProviderBuilderExtensions +override OpenTelemetry.Trace.BaggageActivityProcessor.OnStart(System.Diagnostics.Activity! data) -> void static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AttachLogsToActivityEvent(this OpenTelemetry.Logs.OpenTelemetryLoggerOptions! loggerOptions, System.Action? configure = null) -> OpenTelemetry.Logs.OpenTelemetryLoggerOptions! +static OpenTelemetry.Trace.BaggageActivityProcessor.AllowAllBaggageKeys.get -> System.Predicate! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddAutoFlushActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Func! predicate, int timeoutMilliseconds = 10000) -> OpenTelemetry.Trace.TracerProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddBaggageActivityProcessor(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Predicate! baggageKeyPredicate) -> OpenTelemetry.Trace.TracerProviderBuilder! diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index aa6e70e949..8d4a2eefea 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -25,7 +25,7 @@ internal BaggageActivityProcessor(Predicate baggageKeyPredicate) /// /// Gets a predicate that returns true for all baggage keys. /// - public static Predicate AllowAllBaggageKeys => (baggageKey) => true; + public static Predicate AllowAllBaggageKeys => (_) => true; /// public override void OnStart(Activity data) From d7d98785aaa72913f231c923665da267de82c144 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:55:00 +0100 Subject: [PATCH 16/30] docs cleanup --- src/OpenTelemetry.Extensions/README.md | 14 +++++++------- .../Trace/BaggageActivityProcessor.cs | 2 +- .../Trace/BaggageSpanProcessorTests.cs | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 9274038cba..d0e8ec9732 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -49,15 +49,15 @@ For example, to add all baggage entries to new activities: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor(BaggageSpanProcessor.AllowAllBaggageKeys) + .AddBaggageActivityProcessor(BaggageActivityProcessor.AllowAllBaggageKeys) .Build(); ``` Alternatively, you can select which baggage keys you want to copy using a custom predicate function. -For example, to only copy baggage entries that start with `my-key` using a -custom function: +For example, to only copy baggage entries where the key start with `my-key` +using a custom function: ```cs var tracerProvider = Sdk.CreateTracerProviderBuilder() @@ -65,13 +65,13 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() .Build(); ``` -For example, to only copy baggage entries that start with `my-key` using a -regular expression: +For example, to only copy baggage entries where the key matches the regular +expression `^my-key`: ```cs -var regex = new Regex("^mykey"); +var baggageKeyRegex = new Regex("^mykey", RegexOptions.Compiled); var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddBaggageActivityProcessor((baggageKey) => regex.IsMatch(baggageKey)) + .AddBaggageActivityProcessor((baggageKey) => baggageKeyRegex.IsMatch(baggageKey)) .Build(); ``` diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index 8d4a2eefea..07c50b0425 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -23,7 +23,7 @@ internal BaggageActivityProcessor(Predicate baggageKeyPredicate) } /// - /// Gets a predicate that returns true for all baggage keys. + /// Gets a baggage key predicate that returns true for all baggage keys. /// public static Predicate AllowAllBaggageKeys => (_) => true; diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs index f673b6bed1..f09e897502 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs @@ -67,7 +67,7 @@ public void BaggageSpanProcessor_CanUseRegex() var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); - var regex = new Regex("^mykey"); + var regex = new Regex("^mykey", RegexOptions.Compiled); using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) .AddBaggageActivityProcessor((baggageKey) => regex.IsMatch(baggageKey)) From 7a8609678acd448b2aa6a4597a2a830baf04c1e9 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:55:50 +0100 Subject: [PATCH 17/30] update changelog --- src/OpenTelemetry.Extensions/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/CHANGELOG.md b/src/OpenTelemetry.Extensions/CHANGELOG.md index eb714c2a8c..c1df059f81 100644 --- a/src/OpenTelemetry.Extensions/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* Update BaggageActivityProcessor to use baggage key predicate. +* Update BaggageActivityProcessor to require baggage key predicate. ([#1816](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1816)) ## 1.0.0-beta.5 From e834e707320cf449653ac19303f20fe530914b69 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:57:26 +0100 Subject: [PATCH 18/30] rename test file --- ...geSpanProcessorTests.cs => BaggageActivityProcessorTests.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/OpenTelemetry.Extensions.Tests/Trace/{BaggageSpanProcessorTests.cs => BaggageActivityProcessorTests.cs} (98%) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs similarity index 98% rename from test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs rename to test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index f09e897502..303909de29 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageSpanProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -11,7 +11,7 @@ namespace OpenTelemetry.Extensions.Tests.Trace; -public class BaggageSpanProcessorTests +public class BaggageActivityProcessorTests { [Fact] public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() From bd28cde1edb426ff866f8f5798a4bb657205db98 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 10:59:09 +0100 Subject: [PATCH 19/30] update test names --- .../Trace/BaggageActivityProcessorTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index 303909de29..a10f4109c2 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -14,7 +14,7 @@ namespace OpenTelemetry.Extensions.Tests.Trace; public class BaggageActivityProcessorTests { [Fact] - public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() + public void BaggageActivityProcessor_CanAddAlloAllBaggageKeysPredicate() { var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); @@ -38,7 +38,7 @@ public void BaggageSpanProcessor_CanAddAlloAllBaggageKeysPredicate() } [Fact] - public void BaggageSpanProcessor_CanUseCustomPredicate() + public void BaggageActivityProcessor_CanUseCustomPredicate() { var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); @@ -62,7 +62,7 @@ public void BaggageSpanProcessor_CanUseCustomPredicate() } [Fact] - public void BaggageSpanProcessor_CanUseRegex() + public void BaggageActivityProcessor_CanUseRegex() { var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); @@ -87,7 +87,7 @@ public void BaggageSpanProcessor_CanUseRegex() } [Fact] - public void BaggageSpanProcessor_PredicateThrows_DoesNothing() + public void BaggageActivityProcessor_PredicateThrows_DoesNothing() { var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); From 3ccca82668b9efefbdb471069068edd6e2b74b3e Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Fri, 7 Jun 2024 11:27:02 +0100 Subject: [PATCH 20/30] fix lint errors --- .../Trace/BaggageActivityProcessorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index a10f4109c2..0e294c472a 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -45,7 +45,7 @@ public void BaggageActivityProcessor_CanUseCustomPredicate() using var provider = Sdk.CreateTracerProviderBuilder() .AddProcessor(activityProcessor) - .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key", System.StringComparison.Ordinal)) + .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key", StringComparison.Ordinal)) .AddSource(sourceName) .Build(); From 6c114a3cffb38eecf1d8250bca175271b56a8d09 Mon Sep 17 00:00:00 2001 From: Mike Goldsmith Date: Fri, 7 Jun 2024 15:55:46 +0100 Subject: [PATCH 21/30] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Piotr Kiełkowicz --- .../Internal/OpenTelemetryExtensionsEventSource.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs index d65b2cdd8e..ce7606fa29 100644 --- a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs @@ -45,9 +45,9 @@ public void LogRecordFilterException(string? categoryName, string exception) this.WriteEvent(2, categoryName, exception); } - [Event(1, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Error)] + [Event(3, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Error)] public void BaggageKeyPredicateException(string exception) { - this.WriteEvent(1, exception); + this.WriteEvent(3, exception); } } From 8b7641236eb9483085f2036b7a473ee13050fe6b Mon Sep 17 00:00:00 2001 From: Mike Goldsmith Date: Wed, 12 Jun 2024 09:50:45 +0100 Subject: [PATCH 22/30] Apply suggestions from code review Co-authored-by: Cijo Thomas --- .../Internal/OpenTelemetryExtensionsEventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs index ce7606fa29..f41469087e 100644 --- a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs @@ -45,7 +45,7 @@ public void LogRecordFilterException(string? categoryName, string exception) this.WriteEvent(2, categoryName, exception); } - [Event(3, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Error)] + [Event(3, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Warning)] public void BaggageKeyPredicateException(string exception) { this.WriteEvent(3, exception); From f147ef9b1b9c803c48254a4302adc007e8ef92b4 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Wed, 12 Jun 2024 09:50:00 +0100 Subject: [PATCH 23/30] tweak warning in readme --- src/OpenTelemetry.Extensions/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 6f61fbfc00..4168afb59a 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -76,5 +76,5 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() .Build(); ``` -Warning: The baggage key predicate is executed for every started activity. +Warning: The baggage key predicate is executed for every baggage entry for each started activity. Do not use slow or intensive operations. From bc08c17ae39a79a6b87c282d6e109ad151e47142 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Wed, 12 Jun 2024 09:55:49 +0100 Subject: [PATCH 24/30] update exception message --- .../Internal/OpenTelemetryExtensionsEventSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs index f41469087e..c3bffa0c41 100644 --- a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs @@ -45,7 +45,7 @@ public void LogRecordFilterException(string? categoryName, string exception) this.WriteEvent(2, categoryName, exception); } - [Event(3, Message = "Baggage key predicate function threw exeption: '{0}'", Level = EventLevel.Warning)] + [Event(3, Message = "Baggage key predicate function threw exeption, no baggage entries will not be added to the activity. Exception: '{0}'", Level = EventLevel.Warning)] public void BaggageKeyPredicateException(string exception) { this.WriteEvent(3, exception); From 6661810db7e51cd280d66f79baaaaf799b77d028 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Wed, 12 Jun 2024 10:04:05 +0100 Subject: [PATCH 25/30] fix linter error --- src/OpenTelemetry.Extensions/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Extensions/README.md b/src/OpenTelemetry.Extensions/README.md index 4168afb59a..38dde99d1f 100644 --- a/src/OpenTelemetry.Extensions/README.md +++ b/src/OpenTelemetry.Extensions/README.md @@ -76,5 +76,6 @@ var tracerProvider = Sdk.CreateTracerProviderBuilder() .Build(); ``` -Warning: The baggage key predicate is executed for every baggage entry for each started activity. +Warning: The baggage key predicate is executed for every baggage entry for each +started activity. Do not use slow or intensive operations. From 3239b2ed4fc66509c29d38caa67abf5be3666cb0 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 17 Jun 2024 14:10:22 +0100 Subject: [PATCH 26/30] fix test name --- .../Trace/BaggageActivityProcessorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index 0e294c472a..8d8b8ec920 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -14,7 +14,7 @@ namespace OpenTelemetry.Extensions.Tests.Trace; public class BaggageActivityProcessorTests { [Fact] - public void BaggageActivityProcessor_CanAddAlloAllBaggageKeysPredicate() + public void BaggageActivityProcessor_CanAddAllowAllBaggageKeysPredicate() { var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); From 19fda8cc0eea9d6d9af678672b8199455d9a1c9c Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 17 Jun 2024 14:11:42 +0100 Subject: [PATCH 27/30] remove unused TestActivityProcessor in tests --- .../Trace/BaggageActivityProcessorTests.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index 8d8b8ec920..c015ece3a4 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; -using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; @@ -16,11 +15,9 @@ public class BaggageActivityProcessorTests [Fact] public void BaggageActivityProcessor_CanAddAllowAllBaggageKeysPredicate() { - var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); using var provider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor) .AddBaggageActivityProcessor(BaggageActivityProcessor.AllowAllBaggageKeys) .AddSource(sourceName) .Build(); @@ -40,11 +37,9 @@ public void BaggageActivityProcessor_CanAddAllowAllBaggageKeysPredicate() [Fact] public void BaggageActivityProcessor_CanUseCustomPredicate() { - var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); using var provider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor) .AddBaggageActivityProcessor((baggageKey) => baggageKey.StartsWith("key", StringComparison.Ordinal)) .AddSource(sourceName) .Build(); @@ -64,12 +59,10 @@ public void BaggageActivityProcessor_CanUseCustomPredicate() [Fact] public void BaggageActivityProcessor_CanUseRegex() { - var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); var regex = new Regex("^mykey", RegexOptions.Compiled); using var provider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor) .AddBaggageActivityProcessor((baggageKey) => regex.IsMatch(baggageKey)) .AddSource(sourceName) .Build(); @@ -89,11 +82,9 @@ public void BaggageActivityProcessor_CanUseRegex() [Fact] public void BaggageActivityProcessor_PredicateThrows_DoesNothing() { - var activityProcessor = new TestActivityProcessor(); var sourceName = GetTestMethodName(); using var provider = Sdk.CreateTracerProviderBuilder() - .AddProcessor(activityProcessor) .AddBaggageActivityProcessor(_ => throw new Exception("Predicate throws an exception.")) .AddSource(sourceName) .Build(); From 325f39f0836150f163c088451fe13de2b69ba37f Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 17 Jun 2024 14:22:20 +0100 Subject: [PATCH 28/30] add test to verify only entries that throw are dropped --- .../Trace/BaggageActivityProcessorTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index c015ece3a4..a64c64da4a 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -99,6 +99,40 @@ public void BaggageActivityProcessor_PredicateThrows_DoesNothing() Assert.DoesNotContain(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); } + [Fact] + public void BaggageActivityProcessor_PredicateThrows_OnlyDropsEntriesThatThrow() + { + var sourceName = GetTestMethodName(); + + // First call to predicate should not throw, second call should. + using var provider = Sdk.CreateTracerProviderBuilder() + .AddBaggageActivityProcessor(key => + { + if (key == "key") + { + throw new Exception("Predicate throws an exception."); + } + + return true; + }) + .AddSource(sourceName) + .Build(); + + Baggage.SetBaggage("key", "value"); + Baggage.SetBaggage("other_key", "other_value"); + Baggage.SetBaggage("another_key", "another_value"); + + using var source = new ActivitySource(sourceName); + using var activity = source.StartActivity("name", ActivityKind.Server); + Assert.NotNull(activity); + activity.Stop(); + + // Only keys that do not throw should be added. + Assert.DoesNotContain(activity.Tags, kv => kv.Key == "key" && kv.Value == "value"); + Assert.Contains(activity.Tags, kv => kv.Key == "other_key" && kv.Value == "other_value"); + Assert.Contains(activity.Tags, kv => kv.Key == "another_key" && kv.Value == "another_value"); + } + private static string GetTestMethodName([CallerMemberName] string callingMethodName = "") { return callingMethodName; From 87478e0c629af630e1aaf7a18ad0e497ebcf3f7a Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Mon, 17 Jun 2024 14:33:51 +0100 Subject: [PATCH 29/30] update baggage key event source message with bagage key --- .../Internal/OpenTelemetryExtensionsEventSource.cs | 6 +++--- .../TracerProviderBuilderExtensions.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs index c3bffa0c41..900543b75a 100644 --- a/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs +++ b/src/OpenTelemetry.Extensions/Internal/OpenTelemetryExtensionsEventSource.cs @@ -45,9 +45,9 @@ public void LogRecordFilterException(string? categoryName, string exception) this.WriteEvent(2, categoryName, exception); } - [Event(3, Message = "Baggage key predicate function threw exeption, no baggage entries will not be added to the activity. Exception: '{0}'", Level = EventLevel.Warning)] - public void BaggageKeyPredicateException(string exception) + [Event(3, Message = "Baggage key predicate threw exeption when trying to add baggage entry with key '{0}'. Baggage entry will not be added to the activity. Exception: '{1}'", Level = EventLevel.Warning)] + public void BaggageKeyPredicateException(string baggageKey, string exception) { - this.WriteEvent(3, exception); + this.WriteEvent(3, baggageKey, exception); } } diff --git a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs index ca8fb50b87..3a93575b5a 100644 --- a/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Extensions/TracerProviderBuilderExtensions.cs @@ -70,7 +70,7 @@ public static TracerProviderBuilder AddBaggageActivityProcessor( } catch (Exception exception) { - OpenTelemetryExtensionsEventSource.Log.BaggageKeyPredicateException(exception.Message); + OpenTelemetryExtensionsEventSource.Log.BaggageKeyPredicateException(baggageKey, exception.Message); return false; } })); From ecf316065bc452786bbd07b0258b0c5b4a777ea6 Mon Sep 17 00:00:00 2001 From: Mike Goldsmth Date: Tue, 16 Jul 2024 19:32:04 +0100 Subject: [PATCH 30/30] remove unused usings --- src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs | 1 - .../Trace/BaggageActivityProcessorTests.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs index 07c50b0425..e85845b197 100644 --- a/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs +++ b/src/OpenTelemetry.Extensions/Trace/BaggageActivityProcessor.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System; using System.Diagnostics; namespace OpenTelemetry.Trace; diff --git a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs index a64c64da4a..1f96b7c72d 100644 --- a/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs +++ b/test/OpenTelemetry.Extensions.Tests/Trace/BaggageActivityProcessorTests.cs @@ -1,7 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -using System; using System.Diagnostics; using System.Runtime.CompilerServices; using System.Text.RegularExpressions;