Introduce scope stack abstraction (#1124)

CHANGELOG.md

@@ -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

src/Sentry.AspNet/Internal/HttpContextScopeStackContainer.cs

@@ -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;
+        }
+    }
+}

src/Sentry.AspNet/SentryAspNetOptionsExtensions.cs

@@ -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);

src/Sentry/Internal/Hub.cs

@@ -7,6 +7,7 @@
 using Sentry.Extensibility;
 using Sentry.Infrastructure;
 using Sentry.Integrations;
+using Sentry.Internal.ScopeStack;
 
 namespace Sentry.Internal
 {
@@ -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;
 

src/Sentry/Internal/ScopeStack/AsyncLocalScopeStackContainer.cs

@@ -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;
+        }
+    }
+}

src/Sentry/Internal/ScopeStack/GlobalScopeStackContainer.cs

@@ -0,0 +1,9 @@
+using System.Collections.Generic;
+
+namespace Sentry.Internal.ScopeStack
+{
+    internal class GlobalScopeStackContainer : IScopeStackContainer
+    {
+        public KeyValuePair<Scope, ISentryClient>[]? Stack { get; set; }
+    }
+}

src/Sentry/Internal/ScopeStack/IScopeStackContainer.cs

@@ -0,0 +1,9 @@
+using System.Collections.Generic;
+
+namespace Sentry.Internal.ScopeStack
+{
+    internal interface IScopeStackContainer
+    {
+        KeyValuePair<Scope, ISentryClient>[]? Stack { get; set; }
+    }
+}

src/Sentry/Internal/SentryScopeManager.cs

@@ -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) };
         }
@@ -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];
 
@@ -145,7 +157,7 @@ public void Dispose()
         public void Dispose()
         {
             _options.DiagnosticLogger?.LogDebug($"Disposing {nameof(SentryScopeManager)}.");
-            _asyncLocalScope.Value = null;
+            ScopeStackContainer.Stack = null;
         }
     }
 }

src/Sentry/SentryOptions.cs

@@ -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;
@@ -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; }
 

test/Sentry.Tests/Internals/ScopeStack/AsyncLocalScopeStackContainerTests.cs

@@ -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();
+        }
+    }
+}

test/Sentry.Tests/Internals/ScopeStack/GlobalScopeStackContainerTests.cs

@@ -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});
+        }
+    }
+}