-
Notifications
You must be signed in to change notification settings - Fork 140
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
[RUMF-867] implement stop recording #771
Conversation
packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts
Show resolved
Hide resolved
@@ -79,6 +79,9 @@ export interface RawRumViewEvent { | |||
long_task: Count | |||
resource: Count | |||
} | |||
session: { | |||
has_replay: true | undefined |
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.
Semantically it would be better to have a boolean?
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.
Yes but in practice, this field should not exist for backward and other SDK compatibility. If I want to select all events that don't have replay, using @session.has_replay:false
would lead to incorrect results since not all SDKs implement this flag. Using [email protected]_replay:true
is the way to go.
Using this weird typing forces us to ensure has_replay
is never false
.
Having multiple loose variables to reflect a state does not scale. Instead of adding a new `isStopped` variable, make things more explicit by using a finite number of states, acting like a single source of truth for the whole segment collection. This helps to understand the various state transitions.
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.
LGTM!
Motivation
Implement a method to stop session replay recording
Changes
true
.Testing
Unit, manual
I have gone over the contributing documentation.