Open
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
jpnurmi
force-pushed
the
jpnurmi/feat/strict-trace-continuation
branch
from
2286835 to
e7befcf
Compare
jpnurmi
force-pushed
the
jpnurmi/feat/strict-trace-continuation
branch
from
e7befcf to
61bb274
Compare
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 61bb274. Configure here.
jpnurmi
force-pushed
the
jpnurmi/feat/strict-trace-continuation
branch
2 times, most recently
from
5065a55 to
63a51da
Compare
jpnurmi
changed the title
|
Adds two reusable helpers — `sentry__baggage_iter_next`, which yields the next W3C baggage member as trimmed slices (with property suffixes stripped and malformed members skipped), and `sentry__percent_decode_inplace`, which pct-decodes a buffer in place with malformed escapes passed through verbatim. Both are covered by focused unit tests; no production call sites are rewired in this commit.
Provide an internal object key/value iteration helper with focused unit coverage so later trace propagation code can iterate DSC values without indexed key/value access. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Add `sentry_options_set_strict_trace_continuation` / `_get_` as an experimental API. The option defaults to false and is not wired up to any propagation logic yet; subsequent commits will consume it when the trace-continuation decision path is implemented. Preparation for strict trace continuation: https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `sentry_options_set_org_id` / `_set_org_id_n` / `_get_org_id` as an experimental API. Overrides the organization ID derived from the DSN host, which is required for self-hosted setups whose ingest hostname does not encode the org. Nothing consumes the option yet; subsequent commits will route it through the effective-org_id resolver and the strict-trace-continuation decision. Preparation for strict trace continuation: https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`sentry_set_trace` and `sentry_regenerate_trace` updated the scope's propagation context but left the dynamic sampling context (built once at `sentry_init`) untouched. The DSC's `sample_rand` therefore stayed tied to the trace generated at init, even after the caller switched traces. Outgoing propagation that consumes the scope DSC would emit stale values mismatched against `sentry-trace`. Refresh the scope DSC after each trace change. Surfaced while preparing strict trace continuation, where outgoing baggage will draw all DSC fields from the scope DSC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add `sentry__options_get_effective_org_id` (option > DSN > NULL, empty treated as absent) and consume it in the dynamic sampling context builder. The DSC now only carries `org_id` when the SDK actually has one — the previous code emitted `"org_id":""` for DSNs without an `o<digits>.` host prefix, which ran counter to the trace-propagation spec. Integration and envelope-serialization assertions updated to reflect the absent field. Preparation for strict trace continuation: https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Handle `baggage` alongside `sentry-trace` in `sentry_transaction_context_update_from_header`. Per W3C baggage / RFC 7230 syntax: comma-separated members of the form `key=value` with optional surrounding whitespace. `sentry-*` members are collected (key stripped of the `sentry-` prefix) and their percent-encoded values decoded into a new `incoming_dsc` object on the transaction context's inner state. Non-sentry members are ignored. The `incoming_dsc` object is the input to the next step — the strict trace continuation decision. Nothing consumes it yet. Preparation for strict trace continuation: https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the file-static `set_dynamic_sampling_context` from `sentry_core.c` into `sentry_scope.c` as `sentry__scope_rebuild_dsc_from_options`. The DSC fundamentally belongs to the scope, and the upcoming strict-trace-continuation work needs to call it from outside `sentry_core.c`. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire incoming-baggage org_id and the SDK's effective org_id through `sentry__trace_continuation_allowed` (the spec truth table) when a transaction starts: - Both present and equal, both absent, or only one with strict off: continue. The scope DSC is frozen verbatim from the incoming DSC and propagated as-is from there on. - Both present and differing: never continue. - Exactly one present with strict on: do not continue. When not continuing, the transaction takes a fresh `trace_id`, drops `parent_span_id` and any inherited `sampled` flag (the sampler re-decides), and the scope DSC is rebuilt from the SDK's own options. The internal `incoming_dsc` carrier is stripped from the event before sampling so it never reaches the envelope. Outgoing baggage emission still TODO; spec compliance requires it (`sentry-org_id` MUST be propagated). Coming next. https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the long-standing TODO in `sentry__span_iter_headers` with a proper `baggage` header emitter. The header is built from the scope DSC: when the scope continued an upstream trace, the DSC was frozen verbatim and propagation echoes upstream values "as is" per the trace-propagation spec; otherwise the DSC was rebuilt from the SDK's own options. `sentry-trace_id` is always taken from the span's own `trace_id` and emitted first, so it stays consistent with the `sentry-trace` header. The remaining DSC fields (including `sentry-org_id` when present) are appended with values percent-encoded per RFC 3986. https://develop.sentry.dev/sdk/telemetry/traces/dynamic-sampling-context/#baggage-header Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: OpenAI Codex <noreply@openai.com>
Add unit tests for the new continuation pipeline: - `trace_continuation_truth_table`: pure check of the spec truth table. - `effective_org_id_resolution`: option > DSN > NULL precedence, empty option falls back to DSN. - `parse_baggage_basic_and_filtering`: percent-decoding, OWS trimming, non-`sentry-` members ignored, malformed members skipped. - `strict_continuation_*`: end-to-end via `sentry_transaction_context_update_from_header` → `sentry_transaction_start`, asserting both the resulting trace state (continued vs. forked) and the outgoing baggage emitted via `sentry_transaction_iter_headers` (frozen-from-upstream vs. rebuilt from options, including spec-required `sentry-org_id` propagation). - `set_trace_rebuilds_dsc_sample_rand`: regression for the earlier staleness fix. Also bumps the unreleased CHANGELOG entry now that the feature is observable end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per the trace-propagation spec, the receiving SDK must treat the incoming Dynamic Sampling Context as instantly frozen and propagate its values "as is". `sentry__scope_freeze_dsc_from_incoming` built the DSC but didn't lock it, so a subsequent `sentry_set_release` / `sentry_set_environment` call would overwrite the upstream `release` / `environment` values in the outgoing `baggage` header. Freeze via `sentry_value_freeze` after the merge so the setters silently no-op against the active trace's DSC; the scope's own fields still update and feed the next trace's rebuilt DSC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the strict-continuation decision forks into a new trace, the fork branch cleared `sampled` / `parent_span_id` on `tx` but not on `tx_ctx` (which `parse_sentry_trace` still populated from the incoming `sentry-trace` header). The subsequent `sentry__should_send_transaction(tx_ctx, ...)` call would therefore see the upstream `sampled` flag, treat it as `parent_sampled`, and short-circuit to the upstream decision — bypassing the local `traces_sample_rate` / `traces_sampler`. Pass `tx` (already merged from `tx_ctx` and, in the fork branch, stripped of `sampled`) to the sampler helper so the fork evaluates sampling locally. Non-fork paths are unchanged since `tx` agrees with `tx_ctx` on `sampled` there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`isalnum` from `<ctype.h>` is locale-dependent: in non-"C" locales (e.g. ISO-8859-1) bytes > 127 — such as UTF-8 continuation bytes in release / environment values — can be classified as alphanumeric and left unencoded, producing a malformed baggage header. RFC 3986's `unreserved` set is strict ASCII by definition, so replace the call with a small locale-independent ASCII-range helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Exercise the transaction example with fixed incoming trace and baggage flags so integration tests cover continuation, mismatched org IDs, and missing baggage. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Drop the unused public org ID getter and use a single internal helper for the trace propagation org ID resolved from options or the DSN. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Use shorter internal names for updating the scope DSC from SDK options and freezing it from incoming trace continuation data. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Use a shorter internal predicate name for deciding whether an incoming trace can continue under strict trace continuation rules. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Let the trace continuation predicate derive SDK and incoming organization IDs from the options and incoming DSC so transaction startup only asks whether the incoming trace can continue. Co-Authored-By: OpenAI Codex <noreply@openai.com>
jpnurmi
force-pushed
the
jpnurmi/feat/strict-trace-continuation
branch
from
63a51da to
2794991
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See: https://develop.sentry.dev/sdk/foundations/trace-propagation/#strict-trace-continuation