Uh oh!
There was an error while loading. Please reload this page.
feat(replay): Do not capture replays < 5 seconds - #7949
Conversation
1b9048f to
7049268Comparesize-limit report 📦
|
7049268 to
617e530Compare| // a previous session ID. In this case, we want to buffer events | ||
| // for a set amount of time before flushing. This can help avoid | ||
| // capturing replays of users that immediately close the window. | ||
| setTimeout(() => replay.conditionalFlush(), options._experiments.delayFlushOnCheckout); |
There was a problem hiding this comment.
I wonder if this is worth the overhead, to have a separate initial flush timeout 🤔 If we "enforce" this to be === general flush debounce timeout, we could simply call debounced flush and call it a day? Then it would also ensure we do not flush twice etc. out of the box?
There was a problem hiding this comment.
I think it's fine for now since this is an experiment. We can think of bundling it up with the debounced flush when we GA this. The conditionalFlush here operates on the debounced flush anyway.
There was a problem hiding this comment.
If we allowed the general debounce timeout to be user-controlled, chances are that they set it to something too high that would cause payloads to become too big?
There was a problem hiding this comment.
👍 I don't think this is necessarily an option we expose at all - we just want to verify data integrity on Sentry before moving forward with a change.
Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab. Closesgetsentry/team-replay#63
c0cc254 to
aab4786Compare
Lms24
left a comment
There was a problem hiding this comment.
I think it's worth revisiting this option before we promote it to the stable API but for now as an experiment, this LGTM!
| scrollTimeout: number; | ||
| ignoreSelectors: string[]; | ||
| }; | ||
| delayFlushOnCheckout: number; |
There was a problem hiding this comment.
m: As long as we keep this an experiment, I'm fine with this name but if we make part of the public stable API, should we rename it to something less technical like initialFlushDelay?
There was a problem hiding this comment.
Yeah, as stated in a diff comment, this may not be a public option, but if it is, we will rename it!
Promotes the feature from #7949 to GA. Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab. Closesgetsentry/team-replay#65
Promotes the feature from #7949 to GA. Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab. Closesgetsentry/team-replay#65
Promotes the feature from #7949 to GA. Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab. Closesgetsentry/team-replay#65
Promotes the feature from #7949 to GA. Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab. Closesgetsentry/team-replay#65
Do not immediately flush on snapshot checkouts, instead delay by minimum flush delay (5 seconds). This means that we will not collect replays < 5 seconds. e.g. User opens site and immediately closes the tab.
Semi-reverts #6463
Closeshttps://github.com/getsentry/team-replay/issues/63