diff --git a/src/OpenTelemetry.Extensions.Enrichment/CHANGELOG.md b/src/OpenTelemetry.Extensions.Enrichment/CHANGELOG.md index 72b204df1f..120f6564ef 100644 --- a/src/OpenTelemetry.Extensions.Enrichment/CHANGELOG.md +++ b/src/OpenTelemetry.Extensions.Enrichment/CHANGELOG.md @@ -5,6 +5,10 @@ * Updated OpenTelemetry core component version(s) to `1.15.2`. ([#4080](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4080)) +* Fixed trace enrichment callbacks so exceptions thrown by user-provided + enrichers or enrichment actions no longer interrupt trace processing. + ([#4165](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/4165)) + ## 1.15.0-beta.1 Released 2026-Jan-21 diff --git a/src/OpenTelemetry.Extensions.Enrichment/Internal/EnrichmentEventSource.cs b/src/OpenTelemetry.Extensions.Enrichment/Internal/EnrichmentEventSource.cs new file mode 100644 index 0000000000..77157ac417 --- /dev/null +++ b/src/OpenTelemetry.Extensions.Enrichment/Internal/EnrichmentEventSource.cs @@ -0,0 +1,54 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Diagnostics.Tracing; +using OpenTelemetry.Internal; + +namespace OpenTelemetry.Extensions.Enrichment; + +[EventSource(Name = "OpenTelemetry-Extensions-Enrichment")] +internal sealed class EnrichmentEventSource : EventSource +{ + public static EnrichmentEventSource Log = new(); + + private EnrichmentEventSource() + { + } + + [NonEvent] + public void TraceEnricherException(string operationName, TraceEnricher enricher, Exception ex) + { + if (this.IsEnabled(EventLevel.Warning, EventKeywords.All)) + { + var enricherType = enricher.GetType(); + this.TraceEnricherException(operationName, enricherType.FullName ?? enricherType.Name, ex.ToInvariantString()); + } + } + + [NonEvent] + public void TraceEnrichmentActionException(Action action, Exception ex) + { + if (this.IsEnabled(EventLevel.Warning, EventKeywords.All)) + { + var method = action.Method; + var declaringType = method.DeclaringType; + var actionName = declaringType is null + ? method.Name + : $"{declaringType.FullName ?? declaringType.Name}.{method.Name}"; + + this.TraceEnrichmentActionException(actionName, ex.ToInvariantString()); + } + } + + [Event(1, Message = "Trace enricher '{0}' threw during '{1}'. Trace processing will continue. Exception: '{2}'.", Level = EventLevel.Warning)] + public void TraceEnricherException(string enricherType, string operationName, string exception) + { + this.WriteEvent(1, enricherType, operationName, exception); + } + + [Event(2, Message = "Trace enrichment action '{0}' threw. Trace processing will continue. Exception: '{1}'.", Level = EventLevel.Warning)] + public void TraceEnrichmentActionException(string actionName, string exception) + { + this.WriteEvent(2, actionName, exception); + } +} diff --git a/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentActions.cs b/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentActions.cs index 2988495385..293cb4acd6 100644 --- a/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentActions.cs +++ b/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentActions.cs @@ -18,7 +18,15 @@ public override void Enrich(in TraceEnrichmentBag bag) { for (var i = 0; i < this.actions.Length; i++) { - this.actions[i].Invoke(bag); + var action = this.actions[i]; + try + { + action.Invoke(bag); + } + catch (Exception ex) + { + EnrichmentEventSource.Log.TraceEnrichmentActionException(action, ex); + } } } } diff --git a/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentProcessor.cs b/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentProcessor.cs index 624fe824e0..abe275e2e2 100644 --- a/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentProcessor.cs +++ b/src/OpenTelemetry.Extensions.Enrichment/Internal/TraceEnrichmentProcessor.cs @@ -22,7 +22,14 @@ public override void OnStart(Activity activity) foreach (var enricher in this.traceEnrichers) { - enricher.EnrichOnActivityStart(bag); + try + { + enricher.EnrichOnActivityStart(bag); + } + catch (Exception ex) + { + EnrichmentEventSource.Log.TraceEnricherException(nameof(this.OnStart), enricher, ex); + } } } @@ -32,7 +39,14 @@ public override void OnEnd(Activity activity) foreach (var enricher in this.traceEnrichers) { - enricher.Enrich(bag); + try + { + enricher.Enrich(bag); + } + catch (Exception ex) + { + EnrichmentEventSource.Log.TraceEnricherException(nameof(this.OnEnd), enricher, ex); + } } } } diff --git a/src/OpenTelemetry.Extensions.Enrichment/OpenTelemetry.Extensions.Enrichment.csproj b/src/OpenTelemetry.Extensions.Enrichment/OpenTelemetry.Extensions.Enrichment.csproj index 7502016984..947f803101 100644 --- a/src/OpenTelemetry.Extensions.Enrichment/OpenTelemetry.Extensions.Enrichment.csproj +++ b/src/OpenTelemetry.Extensions.Enrichment/OpenTelemetry.Extensions.Enrichment.csproj @@ -23,6 +23,7 @@ + diff --git a/test/OpenTelemetry.Extensions.Enrichment.Tests/EventSourceTests.cs b/test/OpenTelemetry.Extensions.Enrichment.Tests/EventSourceTests.cs new file mode 100644 index 0000000000..ecaae95216 --- /dev/null +++ b/test/OpenTelemetry.Extensions.Enrichment.Tests/EventSourceTests.cs @@ -0,0 +1,17 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using OpenTelemetry.Tests; +using Xunit; + +namespace OpenTelemetry.Extensions.Enrichment.Tests; + +public class EventSourceTests +{ + [Fact] + public void EventSourceTests_EnrichmentEventSource() + { + var eventSourceType = typeof(TraceEnricher).Assembly.GetType("OpenTelemetry.Extensions.Enrichment.EnrichmentEventSource", throwOnError: true)!; + EventSourceTestHelper.ValidateEventSourceIds(eventSourceType); + } +} diff --git a/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetry.Extensions.Enrichment.Tests.csproj b/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetry.Extensions.Enrichment.Tests.csproj index 3fb222cb44..7bb2716d63 100644 --- a/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetry.Extensions.Enrichment.Tests.csproj +++ b/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetry.Extensions.Enrichment.Tests.csproj @@ -17,4 +17,9 @@ + + + + + diff --git a/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetryEnrichmentServiceCollectionExtensionsTests.cs b/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetryEnrichmentServiceCollectionExtensionsTests.cs index cb1d244a1d..13b6ebaa15 100644 --- a/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetryEnrichmentServiceCollectionExtensionsTests.cs +++ b/test/OpenTelemetry.Extensions.Enrichment.Tests/OpenTelemetryEnrichmentServiceCollectionExtensionsTests.cs @@ -2,8 +2,11 @@ // SPDX-License-Identifier: Apache-2.0 using System.Diagnostics; +using System.Diagnostics.Tracing; +using System.Reflection; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using OpenTelemetry.Tests; using OpenTelemetry.Trace; using Xunit; @@ -183,4 +186,189 @@ public async Task FactoryMethod_RegistersEnricher() await host.StopAsync(); } + + [Fact] + public async Task DelegateMethod_SwallowsExceptionsFromEnrichmentActions() + { + var exportedItems = new List(); + using var eventListener = new InMemoryEventListener(GetEnrichmentEventSource(), EventLevel.Verbose); + + const string testKey = "safe-key"; + const string testValue = "safe-value"; + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => services + .AddOpenTelemetry() + .WithTracing(builder => builder + .AddSource(SourceName) + .AddInMemoryExporter(exportedItems)) + .Services + .AddTraceEnricher(ThrowingEnrichmentAction) + .AddTraceEnricher((Action)(bag => bag.Add(testKey, testValue)))) + .Build(); + + await host.StartAsync(); + + using var source = new ActivitySource(SourceName); + + var exception = Record.Exception(() => + { + using var activity = source.StartActivity(SourceName); + Assert.NotNull(activity); + activity.Stop(); + }); + + Assert.Null(exception); + Assert.Single(exportedItems); + Assert.Equal(testValue, exportedItems[0].TagObjects.Single(tag => tag.Key == testKey).Value); + + var loggedEvent = Assert.Single(eventListener.Events); + var payload = loggedEvent.Payload!.Select(Assert.IsType).ToArray(); + Assert.Equal(2, loggedEvent.EventId); + Assert.Contains(payload, value => value.Contains(nameof(ThrowingEnrichmentAction), StringComparison.Ordinal)); + Assert.Contains(payload, value => value.Contains("boom", StringComparison.Ordinal)); + + await host.StopAsync(); + } + + [Fact] + public async Task InstanceMethod_SwallowsExceptionsFromEnrich() + { + var exportedItems = new List(); + using var eventListener = new InMemoryEventListener(GetEnrichmentEventSource(), EventLevel.Verbose); + var trackingEnricher = new TrackingTraceEnricher(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => services + .AddOpenTelemetry() + .WithTracing(builder => builder + .AddSource(SourceName) + .AddInMemoryExporter(exportedItems)) + .Services + .TryAddTraceEnricher(new ThrowingOnEndTraceEnricher()) + .TryAddTraceEnricher(trackingEnricher)) + .Build(); + + await host.StartAsync(); + + using var source = new ActivitySource(SourceName); + + var exception = Record.Exception(() => + { + using var activity = source.StartActivity(SourceName); + Assert.NotNull(activity); + activity.Stop(); + }); + + Assert.Null(exception); + Assert.Equal(1, trackingEnricher.StartCalls); + Assert.Equal(1, trackingEnricher.EndCalls); + Assert.Single(exportedItems); + Assert.Equal(TrackingTraceEnricher.EndValue, exportedItems[0].TagObjects.Single(tag => tag.Key == TrackingTraceEnricher.EndKey).Value); + + var loggedEvent = Assert.Single(eventListener.Events); + var payload = loggedEvent.Payload!.Select(Assert.IsType).ToArray(); + Assert.Equal(1, loggedEvent.EventId); + Assert.Contains(payload, value => value.Contains(nameof(ThrowingOnEndTraceEnricher), StringComparison.Ordinal)); + Assert.Contains("OnEnd", payload); + Assert.Contains(payload, value => value.Contains("boom", StringComparison.Ordinal)); + + await host.StopAsync(); + } + + [Fact] + public async Task InstanceMethod_SwallowsExceptionsFromEnrichOnActivityStart() + { + var exportedItems = new List(); + using var eventListener = new InMemoryEventListener(GetEnrichmentEventSource(), EventLevel.Verbose); + var trackingEnricher = new TrackingTraceEnricher(); + + using var host = Host.CreateDefaultBuilder() + .ConfigureServices(services => services + .AddOpenTelemetry() + .WithTracing(builder => builder + .AddSource(SourceName) + .AddInMemoryExporter(exportedItems)) + .Services + .TryAddTraceEnricher(new ThrowingOnStartTraceEnricher()) + .TryAddTraceEnricher(trackingEnricher)) + .Build(); + + await host.StartAsync(); + + using var source = new ActivitySource(SourceName); + + var exception = Record.Exception(() => + { + using var activity = source.StartActivity(SourceName); + Assert.NotNull(activity); + activity.Stop(); + }); + + Assert.Null(exception); + Assert.Equal(1, trackingEnricher.StartCalls); + Assert.Equal(1, trackingEnricher.EndCalls); + Assert.Single(exportedItems); + Assert.Equal(TrackingTraceEnricher.StartValue, exportedItems[0].TagObjects.Single(tag => tag.Key == TrackingTraceEnricher.StartKey).Value); + + var loggedEvent = Assert.Single(eventListener.Events); + var payload = loggedEvent.Payload!.Select(Assert.IsType).ToArray(); + Assert.Equal(1, loggedEvent.EventId); + Assert.Contains(payload, value => value.Contains(nameof(ThrowingOnStartTraceEnricher), StringComparison.Ordinal)); + Assert.Contains("OnStart", payload); + Assert.Contains(payload, value => value.Contains("boom", StringComparison.Ordinal)); + + await host.StopAsync(); + } + + private static void ThrowingEnrichmentAction(TraceEnrichmentBag bag) + => throw new InvalidOperationException("boom"); + + private static EventSource GetEnrichmentEventSource() + { + var eventSourceType = typeof(TraceEnricher).Assembly.GetType("OpenTelemetry.Extensions.Enrichment.EnrichmentEventSource", throwOnError: true)!; + return (EventSource)eventSourceType + .GetField("Log", BindingFlags.Public | BindingFlags.Static)! + .GetValue(null)!; + } + + private sealed class ThrowingOnStartTraceEnricher : TraceEnricher + { + public override void Enrich(in TraceEnrichmentBag bag) + { + } + + public override void EnrichOnActivityStart(in TraceEnrichmentBag bag) + => throw new InvalidOperationException("boom"); + } + + private sealed class ThrowingOnEndTraceEnricher : TraceEnricher + { + public override void Enrich(in TraceEnrichmentBag bag) + => throw new InvalidOperationException("boom"); + } + + private sealed class TrackingTraceEnricher : TraceEnricher + { + public const string StartKey = "tracking-start"; + public const string StartValue = "start"; + public const string EndKey = "tracking-end"; + public const string EndValue = "end"; + + public int StartCalls { get; private set; } + + public int EndCalls { get; private set; } + + public override void Enrich(in TraceEnrichmentBag bag) + { + this.EndCalls++; + bag.Add(EndKey, EndValue); + } + + public override void EnrichOnActivityStart(in TraceEnrichmentBag bag) + { + this.StartCalls++; + bag.Add(StartKey, StartValue); + } + } }