feat(core): Send Baggage in Envelope Header

[Lms24]

This PR introduces the first step of getting Dynamic Sampling working with the JS SDKs. It contains a very simple approach of attaching baggage to the envelope header of a transaction's event envelope. It does not take immutability of baggage in multiple transactions per trace scenarios into account. This is something we have to address when we're going to add the baggage header to outgoing requests. However, the simple approach should be enough to send baggage information to relay to get DS working on individual transactions.

Open question which I'm happy to address in this PR or in follow-ups:

• Should we hide this behind an experimental flag?
• I added more properties to be sent than introduced in feat(utils): Introduce Baggage API #5066. My reasoning: At the time of creating the baggage for envelopes, we - theoretically and if provided by the user - have all the data we need. As long as we're not caring about immutability after the first baggage creation in the trace, we're safe to add the other properties as well. Also, this might give the DS teams more testing opportunities on single transactions.

ref: https://getsentry.atlassian.net/browse/WEB-919

[github-actions]

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	18.96 KB (-5.88% 🔽)
@sentry/browser - ES5 CDN Bundle (minified)	58.67 KB (-9.21% 🔽)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	17.82 KB (-5.53% 🔽)
@sentry/browser - ES6 CDN Bundle (minified)	52.57 KB (-9.32% 🔽)
@sentry/browser - Webpack (gzipped + minified)	19.55 KB (-15.85% 🔽)
@sentry/browser - Webpack (minified)	62.03 KB (-24.1% 🔽)
@sentry/react - Webpack (gzipped + minified)	19.58 KB (-15.9% 🔽)
@sentry/nextjs Client - Webpack (gzipped + minified)	43.03 KB (-10.46% 🔽)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	24.6 KB (-5.67% 🔽)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	23.17 KB (-5.37% 🔽)

[Lms24]

This doesn't have anything to do with baggage but it's a no longer necessary check I stumbled across. See this conversation for context

[Sija]

Btw, why not use scope?.getSession() notation?

[AbhiPrasad]

Great question!

It's because we try to optimize our codebase around targeting es6. Optional Chaining is a newer JS feature, ES2020 - "ES11" as per: https://gist.github.com/rajaramtt/7df3702a04c644b0b62c9a64f48f3dbf#64-optional-chaining. This means that it'll have to be down-compiled to es6 compatible code when we generate the final assets to distribute on npm and the CDN (this is for max browser + node version support).

Something like so:

if (hey?.me) {
 console.log('me'); 
}

ends up getting down-compiled to:

if (hey !== null && hey !== void 0 && hey.me) {
  console.log('me');
}

See the babel demo.

which is way more bytes, which is bad for bundle size.

We made a decision as a result to not use any optional chaining in code that might be used in browser environments. See some examples on bundle wins we made here: https://github.com/getsentry/sentry-javascript/pulls?q=is%3Apr+optional+chaining+is%3Aclosed+milestone%3A%22Tree+shaking+%2F+Bundle+Size%22

[AbhiPrasad]

#5066 has been merged, I think we can rebase this accordingly.

[AbhiPrasad]

🚀

[AbhiPrasad]

Should we hide this behind an experimental flag

In Relay we allow for arbitrary headers for forwards compatibility reasons, so I think we should be fine to send these: https://github.com/getsentry/relay/blob/7e63150f486fa6cd35db599aef6287040601f8e7/relay-server/src/envelope.rs#L705

[Lms24]

Thanks for the review, I'll address the feedback first thing tomorrow morning.

[Lms24]

In Relay we allow for arbitrary headers for forwards compatibility reasons, so I think we should be fine to send these

Ahh right, that makes sense. Then I think for now we're good. I was Just thinking that we might have to revisit this when we're adding the header to outgoing requests (due to the potential CORS problems we talked about). If we decide hide the baggage header in outgoing requests behind an experimental flag for v7, we should discuss if we want to continue sending baggage data in the envelope header when the flag is not set. Don't know what this entails in terms of consistency with traces/transactions. But this is a discussion for another time.

Anyway, I implemented the suggestions and I think this should be good to get merged in. Do you wanna take another look @AbhiPrasad ?

[AbhiPrasad]

I was Just thinking that we might have to revisit this when we're adding the header to outgoing requests (due to the potential CORS problems we talked about).

Yes, let’s bring this up as a TSC topic actually.

Everything else looks good for me to ship