Skip to content

Commit

Permalink
Fix: Scope not being applied to OpenTelemetry spans in ASP.NET Core (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jamescrosswell authored and vaind committed Oct 14, 2023
1 parent 7dac060 commit cc21277
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 22 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ API Changes:

- 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.2 ([#2645](https://github.com/getsentry/sentry-dotnet/pull/2645), [#2647](https://github.com/getsentry/sentry-dotnet/pull/2647), [#2698](https://github.com/getsentry/sentry-dotnet/pull/2698))
Expand Down
7 changes: 7 additions & 0 deletions src/Sentry.AspNetCore/SentryMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.Extensions.Options;
using Sentry.AspNetCore.Extensions;
using Sentry.Extensibility;
using Sentry.Internal;
using Sentry.Reflection;

namespace Sentry.AspNetCore;
Expand Down Expand Up @@ -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<IExceptionHandlerFeature?>();
Expand Down
6 changes: 6 additions & 0 deletions src/Sentry.OpenTelemetry/SentrySpanProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,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<Scope>();
if (activityScope is { } savedScope && _hub is Hub hub)
{
hub.RestoreScope(savedScope);
}
}
else
{
Expand Down
11 changes: 6 additions & 5 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Sentry.Extensibility;
using Sentry.Infrastructure;
using Sentry.Integrations;
using Sentry.Internal.ScopeStack;

namespace Sentry.Internal;

Expand Down Expand Up @@ -97,6 +98,8 @@ public async Task ConfigureScopeAsync(Func<Scope, Task> configureScope)

public IDisposable PushScope<TState>(TState state) => ScopeManager.PushScope(state);

public void RestoreScope(Scope savedScope) => ScopeManager.RestoreScope(savedScope);

[Obsolete]
public void WithScope(Action<Scope> scopeCallback) => ScopeManager.WithScope(scopeCallback);

Expand Down Expand Up @@ -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);

Expand All @@ -513,7 +515,6 @@ public void CaptureTransaction(Transaction transaction, Hint? hint)
}
}

var client = currentScopeAndClient.Value;
client.CaptureTransaction(processedTransaction, hint);
}
catch (Exception e)
Expand Down Expand Up @@ -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)
{
Expand Down
1 change: 1 addition & 0 deletions src/Sentry/Internal/IHubEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace Sentry.Internal;
internal interface IHubEx : IHub
{
SentryId CaptureEventInternal(SentryEvent evt, Hint? hint, Scope? scope = null);

T? WithScope<T>(Func<Scope, T?> scopeCallback);
Task WithScopeAsync(Func<Scope, Task> scopeCallback);
Task<T?> WithScopeAsync<T>(Func<Scope, Task<T?>> scopeCallback);
Expand Down
2 changes: 2 additions & 0 deletions src/Sentry/Internal/IInternalScopeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ namespace Sentry.Internal;
internal interface IInternalScopeManager : ISentryScopeManager, IDisposable
{
KeyValuePair<Scope, ISentryClient> GetCurrent();
void RestoreScope(Scope savedScope);

IScopeStackContainer ScopeStackContainer { get; }

// TODO: Move The following to ISentryScopeManager in a future major version.
Expand Down
49 changes: 32 additions & 17 deletions src/Sentry/Internal/SentryScopeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,18 @@ public SentryScopeManager(SentryOptions options, ISentryClient rootClient)
NewStack = () => new[] { new KeyValuePair<Scope, ISentryClient>(new Scope(options), rootClient) };
}

public KeyValuePair<Scope, ISentryClient> GetCurrent()
{
var current = ScopeAndClientStack;
return current[^1];
}
public KeyValuePair<Scope, ISentryClient> GetCurrent() => ScopeAndClientStack[^1];

public void ConfigureScope(Action<Scope>? configureScope)
{
var scope = GetCurrent();
configureScope?.Invoke(scope.Key);
var (scope, _) = GetCurrent();
configureScope?.Invoke(scope);
}

public Task ConfigureScopeAsync(Func<Scope, Task>? configureScope)
{
var scope = GetCurrent();
return configureScope?.Invoke(scope.Key) ?? Task.CompletedTask;
var (scope, _) = GetCurrent();
return configureScope?.Invoke(scope) ?? Task.CompletedTask;
}

public IDisposable PushScope() => PushScope<object>(null);
Expand Down Expand Up @@ -92,39 +88,58 @@ public IDisposable PushScope<TState>(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<Scope, ISentryClient>[currentScopeAndClientStack.Length + 1];
Array.Copy(currentScopeAndClientStack, newScopeAndClientStack, currentScopeAndClientStack.Length);
newScopeAndClientStack[^1] = new KeyValuePair<Scope, ISentryClient>(savedScope, client);

ScopeAndClientStack = newScopeAndClientStack;
}

public void WithScope(Action<Scope> scopeCallback)
{
using (PushScope())
{
var scope = GetCurrent();
scopeCallback.Invoke(scope.Key);
var (scope, _) = GetCurrent();
scopeCallback.Invoke(scope);
}
}

public T? WithScope<T>(Func<Scope, T?> scopeCallback)
{
using (PushScope())
{
var scope = GetCurrent();
return scopeCallback.Invoke(scope.Key);
var (scope, _) = GetCurrent();
return scopeCallback.Invoke(scope);
}
}

public async Task WithScopeAsync(Func<Scope, Task> scopeCallback)
{
using (PushScope())
{
var scope = GetCurrent();
await scopeCallback.Invoke(scope.Key).ConfigureAwait(false);
var (scope, _) = GetCurrent();
await scopeCallback.Invoke(scope).ConfigureAwait(false);
}
}

public async Task<T?> WithScopeAsync<T>(Func<Scope, Task<T?>> scopeCallback)
{
using (PushScope())
{
var scope = GetCurrent();
return await scopeCallback.Invoke(scope.Key).ConfigureAwait(false);
var (scope, _) = GetCurrent();
return await scopeCallback.Invoke(scope).ConfigureAwait(false);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Sentry/TransactionTracer.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sentry.Extensibility;
using Sentry.Internal;
using Sentry.Internal.ScopeStack;
using Sentry.Protocol;

namespace Sentry;
Expand Down
24 changes: 24 additions & 0 deletions test/Sentry.AspNetCore.Tests/SentryMiddlewareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,30 @@ public async Task InvokeAsync_SetsEventIdOnEvent()
_ = _fixture.Hub.Received().CaptureEvent(Arg.Is<SentryEvent>(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<Scope>>(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<Scope>().Should().Be(scope);
}
finally
{
activity.Stop();
}
}

[Fact]
public async Task InvokeAsync_RequestContainsSentryHeaders_ContinuesTrace()
{
Expand Down
22 changes: 22 additions & 0 deletions test/Sentry.OpenTelemetry.Tests/SentrySpanProcessorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,28 @@ public void OnEnd_FinishesSpan()
}
}

[Fact]
public void OnEnd_RestoresSavedScope()
{
// Arrange
_fixture.Options.Instrumenter = Instrumenter.OpenTelemetry;
_fixture.ScopeManager = Substitute.For<IInternalScopeManager>();
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()
{
Expand Down

0 comments on commit cc21277

Please sign in to comment.