diff --git a/CHANGELOG.md b/CHANGELOG.md index a50e5f3d7d..5a6005d9f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,9 @@ - Release of Azure Functions (Isolated Worker/Out-of-Process) support ([#2686](https://github.com/getsentry/sentry-dotnet/pull/2686)) +### Fixes +- Scope is now correctly applied to Transactions when using OpenTelemetry on ASP.NET Core ([#2690](https://github.com/getsentry/sentry-dotnet/pull/2690)) + ### Dependencies - Bump CLI from v2.20.7 to v2.21.1 ([#2645](https://github.com/getsentry/sentry-dotnet/pull/2645), [#2647](https://github.com/getsentry/sentry-dotnet/pull/2647)) diff --git a/src/Sentry.AspNetCore/SentryMiddleware.cs b/src/Sentry.AspNetCore/SentryMiddleware.cs index 7ace286888..ed83eb8f72 100644 --- a/src/Sentry.AspNetCore/SentryMiddleware.cs +++ b/src/Sentry.AspNetCore/SentryMiddleware.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Options; using Sentry.AspNetCore.Extensions; using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Reflection; namespace Sentry.AspNetCore; @@ -132,6 +133,12 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) { var originalMethod = context.Request.Method; await next(context).ConfigureAwait(false); + if (_options.Instrumenter == Instrumenter.OpenTelemetry && Activity.Current is {} activity) + { + // The middleware pipeline finishes up before the Otel Activity.OnEnd callback is invoked so we need + // so save a copy of the scope that can be restored by our SentrySpanProcessor + hub.ConfigureScope(scope => activity.SetFused(scope)); + } // When an exception was handled by other component (i.e: UseExceptionHandler feature). var exceptionFeature = context.Features.Get(); diff --git a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs index f02f7ec70c..217ff88120 100644 --- a/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs +++ b/src/Sentry.OpenTelemetry/SentrySpanProcessor.cs @@ -1,5 +1,6 @@ using OpenTelemetry; using Sentry.Extensibility; +using Sentry.Internal; using Sentry.Internal.Extensions; using Sentry.Internal.OpenTelemetry; @@ -156,6 +157,12 @@ public override void OnEnd(Activity data) // Transactions set otel attributes (and resource attributes) as context. transaction.Contexts["otel"] = GetOtelContext(attributes); + // Events are received/processed in a different AsyncLocal context. Restoring the scope that started it. + var activityScope = data.GetFused(); + if (activityScope is { } savedScope && _hub is Hub hub) + { + hub.RestoreScope(savedScope); + } } else { diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 8a361871c9..aed42e049e 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -1,6 +1,7 @@ using Sentry.Extensibility; using Sentry.Infrastructure; using Sentry.Integrations; +using Sentry.Internal.ScopeStack; namespace Sentry.Internal; @@ -97,6 +98,8 @@ public async Task ConfigureScopeAsync(Func configureScope) public IDisposable PushScope(TState state) => ScopeManager.PushScope(state); + public void RestoreScope(Scope savedScope) => ScopeManager.RestoreScope(savedScope); + [Obsolete] public void WithScope(Action scopeCallback) => ScopeManager.WithScope(scopeCallback); @@ -486,8 +489,7 @@ public void CaptureTransaction(Transaction transaction, Hint? hint) try { // Apply scope data - var currentScopeAndClient = ScopeManager.GetCurrent(); - var scope = currentScopeAndClient.Key; + var (scope, client) = ScopeManager.GetCurrent(); scope.Evaluate(); scope.Apply(transaction); @@ -513,7 +515,6 @@ public void CaptureTransaction(Transaction transaction, Hint? hint) } } - var client = currentScopeAndClient.Value; client.CaptureTransaction(processedTransaction, hint); } catch (Exception e) @@ -543,8 +544,8 @@ public async Task FlushAsync(TimeSpan timeout) { try { - var currentScope = ScopeManager.GetCurrent(); - await currentScope.Value.FlushAsync(timeout).ConfigureAwait(false); + var (_, client) = ScopeManager.GetCurrent(); + await client.FlushAsync(timeout).ConfigureAwait(false); } catch (Exception e) { diff --git a/src/Sentry/Internal/IHubEx.cs b/src/Sentry/Internal/IHubEx.cs index 10c5dd3a22..9a6b200a63 100644 --- a/src/Sentry/Internal/IHubEx.cs +++ b/src/Sentry/Internal/IHubEx.cs @@ -3,6 +3,7 @@ namespace Sentry.Internal; internal interface IHubEx : IHub { SentryId CaptureEventInternal(SentryEvent evt, Hint? hint, Scope? scope = null); + T? WithScope(Func scopeCallback); Task WithScopeAsync(Func scopeCallback); Task WithScopeAsync(Func> scopeCallback); diff --git a/src/Sentry/Internal/IInternalScopeManager.cs b/src/Sentry/Internal/IInternalScopeManager.cs index d9715e7d80..796597b311 100644 --- a/src/Sentry/Internal/IInternalScopeManager.cs +++ b/src/Sentry/Internal/IInternalScopeManager.cs @@ -5,6 +5,8 @@ namespace Sentry.Internal; internal interface IInternalScopeManager : ISentryScopeManager, IDisposable { KeyValuePair GetCurrent(); + void RestoreScope(Scope savedScope); + IScopeStackContainer ScopeStackContainer { get; } // TODO: Move The following to ISentryScopeManager in a future major version. diff --git a/src/Sentry/Internal/SentryScopeManager.cs b/src/Sentry/Internal/SentryScopeManager.cs index 6eb760a922..aab716c867 100644 --- a/src/Sentry/Internal/SentryScopeManager.cs +++ b/src/Sentry/Internal/SentryScopeManager.cs @@ -30,22 +30,18 @@ public SentryScopeManager(SentryOptions options, ISentryClient rootClient) NewStack = () => new[] { new KeyValuePair(new Scope(options), rootClient) }; } - public KeyValuePair GetCurrent() - { - var current = ScopeAndClientStack; - return current[^1]; - } + public KeyValuePair GetCurrent() => ScopeAndClientStack[^1]; public void ConfigureScope(Action? configureScope) { - var scope = GetCurrent(); - configureScope?.Invoke(scope.Key); + var (scope, _) = GetCurrent(); + configureScope?.Invoke(scope); } public Task ConfigureScopeAsync(Func? configureScope) { - var scope = GetCurrent(); - return configureScope?.Invoke(scope.Key) ?? Task.CompletedTask; + var (scope, _) = GetCurrent(); + return configureScope?.Invoke(scope) ?? Task.CompletedTask; } public IDisposable PushScope() => PushScope(null); @@ -92,12 +88,31 @@ public IDisposable PushScope(TState? state) return scopeSnapshot; } + public void RestoreScope(Scope savedScope) + { + if (IsGlobalMode) + { + _options.LogWarning("RestoreScope called in global mode, returning."); + return; + } + + var currentScopeAndClientStack = ScopeAndClientStack; + var (previousScope, client) = currentScopeAndClientStack[^1]; + + _options.LogDebug("Scope restored"); + var newScopeAndClientStack = new KeyValuePair[currentScopeAndClientStack.Length + 1]; + Array.Copy(currentScopeAndClientStack, newScopeAndClientStack, currentScopeAndClientStack.Length); + newScopeAndClientStack[^1] = new KeyValuePair(savedScope, client); + + ScopeAndClientStack = newScopeAndClientStack; + } + public void WithScope(Action scopeCallback) { using (PushScope()) { - var scope = GetCurrent(); - scopeCallback.Invoke(scope.Key); + var (scope, _) = GetCurrent(); + scopeCallback.Invoke(scope); } } @@ -105,8 +120,8 @@ public void WithScope(Action scopeCallback) { using (PushScope()) { - var scope = GetCurrent(); - return scopeCallback.Invoke(scope.Key); + var (scope, _) = GetCurrent(); + return scopeCallback.Invoke(scope); } } @@ -114,8 +129,8 @@ public async Task WithScopeAsync(Func scopeCallback) { using (PushScope()) { - var scope = GetCurrent(); - await scopeCallback.Invoke(scope.Key).ConfigureAwait(false); + var (scope, _) = GetCurrent(); + await scopeCallback.Invoke(scope).ConfigureAwait(false); } } @@ -123,8 +138,8 @@ public async Task WithScopeAsync(Func scopeCallback) { using (PushScope()) { - var scope = GetCurrent(); - return await scopeCallback.Invoke(scope.Key).ConfigureAwait(false); + var (scope, _) = GetCurrent(); + return await scopeCallback.Invoke(scope).ConfigureAwait(false); } } diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index e2de9fa9d2..78b2495b53 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -1,5 +1,6 @@ using Sentry.Extensibility; using Sentry.Internal; +using Sentry.Internal.ScopeStack; using Sentry.Protocol; namespace Sentry; diff --git a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs index ac3152aadf..b315d4d0f8 100644 --- a/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs +++ b/test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs @@ -659,6 +659,30 @@ public async Task InvokeAsync_SetsEventIdOnEvent() _ = _fixture.Hub.Received().CaptureEvent(Arg.Is(e => e.EventId.Equals(eventId))); } + [Fact] + public async Task InvokeAsync_InstrumenterOpenTelemetry_SavesScope() + { + // Arrange + _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; + var scope = new Scope(); + _fixture.Hub.ConfigureScope(Arg.Do>(action => action.Invoke(scope))); + var sut = _fixture.GetSut(); + var activity = new Activity("test").Start(); + + try + { + // Act + await sut.InvokeAsync(_fixture.HttpContext, _fixture.RequestDelegate); + + // Assert + activity.GetFused().Should().Be(scope); + } + finally + { + activity.Stop(); + } + } + [Fact] public async Task InvokeAsync_RequestContainsSentryHeaders_ContinuesTrace() { diff --git a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs index 20c1357f03..dc0de2b27d 100644 --- a/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs +++ b/test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs @@ -263,6 +263,28 @@ public void OnEnd_FinishesSpan() } } + [Fact] + public void OnEnd_RestoresSavedScope() + { + // Arrange + _fixture.Options.Instrumenter = Instrumenter.OpenTelemetry; + _fixture.ScopeManager = Substitute.For(); + var sut = _fixture.GetSut(); + + var scope = new Scope(); + var data = Tracer.StartActivity("transaction")!; + data.SetFused(scope); + sut.OnStart(data); + + sut._map.TryGetValue(data.SpanId, out var span); + + // Act + sut.OnEnd(data); + + // Assert + _fixture.ScopeManager.Received(1).RestoreScope(scope); + } + [Fact] public void OnEnd_SpansEnriched() {