-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: configure session recording sample rate #18068
feat: configure session recording sample rate #18068
Conversation
groupType: TaxonomicFilterGroupType.FeatureFlags, | ||
value: value, | ||
onChange: (_, __, item) => { | ||
'id' in item && item.id && onChange(item.id, item.key) |
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.
@liyiy @neilkakkar I've moved the flag selector into components since I wanted to use it in replay config. And exposed the flag key as well as the id on change... That ok?
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.
Some small points but no show stoppers
frontend/src/scenes/session-recordings/settings/SessionRecordingSettings.tsx
Outdated
Show resolved
Hide resolved
frontend/src/scenes/session-recordings/settings/SessionRecordingSettings.tsx
Show resolved
Hide resolved
frontend/src/scenes/session-recordings/settings/SessionRecordingSettings.tsx
Outdated
Show resolved
Hide resolved
/> | ||
</div> | ||
<p> | ||
Setting a minimum session duration will ensure that only sessions that last longer than that |
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.
I think we should warn that this does mean some useful data may not be collected. Debugging a user issue that starts with a loading screen before redirecting to another domain. With this setting set to say, 10 seconds, the initial redirect screen would not have been recorded as it is only buffered in memory.
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.
This could also be expanded in documentation instead of the modal
* feat: configure session recording sample rate * fix mock * Update query snapshots * Update query snapshots * Update query snapshots * fix * fix * Update query snapshots * fix * fix * Update query snapshots * Update query snapshots * Update query snapshots * Update query snapshots * Update query snapshots * Fix * fix * fix * add minimum duration setting * dropdown instead of text box * start adding linked flag * fix * fix * Update query snapshots * Update query snapshots * Update query snapshots * Update query snapshots * only return flag key in decide * doh * fix * Fix * don't have a restricted list of sample rates * fix * fix * Update query snapshots * all the options * review changes * Fix * fix --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
We want to allow
This
This pairs nicely with PostHog/posthog-js#839