-
-
Notifications
You must be signed in to change notification settings - Fork 229
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
🐝 collect session replays via Sentry #4101
Conversation
Quick links (staging server):
Login:
SVG tester:Number of differences (default views): 0 ✅ Edited: 2024-11-01 01:40:56 UTC |
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.
Sorry, if you already discussed this before, but just a quick fly by review:
- We should be probably using
@sentry/react
instead of@sentry/browser
to get better contextual data - If we keep using Sentry we should stop using Bugsnag, but this can wait until we confirm it works as expected, let's just not forget about it
}), | ||
], | ||
replaysSessionSampleRate: ENV === "development" ? 1 : 0.01, | ||
replaysOnErrorSampleRate: 0, |
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.
Capturing replays on error seems even more useful than non-errors. Is there a good reason to disable it?
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.
It's more so the opposite for me, as I don't want our budget of session replays to get inadvertently consumed by a few pages that raise a lot of the same error over and over (e.g. we end up with 1k monthly session replays on /search
out of our 5k budget just b/c of errors).
Obviously, keeping replaysOnErrorSampleRate
low would help avoid this problem, but it was easier for now to just set this to 0 to get a clear baseline of how well the 5k monthly replays are distributed across our pages (without having to worry about how much errors skew the distribution of replays).
37fdef5
to
0702378
Compare
thanks Martin – done ✅ |
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.
Looks good, no obvious issues with the integration from my end. We can obviously tweak the sampling rates later as needed.
0702378
to
bcb4245
Compare
bcb4245
to
701c71a
Compare
Collect session replays via Sentry for 1% of cookie-consenting user sessions. Closes owid/analytics#155.
Implementation notes and privacy considerations.
Remaining todos before merging:
SENTRY_DSN
to staging env (then remove after merging – we don't need to waste precious replay budget on staging sites going forward). owid/ops#233SENTRY_DSN
to prod env (ops:templates/owid-admin-prod/admin-env.secret
)