fix(develop): clarify trace context's client_sample_rate #13174
Conversation
When reading the Trace Context docs, I assumed that `client_sample_rate` (like all the other fields) was expected to be set by the client. Instead, this field is generated in Relay. If a client sets this field, it will be ignored and overwritten. Added a note to the docs for this field to clarify how it is meant to be used.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
| `client_sample_rate` | ||
|
|
||
| : _Optional_. The client-side sample rate. | ||
| : _Optional_. The client-side sample rate. This field will be populated by relay. Incoming values are ignored. |
There was a problem hiding this comment.
I don't think this is fully correct, we have a code path which re-creates the DSC when the parent project is not in the same org or missing.
Maybe @jan-auer can also chime in?
There was a problem hiding this comment.
Yes, this was the subject of @jan-auer's recent changes. The DSC generation happens here. But the sample rate is taken from the incoming DSC, never from the trace context's client_sample_rate - that field is never read, only set (taken from the DSC, after being recreated if necessary).
Hopefully Jan can confirm.
There was a problem hiding this comment.
This is correct, it happens here: https://github.com/getsentry/relay/blob/cbf72c2930ad01ed0d780e36968088cd6a051878/relay-event-normalization/src/event.rs#L354-L356
Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
|
@cleptric Makes sense. Thanks! 👍 |
When reading the Trace Context docs, I assumed that `client_sample_rate` (like all the other fields) was expected to be set by the client. Instead, this field is generated in Relay. If a client sets this field, it will be ignored and overwritten. Added a note to the docs for this field to clarify how it is meant to be used. Co-authored-by: Alex Krawiec <alex.krawiec@sentry.io>
DESCRIBE YOUR PR
When reading the Trace Context docs, I assumed that
client_sample_rate(like all the other fields) was expected to be set by the client. Instead, this field is generated in Relay. If a client sets this field, it will be ignored and overwritten. Added a note to the docs for this field to clarify how it is meant to be used.IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: