feat(browser): Update propagationContext on spanEnd to keep trace consistent
#11631
Conversation
size-limit report 📦
|
Lms24
force-pushed
the
lms/feat-browser-update-propagationContext-spanEnd
branch
from
4fc17db
to
7197b8c
Compare
|
|
||
| const newPropagationContext = { | ||
| ...oldPropagationContext, | ||
| sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span), |
There was a problem hiding this comment.
l/m: Not 100% sure, but maybe it would be safer to pick this from the DSC, to ensure this is aligned? So something like this:
const dsc = oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span);
const dscSampled = dsc.sampled === 'true' ? true : dsc.sampled === 'false' : false : undefined;
const sampled = oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : dscSampled;There was a problem hiding this comment.
Sure, can change it! I'm just wondering about a concrete scenario where these two might diverge; like spanIsSampled returning false while the dsc.sampled being true or undefined.
There was a problem hiding this comment.
Please do not use any values from the DSC in the SDK. This has potential to cause a lot of issues later on.
There was a problem hiding this comment.
Okay, so we quickly discussed this in person with @cleptric and settled on updating propagationContext.sampled from the propagation context and spanIsSampled instead of the DSC. The reason is that there are talks around removing sampled from the DSC again because it's (likely) unused in ingest/product.
Both of the approaches currently lead to the same outcome, as demonstrated by the integration tests but it's safer in this case to read from propagationContext (i.e. sentry-trace) instead of DSC/baggage.
fix incorrect `baggage` header in <meta> integration test
As spec'd out in #11599 and agreed upon in internal discussions, a trace should to stay consistent over the entire time span of one route. Therefore, when the initial pageload or navigation span ends, we update the scope's propagation context to keep span-specific attributes like the
sampleddecision and the dynamic sampling context on the scope's propagation context, even after the transaction has ended.This ensures that the trace data is consistent for the entire duration of the route. Subsequent navigations will reset the propagation context (see #11377).
cc @cleptric
ref #11599