feat: Defer capturing root replay until upload#106
Conversation
6332727 to
aeed92f
Compare
|
Ah this isn't correct yet, we should only create the event if a session was newly created. |
e1ae1e2 to
1b55f12
Compare
56ad54b to
f7061df
Compare
src/index.ts
Outdated
| this.shouldCreateReplay = false; | ||
| } | ||
|
|
||
| // TEMP: keep sending a replay event just for the duration |
There was a problem hiding this comment.
going forward we'll send replay events pretty frequently for things like url / transaction name, so not sure if this is just temp
src/index.ts
Outdated
| * event per session and it should only be created | ||
| * immediately before sending recording) | ||
| */ | ||
| private shouldCreateReplay = false; |
There was a problem hiding this comment.
nitpicky, but i'd prefer a name like:
private hasInitializedReplay = false
private didCaptureRootEvent = false
idk. i guess i'm thinking something more definitive sounding instead of 'should'
Alternatively it could be a property of the session, that's what is being passed into captureReplay and call captureReplay unconditionally, do the check and update the flag in there, and keep options private.
There was a problem hiding this comment.
I can make it more affirmative sounding like needsCaptureReplay - the suggested names imply it can only be initialized once, but a session could expire and we would need to create a new root event.
re: property of Session - this does feel icky having it live in options (it's not really an option, and it will be a bit annoying having to sync its state). I may refactor this actually because what we need to know is if the session has been newly created, or if it was loaded from storage, which can be handled when calling getSession rather than have it live as a property on the Session.
Co-authored-by: Ryan Albrecht <ryan@ryanalbrecht.ca>
…return if session was newly created or loaded from storage
…ay#106) I suspect we have a lot of "empty" replays due to this issue where we are immediately creating the root replay event. This had the possibility of having a replay w/ no recording events. Unfortunately, after some testing, a lot of short replays still get through because we immediately flush updates after a full DOM checkout. However, this is still needed in order to implement getsentry/sentry-replay#101.
I suspect we have a lot of "empty" replays due to this issue where we are immediately creating the root replay event. This had the possibility of having a replay w/ no recording events. Unfortunately, after some testing, a lot of short replays still get through because we immediately flush updates after a full DOM checkout. However, this is still needed in order to implement #101.