From 31d722f44a4d2d31b83201b34a2a37802f668dfa Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 25 Jun 2026 17:02:22 +1200 Subject: [PATCH] feat: Add SentryOptions.AutoSetScopeTransactions to auto-store transactions on the scope When enabled (opt-in, defaults to false), Hub.StartTransaction automatically stores the started transaction on the current scope, but only when the scope does not already have a transaction set - so a transaction set manually or by another integration is never overwritten. No explicit clearing is needed: TransactionTracer.Finish already calls Scope.ResetTransaction, which only clears the scope transaction when it still references that exact instance. Also adds an internal per-call override (StartSpan/StartTransaction) so integrations can opt in/out independently of the global option. The MAUI CommunityToolkit binder uses it to keep its existing behaviour of not storing the transaction on the (global, AsyncLocal-backed) scope. Closes #2399 Co-Authored-By: Claude Opus 4.8 --- .../MauiCommunityToolkitMvvmEventsBinder.cs | 11 +- src/Sentry/BindableSentryOptions.cs | 2 + src/Sentry/HubExtensions.cs | 33 +++- src/Sentry/Internal/Hub.cs | 20 ++- src/Sentry/Scope.cs | 58 +++++-- src/Sentry/SentryOptions.cs | 18 +++ ...iApprovalTests.Run.DotNet10_0.verified.txt | 1 + ...piApprovalTests.Run.DotNet8_0.verified.txt | 1 + ...piApprovalTests.Run.DotNet9_0.verified.txt | 1 + .../ApiApprovalTests.Run.Net4_8.verified.txt | 1 + test/Sentry.Tests/HubTests.cs | 143 ++++++++++++++++++ 11 files changed, 262 insertions(+), 27 deletions(-) diff --git a/src/Sentry.Maui.CommunityToolkit.Mvvm/MauiCommunityToolkitMvvmEventsBinder.cs b/src/Sentry.Maui.CommunityToolkit.Mvvm/MauiCommunityToolkitMvvmEventsBinder.cs index 39c8a76f0e..8fe9ec153d 100644 --- a/src/Sentry.Maui.CommunityToolkit.Mvvm/MauiCommunityToolkitMvvmEventsBinder.cs +++ b/src/Sentry.Maui.CommunityToolkit.Mvvm/MauiCommunityToolkitMvvmEventsBinder.cs @@ -144,14 +144,11 @@ private void RelayCommandOnPropertyChanged(object? sender, PropertyChangedEventA { // Note that we may be creating a transaction here and if so we explicitly don't store it on // Scope.Transaction, because Scope.Transaction is AsyncLocal and MAUI Apps have a global scope. The - // results would be that we would store the transaction on the scope, but it would never be cleared again, + // result would be that we would store the transaction on the scope, but it would never be cleared again, // since the next call to OnPropertyChanged for this RelayCommand will (likely) be from a different thread. - var span = hub.StartSpan(SpanName, SpanOp); - if (span is ITransactionTracer transaction) - { - hub.ConfigureScope(scope => scope.Transaction = transaction); - } - + // We pass autoSetScopeTransaction: false so that this holds even when the user has globally enabled + // SentryOptions.AutoSetScopeTransactions. + var span = hub.StartSpan(SpanName, SpanOp, autoSetScopeTransaction: false); relay.SetFused(span); } else if (relay.GetFused() is { } span) diff --git a/src/Sentry/BindableSentryOptions.cs b/src/Sentry/BindableSentryOptions.cs index 6815aa0d95..5e39356dbc 100644 --- a/src/Sentry/BindableSentryOptions.cs +++ b/src/Sentry/BindableSentryOptions.cs @@ -10,6 +10,7 @@ internal partial class BindableSentryOptions public bool? IsGlobalModeEnabled { get; set; } public bool? EnableScopeSync { get; set; } public bool? EnableBackpressureHandling { get; set; } + public bool? AutoSetScopeTransactions { get; set; } public List? TagFilters { get; set; } public bool? SendDefaultPii { get; set; } public bool? IsEnvironmentUser { get; set; } @@ -65,6 +66,7 @@ public void ApplyTo(SentryOptions options) options.IsGlobalModeEnabled = IsGlobalModeEnabled ?? options.IsGlobalModeEnabled; options.EnableScopeSync = EnableScopeSync ?? options.EnableScopeSync; options.EnableBackpressureHandling = EnableBackpressureHandling ?? options.EnableBackpressureHandling; + options.AutoSetScopeTransactions = AutoSetScopeTransactions ?? options.AutoSetScopeTransactions; options.TagFilters = TagFilters?.Select(s => new StringOrRegex(s)).ToList() ?? options.TagFilters; options.SendDefaultPii = SendDefaultPii ?? options.SendDefaultPii; options.IsEnvironmentUser = IsEnvironmentUser ?? options.IsEnvironmentUser; diff --git a/src/Sentry/HubExtensions.cs b/src/Sentry/HubExtensions.cs index 07639f4eea..d87118c35c 100644 --- a/src/Sentry/HubExtensions.cs +++ b/src/Sentry/HubExtensions.cs @@ -54,11 +54,36 @@ public static ITransactionTracer StartTransaction( /// /// Starts a span or transaction if a transaction is not already active on the scope /// - public static ISpan StartSpan(this IHub hub, string operation, string description) + public static ISpan StartSpan(this IHub hub, string operation, string description) => + hub.StartSpan(operation, description, null); + + /// + /// Starts a span or transaction if a transaction is not already active on the scope. + /// overrides for a + /// newly started transaction (it has no effect when a child span is started off an existing transaction). This lets + /// integrations control whether a transaction is stored on the scope independently of the user's global setting - + /// e.g. the MAUI CommunityToolkit binder opts out because the scope is global and AsyncLocal-backed. + /// + internal static ISpan StartSpan(this IHub hub, string operation, string description, bool? autoSetScopeTransaction) { - return hub.GetTransaction() is { } transaction - ? transaction.StartChild(operation, description) - : hub.StartTransaction(operation, description); // this is actually in the wrong order but changing it may break other things + if (hub.GetTransaction() is { } transaction) + { + return transaction.StartChild(operation, description); + } + + // No active transaction - start a new one. Note the operation/description argument order below is intentionally + // preserved from the original (arguably wrong) behaviour of StartSpan; changing it may break other things. + if (hub is Hub fullHub) + { + return fullHub.StartTransaction( + new TransactionContext(operation, description), + new Dictionary(), + null, + autoSetScopeTransaction); + } + + // Fallback for non-Hub IHub implementations, which can't honour the override. + return hub.StartTransaction(operation, description); } /// diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index dade93b89f..6197612734 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -176,7 +176,8 @@ public ITransactionTracer StartTransaction( internal ITransactionTracer StartTransaction( ITransactionContext context, IReadOnlyDictionary customSamplingContext, - DynamicSamplingContext? dynamicSamplingContext) + DynamicSamplingContext? dynamicSamplingContext, + bool? autoSetScopeTransaction = null) { // If the hub is disabled, we will always sample out. In other words, starting a transaction // after disposing the hub will result in that transaction not being sent to Sentry. @@ -258,6 +259,7 @@ internal ITransactionTracer StartTransaction( // If no DSC was provided, create one based on this transaction. // Must be done AFTER the sampling decision has been made (the DSC propagates sampling decisions). unsampledTransaction.DynamicSamplingContext ??= unsampledTransaction.CreateDynamicSamplingContext(_options, _replaySession); + TryAutoSetScopeTransaction(unsampledTransaction, autoSetScopeTransaction); return unsampledTransaction; } @@ -280,9 +282,25 @@ internal ITransactionTracer StartTransaction( // A sampled out transaction still appears fully functional to the user // but will be dropped by the client and won't reach Sentry's servers. + TryAutoSetScopeTransaction(transaction, autoSetScopeTransaction); return transaction; } + private void TryAutoSetScopeTransaction(ITransactionTracer transaction, bool? autoSetScopeTransaction) + { + // The per-call override (used by some integrations) takes precedence over the global option. + if (!(autoSetScopeTransaction ?? _options.AutoSetScopeTransactions)) + { + return; + } + + // Only set the transaction on the scope if there isn't already one set (manually by the user or by another + // integration). We never overwrite an existing transaction. There's no need to track whether we were the one + // to set it for clearing purposes: TransactionTracer.Finish calls Scope.ResetTransaction, which only clears + // the scope's transaction if it still references this exact instance. + ConfigureScope(static (scope, t) => scope.SetTransactionIfNull(t), transaction); + } + public void BindException(Exception exception, ISpan span) { // Don't bind on sampled out spans diff --git a/src/Sentry/Scope.cs b/src/Sentry/Scope.cs index 62b926cb7f..1eb16b07cd 100644 --- a/src/Sentry/Scope.cs +++ b/src/Sentry/Scope.cs @@ -221,21 +221,7 @@ public ITransactionTracer? Transaction _transactionLock.EnterWriteLock(); try { - _transaction.Value = value; - - if (Options.EnableScopeSync) - { - if (_transaction.Value != null) - { - // If there is a transaction set we propagate the trace to the native layer - Options.ScopeObserver?.SetTrace(_transaction.Value.TraceId, _transaction.Value.SpanId); - } - else - { - // If the transaction is being removed from the scope, reset and sync the trace as well - Options.ScopeObserver?.SetTrace(PropagationContext.TraceId, PropagationContext.SpanId); - } - } + SetTransactionValue(value); } finally { @@ -244,6 +230,48 @@ public ITransactionTracer? Transaction } } + // Must be called while holding the write lock on _transactionLock. + private void SetTransactionValue(ITransactionTracer? value) + { + _transaction.Value = value; + + if (Options.EnableScopeSync) + { + if (_transaction.Value != null) + { + // If there is a transaction set we propagate the trace to the native layer + Options.ScopeObserver?.SetTrace(_transaction.Value.TraceId, _transaction.Value.SpanId); + } + else + { + // If the transaction is being removed from the scope, reset and sync the trace as well + Options.ScopeObserver?.SetTrace(PropagationContext.TraceId, PropagationContext.SpanId); + } + } + } + + /// + /// Atomically sets on the scope only if there isn't already an (unfinished) + /// transaction set. Used to implement without overwriting a + /// transaction that was set manually or by another integration. + /// + internal void SetTransactionIfNull(ITransactionTracer transaction) + { + _transactionLock.EnterWriteLock(); + try + { + // Treat a finished transaction as "not set" - this mirrors the getter, which returns null in that case. + if (_transaction.Value is null or { IsFinished: true }) + { + SetTransactionValue(transaction); + } + } + finally + { + _transactionLock.ExitWriteLock(); + } + } + internal SentryPropagationContext PropagationContext { get; private set; } internal SessionUpdate? SessionUpdate { get; set; } diff --git a/src/Sentry/SentryOptions.cs b/src/Sentry/SentryOptions.cs index 187da749e0..004581c40b 100644 --- a/src/Sentry/SentryOptions.cs +++ b/src/Sentry/SentryOptions.cs @@ -103,6 +103,24 @@ public bool IsGlobalModeEnabled /// public bool EnableBackpressureHandling { get; set; } = true; + /// + /// When enabled, starting a transaction (for example via ) + /// automatically stores it on the current scope, provided the scope does not already have a transaction set. This + /// means APIs such as work without having to manually call + /// SentrySdk.ConfigureScope(scope => scope.Transaction = transaction). + /// + /// + /// Defaults to false. An automatically stored transaction is only ever cleared from the scope by the SDK if + /// the scope still references that exact transaction when it finishes, so a transaction set manually (or by another + /// integration) is never overwritten or cleared. + /// + /// Note that is backed by an . In global + /// scope mode (see ) the same caution that applies to setting + /// manually applies to enabling this option. + /// + /// + public bool AutoSetScopeTransactions { get; set; } = false; + /// /// This holds a reference to the current transport, when one is active. /// If set manually before initialization, the provided transport will be used instead of the default transport. diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt index 5b0c537022..9c332b479b 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt @@ -789,6 +789,7 @@ namespace Sentry public bool AttachStacktrace { get; set; } public bool AutoSessionTracking { get; set; } public System.TimeSpan AutoSessionTrackingInterval { get; set; } + public bool AutoSetScopeTransactions { get; set; } public Sentry.Extensibility.IBackgroundWorker? BackgroundWorker { get; set; } public string? CacheDirectoryPath { get; set; } public bool CaptureFailedRequests { get; set; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 5b0c537022..9c332b479b 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -789,6 +789,7 @@ namespace Sentry public bool AttachStacktrace { get; set; } public bool AutoSessionTracking { get; set; } public System.TimeSpan AutoSessionTrackingInterval { get; set; } + public bool AutoSetScopeTransactions { get; set; } public Sentry.Extensibility.IBackgroundWorker? BackgroundWorker { get; set; } public string? CacheDirectoryPath { get; set; } public bool CaptureFailedRequests { get; set; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index 5b0c537022..9c332b479b 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -789,6 +789,7 @@ namespace Sentry public bool AttachStacktrace { get; set; } public bool AutoSessionTracking { get; set; } public System.TimeSpan AutoSessionTrackingInterval { get; set; } + public bool AutoSetScopeTransactions { get; set; } public Sentry.Extensibility.IBackgroundWorker? BackgroundWorker { get; set; } public string? CacheDirectoryPath { get; set; } public bool CaptureFailedRequests { get; set; } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index f9068edea7..fa987cfee8 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -776,6 +776,7 @@ namespace Sentry public bool AttachStacktrace { get; set; } public bool AutoSessionTracking { get; set; } public System.TimeSpan AutoSessionTrackingInterval { get; set; } + public bool AutoSetScopeTransactions { get; set; } public Sentry.Extensibility.IBackgroundWorker? BackgroundWorker { get; set; } public string? CacheDirectoryPath { get; set; } public bool CaptureFailedRequests { get; set; } diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index c669a97060..2b12ed8909 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -654,6 +654,149 @@ public void StartTransaction_NameOpDescription_Works() transaction.Description.Should().Be("description"); } + [Fact] + public void StartTransaction_AutoSetScopeTransactionsDisabled_DoesNotSetOnScope() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = false; // the default + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction("name", "operation"); + + // Assert + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeNull(); + hub.GetSpan().Should().BeNull(); + } + + [Fact] + public void StartTransaction_AutoSetScopeTransactionsEnabled_SetsOnScopeWhenEmpty() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction("name", "operation"); + + // Assert + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeSameAs(transaction); + hub.GetSpan().Should().BeSameAs(transaction); + } + + [Fact] + public void StartTransaction_AutoSetScopeTransactionsEnabled_DoesNotOverwriteExistingTransaction() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + var existing = hub.StartTransaction("existing", "operation"); + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction = existing; + + // Act + var newTransaction = hub.StartTransaction("new", "operation"); + + // Assert - the manually set transaction must not be overwritten + scope.Transaction.Should().BeSameAs(existing); + newTransaction.Should().NotBeSameAs(existing); + } + + [Fact] + public void StartTransaction_AutoSetScopeTransactionsEnabled_ClearedFromScopeOnFinish() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + var transaction = hub.StartTransaction("name", "operation"); + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeSameAs(transaction); + + // Act + transaction.Finish(); + + // Assert + scope.Transaction.Should().BeNull(); + } + + [Fact] + public void StartTransaction_AutoSetScopeTransactionsEnabled_FinishDoesNotClearOverriddenTransaction() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + var autoSet = hub.StartTransaction("auto", "operation"); + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeSameAs(autoSet); + + // The user (or another integration) replaces the scope transaction after ours was auto-set. + var replacement = hub.StartTransaction("replacement", "operation"); + scope.Transaction = replacement; + + // Act - finishing the originally auto-set transaction must not clear the replacement + autoSet.Finish(); + + // Assert + scope.Transaction.Should().BeSameAs(replacement); + } + + [Fact] + public void StartTransaction_AutoSetScopeTransactionsEnabled_SetsUnsampledTransactionOnScope() + { + // Arrange - sampled out so StartTransaction returns an UnsampledTransaction + _fixture.Options.TracesSampleRate = 0.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + + // Act + var transaction = hub.StartTransaction("name", "operation"); + + // Assert + transaction.IsSampled.Should().BeFalse(); + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeSameAs(transaction); + } + + [Fact] + public void StartSpan_AutoSetScopeTransactionOverrideFalse_DoesNotSetOnScope_EvenWhenOptionEnabled() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = true; + var hub = _fixture.GetSut(); + + // Act - the override wins over the global option (this is what the MAUI binder relies on) + var span = hub.StartSpan("operation", "description", autoSetScopeTransaction: false); + + // Assert + span.Should().BeAssignableTo(); + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeNull(); + } + + [Fact] + public void StartSpan_AutoSetScopeTransactionOverrideTrue_SetsOnScope_EvenWhenOptionDisabled() + { + // Arrange + _fixture.Options.TracesSampleRate = 1.0; + _fixture.Options.AutoSetScopeTransactions = false; + var hub = _fixture.GetSut(); + + // Act - the override wins over the (disabled) global option + var span = hub.StartSpan("operation", "description", autoSetScopeTransaction: true); + + // Assert + var scope = hub.ScopeManager.GetCurrent().Key; + scope.Transaction.Should().BeSameAs(span); + } + [Fact] public void StartTransaction_FromTraceHeader_CopiesContext() {