-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(types): Upstream some replay types #6506
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
Conversation
size-limit report 📦
|
packages/replay/src/replay.ts
Outdated
@@ -942,6 +942,7 @@ export class ReplayContainer implements ReplayContainerInterface { | |||
trace_ids: traceIds, | |||
urls, | |||
replay_id: replayId, | |||
event_id: replayId, |
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 don't think this is what we want as we will then have multiple replay events with the same event id?
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 can get of this line, right? The uuid is assigned in _prepareEvent.
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.
done!
replay_event: 'replay_event', | ||
replay_recording: 'replay_recording', |
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.
damn we should have named these replay
and recording
to be consistent with our other event types
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 apart from the event_id as mentioned by Billy.
a345409
to
8a6a338
Compare
This PR upstreams some types from
@sentry/replay
to@sentry/types
, in preparation for moving some stuff there.Especially the
ReplayEvent
may still change a bit, but I think this should be fine.