From 8d40272c756407c7fce41bb08ad56de782c1e30e Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 2 Apr 2026 14:35:33 +1300 Subject: [PATCH 01/18] feat: Auto-create traces for MAUI navigation events Resolves: #5109 - #5109 --- src/Sentry.Maui/BindableSentryMauiOptions.cs | 4 + src/Sentry.Maui/Internal/MauiEventsBinder.cs | 71 +++++++++++- src/Sentry.Maui/SentryMauiOptions.cs | 17 +++ src/Sentry/Extensibility/DisabledHub.cs | 8 +- src/Sentry/Extensibility/HubAdapter.cs | 10 +- src/Sentry/Internal/Hub.cs | 10 +- src/Sentry/Internal/IHubInternal.cs | 15 +++ src/Sentry/SentrySdk.cs | 10 ++ ...piApprovalTests.Run.DotNet9_0.verified.txt | 2 + .../MauiEventsBinderFixture.cs | 5 +- .../MauiEventsBinderTests.Application.cs | 57 ++++++++++ .../MauiEventsBinderTests.Shell.cs | 101 ++++++++++++++++++ ...piApprovalTests.Run.DotNet9_0.verified.txt | 1 + 13 files changed, 299 insertions(+), 12 deletions(-) create mode 100644 src/Sentry/Internal/IHubInternal.cs diff --git a/src/Sentry.Maui/BindableSentryMauiOptions.cs b/src/Sentry.Maui/BindableSentryMauiOptions.cs index f4f98e1ad0..3f901a40a7 100644 --- a/src/Sentry.Maui/BindableSentryMauiOptions.cs +++ b/src/Sentry.Maui/BindableSentryMauiOptions.cs @@ -10,6 +10,8 @@ internal class BindableSentryMauiOptions : BindableSentryLoggingOptions public bool? IncludeBackgroundingStateInBreadcrumbs { get; set; } public bool? CreateElementEventsBreadcrumbs { get; set; } = false; public bool? AttachScreenshot { get; set; } + public bool? EnableNavigationTransactions { get; set; } + public TimeSpan? NavigationTransactionIdleTimeout { get; set; } public void ApplyTo(SentryMauiOptions options) { @@ -19,5 +21,7 @@ public void ApplyTo(SentryMauiOptions options) options.IncludeBackgroundingStateInBreadcrumbs = IncludeBackgroundingStateInBreadcrumbs ?? options.IncludeBackgroundingStateInBreadcrumbs; options.CreateElementEventsBreadcrumbs = CreateElementEventsBreadcrumbs ?? options.CreateElementEventsBreadcrumbs; options.AttachScreenshot = AttachScreenshot ?? options.AttachScreenshot; + options.EnableNavigationTransactions = EnableNavigationTransactions ?? options.EnableNavigationTransactions; + options.NavigationTransactionIdleTimeout = NavigationTransactionIdleTimeout ?? options.NavigationTransactionIdleTimeout; } } diff --git a/src/Sentry.Maui/Internal/MauiEventsBinder.cs b/src/Sentry.Maui/Internal/MauiEventsBinder.cs index 78e619bed9..614114d053 100644 --- a/src/Sentry.Maui/Internal/MauiEventsBinder.cs +++ b/src/Sentry.Maui/Internal/MauiEventsBinder.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Options; +using Sentry.Internal; namespace Sentry.Maui.Internal; @@ -13,6 +14,10 @@ internal class MauiEventsBinder : IMauiEventsBinder private readonly SentryMauiOptions _options; internal readonly IEnumerable _elementEventBinders; + // Tracks the active auto-finishing navigation transaction so we can explicitly finish it early + // (e.g. when the next navigation begins) before the idle timeout would fire. + private ITransactionTracer? _currentTransaction; + // https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/#breadcrumb-types // https://github.com/getsentry/sentry/blob/master/static/app/types/breadcrumbs.tsx internal const string NavigationType = "navigation"; @@ -319,16 +324,50 @@ internal void HandlePageEvents(Page page, bool bind = true) } } + private ITransactionTracer StartNavigationTransaction(string name) + { + // Finish any previous navigation transaction before starting a new one + _currentTransaction?.Finish(SpanStatus.Ok); + + var context = new TransactionContext(name, "ui.load") + { + NameSource = TransactionNameSource.Route + }; + + var transaction = _hub is IHubInternal internalHub + ? internalHub.StartTransaction(context, _options.NavigationTransactionIdleTimeout) + : _hub.StartTransaction(context); + + _hub.ConfigureScope(static (scope, t) => scope.Transaction = t, transaction); + _currentTransaction = transaction; + return transaction; + } + // Application Events private void OnApplicationOnPageAppearing(object? sender, Page page) => _hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.PageAppearing), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, page, nameof(Page))); private void OnApplicationOnPageDisappearing(object? sender, Page page) => _hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.PageDisappearing), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, page, nameof(Page))); - private void OnApplicationOnModalPushed(object? sender, ModalPushedEventArgs e) => + + private void OnApplicationOnModalPushed(object? sender, ModalPushedEventArgs e) + { _hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.ModalPushed), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, e.Modal, nameof(e.Modal))); - private void OnApplicationOnModalPopped(object? sender, ModalPoppedEventArgs e) => + if (_options.EnableNavigationTransactions) + { + StartNavigationTransaction(e.Modal.GetType().Name); + } + } + + private void OnApplicationOnModalPopped(object? sender, ModalPoppedEventArgs e) + { _hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.ModalPopped), NavigationType, NavigationCategory, data => data.AddElementInfo(_options, e.Modal, nameof(e.Modal))); + if (_options.EnableNavigationTransactions) + { + _currentTransaction?.Finish(SpanStatus.Ok); + _currentTransaction = null; + } + } private void OnApplicationOnRequestedThemeChanged(object? sender, AppThemeChangedEventArgs e) => _hub.AddBreadcrumbForEvent(_options, sender, nameof(Application.RequestedThemeChanged), SystemType, RenderingCategory, data => data.Add(nameof(e.RequestedTheme), e.RequestedTheme.ToString())); @@ -340,8 +379,15 @@ private void OnWindowOnActivated(object? sender, EventArgs _) => private void OnWindowOnDeactivated(object? sender, EventArgs _) => _hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Deactivated), SystemType, LifecycleCategory); - private void OnWindowOnStopped(object? sender, EventArgs _) => + private void OnWindowOnStopped(object? sender, EventArgs _) + { _hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Stopped), SystemType, LifecycleCategory); + if (_options.EnableNavigationTransactions) + { + _currentTransaction?.Finish(SpanStatus.Ok); + _currentTransaction = null; + } + } private void OnWindowOnResumed(object? sender, EventArgs _) => _hub.AddBreadcrumbForEvent(_options, sender, nameof(Window.Resumed), SystemType, LifecycleCategory); @@ -419,7 +465,8 @@ private void OnElementOnUnfocused(object? sender, FocusEventArgs _) => // Shell Events - private void OnShellOnNavigating(object? sender, ShellNavigatingEventArgs e) => + private void OnShellOnNavigating(object? sender, ShellNavigatingEventArgs e) + { _hub.AddBreadcrumbForEvent(_options, sender, nameof(Shell.Navigating), NavigationType, NavigationCategory, data => { data.Add("from", e.Current?.Location.ToString() ?? ""); @@ -427,7 +474,14 @@ private void OnShellOnNavigating(object? sender, ShellNavigatingEventArgs e) => data.Add(nameof(e.Source), e.Source.ToString()); }); - private void OnShellOnNavigated(object? sender, ShellNavigatedEventArgs e) => + if (_options.EnableNavigationTransactions) + { + StartNavigationTransaction(e.Target?.Location.ToString() ?? "Unknown"); + } + } + + private void OnShellOnNavigated(object? sender, ShellNavigatedEventArgs e) + { _hub.AddBreadcrumbForEvent(_options, sender, nameof(Shell.Navigated), NavigationType, NavigationCategory, data => { data.Add("from", e.Previous?.Location.ToString() ?? ""); @@ -435,6 +489,13 @@ private void OnShellOnNavigated(object? sender, ShellNavigatedEventArgs e) => data.Add(nameof(e.Source), e.Source.ToString()); }); + // Update the transaction name to the final resolved route now that navigation is confirmed + if (_options.EnableNavigationTransactions && _currentTransaction != null) + { + _currentTransaction.Name = e.Current?.Location.ToString() ?? _currentTransaction.Name; + } + } + // Page Events private void OnPageOnAppearing(object? sender, EventArgs _) => diff --git a/src/Sentry.Maui/SentryMauiOptions.cs b/src/Sentry.Maui/SentryMauiOptions.cs index 17038bf747..35e017522c 100644 --- a/src/Sentry.Maui/SentryMauiOptions.cs +++ b/src/Sentry.Maui/SentryMauiOptions.cs @@ -76,6 +76,23 @@ public SentryMauiOptions() /// public bool AttachScreenshot { get; set; } + /// + /// Automatically starts a Sentry transaction when the user navigates to a new page and sets it on the scope, + /// allowing child spans (e.g. HTTP requests, database calls) to be attached during page load. + /// The transaction finishes automatically after if not + /// finished explicitly first (e.g. by a subsequent navigation). + /// Requires or to + /// be configured. + /// The default is true. + /// + public bool EnableNavigationTransactions { get; set; } = true; + + /// + /// Controls how long an automatic navigation transaction waits before finishing itself when not explicitly + /// finished. Defaults to 3 seconds. + /// + public TimeSpan NavigationTransactionIdleTimeout { get; set; } = TimeSpan.FromSeconds(3); + private Func? _beforeCapture; /// /// Action performed before attaching a screenshot diff --git a/src/Sentry/Extensibility/DisabledHub.cs b/src/Sentry/Extensibility/DisabledHub.cs index 9bf79277f0..49f666dc1c 100644 --- a/src/Sentry/Extensibility/DisabledHub.cs +++ b/src/Sentry/Extensibility/DisabledHub.cs @@ -6,7 +6,7 @@ namespace Sentry.Extensibility; /// /// Disabled Hub. /// -public class DisabledHub : IHub, IDisposable +public class DisabledHub : IHub, IHubInternal, IDisposable { /// /// The singleton instance. @@ -81,6 +81,12 @@ public void UnsetTag(string key) public ITransactionTracer StartTransaction(ITransactionContext context, IReadOnlyDictionary customSamplingContext) => NoOpTransaction.Instance; + /// + /// Returns a dummy transaction. + /// + public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) + => NoOpTransaction.Instance; + /// /// No-Op. /// diff --git a/src/Sentry/Extensibility/HubAdapter.cs b/src/Sentry/Extensibility/HubAdapter.cs index 055498c2fc..5bca0defe5 100644 --- a/src/Sentry/Extensibility/HubAdapter.cs +++ b/src/Sentry/Extensibility/HubAdapter.cs @@ -1,4 +1,5 @@ using Sentry.Infrastructure; +using Sentry.Internal; using Sentry.Protocol.Envelopes; namespace Sentry.Extensibility; @@ -12,7 +13,7 @@ namespace Sentry.Extensibility; /// /// [DebuggerStepThrough] -public sealed class HubAdapter : IHub +public sealed class HubAdapter : IHub, IHubInternal { /// /// The single instance which forwards all calls to @@ -121,6 +122,13 @@ internal ITransactionTracer StartTransaction( DynamicSamplingContext? dynamicSamplingContext) => SentrySdk.StartTransaction(context, customSamplingContext, dynamicSamplingContext); + /// + /// Forwards the call to . + /// + [DebuggerStepThrough] + ITransactionTracer IHubInternal.StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) + => SentrySdk.StartTransaction(context, idleTimeout); + /// /// Forwards the call to . /// diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 5480d9e107..0eb30dc4db 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -6,7 +6,7 @@ namespace Sentry.Internal; -internal class Hub : IHub, IDisposable +internal class Hub : IHub, IHubInternal, IDisposable { private readonly Lock _sessionPauseLock = new(); @@ -173,10 +173,14 @@ public ITransactionTracer StartTransaction( IReadOnlyDictionary customSamplingContext) => StartTransaction(context, customSamplingContext, null); + public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) + => StartTransaction(context, new Dictionary(), null, idleTimeout); + internal ITransactionTracer StartTransaction( ITransactionContext context, IReadOnlyDictionary customSamplingContext, - DynamicSamplingContext? dynamicSamplingContext) + DynamicSamplingContext? dynamicSamplingContext, + TimeSpan? idleTimeout = 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. @@ -255,7 +259,7 @@ internal ITransactionTracer StartTransaction( return unsampledTransaction; } - var transaction = new TransactionTracer(this, context) + var transaction = new TransactionTracer(this, context, idleTimeout) { SampleRate = sampleRate, SampleRand = sampleRand, diff --git a/src/Sentry/Internal/IHubInternal.cs b/src/Sentry/Internal/IHubInternal.cs new file mode 100644 index 0000000000..8051c5189e --- /dev/null +++ b/src/Sentry/Internal/IHubInternal.cs @@ -0,0 +1,15 @@ +namespace Sentry.Internal; + +/// +/// Internal hub interface exposing additional overloads not part of the public contract. +/// Implemented by , , and +/// . +/// +internal interface IHubInternal : IHub +{ + /// + /// Starts a transaction that will automatically finish after if not + /// finished explicitly first. + /// + ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); +} diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs index fd7a6bd90f..d7b11c1ced 100644 --- a/src/Sentry/SentrySdk.cs +++ b/src/Sentry/SentrySdk.cs @@ -663,6 +663,16 @@ internal static ITransactionTracer StartTransaction( DynamicSamplingContext? dynamicSamplingContext) => CurrentHub.StartTransaction(context, customSamplingContext, dynamicSamplingContext); + /// + /// Starts a transaction that will automatically finish after if not + /// finished explicitly first. + /// + [DebuggerStepThrough] + internal static ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) + => CurrentHub is IHubInternal internalHub + ? internalHub.StartTransaction(context, idleTimeout) + : CurrentHub.StartTransaction(context); + /// /// Starts a transaction. /// diff --git a/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index f2790100c2..db0810300e 100644 --- a/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -32,9 +32,11 @@ namespace Sentry.Maui public SentryMauiOptions() { } public bool AttachScreenshot { get; set; } public bool CreateElementEventsBreadcrumbs { get; set; } + public bool EnableNavigationTransactions { get; set; } public bool IncludeBackgroundingStateInBreadcrumbs { get; set; } public bool IncludeTextInBreadcrumbs { get; set; } public bool IncludeTitleInBreadcrumbs { get; set; } + public System.TimeSpan NavigationTransactionIdleTimeout { get; set; } public void SetBeforeScreenshotCapture(System.Func beforeCapture) { } } public static class SessionReplay diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs b/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs index 38cb143a7a..8ef0741e5e 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs @@ -1,10 +1,11 @@ +using Sentry.Internal; using Sentry.Maui.Internal; namespace Sentry.Maui.Tests; internal class MauiEventsBinderFixture { - public IHub Hub { get; } + public IHubInternal Hub { get; } public MauiEventsBinder Binder { get; } @@ -14,7 +15,7 @@ internal class MauiEventsBinderFixture public MauiEventsBinderFixture(params IEnumerable elementEventBinders) { - Hub = Substitute.For(); + Hub = Substitute.For(); Hub.SubstituteConfigureScope(Scope); Scope.Transaction = Substitute.For(); diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Application.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Application.cs index 3d659faaaf..52a123b83f 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Application.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Application.cs @@ -1,3 +1,4 @@ +using Sentry.Internal; using Sentry.Maui.Internal; using Sentry.Maui.Tests.Mocks; @@ -193,6 +194,62 @@ public static IEnumerable ApplicationModalEventsData } } + [Fact] + public void Application_ModalPushed_StartsNavigationTransaction() + { + // Arrange + var application = MockApplication.Create(); + _fixture.Binder.HandleApplicationEvents(application); + var mockTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(mockTransaction); + var modalPage = new ContentPage { StyleId = "TestModalPage" }; + + // Act + application.RaiseEvent(nameof(Application.ModalPushed), new ModalPushedEventArgs(modalPage)); + + // Assert + _fixture.Hub.Received(1).StartTransaction( + Arg.Is(c => c.Name == nameof(ContentPage) && c.Operation == "ui.load"), + Arg.Any()); + } + + [Fact] + public void Application_ModalPushed_DisabledOption_DoesNotStartTransaction() + { + // Arrange + _fixture.Options.EnableNavigationTransactions = false; + var application = MockApplication.Create(); + _fixture.Binder.HandleApplicationEvents(application); + var modalPage = new ContentPage { StyleId = "TestModalPage" }; + + // Act + application.RaiseEvent(nameof(Application.ModalPushed), new ModalPushedEventArgs(modalPage)); + + // Assert + _fixture.Hub.DidNotReceive().StartTransaction(Arg.Any(), Arg.Any()); + } + + [Fact] + public void Application_ModalPopped_FinishesActiveTransaction() + { + // Arrange + var application = MockApplication.Create(); + _fixture.Binder.HandleApplicationEvents(application); + var mockTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(mockTransaction); + var modalPage = new ContentPage { StyleId = "TestModalPage" }; + + application.RaiseEvent(nameof(Application.ModalPushed), new ModalPushedEventArgs(modalPage)); + + // Act + application.RaiseEvent(nameof(Application.ModalPopped), new ModalPoppedEventArgs(modalPage)); + + // Assert + mockTransaction.Received(1).Finish(SpanStatus.Ok); + } + [Fact] public void Application_RequestedThemeChanged_AddsBreadcrumb() { diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs index 909a580c69..79e43894f4 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs @@ -1,3 +1,4 @@ +using Sentry.Internal; using Sentry.Maui.Internal; namespace Sentry.Maui.Tests; @@ -113,4 +114,104 @@ public void Shell_UnbindNavigated_DoesNotAddBreadcrumb() // Assert Assert.Single(_fixture.Scope.Breadcrumbs); } + + [Fact] + public void Shell_Navigating_StartsNavigationTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var mockTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(mockTransaction); + + // Act + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert + _fixture.Hub.Received(1).StartTransaction( + Arg.Is(c => c.Name == "bar" && c.Operation == "ui.load"), + Arg.Any()); + } + + [Fact] + public void Shell_Navigating_DisabledOption_DoesNotStartTransaction() + { + // Arrange + _fixture.Options.EnableNavigationTransactions = false; + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + + // Act + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert + _fixture.Hub.DidNotReceive().StartTransaction(Arg.Any(), Arg.Any()); + } + + [Fact] + public void Shell_Navigating_FinishesPreviousTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var firstTransaction = Substitute.For(); + var secondTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(firstTransaction, secondTransaction); + + // Act - navigate twice + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("bar"), new ShellNavigationState("baz"), ShellNavigationSource.Push, false)); + + // Assert - first transaction was finished before the second started + firstTransaction.Received(1).Finish(SpanStatus.Ok); + } + + [Fact] + public void Shell_Navigated_UpdatesTransactionName() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var mockTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(mockTransaction); + + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Act + shell.RaiseEvent(nameof(Shell.Navigated), + new ShellNavigatedEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("//resolved/bar"), ShellNavigationSource.Push)); + + // Assert + mockTransaction.Name.Should().Be("//resolved/bar"); + } + + [Fact] + public void Window_Stopped_FinishesActiveTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var window = new Window(); + _fixture.Binder.HandleWindowEvents(window); + var mockTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(mockTransaction); + + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Act + window.RaiseEvent(nameof(Window.Stopped), EventArgs.Empty); + + // Assert + mockTransaction.Received(1).Finish(SpanStatus.Ok); + } } diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index 86aa068b07..e9d36a4201 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -1517,6 +1517,7 @@ namespace Sentry.Extensibility public void SetTag(string key, string value) { } public void StartSession() { } public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.Collections.Generic.IReadOnlyDictionary customSamplingContext) { } + public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.TimeSpan? idleTimeout) { } public void UnsetTag(string key) { } } public class FormRequestPayloadExtractor : Sentry.Extensibility.BaseRequestPayloadExtractor From 40411194bf00e5c3ab48f05528949ce5b92a90f2 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Thu, 2 Apr 2026 02:09:29 +0000 Subject: [PATCH 02/18] Format code --- src/Sentry/Internal/IHubInternal.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry/Internal/IHubInternal.cs b/src/Sentry/Internal/IHubInternal.cs index 8051c5189e..67d11b770a 100644 --- a/src/Sentry/Internal/IHubInternal.cs +++ b/src/Sentry/Internal/IHubInternal.cs @@ -11,5 +11,5 @@ internal interface IHubInternal : IHub /// Starts a transaction that will automatically finish after if not /// finished explicitly first. /// - ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); + public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); } From 09760b3cd5f5774c19c39e5b6af9aefdf00ccb0f Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 12:08:12 +1200 Subject: [PATCH 03/18] Discard UI event transactions without child spans --- samples/Sentry.Samples.Maui/MauiProgram.cs | 1 + src/Sentry.Maui/Internal/MauiEventsBinder.cs | 30 ++++- src/Sentry/ITransactionTracer.cs | 5 + src/Sentry/Infrastructure/ITimer.cs | 17 +++ src/Sentry/Infrastructure/SystemTimer.cs | 22 ++++ src/Sentry/Internal/NoOpTransaction.cs | 2 + src/Sentry/SpanTracer.cs | 1 + src/Sentry/TransactionTracer.cs | 105 +++++++++++---- .../MauiEventsBinderFixture.cs | 2 - .../MauiEventsBinderTests.Shell.cs | 48 ++++++- test/Sentry.Testing/MockTimer.cs | 58 +++++++++ ...iApprovalTests.Run.DotNet10_0.verified.txt | 3 + ...piApprovalTests.Run.DotNet8_0.verified.txt | 3 + ...piApprovalTests.Run.DotNet9_0.verified.txt | 2 + .../Protocol/SentryTransactionTests.cs | 56 +++++--- test/Sentry.Tests/TransactionTracerTests.cs | 122 ++++++++++++++++++ 16 files changed, 427 insertions(+), 50 deletions(-) create mode 100644 src/Sentry/Infrastructure/ITimer.cs create mode 100644 src/Sentry/Infrastructure/SystemTimer.cs create mode 100644 test/Sentry.Testing/MockTimer.cs diff --git a/samples/Sentry.Samples.Maui/MauiProgram.cs b/samples/Sentry.Samples.Maui/MauiProgram.cs index c1789d3850..0351931a0a 100644 --- a/samples/Sentry.Samples.Maui/MauiProgram.cs +++ b/samples/Sentry.Samples.Maui/MauiProgram.cs @@ -41,6 +41,7 @@ public static MauiApp CreateMauiApp() // but only if tracing is enabled. Here we capture all traces (in a production app you'd probably only // capture a certain percentage) options.TracesSampleRate = 1.0F; + options.EnableNavigationTransactions = true; // Automatically create traces for async relay commands in the MVVM Community Toolkit options.AddCommunityToolkitIntegration(); diff --git a/src/Sentry.Maui/Internal/MauiEventsBinder.cs b/src/Sentry.Maui/Internal/MauiEventsBinder.cs index 614114d053..0db0fd9c5e 100644 --- a/src/Sentry.Maui/Internal/MauiEventsBinder.cs +++ b/src/Sentry.Maui/Internal/MauiEventsBinder.cs @@ -324,9 +324,31 @@ internal void HandlePageEvents(Page page, bool bind = true) } } - private ITransactionTracer StartNavigationTransaction(string name) + private ITransactionTracer? StartNavigationTransaction(string name) { - // Finish any previous navigation transaction before starting a new one + // If there's already a transaction on the scope that we didn't create, it was put there + // manually by the user — don't override it. + _hub.ConfigureScope(scope => + { + if (scope.Transaction is { } existing && !ReferenceEquals(existing, _currentTransaction)) + { + _manualTransactionOnScope = true; + } + }); + if (_manualTransactionOnScope) + { + return null; + } + + // Same destination as the current transaction — reset the idle timeout instead of + // creating a new transaction. + if (_currentTransaction is { IsFinished: false } current && current.Name == name) + { + current.ResetIdleTimeout(); + return current; + } + + // Finish any previous SDK-owned navigation transaction before starting a new one. _currentTransaction?.Finish(SpanStatus.Ok); var context = new TransactionContext(name, "ui.load") @@ -343,6 +365,10 @@ private ITransactionTracer StartNavigationTransaction(string name) return transaction; } + // Set to true when we detect a user-created transaction on the scope; cleared on the next + // navigation so we re-evaluate (the user's transaction may have finished by then). + private bool _manualTransactionOnScope; + // Application Events private void OnApplicationOnPageAppearing(object? sender, Page page) => diff --git a/src/Sentry/ITransactionTracer.cs b/src/Sentry/ITransactionTracer.cs index 9971321f2c..aa6650a5f2 100644 --- a/src/Sentry/ITransactionTracer.cs +++ b/src/Sentry/ITransactionTracer.cs @@ -26,4 +26,9 @@ public interface ITransactionTracer : ITransactionData, ISpan /// Gets the last active (not finished) span in this transaction. /// public ISpan? GetLastActiveSpan(); + + /// + /// Resets the idle timeout for auto-finishing transactions. No-op for transactions without an idle timeout. + /// + public void ResetIdleTimeout(); } diff --git a/src/Sentry/Infrastructure/ITimer.cs b/src/Sentry/Infrastructure/ITimer.cs new file mode 100644 index 0000000000..910b9f8a30 --- /dev/null +++ b/src/Sentry/Infrastructure/ITimer.cs @@ -0,0 +1,17 @@ +namespace Sentry.Infrastructure; + +/// +/// Abstraction over a one-shot timer, to allow deterministic testing. +/// +internal interface ISentryTimer : IDisposable +{ + /// + /// Starts (or restarts) the timer to fire after . + /// + void Start(TimeSpan timeout); + + /// + /// Cancels any pending fire. Has no effect if the timer is already cancelled. + /// + void Cancel(); +} diff --git a/src/Sentry/Infrastructure/SystemTimer.cs b/src/Sentry/Infrastructure/SystemTimer.cs new file mode 100644 index 0000000000..3d66ca3043 --- /dev/null +++ b/src/Sentry/Infrastructure/SystemTimer.cs @@ -0,0 +1,22 @@ +namespace Sentry.Infrastructure; + +/// +/// Production backed by . +/// +internal sealed class SystemTimer : ISentryTimer +{ + private readonly Timer _timer; + + public SystemTimer(Action callback) + { + _timer = new Timer(_ => callback(), null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); + } + + public void Start(TimeSpan timeout) => + _timer.Change(timeout, Timeout.InfiniteTimeSpan); + + public void Cancel() => + _timer.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); + + public void Dispose() => _timer.Dispose(); +} diff --git a/src/Sentry/Internal/NoOpTransaction.cs b/src/Sentry/Internal/NoOpTransaction.cs index ebbcb38d71..3e2a3227ee 100644 --- a/src/Sentry/Internal/NoOpTransaction.cs +++ b/src/Sentry/Internal/NoOpTransaction.cs @@ -93,5 +93,7 @@ public IReadOnlyList Fingerprint public ISpan? GetLastActiveSpan() => default; + public void ResetIdleTimeout() { } + public void AddBreadcrumb(Breadcrumb breadcrumb) { } } diff --git a/src/Sentry/SpanTracer.cs b/src/Sentry/SpanTracer.cs index b221918694..608e0b404c 100644 --- a/src/Sentry/SpanTracer.cs +++ b/src/Sentry/SpanTracer.cs @@ -156,6 +156,7 @@ public void Finish() { Status ??= SpanStatus.Ok; EndTimestamp ??= _stopwatch.CurrentDateTimeOffset; + Transaction?.ChildSpanFinished(); } /// diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 54464668d8..93380a2326 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -1,4 +1,5 @@ using Sentry.Extensibility; +using Sentry.Infrastructure; using Sentry.Internal; using Sentry.Protocol; @@ -11,12 +12,12 @@ public sealed class TransactionTracer : IBaseTracer, ITransactionTracer { private readonly IHub _hub; private readonly SentryOptions? _options; - private readonly Timer? _idleTimer; + private readonly ISentryTimer? _idleTimer; + private readonly TimeSpan? _idleTimeout; + private int _activeSpanCount; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); private InterlockedBoolean _hasFinished; - private InterlockedBoolean _cancelIdleTimeout; - private readonly Instrumenter _instrumenter = Instrumenter.Sentry; bool IBaseTracer.IsOtelInstrumenter => _instrumenter == Instrumenter.OpenTelemetry; @@ -223,7 +224,8 @@ internal TransactionTracer(IHub hub, string name, string operation, TransactionN /// /// Initializes an instance of . /// - internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idleTimeout = null) + internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idleTimeout = null, + Func? timerFactory = null) { _hub = hub; _options = _hub.GetSentryOptions(); @@ -243,24 +245,34 @@ internal TransactionTracer(IHub hub, ITransactionContext context, TimeSpan? idle Origin = transactionContext.Origin; } - // Set idle timer only if an idle timeout has been provided directly if (idleTimeout.HasValue) { - _cancelIdleTimeout = true; // Timer will be cancelled once, atomically setting this back to false - _idleTimer = new Timer(state => - { - if (state is not TransactionTracer transactionTracer) - { - _options?.LogDebug( - $"Idle timeout callback received nor non-TransactionTracer state. " + - "Unable to finish transaction automatically." - ); - return; - } + _idleTimeout = idleTimeout; + var factory = timerFactory ?? (cb => new SystemTimer(cb)); + _idleTimer = factory(OnIdleTimeout); + _idleTimer.Start(idleTimeout.Value); + } + } + + private void OnIdleTimeout() + { + if (IsSentryRequest) + { + _options?.LogDebug("Transaction '{0}' is a Sentry Request. Don't complete.", SpanId); + return; + } - transactionTracer.Finish(Status ?? SpanStatus.Ok); - }, this, idleTimeout.Value, Timeout.InfiniteTimeSpan); + // Discard if no child spans were ever started + if (_spans.IsEmpty) + { + _options?.LogDebug("Idle transaction '{0}' has no child spans. Discarding.", SpanId); + _hasFinished.Exchange(true); + _idleTimer?.Dispose(); + _hub.ConfigureScope(static (scope, tracer) => scope.ResetTransaction(tracer), this); + return; } + + Finish(Status ?? SpanStatus.Ok); } /// @@ -308,6 +320,28 @@ private void AddChildSpan(SpanTracer span) { _spans.Add(span); _activeSpanTracker.Push(span); + // Pause the idle timer while a child span is in flight + if (_idleTimeout.HasValue) + { + Interlocked.Increment(ref _activeSpanCount); + _idleTimer?.Cancel(); + } + } + } + + internal void ChildSpanFinished() + { + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + + // Only restart the idle timer when there are no more active (unfinished) child spans + var remaining = Interlocked.Decrement(ref _activeSpanCount); + if (remaining <= 0) + { + _activeSpanCount = 0; // guard against underflow + _idleTimer?.Start(_idleTimeout.Value); } } @@ -357,6 +391,17 @@ public void Clear() /// public ISpan? GetLastActiveSpan() => _activeSpanTracker.PeekActive(); + /// + /// Resets the idle timer. Only has an effect on transactions created with an idle timeout. + /// + public void ResetIdleTimeout() + { + if (_idleTimeout.HasValue && !_hasFinished) + { + _idleTimer?.Start(_idleTimeout.Value); + } + } + /// public void Finish() { @@ -366,12 +411,8 @@ public void Finish() } _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); - if (_cancelIdleTimeout.Exchange(false) == true) - { - _options?.LogDebug("Disposing of idle timer for Transaction '{0}'.", SpanId); - _idleTimer?.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); - _idleTimer?.Dispose(); - } + _idleTimer?.Cancel(); + _idleTimer?.Dispose(); if (IsSentryRequest) { @@ -384,7 +425,21 @@ public void Finish() TransactionProfiler?.Finish(); Status ??= SpanStatus.Ok; - EndTimestamp ??= _stopwatch.CurrentDateTimeOffset; + + // For idle transactions, trim end time to the last finished child span + if (_idleTimeout.HasValue) + { + var latestSpanEnd = _spans + .Where(s => s.IsFinished) + .Select(s => s.EndTimestamp) + .Max(); + EndTimestamp = latestSpanEnd ?? _stopwatch.CurrentDateTimeOffset; + } + else + { + EndTimestamp ??= _stopwatch.CurrentDateTimeOffset; + } + _options?.LogDebug("Finished Transaction '{0}'.", SpanId); // Clear the transaction from the scope and regenerate the Propagation Context diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs b/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs index 8ef0741e5e..b45ab996e0 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderFixture.cs @@ -18,8 +18,6 @@ public MauiEventsBinderFixture(params IEnumerable eleme Hub = Substitute.For(); Hub.SubstituteConfigureScope(Scope); - Scope.Transaction = Substitute.For(); - Options.Debug = true; var logger = Substitute.For(); logger.IsEnabled(Arg.Any()).Returns(true); diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs index 79e43894f4..d3a639f1aa 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs @@ -116,7 +116,7 @@ public void Shell_UnbindNavigated_DoesNotAddBreadcrumb() } [Fact] - public void Shell_Navigating_StartsNavigationTransaction() + public void Shell_Navigating_EnableNavigationTransactions_StartsNavigationTransaction() { // Arrange var shell = new Shell { StyleId = "shell" }; @@ -136,7 +136,7 @@ public void Shell_Navigating_StartsNavigationTransaction() } [Fact] - public void Shell_Navigating_DisabledOption_DoesNotStartTransaction() + public void Shell_Navigating_DisableNavigationTransactions_DoesNotStartTransaction() { // Arrange _fixture.Options.EnableNavigationTransactions = false; @@ -193,6 +193,50 @@ public void Shell_Navigated_UpdatesTransactionName() mockTransaction.Name.Should().Be("//resolved/bar"); } + [Fact] + public void Shell_Navigating_ManualTransactionOnScope_IsNotOverridden() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + + // Simulate the user setting their own transaction on the scope before navigation + var userTransaction = Substitute.For(); + _fixture.Scope.Transaction = userTransaction; + + // Act + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert - SDK should NOT start a new transaction when there's a user-created one + _fixture.Hub.DidNotReceive().StartTransaction(Arg.Any(), Arg.Any()); + Assert.Same(userTransaction, _fixture.Scope.Transaction); + } + + [Fact] + public void Shell_Navigating_SameRoute_ResetsTimeoutButDoesNotStartNewTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var firstTransaction = Substitute.For(); + firstTransaction.Name.Returns("bar"); + firstTransaction.IsFinished.Returns(false); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(firstTransaction); + + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Act - navigate to the same route again + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("bar"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert - only one transaction was started, and its idle timeout was reset + _fixture.Hub.Received(1).StartTransaction(Arg.Any(), Arg.Any()); + firstTransaction.Received(1).ResetIdleTimeout(); + } + [Fact] public void Window_Stopped_FinishesActiveTransaction() { diff --git a/test/Sentry.Testing/MockTimer.cs b/test/Sentry.Testing/MockTimer.cs new file mode 100644 index 0000000000..fb47ec029e --- /dev/null +++ b/test/Sentry.Testing/MockTimer.cs @@ -0,0 +1,58 @@ +using Sentry.Infrastructure; + +namespace Sentry.Testing; + +/// +/// A deterministic for use in tests. Call to +/// simulate the timeout elapsing without any real waiting. +/// +public class MockTimer : ISentryTimer +{ + private Action _callback; + private bool _disposed; + + /// Number of times has been called. + public int StartCount { get; private set; } + + /// Whether the timer is currently cancelled (not ticking). + public bool IsCancelled { get; private set; } = true; + + /// The most recent timeout passed to . + public TimeSpan? LastTimeout { get; private set; } + + public MockTimer(Action callback) + { + _callback = callback; + } + + /// + public void Start(TimeSpan timeout) + { + LastTimeout = timeout; + StartCount++; + IsCancelled = false; + } + + /// + public void Cancel() + { + IsCancelled = true; + } + + /// + /// Manually triggers the timer callback, simulating the idle timeout elapsing. + /// + public void Fire() + { + if (!_disposed) + { + _callback.Invoke(); + } + } + + /// + public void Dispose() + { + _disposed = true; + } +} diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt index 86aa068b07..4168d3c4cc 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt @@ -309,6 +309,7 @@ namespace Sentry new string Name { get; set; } System.Collections.Generic.IReadOnlyCollection Spans { get; } Sentry.ISpan? GetLastActiveSpan(); + void ResetIdleTimeout(); } public enum InstructionAddressAdjustment { @@ -1386,6 +1387,7 @@ namespace Sentry public void Finish(System.Exception exception, Sentry.SpanStatus status) { } public Sentry.ISpan? GetLastActiveSpan() { } public Sentry.SentryTraceHeader GetTraceHeader() { } + public void ResetIdleTimeout() { } public void SetData(string key, object? value) { } [System.Obsolete("Use SetData")] public void SetExtra(string key, object? value) { } @@ -1517,6 +1519,7 @@ namespace Sentry.Extensibility public void SetTag(string key, string value) { } public void StartSession() { } public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.Collections.Generic.IReadOnlyDictionary customSamplingContext) { } + public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.TimeSpan? idleTimeout) { } public void UnsetTag(string key) { } } public class FormRequestPayloadExtractor : Sentry.Extensibility.BaseRequestPayloadExtractor diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt index 86aa068b07..4168d3c4cc 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt @@ -309,6 +309,7 @@ namespace Sentry new string Name { get; set; } System.Collections.Generic.IReadOnlyCollection Spans { get; } Sentry.ISpan? GetLastActiveSpan(); + void ResetIdleTimeout(); } public enum InstructionAddressAdjustment { @@ -1386,6 +1387,7 @@ namespace Sentry public void Finish(System.Exception exception, Sentry.SpanStatus status) { } public Sentry.ISpan? GetLastActiveSpan() { } public Sentry.SentryTraceHeader GetTraceHeader() { } + public void ResetIdleTimeout() { } public void SetData(string key, object? value) { } [System.Obsolete("Use SetData")] public void SetExtra(string key, object? value) { } @@ -1517,6 +1519,7 @@ namespace Sentry.Extensibility public void SetTag(string key, string value) { } public void StartSession() { } public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.Collections.Generic.IReadOnlyDictionary customSamplingContext) { } + public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.TimeSpan? idleTimeout) { } public void UnsetTag(string key) { } } public class FormRequestPayloadExtractor : Sentry.Extensibility.BaseRequestPayloadExtractor diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt index e9d36a4201..4168d3c4cc 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt @@ -309,6 +309,7 @@ namespace Sentry new string Name { get; set; } System.Collections.Generic.IReadOnlyCollection Spans { get; } Sentry.ISpan? GetLastActiveSpan(); + void ResetIdleTimeout(); } public enum InstructionAddressAdjustment { @@ -1386,6 +1387,7 @@ namespace Sentry public void Finish(System.Exception exception, Sentry.SpanStatus status) { } public Sentry.ISpan? GetLastActiveSpan() { } public Sentry.SentryTraceHeader GetTraceHeader() { } + public void ResetIdleTimeout() { } public void SetData(string key, object? value) { } [System.Obsolete("Use SetData")] public void SetExtra(string key, object? value) { } diff --git a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs index 604e1cdda7..c851827da4 100644 --- a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs +++ b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs @@ -28,31 +28,49 @@ public void NewTransactionTracer_ConstructingWithContext_HasValidStartTime() } [Fact] - public async Task NewTransactionTracer_IdleTimeoutProvided_AutomaticallyFinishes() + public void NewTransactionTracer_IdleTimeoutProvided_WithChildSpan_AutomaticallyFinishes() { // Arrange - var client = Substitute.For(); - var options = new SentryOptions - { - Dsn = ValidDsn, - Debug = true - }; - var hub = new Hub(options, client); - var context = new TransactionContext("my name", - "my operation", - SpanId.Create(), - SpanId.Create(), - SentryId.Create(), - "description", - SpanStatus.Ok, null, true, TransactionNameSource.Component); + var hub = Substitute.For(); + var context = new TransactionContext("my name", "my operation", + SpanId.Create(), SpanId.Create(), SentryId.Create(), + "description", SpanStatus.Ok, null, true, TransactionNameSource.Component); - var transaction = new TransactionTracer(hub, context, TimeSpan.FromMilliseconds(2)); + MockTimer mockTimer = null; + var transaction = new TransactionTracer(hub, context, + idleTimeout: TimeSpan.FromSeconds(30), + timerFactory: cb => { mockTimer = new MockTimer(cb); return mockTimer; }); - // Act - await Task.Delay(TimeSpan.FromSeconds(2)); + var span = transaction.StartChild("child"); + span.Finish(); - // Assert + // Act — simulate idle timeout elapsing after the child span finished + mockTimer.Fire(); + + // Assert — transaction captured and marked finished transaction.IsFinished.Should().BeTrue(); + hub.Received(1).CaptureTransaction(Arg.Any()); + } + + [Fact] + public void NewTransactionTracer_IdleTimeoutProvided_NoChildSpans_IsDiscarded() + { + // Arrange + var hub = Substitute.For(); + var context = new TransactionContext("my name", "my operation", + SpanId.Create(), SpanId.Create(), SentryId.Create(), + "description", SpanStatus.Ok, null, true, TransactionNameSource.Component); + + MockTimer mockTimer = null; + _ = new TransactionTracer(hub, context, + idleTimeout: TimeSpan.FromSeconds(30), + timerFactory: cb => { mockTimer = new MockTimer(cb); return mockTimer; }); + + // Act — simulate idle timeout elapsing with no child spans + mockTimer.Fire(); + + // Assert — transaction discarded, not captured + hub.DidNotReceive().CaptureTransaction(Arg.Any()); } [Fact] diff --git a/test/Sentry.Tests/TransactionTracerTests.cs b/test/Sentry.Tests/TransactionTracerTests.cs index 9ad72fce5c..a8bac240c1 100644 --- a/test/Sentry.Tests/TransactionTracerTests.cs +++ b/test/Sentry.Tests/TransactionTracerTests.cs @@ -1,7 +1,27 @@ +using Sentry.Testing; + namespace Sentry.Tests; public class TransactionTracerTests { + private static readonly TimeSpan AnyTimeout = TimeSpan.FromSeconds(30); + + private static (TransactionTracer transaction, MockTimer timer) CreateIdleTransaction( + IHub hub, string name = "name", string op = "op") + { + MockTimer mockTimer = null; + var transaction = new TransactionTracer( + hub, + new TransactionContext(name, op), + idleTimeout: AnyTimeout, + timerFactory: cb => + { + mockTimer = new MockTimer(cb); + return mockTimer; + }); + return (transaction, mockTimer); + } + [Fact] public void Dispose_Unfinished_Finishes() { @@ -76,4 +96,106 @@ public void Dispose_WithTrackedSpans_ClearsTrackedSpans() // Assert Assert.Empty(transaction.Spans); } + + // --- Idle timeout scenarios (only apply when idleTimeout is non-null) --- + + [Fact] + public void IdleTimeout_NoChildSpans_TransactionIsDiscarded() + { + // Given an auto-generated UI event transaction with no child spans + var hub = Substitute.For(); + var (_, timer) = CreateIdleTransaction(hub); + + // When the idleTimeout fires + timer.Fire(); + + // Then the SDK discards the transaction (does not capture it) + hub.DidNotReceive().CaptureTransaction(Arg.Any()); + } + + [Fact] + public void IdleTimeout_WithFinishedChildSpan_TrimsEndTimestampToLatestSpan() + { + // Given an auto-generated UI event transaction with one finished child span + var hub = Substitute.For(); + var (transaction, timer) = CreateIdleTransaction(hub); + var span = transaction.StartChild("child"); + span.Finish(); + var expectedEndTime = span.EndTimestamp!.Value; + + // When the idleTimeout fires + timer.Fire(); + + // Then the transaction is captured with EndTimestamp trimmed to the last finished span + hub.Received(1).CaptureTransaction( + Arg.Is(t => t.EndTimestamp == expectedEndTime)); + } + + [Fact] + public void StartChild_CancelsIdleTimeout() + { + // Given an auto-generated UI event transaction + var hub = Substitute.For(); + var (transaction, timer) = CreateIdleTransaction(hub); + timer.StartCount.Should().Be(1); // started on creation + + // When the SDK starts a child span + _ = transaction.StartChild("child"); + + // Then the idle timer is cancelled while the span is in flight + timer.IsCancelled.Should().BeTrue(); + } + + [Fact] + public void LastSpan_Finish_ResetsIdleTimeout() + { + // Given an auto-generated UI event transaction with two child spans + var hub = Substitute.For(); + var (transaction, timer) = CreateIdleTransaction(hub); + var span1 = transaction.StartChild("child1"); + var span2 = transaction.StartChild("child2"); + + // When the first span finishes the timer stays cancelled (span2 still active) + span1.Finish(); + timer.IsCancelled.Should().BeTrue(); + + // When the last span finishes, the idle timer is restarted + span2.Finish(); + timer.IsCancelled.Should().BeFalse(); + timer.StartCount.Should().Be(2); // initial start + restart after last span + } + + [Fact] + public void NonLastSpan_Finish_DoesNotResetIdleTimeout() + { + // Given an auto-generated UI event transaction with two child spans + var hub = Substitute.For(); + var (transaction, timer) = CreateIdleTransaction(hub); + var span1 = transaction.StartChild("child1"); + _ = transaction.StartChild("child2"); + + // When the first (non-last) span finishes, the timer is NOT restarted + span1.Finish(); + timer.IsCancelled.Should().BeTrue(); + timer.StartCount.Should().Be(1); // only the initial start + } + + [Fact] + public void LastSpan_Finish_ThenTimerFires_CapturesTransaction() + { + // Given an auto-generated UI event transaction + var hub = Substitute.For(); + var (transaction, timer) = CreateIdleTransaction(hub); + var span = transaction.StartChild("child"); + span.Finish(); + + // Timer hasn't fired yet — not captured + hub.DidNotReceive().CaptureTransaction(Arg.Any()); + + // When the idle timer fires + timer.Fire(); + + // Then the transaction is captured + hub.Received(1).CaptureTransaction(Arg.Any()); + } } From 3a8c8f0435e036cecd3cc7cdb9bc721ed05b9343 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 12:41:02 +1200 Subject: [PATCH 04/18] missing verify files --- .../ApiApprovalTests.Run.DotNet10_0.verified.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt b/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt index db0645d89c..f6ab374dad 100644 --- a/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt +++ b/test/Sentry.Maui.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt @@ -39,9 +39,11 @@ namespace Sentry.Maui public SentryMauiOptions() { } public bool AttachScreenshot { get; set; } public bool CreateElementEventsBreadcrumbs { get; set; } + public bool EnableNavigationTransactions { get; set; } public bool IncludeBackgroundingStateInBreadcrumbs { get; set; } public bool IncludeTextInBreadcrumbs { get; set; } public bool IncludeTitleInBreadcrumbs { get; set; } + public System.TimeSpan NavigationTransactionIdleTimeout { get; set; } public void SetBeforeScreenshotCapture(System.Func beforeCapture) { } } public static class SessionReplay From 86a36dbad5705e41f594279f74626313f7c2961c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 12:41:27 +1200 Subject: [PATCH 05/18] Clean up duplicate tests --- .../Protocol/SentryTransactionTests.cs | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs index c851827da4..e774e7a0f3 100644 --- a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs +++ b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs @@ -27,52 +27,6 @@ public void NewTransactionTracer_ConstructingWithContext_HasValidStartTime() Assert.NotEqual(DateTimeOffset.MinValue, actualTransaction.StartTimestamp); } - [Fact] - public void NewTransactionTracer_IdleTimeoutProvided_WithChildSpan_AutomaticallyFinishes() - { - // Arrange - var hub = Substitute.For(); - var context = new TransactionContext("my name", "my operation", - SpanId.Create(), SpanId.Create(), SentryId.Create(), - "description", SpanStatus.Ok, null, true, TransactionNameSource.Component); - - MockTimer mockTimer = null; - var transaction = new TransactionTracer(hub, context, - idleTimeout: TimeSpan.FromSeconds(30), - timerFactory: cb => { mockTimer = new MockTimer(cb); return mockTimer; }); - - var span = transaction.StartChild("child"); - span.Finish(); - - // Act — simulate idle timeout elapsing after the child span finished - mockTimer.Fire(); - - // Assert — transaction captured and marked finished - transaction.IsFinished.Should().BeTrue(); - hub.Received(1).CaptureTransaction(Arg.Any()); - } - - [Fact] - public void NewTransactionTracer_IdleTimeoutProvided_NoChildSpans_IsDiscarded() - { - // Arrange - var hub = Substitute.For(); - var context = new TransactionContext("my name", "my operation", - SpanId.Create(), SpanId.Create(), SentryId.Create(), - "description", SpanStatus.Ok, null, true, TransactionNameSource.Component); - - MockTimer mockTimer = null; - _ = new TransactionTracer(hub, context, - idleTimeout: TimeSpan.FromSeconds(30), - timerFactory: cb => { mockTimer = new MockTimer(cb); return mockTimer; }); - - // Act — simulate idle timeout elapsing with no child spans - mockTimer.Fire(); - - // Assert — transaction discarded, not captured - hub.DidNotReceive().CaptureTransaction(Arg.Any()); - } - [Fact] public void NewTransactionTracer_PropagationContextHasReplayId_UsesActiveSessionReplayIdInstead() { From e02ac47657688975c0b16c85fca9c0ab3e68e900 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 12:41:47 +1200 Subject: [PATCH 06/18] Fix scope issue for detecting manual transactions --- src/Sentry.Maui/Internal/MauiEventsBinder.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Sentry.Maui/Internal/MauiEventsBinder.cs b/src/Sentry.Maui/Internal/MauiEventsBinder.cs index 0db0fd9c5e..9450b4c30f 100644 --- a/src/Sentry.Maui/Internal/MauiEventsBinder.cs +++ b/src/Sentry.Maui/Internal/MauiEventsBinder.cs @@ -328,20 +328,20 @@ internal void HandlePageEvents(Page page, bool bind = true) { // If there's already a transaction on the scope that we didn't create, it was put there // manually by the user — don't override it. + var manualTransactionOnScope = false; _hub.ConfigureScope(scope => { if (scope.Transaction is { } existing && !ReferenceEquals(existing, _currentTransaction)) { - _manualTransactionOnScope = true; + manualTransactionOnScope = true; } }); - if (_manualTransactionOnScope) + if (manualTransactionOnScope) { return null; } - // Same destination as the current transaction — reset the idle timeout instead of - // creating a new transaction. + // Reset the idle timeout instead of creating a new transaction if the destination is the same if (_currentTransaction is { IsFinished: false } current && current.Name == name) { current.ResetIdleTimeout(); @@ -365,10 +365,6 @@ internal void HandlePageEvents(Page page, bool bind = true) return transaction; } - // Set to true when we detect a user-created transaction on the scope; cleared on the next - // navigation so we re-evaluate (the user's transaction may have finished by then). - private bool _manualTransactionOnScope; - // Application Events private void OnApplicationOnPageAppearing(object? sender, Page page) => From e0145f4d1fc9ef7cb58d96c966ff759bdc505255 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 12:49:55 +1200 Subject: [PATCH 07/18] Fix race conditions --- src/Sentry/TransactionTracer.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 93380a2326..da9e40a0f0 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -340,7 +340,8 @@ internal void ChildSpanFinished() var remaining = Interlocked.Decrement(ref _activeSpanCount); if (remaining <= 0) { - _activeSpanCount = 0; // guard against underflow + // Guard against underflow atomically to avoid racing with concurrent Increment + Interlocked.CompareExchange(ref _activeSpanCount, 0, remaining); _idleTimer?.Start(_idleTimeout.Value); } } @@ -396,10 +397,19 @@ public void Clear() /// public void ResetIdleTimeout() { - if (_idleTimeout.HasValue && !_hasFinished) + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + try { _idleTimer?.Start(_idleTimeout.Value); } + catch (ObjectDisposedException) + { + // Finish() may dispose the timer concurrently between the _hasFinished check and Start(). + // Swallow the exception — the transaction is already finishing. + } } /// From 67c36c321409c5d3467c07eb375c2679d12611ac Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 13:04:22 +1200 Subject: [PATCH 08/18] Fix no transactions being created when user has a transaction on the scope --- src/Sentry.Maui/Internal/MauiEventsBinder.cs | 32 +++++++++---------- .../MauiEventsBinderTests.Shell.cs | 12 +++++-- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/Sentry.Maui/Internal/MauiEventsBinder.cs b/src/Sentry.Maui/Internal/MauiEventsBinder.cs index 9450b4c30f..45c7e30675 100644 --- a/src/Sentry.Maui/Internal/MauiEventsBinder.cs +++ b/src/Sentry.Maui/Internal/MauiEventsBinder.cs @@ -326,21 +326,6 @@ internal void HandlePageEvents(Page page, bool bind = true) private ITransactionTracer? StartNavigationTransaction(string name) { - // If there's already a transaction on the scope that we didn't create, it was put there - // manually by the user — don't override it. - var manualTransactionOnScope = false; - _hub.ConfigureScope(scope => - { - if (scope.Transaction is { } existing && !ReferenceEquals(existing, _currentTransaction)) - { - manualTransactionOnScope = true; - } - }); - if (manualTransactionOnScope) - { - return null; - } - // Reset the idle timeout instead of creating a new transaction if the destination is the same if (_currentTransaction is { IsFinished: false } current && current.Name == name) { @@ -360,7 +345,22 @@ internal void HandlePageEvents(Page page, bool bind = true) ? internalHub.StartTransaction(context, _options.NavigationTransactionIdleTimeout) : _hub.StartTransaction(context); - _hub.ConfigureScope(static (scope, t) => scope.Transaction = t, transaction); + // Only bind to scope if there is no user-created transaction already there. + // Re-evaluated on each navigation so a user transaction that finishes later is handled correctly. + var hasUserTransaction = false; + _hub.ConfigureScope(scope => + { + if (scope.Transaction is { } existing && !ReferenceEquals(existing, _currentTransaction)) + { + hasUserTransaction = true; + } + }); + + if (!hasUserTransaction) + { + _hub.ConfigureScope(static (scope, t) => scope.Transaction = t, transaction); + } + _currentTransaction = transaction; return transaction; } diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs index d3a639f1aa..1f9cc33a28 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs @@ -194,7 +194,7 @@ public void Shell_Navigated_UpdatesTransactionName() } [Fact] - public void Shell_Navigating_ManualTransactionOnScope_IsNotOverridden() + public void Shell_Navigating_ManualTransactionOnScope_AutoTransactionCreatedButNotBoundToScope() { // Arrange var shell = new Shell { StyleId = "shell" }; @@ -204,12 +204,18 @@ public void Shell_Navigating_ManualTransactionOnScope_IsNotOverridden() var userTransaction = Substitute.For(); _fixture.Scope.Transaction = userTransaction; + var autoTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(autoTransaction); + // Act shell.RaiseEvent(nameof(Shell.Navigating), new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); - // Assert - SDK should NOT start a new transaction when there's a user-created one - _fixture.Hub.DidNotReceive().StartTransaction(Arg.Any(), Arg.Any()); + // Assert - SDK still starts the auto transaction, but does NOT replace the user's scope transaction + _fixture.Hub.Received(1).StartTransaction( + Arg.Is(c => c.Name == "bar" && c.Operation == "ui.load"), + Arg.Any()); Assert.Same(userTransaction, _fixture.Scope.Transaction); } From 2b71e0cd08f3c4a265b45318f804bd8dd77ae033 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 13:15:52 +1200 Subject: [PATCH 09/18] Address test gaps --- .../MauiEventsBinderTests.Shell.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs index 1f9cc33a28..22fadca6f3 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs @@ -172,6 +172,30 @@ public void Shell_Navigating_FinishesPreviousTransaction() firstTransaction.Received(1).Finish(SpanStatus.Ok); } + [Fact] + public void Shell_Navigating_DifferentRoute_ReplacesScopeTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var firstTransaction = Substitute.For(); + var secondTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(firstTransaction, secondTransaction); + + // Act - first navigation binds firstTransaction to scope + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + Assert.Same(firstTransaction, _fixture.Scope.Transaction); + + // Act - second navigation to a different route + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("bar"), new ShellNavigationState("baz"), ShellNavigationSource.Push, false)); + + // Assert - scope transaction replaced with the new one + Assert.Same(secondTransaction, _fixture.Scope.Transaction); + } + [Fact] public void Shell_Navigated_UpdatesTransactionName() { @@ -243,6 +267,33 @@ public void Shell_Navigating_SameRoute_ResetsTimeoutButDoesNotStartNewTransactio firstTransaction.Received(1).ResetIdleTimeout(); } + [Fact] + public void Shell_Navigating_SameRoute_PreviousTransactionFinished_StartsNewTransaction() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var firstTransaction = Substitute.For(); + var secondTransaction = Substitute.For(); + firstTransaction.Name.Returns("bar"); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(firstTransaction, secondTransaction); + + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Simulate the idle timeout firing and auto-finishing the transaction + firstTransaction.IsFinished.Returns(true); + + // Act - same route, but the previous transaction has already finished + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("bar"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert - a new transaction is started rather than reusing the finished one + _fixture.Hub.Received(2).StartTransaction(Arg.Any(), Arg.Any()); + Assert.Same(secondTransaction, _fixture.Scope.Transaction); + } + [Fact] public void Window_Stopped_FinishesActiveTransaction() { From 430ec806cd83d9ed8b23e7cddc15dd6e972bada6 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 14:30:23 +1200 Subject: [PATCH 10/18] Matched tests with Android implementation --- samples/Sentry.Samples.Maui/MauiProgram.cs | 2 + .../MauiEventsBinderTests.Shell.cs | 67 +++++++++++++++++-- test/Sentry.Tests/TransactionTracerTests.cs | 18 +++++ 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/samples/Sentry.Samples.Maui/MauiProgram.cs b/samples/Sentry.Samples.Maui/MauiProgram.cs index 0351931a0a..2dadfc55d0 100644 --- a/samples/Sentry.Samples.Maui/MauiProgram.cs +++ b/samples/Sentry.Samples.Maui/MauiProgram.cs @@ -41,6 +41,8 @@ public static MauiApp CreateMauiApp() // but only if tracing is enabled. Here we capture all traces (in a production app you'd probably only // capture a certain percentage) options.TracesSampleRate = 1.0F; + + // Automatically create traces for navigation events options.EnableNavigationTransactions = true; // Automatically create traces for async relay commands in the MVVM Community Toolkit diff --git a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs index 22fadca6f3..ad44c3ed5e 100644 --- a/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs +++ b/test/Sentry.Maui.Tests/MauiEventsBinderTests.Shell.cs @@ -116,7 +116,7 @@ public void Shell_UnbindNavigated_DoesNotAddBreadcrumb() } [Fact] - public void Shell_Navigating_EnableNavigationTransactions_StartsNavigationTransaction() + public void Shell_Navigating_FirstNavigation_SetsTransactionOnScope() { // Arrange var shell = new Shell { StyleId = "shell" }; @@ -129,14 +129,50 @@ public void Shell_Navigating_EnableNavigationTransactions_StartsNavigationTransa shell.RaiseEvent(nameof(Shell.Navigating), new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + // Assert + Assert.Same(mockTransaction, _fixture.Scope.Transaction); + } + + [Fact] + public void Shell_Navigating_UsesRouteAsTransactionName() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(Substitute.For()); + + // Act + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + // Assert _fixture.Hub.Received(1).StartTransaction( - Arg.Is(c => c.Name == "bar" && c.Operation == "ui.load"), + Arg.Is(c => c.Name == "bar"), Arg.Any()); } [Fact] - public void Shell_Navigating_DisableNavigationTransactions_DoesNotStartTransaction() + public void Shell_Navigating_UsesUiLoadAsOperation() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(Substitute.For()); + + // Act + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Assert + _fixture.Hub.Received(1).StartTransaction( + Arg.Is(c => c.Operation == "ui.load"), + Arg.Any()); + } + + [Fact] + public void Shell_Navigating_NavigationTransactionsDisabled_DoesNotStartTransaction() { // Arrange _fixture.Options.EnableNavigationTransactions = false; @@ -151,6 +187,28 @@ public void Shell_Navigating_DisableNavigationTransactions_DoesNotStartTransacti _fixture.Hub.DidNotReceive().StartTransaction(Arg.Any(), Arg.Any()); } + [Fact] + public void Shell_Navigating_DifferentDestination_ClearsTransactionFromScope() + { + // Arrange + var shell = new Shell { StyleId = "shell" }; + _fixture.Binder.HandleShellEvents(shell); + var firstTransaction = Substitute.For(); + var secondTransaction = Substitute.For(); + _fixture.Hub.StartTransaction(Arg.Any(), Arg.Any()) + .Returns(firstTransaction, secondTransaction); + + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("foo"), new ShellNavigationState("bar"), ShellNavigationSource.Push, false)); + + // Act - navigate to a different destination, finishing the first transaction + shell.RaiseEvent(nameof(Shell.Navigating), + new ShellNavigatingEventArgs(new ShellNavigationState("bar"), new ShellNavigationState("baz"), ShellNavigationSource.Push, false)); + + // Assert - scope now holds the new transaction, not the old one + Assert.Same(secondTransaction, _fixture.Scope.Transaction); + } + [Fact] public void Shell_Navigating_FinishesPreviousTransaction() { @@ -244,7 +302,7 @@ public void Shell_Navigating_ManualTransactionOnScope_AutoTransactionCreatedButN } [Fact] - public void Shell_Navigating_SameRoute_ResetsTimeoutButDoesNotStartNewTransaction() + public void Shell_Navigating_SameRoute_ActiveTransaction_ResetsIdleTimeout() { // Arrange var shell = new Shell { StyleId = "shell" }; @@ -315,4 +373,5 @@ public void Window_Stopped_FinishesActiveTransaction() // Assert mockTransaction.Received(1).Finish(SpanStatus.Ok); } + } diff --git a/test/Sentry.Tests/TransactionTracerTests.cs b/test/Sentry.Tests/TransactionTracerTests.cs index a8bac240c1..6aaa408e19 100644 --- a/test/Sentry.Tests/TransactionTracerTests.cs +++ b/test/Sentry.Tests/TransactionTracerTests.cs @@ -97,6 +97,24 @@ public void Dispose_WithTrackedSpans_ClearsTrackedSpans() Assert.Empty(transaction.Spans); } + [Fact] + public void Finish_ClearsTransactionFromScope() + { + // Arrange + var hub = Substitute.For(); + var scope = new Scope(); + hub.SubstituteConfigureScope(scope); + + var transaction = new TransactionTracer(hub, new TransactionContext("name", "op")); + scope.Transaction = transaction; + + // Act + transaction.Finish(); + + // Assert + Assert.Null(scope.Transaction); + } + // --- Idle timeout scenarios (only apply when idleTimeout is non-null) --- [Fact] From 54b3d45146acc69a6c00fede161ddbc40fe370b5 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Tue, 7 Apr 2026 15:09:03 +1200 Subject: [PATCH 11/18] Review feedback --- src/Sentry/TransactionTracer.cs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index da9e40a0f0..60b72beaee 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -14,7 +14,6 @@ public sealed class TransactionTracer : IBaseTracer, ITransactionTracer private readonly SentryOptions? _options; private readonly ISentryTimer? _idleTimer; private readonly TimeSpan? _idleTimeout; - private int _activeSpanCount; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); private InterlockedBoolean _hasFinished; @@ -320,12 +319,7 @@ private void AddChildSpan(SpanTracer span) { _spans.Add(span); _activeSpanTracker.Push(span); - // Pause the idle timer while a child span is in flight - if (_idleTimeout.HasValue) - { - Interlocked.Increment(ref _activeSpanCount); - _idleTimer?.Cancel(); - } + _idleTimer?.Cancel(); // Pause the idle timer while a child span is in flight } } @@ -337,11 +331,8 @@ internal void ChildSpanFinished() } // Only restart the idle timer when there are no more active (unfinished) child spans - var remaining = Interlocked.Decrement(ref _activeSpanCount); - if (remaining <= 0) + if (_activeSpanTracker.PeekActive() == null) { - // Guard against underflow atomically to avoid racing with concurrent Increment - Interlocked.CompareExchange(ref _activeSpanCount, 0, remaining); _idleTimer?.Start(_idleTimeout.Value); } } From a3b5e22d8eedd21dee46e60daffdd094436d0e74 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Wed, 8 Apr 2026 01:54:01 +0000 Subject: [PATCH 12/18] Format code --- src/Sentry/Infrastructure/ITimer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sentry/Infrastructure/ITimer.cs b/src/Sentry/Infrastructure/ITimer.cs index 910b9f8a30..516ba57ca2 100644 --- a/src/Sentry/Infrastructure/ITimer.cs +++ b/src/Sentry/Infrastructure/ITimer.cs @@ -8,10 +8,10 @@ internal interface ISentryTimer : IDisposable /// /// Starts (or restarts) the timer to fire after . /// - void Start(TimeSpan timeout); + public void Start(TimeSpan timeout); /// /// Cancels any pending fire. Has no effect if the timer is already cancelled. /// - void Cancel(); + public void Cancel(); } From 0d5a5f4f7f9f0285519d94860a5b584fc49baee9 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 8 Apr 2026 14:09:45 +1200 Subject: [PATCH 13/18] Windows verify tests --- test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt index 58020d548b..c4c8b8505a 100644 --- a/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt +++ b/test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt @@ -297,6 +297,7 @@ namespace Sentry new string Name { get; set; } System.Collections.Generic.IReadOnlyCollection Spans { get; } Sentry.ISpan? GetLastActiveSpan(); + void ResetIdleTimeout(); } public enum InstructionAddressAdjustment { @@ -1367,6 +1368,7 @@ namespace Sentry public void Finish(System.Exception exception, Sentry.SpanStatus status) { } public Sentry.ISpan? GetLastActiveSpan() { } public Sentry.SentryTraceHeader GetTraceHeader() { } + public void ResetIdleTimeout() { } public void SetData(string key, object? value) { } [System.Obsolete("Use SetData")] public void SetExtra(string key, object? value) { } @@ -1498,6 +1500,7 @@ namespace Sentry.Extensibility public void SetTag(string key, string value) { } public void StartSession() { } public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.Collections.Generic.IReadOnlyDictionary customSamplingContext) { } + public Sentry.ITransactionTracer StartTransaction(Sentry.ITransactionContext context, System.TimeSpan? idleTimeout) { } public void UnsetTag(string key) { } } public class FormRequestPayloadExtractor : Sentry.Extensibility.BaseRequestPayloadExtractor From 0418c99956d495dde6912a429fb189a8df2bde3f Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Wed, 8 Apr 2026 15:09:13 +1200 Subject: [PATCH 14/18] Add runtime guard to ensure HubAdaptes is never set to SentrySdk.CurrentHub --- src/Sentry/SentrySdk.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Sentry/SentrySdk.cs b/src/Sentry/SentrySdk.cs index d7b11c1ced..1f378375d8 100644 --- a/src/Sentry/SentrySdk.cs +++ b/src/Sentry/SentrySdk.cs @@ -210,6 +210,11 @@ public static IDisposable Init(Action? configureOptions) internal static IDisposable UseHub(IHub hub) { + if (hub is HubAdapter) + { + hub.GetSentryOptions()?.LogError("Attempting to initianise the SentrySdk with a HubAdapter can lead to infinite recursion. Initialisation cancelled."); + return DisabledHub.Instance; + } var oldHub = Interlocked.Exchange(ref CurrentHub, hub); (oldHub as IDisposable)?.Dispose(); return new DisposeHandle(hub); From 2995289bb14870c67d1ad41058aae4853fe5ec36 Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 9 Apr 2026 11:00:56 +1200 Subject: [PATCH 15/18] Fix https://github.com/getsentry/sentry-dotnet/pull/5111#discussion_r3049011107 --- src/Sentry/TransactionTracer.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 60b72beaee..b3e6a458be 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -433,6 +433,7 @@ public void Finish() var latestSpanEnd = _spans .Where(s => s.IsFinished) .Select(s => s.EndTimestamp) + .DefaultIfEmpty() .Max(); EndTimestamp = latestSpanEnd ?? _stopwatch.CurrentDateTimeOffset; } From b75839b39f1afdf00cb337c3177c2a928f811a6c Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 9 Apr 2026 12:53:33 +1200 Subject: [PATCH 16/18] Change locking mechanism on _hasFinished to prevent adding spans to finished transactions --- src/Sentry/TransactionTracer.cs | 84 ++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 11 deletions(-) diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index b3e6a458be..1a4224a078 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -10,12 +10,15 @@ namespace Sentry; /// public sealed class TransactionTracer : IBaseTracer, ITransactionTracer { + private const int SpanLimit = 1000; + private readonly IHub _hub; private readonly SentryOptions? _options; private readonly ISentryTimer? _idleTimer; private readonly TimeSpan? _idleTimeout; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); - private InterlockedBoolean _hasFinished; + private bool _hasFinished; + private readonly ReaderWriterLockSlim _finishLock = new(); private readonly Instrumenter _instrumenter = Instrumenter.Sentry; @@ -262,10 +265,28 @@ private void OnIdleTimeout() } // Discard if no child spans were ever started - if (_spans.IsEmpty) + bool shouldDiscard; + _finishLock.EnterWriteLock(); + try + { + if (_spans.IsEmpty && !_hasFinished) + { + _hasFinished = true; + shouldDiscard = true; + } + else + { + shouldDiscard = false; + } + } + finally + { + _finishLock.ExitWriteLock(); + } + + if (shouldDiscard) { _options?.LogDebug("Idle transaction '{0}' has no child spans. Discarding.", SpanId); - _hasFinished.Exchange(true); _idleTimer?.Dispose(); _hub.ConfigureScope(static (scope, tracer) => scope.ResetTransaction(tracer), this); return; @@ -312,22 +333,46 @@ internal ISpan StartChild(SpanId? spanId, SpanId parentSpanId, string operation, private void AddChildSpan(SpanTracer span) { // Limit spans to 1000 - var isOutOfLimit = _spans.Count >= 1000; + var isOutOfLimit = _spans.Count >= SpanLimit; span.IsSampled = isOutOfLimit ? false : IsSampled; + if (isOutOfLimit) + { + _options?.LogDebug("Discarding child span '{0}' due to {1} span limit", SpanId, SpanLimit); + return; + } - if (!isOutOfLimit) + _finishLock.EnterReadLock(); + try { + if (_hasFinished) + { + _options?.LogDebug("Discarding child span '{0}' as the trace has already finished", SpanId); + return; + } + _spans.Add(span); _activeSpanTracker.Push(span); _idleTimer?.Cancel(); // Pause the idle timer while a child span is in flight } + finally + { + _finishLock.ExitReadLock(); + } } internal void ChildSpanFinished() { - if (!_idleTimeout.HasValue || _hasFinished) + _finishLock.EnterReadLock(); + try { - return; + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + } + finally + { + _finishLock.ExitReadLock(); } // Only restart the idle timer when there are no more active (unfinished) child spans @@ -388,9 +433,17 @@ public void Clear() /// public void ResetIdleTimeout() { - if (!_idleTimeout.HasValue || _hasFinished) + _finishLock.EnterReadLock(); + try { - return; + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + } + finally + { + _finishLock.ExitReadLock(); } try { @@ -406,9 +459,18 @@ public void ResetIdleTimeout() /// public void Finish() { - if (_hasFinished.Exchange(true)) + _finishLock.EnterWriteLock(); + try { - return; + if (_hasFinished) + { + return; + } + _hasFinished = true; + } + finally + { + _finishLock.ExitWriteLock(); } _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); From b9e4ff10d45792e7376d223de3e2aac4e9ebbf6a Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 9 Apr 2026 13:50:01 +1200 Subject: [PATCH 17/18] Renamed file to match the class name --- src/Sentry/Infrastructure/{ITimer.cs => ISentryTimer.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Sentry/Infrastructure/{ITimer.cs => ISentryTimer.cs} (100%) diff --git a/src/Sentry/Infrastructure/ITimer.cs b/src/Sentry/Infrastructure/ISentryTimer.cs similarity index 100% rename from src/Sentry/Infrastructure/ITimer.cs rename to src/Sentry/Infrastructure/ISentryTimer.cs From 7a0395a10ef668eaa0a33f08101a28cc2b1d7f6a Mon Sep 17 00:00:00 2001 From: James Crosswell Date: Thu, 9 Apr 2026 15:15:04 +1200 Subject: [PATCH 18/18] Fix threading issues around idleTimer --- src/Sentry.Maui/Internal/MauiEventsBinder.cs | 2 - src/Sentry/TransactionTracer.cs | 48 ++++++++++---------- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/Sentry.Maui/Internal/MauiEventsBinder.cs b/src/Sentry.Maui/Internal/MauiEventsBinder.cs index 45c7e30675..776540f71f 100644 --- a/src/Sentry.Maui/Internal/MauiEventsBinder.cs +++ b/src/Sentry.Maui/Internal/MauiEventsBinder.cs @@ -346,7 +346,6 @@ internal void HandlePageEvents(Page page, bool bind = true) : _hub.StartTransaction(context); // Only bind to scope if there is no user-created transaction already there. - // Re-evaluated on each navigation so a user transaction that finishes later is handled correctly. var hasUserTransaction = false; _hub.ConfigureScope(scope => { @@ -355,7 +354,6 @@ internal void HandlePageEvents(Page page, bool bind = true) hasUserTransaction = true; } }); - if (!hasUserTransaction) { _hub.ConfigureScope(static (scope, t) => scope.Transaction = t, transaction); diff --git a/src/Sentry/TransactionTracer.cs b/src/Sentry/TransactionTracer.cs index 1a4224a078..4415899659 100644 --- a/src/Sentry/TransactionTracer.cs +++ b/src/Sentry/TransactionTracer.cs @@ -272,6 +272,7 @@ private void OnIdleTimeout() if (_spans.IsEmpty && !_hasFinished) { _hasFinished = true; + _idleTimer?.Dispose(); shouldDiscard = true; } else @@ -287,7 +288,7 @@ private void OnIdleTimeout() if (shouldDiscard) { _options?.LogDebug("Idle transaction '{0}' has no child spans. Discarding.", SpanId); - _idleTimer?.Dispose(); + EndTimestamp = _stopwatch.CurrentDateTimeOffset; // Prevent MauiEventsBinder from reusing _hub.ConfigureScope(static (scope, tracer) => scope.ResetTransaction(tracer), this); return; } @@ -332,7 +333,6 @@ internal ISpan StartChild(SpanId? spanId, SpanId parentSpanId, string operation, private void AddChildSpan(SpanTracer span) { - // Limit spans to 1000 var isOutOfLimit = _spans.Count >= SpanLimit; span.IsSampled = isOutOfLimit ? false : IsSampled; if (isOutOfLimit) @@ -350,9 +350,9 @@ private void AddChildSpan(SpanTracer span) return; } + _idleTimer?.Cancel(); // Pause the idle timer while a child span is in flight _spans.Add(span); _activeSpanTracker.Push(span); - _idleTimer?.Cancel(); // Pause the idle timer while a child span is in flight } finally { @@ -369,17 +369,17 @@ internal void ChildSpanFinished() { return; } + + // Only restart the idle timer when there are no more active (unfinished) child spans + if (_activeSpanTracker.PeekActive() == null) + { + _idleTimer?.Start(_idleTimeout.Value); + } } finally { _finishLock.ExitReadLock(); } - - // Only restart the idle timer when there are no more active (unfinished) child spans - if (_activeSpanTracker.PeekActive() == null) - { - _idleTimer?.Start(_idleTimeout.Value); - } } private class LastActiveSpanTracker @@ -440,43 +440,43 @@ public void ResetIdleTimeout() { return; } + _idleTimer?.Start(_idleTimeout.Value); } finally { _finishLock.ExitReadLock(); } - try - { - _idleTimer?.Start(_idleTimeout.Value); - } - catch (ObjectDisposedException) - { - // Finish() may dispose the timer concurrently between the _hasFinished check and Start(). - // Swallow the exception — the transaction is already finishing. - } } - /// - public void Finish() + private bool TryFinishOnce() { _finishLock.EnterWriteLock(); try { if (_hasFinished) { - return; + return false; } _hasFinished = true; + _idleTimer?.Cancel(); + _idleTimer?.Dispose(); + return true; } finally { _finishLock.ExitWriteLock(); } + } - _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); - _idleTimer?.Cancel(); - _idleTimer?.Dispose(); + /// + public void Finish() + { + if (!TryFinishOnce()) + { + return; + } + _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); if (IsSentryRequest) { // Normally we wouldn't start transactions for Sentry requests but when instrumenting with OpenTelemetry