Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce scope stack abstraction #1124

Merged
merged 10 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
- Persisted Sessions logging ([#1125](https://github.com/getsentry/sentry-dotnet/pull/1125))
- Don't log an error when attempting to recover a persisted session but none exists ([#1123](https://github.com/getsentry/sentry-dotnet/pull/1123))

### Features

- Introduce scope stack abstraction to support global scope on desktop and mobile applications and `HttpContext`-backed scoped on legacy ASP.NET ([#1124](https://github.com/getsentry/sentry-dotnet/pull/1124))

## 3.8.0

### Fixes
Expand Down
17 changes: 17 additions & 0 deletions src/Sentry.AspNet/Internal/HttpContextScopeStackContainer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System.Collections.Generic;
using System.Web;
using Sentry.Internal.ScopeStack;

namespace Sentry.AspNet.Internal
{
internal class HttpContextScopeStackContainer : IScopeStackContainer
{
private const string FieldName = "__SentryScopeStack";

public KeyValuePair<Scope, ISentryClient>[]? Stack
{
get => HttpContext.Current.Items[FieldName] as KeyValuePair<Scope, ISentryClient>[];
set => HttpContext.Current.Items[FieldName] = value;
}
}
}
3 changes: 3 additions & 0 deletions src/Sentry.AspNet/SentryAspNetOptionsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ public static void AddAspNet(this SentryOptions options, RequestSize maxRequestB

var eventProcessor = new SystemWebRequestEventProcessor(payloadExtractor, options);

// Ignore options.IsGlobalModeEnable, we always want to use HttpContext as backing store here
options.ScopeStackContainer ??= new HttpContextScopeStackContainer();

options.DiagnosticLogger ??= new TraceDiagnosticLogger(options.DiagnosticLevel);
options.Release ??= SystemWebVersionLocator.Resolve(options, HttpContext.Current);
options.AddEventProcessor(eventProcessor);
Expand Down
7 changes: 6 additions & 1 deletion src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Sentry.Extensibility;
using Sentry.Infrastructure;
using Sentry.Integrations;
using Sentry.Internal.ScopeStack;

namespace Sentry.Internal
{
Expand Down Expand Up @@ -48,7 +49,11 @@ internal Hub(ISentryClient client, ISystemClock clock, ISessionManager sessionMa

options.DiagnosticLogger?.LogDebug("Initializing Hub for Dsn: '{0}'.", options.Dsn);

ScopeManager = new SentryScopeManager(options, _ownedClient);
ScopeManager = new SentryScopeManager(
options.ScopeStackContainer ?? new AsyncLocalScopeStackContainer(),
options,
_ownedClient
);

_integrations = options.Integrations;

Expand Down
16 changes: 16 additions & 0 deletions src/Sentry/Internal/ScopeStack/AsyncLocalScopeStackContainer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System.Collections.Generic;
using System.Threading;

namespace Sentry.Internal.ScopeStack
{
internal class AsyncLocalScopeStackContainer : IScopeStackContainer
{
private readonly AsyncLocal<KeyValuePair<Scope, ISentryClient>[]?> _asyncLocalScope = new();

public KeyValuePair<Scope, ISentryClient>[]? Stack
{
get => _asyncLocalScope.Value;
set => _asyncLocalScope.Value = value;
}
}
}
9 changes: 9 additions & 0 deletions src/Sentry/Internal/ScopeStack/GlobalScopeStackContainer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Collections.Generic;

namespace Sentry.Internal.ScopeStack
{
internal class GlobalScopeStackContainer : IScopeStackContainer
{
public KeyValuePair<Scope, ISentryClient>[]? Stack { get; set; }
}
}
9 changes: 9 additions & 0 deletions src/Sentry/Internal/ScopeStack/IScopeStackContainer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Collections.Generic;

namespace Sentry.Internal.ScopeStack
{
internal interface IScopeStackContainer
{
KeyValuePair<Scope, ISentryClient>[]? Stack { get; set; }
}
}
26 changes: 19 additions & 7 deletions src/Sentry/Internal/SentryScopeManager.cs
Original file line number Diff line number Diff line change
@@ -1,28 +1,34 @@
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Sentry.Extensibility;
using Sentry.Internal.ScopeStack;

namespace Sentry.Internal
{
internal sealed class SentryScopeManager : IInternalScopeManager, IDisposable
{
// Internal for testing
internal IScopeStackContainer ScopeStackContainer { get; }

private readonly SentryOptions _options;
private readonly AsyncLocal<KeyValuePair<Scope, ISentryClient>[]?> _asyncLocalScope = new();

internal KeyValuePair<Scope, ISentryClient>[] ScopeAndClientStack
{
get => _asyncLocalScope.Value ?? (_asyncLocalScope.Value = NewStack());
set => _asyncLocalScope.Value = value;
get => ScopeStackContainer.Stack ??= NewStack();
set => ScopeStackContainer.Stack = value;
}

private Func<KeyValuePair<Scope, ISentryClient>[]> NewStack { get; }

private bool IsGlobalMode => ScopeStackContainer is GlobalScopeStackContainer;

public SentryScopeManager(
IScopeStackContainer scopeStackContainer,
SentryOptions options,
ISentryClient rootClient)
{
ScopeStackContainer = scopeStackContainer;
_options = options;
NewStack = () => new [] { new KeyValuePair<Scope, ISentryClient>(new Scope(options), rootClient) };
}
Expand All @@ -47,10 +53,16 @@ public Task ConfigureScopeAsync(Func<Scope, Task>? configureScope)
return configureScope?.Invoke(scope.Key) ?? Task.CompletedTask;
}

public IDisposable PushScope() => PushScope<object>(null!); // NRTs don't work well with generics
public IDisposable PushScope() => PushScope<object>(null);

public IDisposable PushScope<TState>(TState state)
public IDisposable PushScope<TState>(TState? state)
{
if (IsGlobalMode)
{
_options.DiagnosticLogger?.LogWarning("Push scope called in global mode, returning.");
return DisabledHub.Instance;
}

var currentScopeAndClientStack = ScopeAndClientStack;
var scope = currentScopeAndClientStack[currentScopeAndClientStack.Length - 1];

Expand Down Expand Up @@ -145,7 +157,7 @@ public void Dispose()
public void Dispose()
{
_options.DiagnosticLogger?.LogDebug($"Disposing {nameof(SentryScopeManager)}.");
_asyncLocalScope.Value = null;
ScopeStackContainer.Stack = null;
}
}
}
12 changes: 12 additions & 0 deletions src/Sentry/SentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Sentry.Http;
using Sentry.Integrations;
using Sentry.Internal;
using Sentry.Internal.ScopeStack;
using static Sentry.Internal.Constants;
using static Sentry.Constants;
using Runtime = Sentry.PlatformAbstractions.Runtime;
Expand All @@ -21,6 +22,17 @@ public class SentryOptions
{
private Dictionary<string, string>? _defaultTags;

internal IScopeStackContainer? ScopeStackContainer { get; set; }

/// <summary>
/// Specifies whether to use global scope management mode.
/// </summary>
public bool IsGlobalModeEnabled
{
get => ScopeStackContainer is GlobalScopeStackContainer;
set => ScopeStackContainer = value ? new GlobalScopeStackContainer() : new AsyncLocalScopeStackContainer();
}

// Override for tests
internal ITransport? Transport { get; set; }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using FluentAssertions;
using NSubstitute;
using Sentry.Internal.ScopeStack;
using Xunit;

namespace Sentry.Tests.Internals.ScopeStack
{
public class AsyncLocalScopeStackContainerTests
{
[Fact]
public async Task Scopes_are_not_shared_between_parallel_async_executions()
{
// Arrange
var container = new AsyncLocalScopeStackContainer();

var scope1 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

var scope2 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

// Act & assert
var task1 = Task.Run(async () =>
{
container.Stack.Should().BeNull();

await Task.Yield();

container.Stack.Should().BeNull();
});

var task2 = Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope1};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope1});
});

var task3 = Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope2};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope2});
});

await Task.WhenAll(task1, task2, task3);

container.Stack.Should().BeNull();
}

