Skip to content

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Mar 28, 2025

This PR contains several fixes to the Envelope processor surrounding DSC
validation. Together, these fixes ensure that the _dsc metadata field
in the event payload contains the same values that were used during
sampling and normalization. It also fixes the trace.client_sample_rate
field in case a foreign DSC was reset.

Background

Relay validates whether the DSC originates in a project of the same
organization. If this is not the case and we were to use this DSC for
dynamic sampling, we would be applying sampling rules and rates from a
different organization.

If the DSC is invalid, we try to compute a new one from the information
in the event payload: We take the trace ID, environment, release, and
project key from event data. This simplifies the sampling routines,
since they can afterwards use the DSC and do not have to access the
event payload.

Fixes

  1. If computing a fallback DSC from the event payload fails, remove the
    DSC from the event payload. Previously, we would leave the old DSC
    in.
  2. Retain the client sampling rate from the original DSC. While the
    remaining DSC contents cannot be used, the sampling rate was actually
    applied and must be retained for extrapolation. Previously, we
    removed it along with the rest of the foreign DSC.
  3. Validate the DSC before finalize_event, where it gets written into
    the event payload _dsc field. Since we validate the DSC both in
    PoPs and processing Relays, in practice the field always received the
    correct value. However, we now ensure correct order.
  4. Validate the DSC before normalization, where we materialize the
    client sampling rate into the event's trace context. Previously,
    these two functions were switched and potentially wrong data was
    written.
  5. Apply DSC validation in the errors pipeline so that the correct
    _dsc metadata is materialized there, too.

@jan-auer jan-auer requested a review from a team as a code owner March 28, 2025 15:40
@jan-auer jan-auer enabled auto-merge (squash) March 28, 2025 16:03
@jan-auer jan-auer merged commit 9d9bea0 into master Mar 28, 2025
25 checks passed
@jan-auer jan-auer deleted the fix/trace-ctx-client-sample-rate branch March 28, 2025 16:09
mjq added a commit that referenced this pull request Apr 3, 2025
Wrote this integration test while confirming my understanding of #4625.
It confirms that when the DSC and envelope's org IDs disagree, the DSC
is recreated but the original sample rate is propagated.

I don't think this case is quite covered by existing integration tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants