feat(tracing): end-to-end trace propagation across the queue (#110, ADR-039)#406
Conversation
…DR-039) Capture the ambient OpenTelemetry span context at enqueue as a W3C traceparent under the reserved awa:traceparent metadata key (single choke point: prepare_raw_job_insert — typed, raw, batch, COPY, transactional and Python producers all funnel through it; no migration, both engines, SQL producers set the key themselves). The worker's job.execute span continues the trace: first attempts join as a remote child; retries detach into a fresh root trace with a span link back to the enqueue site, per OTel messaging semantics for deferred processing. Producer and consumer spans carry messaging semantic conventions. Default-on (E8: ~0.23µs/enqueue, +0.023% of a 1ms roundtrip), AWA_TRACE_CAPTURE=off kill switch, ctx.traceparent() / job.traceparent for handler-side forwarding.
hardbyte
added
the
full-ci
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements end-to-end distributed tracing across enqueue and worker execution. It adds a ChangesDistributed tracing propagation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Producer
participant InsertRs as insert.rs (prepare_raw_job_insert)
participant Metadata as job.metadata
participant Executor as JobExecutor
participant Span as job.execute span
Producer->>InsertRs: enqueue job
InsertRs->>Metadata: inject_traceparent(awa:traceparent)
Executor->>Metadata: traceparent_from_metadata()
alt first attempt
Executor->>Span: set_parent(remote enqueue context)
else retry
Executor->>Span: clear parent, add link to enqueue span
end
Span-->>Producer: exported spans joined/linked to producer trace
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c34d27463d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// W3C traceparent captured at the enqueue site, if any. The execution | ||
| /// span already joins that trace automatically; use this to hand the | ||
| /// context onward (outgoing HTTP headers, child processes). | ||
| pub fn traceparent(&self) -> Option<&str> { | ||
| awa_model::trace::traceparent_from_metadata(&self.job.metadata) |
There was a problem hiding this comment.
When a handler follows the new ctx.traceparent() guidance for an outgoing HTTP call, this returns the stored enqueue-site traceparent, not the current job.execute span context. Because execute_task has already made job.execute a child of that enqueue span, downstream spans created from this header become siblings of job.execute rather than children, which breaks the trace topology for the exact forwarding scenario documented here; build the header from the current execution span context or avoid recommending this helper for propagation.
Useful? React with 👍 / 👎.
ctx.traceparent()/job.traceparent return the stored enqueue-site context; propagating that onward would make downstream spans siblings of job.execute. Rust handlers forward with trace::current_traceparent() (handlers run inside the execution span); the Python snippet documents the enqueue site as the best available parent until binding-layer context attach lands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
awa-model/src/trace.rs (2)
121-191: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a test for the
capture_disabledenv-var path.The
capture_disabled()function and its interaction withinject_traceparentwhenAWA_TRACE_CAPTURE=offis set are untested. A test usingserial_testor similar (to avoid env-var races in parallel test execution) would verify the kill switch works end-to-end. This is low priority since the logic is straightforward.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-model/src/trace.rs` around lines 121 - 191, Add a test covering the AWA_TRACE_CAPTURE=off path, since `capture_disabled()` and `inject_traceparent` are not exercised together yet. Create a serial test (using `serial_test` or equivalent) that sets the env var off, calls `inject_traceparent`, and verifies no `traceparent` is injected; reference `capture_disabled`, `inject_traceparent`, and `TRACEPARENT_METADATA_KEY` so the test stays easy to locate.
93-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider whether
remote_contextis needed as a public API.The executor constructs the
Contextinline fromparse_traceparentrather than using this wrapper. If no external consumer currently calls it, consider making itpub(crate)or removing it to reduce the public API surface. If it's intended for future use or external consumers, a doc comment noting the intended use case would help.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa-model/src/trace.rs` around lines 93 - 96, The `remote_context` wrapper in `trace.rs` appears unused by internal code and may not need to be part of the public API. Either reduce its visibility to `pub(crate)` or remove it if no external consumers rely on it; if you keep it public, add a doc comment on `remote_context` explaining its intended use so the API surface is clear.awa/tests/trace_propagation_test.rs (1)
126-158: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise
ctx.traceparent()in the propagation test.This validates raw metadata but not the new Rust accessor contract. Send
ctx.traceparent()from the handler too, so a wrong reserved key or extraction regression is caught.Proposed test coverage tweak
- let (tx, mut rx) = tokio::sync::mpsc::channel::<serde_json::Value>(1); + let (tx, mut rx) = tokio::sync::mpsc::channel::<(serde_json::Value, Option<String>)>(1); @@ let tx = tx.clone(); let handler_metadata = ctx.job.metadata.clone(); + let handler_traceparent = ctx.traceparent().map(str::to_owned); async move { - let _ = tx.send(handler_metadata).await; + let _ = tx.send((handler_metadata, handler_traceparent)).await; Ok(JobResult::Completed) } @@ - let handler_metadata = tokio::time::timeout(Duration::from_secs(20), rx.recv()) + let (handler_metadata, handler_traceparent) = + tokio::time::timeout(Duration::from_secs(20), rx.recv()) .await .expect("job should execute within 20s") .expect("handler should send metadata"); @@ "handler-visible metadata should carry the captured traceparent" ); + assert_eq!( + handler_traceparent.as_deref(), + Some(stored.as_str()), + "ctx.traceparent() should expose the captured traceparent" + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@awa/tests/trace_propagation_test.rs` around lines 126 - 158, The trace propagation test currently verifies only raw job metadata, so it can miss regressions in the new accessor contract. Update the handler registered in Client::builder(...).register::<TracedJob, _, _>(...) to also read ctx.traceparent() and send that value through the existing channel alongside handler_metadata, then extend the assertions to compare the accessor result against the captured stored traceparent. This ensures the test covers both the reserved metadata key and the ctx.traceparent() extraction path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/039-trace-propagation.md`:
- Around line 25-28: The enqueue-path scope in the ADR text is too broad because
it implies Python producers are included in ambient trace capture. Update the
wording around prepare_raw_job_insert so it only refers to Rust-backed enqueue
paths (typed, raw, batch, COPY, transactional) or otherwise explicitly qualify
that Python producer-side auto-capture is not included yet.
---
Nitpick comments:
In `@awa-model/src/trace.rs`:
- Around line 121-191: Add a test covering the AWA_TRACE_CAPTURE=off path, since
`capture_disabled()` and `inject_traceparent` are not exercised together yet.
Create a serial test (using `serial_test` or equivalent) that sets the env var
off, calls `inject_traceparent`, and verifies no `traceparent` is injected;
reference `capture_disabled`, `inject_traceparent`, and
`TRACEPARENT_METADATA_KEY` so the test stays easy to locate.
- Around line 93-96: The `remote_context` wrapper in `trace.rs` appears unused
by internal code and may not need to be part of the public API. Either reduce
its visibility to `pub(crate)` or remove it if no external consumers rely on it;
if you keep it public, add a doc comment on `remote_context` explaining its
intended use so the API surface is clear.
In `@awa/tests/trace_propagation_test.rs`:
- Around line 126-158: The trace propagation test currently verifies only raw
job metadata, so it can miss regressions in the new accessor contract. Update
the handler registered in Client::builder(...).register::<TracedJob, _, _>(...)
to also read ctx.traceparent() and send that value through the existing channel
alongside handler_metadata, then extend the assertions to compare the accessor
result against the captured stored traceparent. This ensures the test covers
both the reserved metadata key and the ctx.traceparent() extraction path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc362c12-e943-4f02-b3db-37be800a4397
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
CHANGELOG.mdCargo.tomlawa-model/Cargo.tomlawa-model/src/insert.rsawa-model/src/lib.rsawa-model/src/trace.rsawa-python/python/awa/_awa.pyiawa-python/src/job.rsawa-worker/Cargo.tomlawa-worker/src/context.rsawa-worker/src/executor.rsawa/Cargo.tomlawa/tests/trace_propagation_test.rsdocs/adr/039-trace-propagation.mddocs/adr/README.mddocs/configuration.mddocs/getting-started-python.mddocs/getting-started-rust.mddocs/stability.md
…ir OTel context is not ambient-captured
1 participant
Closes #110 (Rust path + Python handler-side exposure; see "Remaining" below). Implements roadmap decision D8. Design: ADR-039.
What this does
Capture (enqueue). When a producer enqueues inside an OpenTelemetry span, the W3C
traceparentis serialized into the job's metadata under the reservedawa:traceparentkey. Capture happens inprepare_raw_job_insert— the single choke point that every Rust producer path funnels through (typed, raw, batch, COPY, transactional, Python bindings). No migration, identical on both engines, and SQL-native producers (#342 / ADR-032) participate by setting the key themselves. A caller-supplied value always wins, which keeps the pre-0.7 opt-in pattern working verbatim.Restore (execute), per OTel messaging semantics:
Semantic conventions on both sides: producer spans are
SpanKind::Producernamedsend {queue}(sendfor batch), execution spans areSpanKind::Consumerwithmessaging.system="awa",messaging.destination.name,messaging.operation.type,messaging.message.id. The execution span keeps its establishedjob.execute {kind}name — kind is the operationally useful dimension for a job queue; the semconv shape rides on kind + attributes (trade-off recorded in the ADR).Handler-side forwarding:
ctx.traceparent()(Rust) andjob.traceparent(Python, typed in the.pyi).E8 gate (< 2% enqueue overhead → default-on)
The added work lives entirely in
prepare_raw_job_insert, so the rerunnable#[ignore]bench measures that path directly (end-to-end DB timing cannot resolve a µs delta — it swung ±20% run-to-run on the same code):Decisively under the gate → capture defaults on, with
AWA_TRACE_CAPTURE=offas a process-wide kill switch.Tests
trace_propagation_test.rs(in-memory span exporter, runs in therestCI shard):test.enqueue → send {queue} → job.execute: same trace id, correct parent ids, producer/consumer span kinds, semconv attributes, handler-visible metadata carries the key;--ignored).TLA+
No lifecycle or protocol change — the traceparent is opaque metadata passthrough; capture/restore don't touch state transitions. No model updates required.
Stability policy
Adds one normative paragraph to
docs/stability.md: metadata keys prefixedawa:are a reserved namespace (this is its first citizen). Flagging explicitly for review since the policy is now Accepted.Remaining (#110 follow-up)
Automatic Python producer capture: the Rust core can't see opentelemetry-python's ambient context across the FFI boundary. The documented pattern (set
metadata["awa:traceparent"], snippet ingetting-started-python.md) covers it manually; automatic capture belongs in the awa-python binding layer and is noted in the ADR.tracestate(vendor state) is intentionally not carried.Docs: ADR-039,
configuration.md§ Distributed tracing, getting-started (Rust + Python), CHANGELOG.Summary by CodeRabbit