Skip to content
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

IdleTransaction duration should not exceed final timeout #8504

Closed
AbhiPrasad opened this issue Jul 11, 2023 · 5 comments · Fixed by #8653
Closed

IdleTransaction duration should not exceed final timeout #8504

AbhiPrasad opened this issue Jul 11, 2023 · 5 comments · Fixed by #8653
Assignees
Labels
Feature: Spans Package: browser Issues related to the Sentry Browser SDK Sync: Jira apply to auto-create a Jira shadow ticket Type: Bug

Comments

@AbhiPrasad
Copy link
Member

AbhiPrasad commented Jul 11, 2023

Right now finalTimeout just sets a setTimeout, but doesn't actually enforce a maximum transaction length. This means there are some scenarios where a transaction has started, but has a super long duration (1 min+).

If we see that a transaction's duration has exceeded the final timeout, we should drop it, because it means that the duration is effectively invalid (didn't respect setTimeout of finalTimeout).

┆Issue is synchronized with this Jira Improvement by Unito

@AbhiPrasad AbhiPrasad added Type: Bug Feature: Spans Package: browser Issues related to the Sentry Browser SDK labels Jul 11, 2023
@AbhiPrasad AbhiPrasad changed the title IdleTransaction duration should not exceed 30s IdleTransaction duration should not exceed final timeout Jul 11, 2023
@ndmanvar ndmanvar added the Sync: Jira apply to auto-create a Jira shadow ticket label Jul 17, 2023
@AbhiPrasad
Copy link
Member Author

we might be able to share the solution here with react native as they also have issues with final timeout: getsentry/sentry-react-native#3165

There's two possible approaches:
a) we trim all transactions to finalTimeout. This has the unfortunate side effect of produced a bunch of transactions at the final timeout, which will skew aggregate statistics. I prefer not to do this.
b) we drop all transactions with a duration that exceeds finalTimeout + idleTimeout. This is the solution in the draft PR. We might have to create a new client outcome for this.

@krystofwoldrich
Copy link
Member

Yes, we could share the fix, that would be nice.

Although we incline to the option a) for the fix. At the moment the idleTx has heartbeat when no spans change in x number of beats the transaction is finished. This is also implemented as setTimeout and doesn't have to be executed the same as finalTimeout.

In RN the reason for the timeouts being executed with hours/days delay is simple, the apps are in the background. Other mobile SDKs, for example, Android calls its version of finalTimeout when the app is in the background -> the tx will be finished with the deadline exceeded.

@danielkhan
Copy link

@AbhiPrasad, what triggers the end of a transaction? Are there any heuristics in place?
I am asking because sometimes it defeats the purpose when we bundle the page load and the user interactions after the page is loaded into one transaction.

@AbhiPrasad
Copy link
Member Author

@danielkhan, idle transactions (pageloads and navigations) end themselves when the page is at an idle state, which means there is no components rendering or active http requests. We have some extra heuristics in place for polling scenarios and artificially long activities to make sure transactions always finished.

@krystofwoldrich
Copy link
Member

IMG_0282

Here is a visual of what is happening. Blue is the current state and green is how we could solve it. Also note this happens only when there is at least one active span in the transaction at the moment when JS Thread is paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Spans Package: browser Issues related to the Sentry Browser SDK Sync: Jira apply to auto-create a Jira shadow ticket Type: Bug
Projects
None yet
4 participants