[HEAVILY WIP] ref(tracing): Refactor IdleTimeout Algorithm #4471
Conversation
In prep for adding a double timeout to tracing, we first start by removing the heartbeat from transactions, which should be covered by the existence of the final timeout.
size-limit report
|
There was a problem hiding this comment.
We should probably just implement finalTimeout in this PR but have it be optional (set the default to undefined, only if passed a finalTimeout will the setTimeout for final be called). That way after this is merge we can try it right away on ourselves.
Other than that, a couple things around how idleTimeout could be changed, I think the removal of heartbeat will cause it to be a bit more sensitive. Not sure if we should leave improving idleTimeout for later, or we want to try this on ourselves first to see the % difference in idleTimeout reasons.
| }); | ||
|
|
||
| function createIdleTransction( |
| * | ||
| * Time is in ms | ||
| * | ||
| * Default: 10000 |
There was a problem hiding this comment.
Didn't we say 10_000 is a bit too short given that many transactions will be on 3g connections etc?
| export const DEFAULT_IDLE_TIMEOUT = 1000; | ||
| export const HEARTBEAT_INTERVAL = 5000; | ||
| // 10 seconds | ||
| export const DEFAULT_FINAL_TIMEOUT = 10000; |
There was a problem hiding this comment.
See comment above, should likely bump final timeout.
| this._finalTimeoutID = setTimeout(() => { | ||
|
|
||
| }, this._finalTimeout) |
There was a problem hiding this comment.
this.setTag(FINISH_REASON_TAG, <final>);
this.finish();
???
| } | ||
|
|
||
| if (Object.keys(this.activities).length === 0) { | ||
| if (Object.keys(this._activities).length === 0) { |
There was a problem hiding this comment.
Can you confirm if activities includes ongoing http requests? I was thinking along the lines of TTI quiescence (see https://github.com/WICG/time-to-interactive#definition)
There was a problem hiding this comment.
Yup can confirm it does.
|
Closing this in favour of alternate implementation |
🚧 WIP 🏗️
Based on #4442 (comment)
I just pushed this up for size bot