-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(replay): Bump limit for minReplayDuration #18190
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
Conversation
dev-packages/browser-integration-tests/suites/replay/minReplayDurationLimit/init.js
Show resolved
Hide resolved
dev-packages/browser-integration-tests/suites/replay/minReplayDurationLimit/test.ts
Outdated
Show resolved
Hide resolved
…DurationLimit/init.js Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
…DurationLimit/test.ts Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
|
@billyvg just wondering, would we not also drop buffered replays here since they can be anywhere from 0-60s? |
Yeah i think we just have to strongly warn users about this if we are going to add it. Though looking at [the code]... it seems like we only consider the session start to current flush time as the "duration" - |
|
|
||
| /* | ||
| The max. allowed value that the minReplayDuration can be set to. | ||
| This needs to be below 60s, so we don't unintentionally drop buffered replays that are longer than 60s. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add the comment in the replay options type so that it's visible to users. We should also be clear that this can drop onError-sampled replays
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
With this PR users can set their min replay duration to max 50s, previously this was capped at 15s.
We cannot bump this value further as this would lead to dropping buffered replays (we keep max. 60s in-memory at this point)
closes #18109