-
-
Notifications
You must be signed in to change notification settings - Fork 230
feat: Auto-create traces for MAUI navigation events #5111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8d40272
4041119
09760b3
3a8c8f0
86a36db
e02ac47
e0145f4
67c36c3
2b71e0c
430ec80
54b3d45
5f73295
a3b5e22
0d5a5f4
0418c99
2995289
b75839b
b9e4ff1
7a0395a
a0778cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<IMauiElementEventBinder> _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,70 @@ 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); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In practice, everything that implements Potentially we could replace it with an Maybe rather than using an @Flash0ver thoughts? |
||
|
|
||
| // Only bind to scope if there is no user-created transaction already there. | ||
| 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; | ||
| } | ||
|
jamescrosswell marked this conversation as resolved.
|
||
| } | ||
| 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 +399,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,22 +485,37 @@ 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() ?? ""); | ||
| data.Add("to", e.Target?.Location.ToString() ?? ""); | ||
| 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() ?? ""); | ||
| data.Add("to", e.Current?.Location.ToString() ?? ""); | ||
| 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 _) => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,4 +26,9 @@ public interface ITransactionTracer : ITransactionData, ISpan | |
| /// Gets the last active (not finished) span in this transaction. | ||
| /// </summary> | ||
| public ISpan? GetLastActiveSpan(); | ||
|
|
||
| /// <summary> | ||
| /// Resets the idle timeout for auto-finishing transactions. No-op for transactions without an idle timeout. | ||
| /// </summary> | ||
| public void ResetIdleTimeout(); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically this would be a breaking change as well... we could put this (as an @Flash0ver what do you think? |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| namespace Sentry.Infrastructure; | ||
|
|
||
| /// <summary> | ||
| /// Abstraction over a one-shot timer, to allow deterministic testing. | ||
| /// </summary> | ||
| internal interface ISentryTimer : IDisposable | ||
| { | ||
| /// <summary> | ||
| /// Starts (or restarts) the timer to fire after <paramref name="timeout"/>. | ||
| /// </summary> | ||
| public void Start(TimeSpan timeout); | ||
|
|
||
| /// <summary> | ||
| /// Cancels any pending fire. Has no effect if the timer is already cancelled. | ||
| /// </summary> | ||
| public void Cancel(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| namespace Sentry.Infrastructure; | ||
|
|
||
| /// <summary> | ||
| /// Production <see cref="ISentryTimer"/> backed by <see cref="System.Threading.Timer"/>. | ||
| /// </summary> | ||
| 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(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| namespace Sentry.Internal; | ||
|
|
||
| /// <summary> | ||
| /// Internal hub interface exposing additional overloads not part of the public <see cref="IHub"/> contract. | ||
| /// Implemented by <see cref="Hub"/>, <see cref="Extensibility.DisabledHub"/>, and | ||
| /// <see cref="Extensibility.HubAdapter"/>. | ||
| /// </summary> | ||
| internal interface IHubInternal : IHub | ||
| { | ||
| /// <summary> | ||
| /// Starts a transaction that will automatically finish after <paramref name="idleTimeout"/> if not | ||
| /// finished explicitly first. | ||
| /// </summary> | ||
| public ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically it'd be a breaking change if we added this method to the public |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -210,6 +210,11 @@ public static IDisposable Init(Action<SentryOptions>? 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; | ||
| } | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this to (try to) stop Warden from complaining about a potential infinite recursion... since We could throw an exception here instead of logging and returning a |
||
| 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); | ||
|
|
||
| /// <summary> | ||
| /// Starts a transaction that will automatically finish after <paramref name="idleTimeout"/> if not | ||
| /// finished explicitly first. | ||
| /// </summary> | ||
| [DebuggerStepThrough] | ||
| internal static ITransactionTracer StartTransaction(ITransactionContext context, TimeSpan? idleTimeout) | ||
| => CurrentHub is IHubInternal internalHub | ||
| ? internalHub.StartTransaction(context, idleTimeout) | ||
| : CurrentHub.StartTransaction(context); | ||
|
|
||
| /// <summary> | ||
| /// Starts a transaction. | ||
| /// </summary> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.