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

Change Replay sampling to sample on each error #7735

Closed
Tracked by #7540
billyvg opened this issue Apr 4, 2023 · 2 comments · Fixed by #7768
Closed
Tracked by #7540

Change Replay sampling to sample on each error #7735

billyvg opened this issue Apr 4, 2023 · 2 comments · Fixed by #7768
Assignees
Labels
Package: replay Issues related to the Sentry Replay SDK

Comments

@billyvg
Copy link
Member

billyvg commented Apr 4, 2023

Current State

Our current sampling logic for error-based samples is a bit confusing: we take the sample at the beginning of the session, before any error even occurs. This was a decision we made when the project first started in order to be able quickly kill the SDK without having to deploy anything, as well as being able to avoid the SDK overhead for un-sampled sessions.

The sampling logic makes sense for sessions with only 1 error. For example, at an error sample rate of 50%, if you have 100 errors you would expect ~50 replays. However, if the average session has 10 errors, our existing sampling logic would produce ~5 replays (100 errors / 10 errors per session = 10 sessions * 50% sample rate = 5 replays).

This outcome is quite unexpected and can simply be considered wrong. It also prevents us from a very requested feature: custom logic to determine what errors will produce a replay.

Proposal

Sample on each error

Change the sampling sequence so that we make sampling decisions when an error occurs, instead of when the Replay integration is initialized and session is created.

  • if both sample rates are 0, replay recording does not start
  • if session sample rate > 0
    • make sampling decision for session-based replay
      • if sampled, call start()
      • else, move on to error sample logic
  • if error sample rate > 0
    • start recording only (same behavior as our current onError flow, buffers events and does not upload replay) and wait until an error occurs
@aberres
Copy link

aberres commented Apr 6, 2023

This behavior is confusing.

Especially as it can lead to situations where not only 60 seconds but longer periods before an error are recorded. Leading to the same privacy concerns as described in #7770.

@billyvg
Copy link
Member Author

billyvg commented Apr 11, 2023

@aberres It will not affect the buffering duration, it will still only buffer up to 60 seconds prior to the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: replay Issues related to the Sentry Replay SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants