diff --git a/samples/Sentry.Samples.Maui/MauiProgram.cs b/samples/Sentry.Samples.Maui/MauiProgram.cs index c1789d3850..2dadfc55d0 100644 --- a/samples/Sentry.Samples.Maui/MauiProgram.cs +++ b/samples/Sentry.Samples.Maui/MauiProgram.cs @@ -42,6 +42,9 @@ public static MauiApp CreateMauiApp() // 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 options.AddCommunityToolkitIntegration(); 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..45c7e30675 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,72 @@ internal void HandlePageEvents(Page page, bool bind = true) } } + private ITransactionTracer? StartNavigationTransaction(string name) + { + // 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(); + 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") + { + NameSource = TransactionNameSource.Route + }; + + var transaction = _hub is IHubInternal internalHub + ? internalHub.StartTransaction(context, _options.NavigationTransactionIdleTimeout) + : _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 => + { + 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; + } + // 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 +401,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 +487,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 +496,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 +511,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/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/ISentryTimer.cs b/src/Sentry/Infrastructure/ISentryTimer.cs new file mode 100644 index 0000000000..516ba57ca2 --- /dev/null +++ b/src/Sentry/Infrastructure/ISentryTimer.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 . + /// + public void Start(TimeSpan timeout); + + /// + /// Cancels any pending fire. Has no effect if the timer is already cancelled. + /// + public 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/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..67d11b770a --- /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. + /// + public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); +} 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/SentrySdk.cs b/src/Sentry/SentrySdk.cs index fd7a6bd90f..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); @@ -663,6 +668,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/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..1a4224a078 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; @@ -9,13 +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 Timer? _idleTimer; + private readonly ISentryTimer? _idleTimer; + private readonly TimeSpan? _idleTimeout; private readonly SentryStopwatch _stopwatch = SentryStopwatch.StartNew(); - private InterlockedBoolean _hasFinished; - - private InterlockedBoolean _cancelIdleTimeout; + private bool _hasFinished; + private readonly ReaderWriterLockSlim _finishLock = new(); private readonly Instrumenter _instrumenter = Instrumenter.Sentry; @@ -223,7 +226,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 +247,52 @@ 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 => + _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; + } + + // Discard if no child spans were ever started + bool shouldDiscard; + _finishLock.EnterWriteLock(); + try + { + if (_spans.IsEmpty && !_hasFinished) { - if (state is not TransactionTracer transactionTracer) - { - _options?.LogDebug( - $"Idle timeout callback received nor non-TransactionTracer state. " + - "Unable to finish transaction automatically." - ); - return; - } + _hasFinished = true; + shouldDiscard = true; + } + else + { + shouldDiscard = false; + } + } + finally + { + _finishLock.ExitWriteLock(); + } - transactionTracer.Finish(Status ?? SpanStatus.Ok); - }, this, idleTimeout.Value, Timeout.InfiniteTimeSpan); + if (shouldDiscard) + { + _options?.LogDebug("Idle transaction '{0}' has no child spans. Discarding.", SpanId); + _idleTimer?.Dispose(); + _hub.ConfigureScope(static (scope, tracer) => scope.ResetTransaction(tracer), this); + return; } + + Finish(Status ?? SpanStatus.Ok); } /// @@ -301,13 +333,52 @@ 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() + { + _finishLock.EnterReadLock(); + try + { + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + } + 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); } } @@ -357,22 +428,55 @@ 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() + { + _finishLock.EnterReadLock(); + try + { + if (!_idleTimeout.HasValue || _hasFinished) + { + return; + } + } + 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() { - if (_hasFinished.Exchange(true)) + _finishLock.EnterWriteLock(); + try { - return; + if (_hasFinished) + { + return; + } + _hasFinished = true; } - - _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); - if (_cancelIdleTimeout.Exchange(false) == true) + finally { - _options?.LogDebug("Disposing of idle timer for Transaction '{0}'.", SpanId); - _idleTimer?.Change(Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); - _idleTimer?.Dispose(); + _finishLock.ExitWriteLock(); } + _options?.LogDebug("Attempting to finish Transaction '{0}'.", SpanId); + _idleTimer?.Cancel(); + _idleTimer?.Dispose(); + if (IsSentryRequest) { // Normally we wouldn't start transactions for Sentry requests but when instrumenting with OpenTelemetry @@ -384,7 +488,22 @@ 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) + .DefaultIfEmpty() + .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/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 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..b45ab996e0 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,11 +15,9 @@ internal class MauiEventsBinderFixture public MauiEventsBinderFixture(params IEnumerable elementEventBinders) { - Hub = Substitute.For(); + 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.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..ad44c3ed5e 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,264 @@ public void Shell_UnbindNavigated_DoesNotAddBreadcrumb() // Assert Assert.Single(_fixture.Scope.Breadcrumbs); } + + [Fact] + public void Shell_Navigating_FirstNavigation_SetsTransactionOnScope() + { + // 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 + 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"), + Arg.Any()); + } + + [Fact] + 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; + 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_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() + { + // 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_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() + { + // 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 Shell_Navigating_ManualTransactionOnScope_AutoTransactionCreatedButNotBoundToScope() + { + // 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; + + 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 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); + } + + [Fact] + public void Shell_Navigating_SameRoute_ActiveTransaction_ResetsIdleTimeout() + { + // 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 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() + { + // 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.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 86aa068b07..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) { } @@ -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.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 diff --git a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs index 604e1cdda7..e774e7a0f3 100644 --- a/test/Sentry.Tests/Protocol/SentryTransactionTests.cs +++ b/test/Sentry.Tests/Protocol/SentryTransactionTests.cs @@ -27,34 +27,6 @@ public void NewTransactionTracer_ConstructingWithContext_HasValidStartTime() Assert.NotEqual(DateTimeOffset.MinValue, actualTransaction.StartTimestamp); } - [Fact] - public async Task NewTransactionTracer_IdleTimeoutProvided_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 transaction = new TransactionTracer(hub, context, TimeSpan.FromMilliseconds(2)); - - // Act - await Task.Delay(TimeSpan.FromSeconds(2)); - - // Assert - transaction.IsFinished.Should().BeTrue(); - } - [Fact] public void NewTransactionTracer_PropagationContextHasReplayId_UsesActiveSessionReplayIdInstead() { diff --git a/test/Sentry.Tests/TransactionTracerTests.cs b/test/Sentry.Tests/TransactionTracerTests.cs index 9ad72fce5c..6aaa408e19 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,124 @@ public void Dispose_WithTrackedSpans_ClearsTrackedSpans() // Assert 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] + 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()); + } }