fix(dsc): Stop using cross-org DSC data in v2 span normalization#6073
fix(dsc): Stop using cross-org DSC data in v2 span normalization#6073elramen wants to merge 15 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| /// An exception exists for our OTeL integration, which currently never sets a dynamic sampling | ||
| /// context. In the future we may want to extract a DSC from the OTeL payload to allow dynamic | ||
| /// sampling if the necessary attributes are present. | ||
| pub fn validate_dsc_presence(spans: &SerializedSpans) -> Result<()> { |
There was a problem hiding this comment.
Ideally we keep this function alive to keep the documentation and reasoning comments alive.
It's fine if this becomes a private function and is just called from a "validate_and_set" step. If that's not really possible, we definitely still want to keep the reasoning and comments alive.
There was a problem hiding this comment.
Tried to make the comments more to the point and avoid repetition. I'll bring back context that was lost and we can see if something is still missing.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21ded61. Configure here.
| Ok(()) | ||
| match ctx.sampling_project_info { | ||
| None => { | ||
| *dsc = DynamicSamplingContext { |
There was a problem hiding this comment.
Is it not possible here that spans.headers already has a DSC with more detailed information (release, environment, etc.)?
3 participants
Stop using cross-org DSC data in v2 span normalization. Instead, nullify DSC attributes when the DSC/trace comes from a different org.
Reason for nullifying instead of using span attributes like
sentry.segment_name: there is no guarantee that all spans in the envelope will have the same value set for these attributes. We could check if the spans all align but this is not good because how an SDK groups spans per envelope (e.g. 1 vs 2 segments per envelope) should not impact dynamic sampling.This change also includes two other improvements:
project_iddirectly in the DSC, removing the need for theEnrichedDscstruct.dsc_muttoEnvelopeHeadersto avoid having to clone and set the DSC when updating only a single attribute.Fixes RELAY-254