fix(apm): Check activities count before finishing idle transaction #2672
Conversation
AbhiPrasad
force-pushed
the
abhi/fix/idle-transaction-finish
branch
from
efbdc9e to
ed26a7e
Compare
AbhiPrasad
changed the title
| if (Object.keys(Tracing._activities).length === 0 && Tracing._activeTransaction) { | ||
| Tracing.finishIdleTransaction(end); | ||
| } |
There was a problem hiding this comment.
We have to be careful here. I tinkered around with this for a long time.
While in your specific example it might improve the transaction, in other cases this has the potential of not sending the transaction at all.
Also, this change was made on purpose, before (see git history) we have a debouce timer that kept the Transaction alive which resulted in may weird very long Transaction. This change was made so after all the initial requests are done and the first "idle" occurs, we finish the Transaction no matter what (to prevent long transactions).
We finish Span that would have been started and mark them as cancelled.
There was a problem hiding this comment.
I see. The issue here is that right now the moment the activities goes to 0, the idle transaction ends.
I see what you mean by long running transaction (like if a page keeps creating single activities), but a user can easily just adjust the timeout option to instantly call finish idle transaction.
We could also apply a heuristic here, and say something like if the activities went from 0 -> 1 -> 0 -> 1 a certain amount of times, we should just end the idle transaction.
This change should still lead to transactions finishing through the heartbeat though right?
Right now testing on multiple Sentry pages, a single page load fails to account for a lot of our requests, and I’d rather optimize for that use case then 0 -> 1 -> 0 ->1 causing long running transactions.
|
After discussing with Daniel, we are closing this PR to investigate this issue some more. Points to figure out
I will be cherry-pick the spelling changes from this branch into another PR. |
After an idle transaction timeout, we should check to see if the activities count is still 0 before finishing a transaction.
Also fixed some spelling/grammar mistakes.