[Fact]
public async Task Scopes_are_not_shared_between_sequential_async_executions()
{
// Arrange
var container = new AsyncLocalScopeStackContainer();

var scope1 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

var scope2 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

// Act & assert
await Task.Run(async () =>
{
container.Stack.Should().BeNull();

await Task.Yield();

container.Stack.Should().BeNull();
});

await Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope1};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope1});
});

await Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope2};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope2});
});

container.Stack.Should().BeNull();
}

[Fact]
public async Task Scopes_are_shared_between_nested_async_executions()
{
// Arrange
var container = new AsyncLocalScopeStackContainer();

var scope1 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

var scope2 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

// Act & assert
await Task.Run(async () =>
{
container.Stack.Should().BeNull();

await Task.Yield();

container.Stack.Should().BeNull();

await Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope1};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope1});

await Task.Run(async () =>
{
container.Stack.Should().BeEquivalentTo(new[] {scope1});

await Task.Yield();
container.Stack = new[] {scope2};

container.Stack.Should().BeEquivalentTo(new[] {scope2});
}).ConfigureAwait(false);
}).ConfigureAwait(false);
});

container.Stack.Should().BeNull();
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System.Collections.Generic;
using System.Threading.Tasks;
using FluentAssertions;
using NSubstitute;
using Sentry.Internal.ScopeStack;
using Xunit;

namespace Sentry.Tests.Internals.ScopeStack
{
public class GlobalScopeStackContainerTests
{
[Fact]
public async Task Scopes_are_shared_between_parallel_async_executions()
{
// Arrange
var container = new GlobalScopeStackContainer();

var scope1 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

var scope2 = new KeyValuePair<Scope, ISentryClient>(
Substitute.For<Scope>(),
Substitute.For<ISentryClient>()
);

// Act & assert
await Task.Run(async () =>
{
container.Stack.Should().BeNull();

container.Stack = new[] {scope1, scope2};
await Task.Yield();

container.Stack.Should().BeEquivalentTo(new[] {scope1, scope2});
});

container.Stack.Should().BeEquivalentTo(new[] {scope1, scope2});
}
}
}
Loading