ref(replay): Send SDK's name in replay event #6514
Conversation
size-limit report 📦
|
| return createEnvelope( | ||
| { | ||
| event_id: replayId, | ||
| sent_at: new Date().toISOString(), | ||
| sdk: REPLAY_SDK_INFO, | ||
| sdk: { name, version }, |
There was a problem hiding this comment.
l: Should we maybe just pass sdk: replayEvent.sdk here?
There was a problem hiding this comment.
For envelopes in the SDK, we only send name and version in the header:
sentry-javascript/packages/core/src/envelope.ts
Lines 16 to 24 in 2591d2a
According to the dev spec, we can send the entire sdk object though, so we could change it... I think for now we should go with SDK consistency, WDYT? Keeps the envelope a little smaller if we don't pass the integrations array twice for instance
There was a problem hiding this comment.
ahh, ok, I see. Sorry, this was/is a bit confusing for me, what goes on the event vs. what goes on the envelope 😅
| // extract the SDK name because `client._prepareEvent` doesn't add it to the event | ||
| const metadata = client.getOptions() && client.getOptions()._metadata; | ||
| const { name } = (metadata && metadata.sdk) || {}; | ||
|
|
||
| preparedEvent.sdk = { |
There was a problem hiding this comment.
l: IMHO we don't need to overwrite this like that, as it will never be set. I'd just do:
preparedEvent.sdk = {
name,
version: 'xxx'
}Or, maybe export the getSdkMetadataForEnvelopeHeader function or similar from core and use that somehow (but I'd say that's optional).
There was a problem hiding this comment.
If we don't use the ...preparedEvent.sdk spread, we don't add the entire SDK metadata to the event. For instance, the integrations array won't be added. I think it's fine to omit it in the envelope header but I don't know if we should remove it from the event.
getSdkMetadataForEnvelopeHeader would only make sense for constructing the envelope header. I'll take a look if this reduces bundle size
There was a problem hiding this comment.
Ok, we can use getSdkMetadataForEnvelopeHeader. Will do this in a follow-up PR though, as it involves moving stuff around.
Lms24
force-pushed
the
lms-replay-ref-sdkname
branch
from
e51d36c to
73d63b3
Compare
) As outlined in #6539, we were previosuly not sending the `dsn` key in the replay event [envelope header](https://develop.sentry.dev/sdk/envelopes/#envelope-headers), thereby not including the necessary information to forward the event to Sentry. This patch fixes that by extracting the `createEventEnvelopeHeaders` function to utils so that we can use it in Replay (analogously to #6514), as well as using this function to create all headers. We tested the change with NextJS' `tunnelRoute` as well as with the conventional `tunnel` option and everything seems to work now.
This PR replaces the
sentry.javascript.integration.replayname inevent.sdk.namewith the actual Sentry SDK name (e.g.sentry.javascript.browser), analogously to how we send SDK metadata in other event types.Going forward, we can can also use this information e.g. for data analysis to filter replay events by SDK
ref #6366