feat(ingester): continue the caller's trace across the ingest spawn (RFC 0039 slice 2) - #628
Conversation
…RFC 0039 slice 2) Both OTLP receivers create their `ingest logs` span inside `ingest_bound`, past a `tokio::spawn` that ambient context does not cross, so the caller's `traceparent` never reached it. Each handler now extracts the context from its own carrier before the spawn and attaches it to the whole spawned block, so the span is minted under it (RFC0039.3). The gRPC arm extracts from tonic metadata in `export` rather than from the tower auth layer as RFC 0039 §3.3 first proposed. That design contradicted §6 -- whose test drives `export` directly, with no tower stack, and so would never have exercised the extraction -- and put propagation inside a layer named for authentication. OTel prescribes extracting in the handler that receives the call, as the OTel Demo's own C++ gRPC service does with a carrier over `client_metadata()`. Cost is the `MetadataExtractor` that §3.4 had hoped to avoid; both RFC sections are amended in place. The new test is its own binary because it needs the process-global tracer (RFC0028.2) and `rfc0038_3_spawn_boundary.rs` owns the no-inbound-context case; leaving that file untouched keeps both cases covered. It fails with each arm's span in a freshly minted trace when the two handler changes are reverted. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 52 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughInbound trace context propagation now supports HTTP headers and gRPC metadata. Both receivers extract the caller context before ChangesInbound trace context propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Receiver
participant SpawnedIngest
participant SpanExporter
Caller->>Receiver: Sends traceparent via HTTP headers or gRPC metadata
Receiver->>Receiver: Extracts caller context
Receiver->>SpawnedIngest: Spawns ingest task with attached context
SpawnedIngest->>SpanExporter: Records ingest logs and commit wal spans
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 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/rfcs/0039-inbound-trace-context-propagation.md`:
- Line 73: Update the stale source-location references in
docs/rfcs/0039-inbound-trace-context-propagation.md: line 73 should reference
the gRPC LogsReceiver::export entrypoint at grpc.rs:155 instead of grpc.rs:152,
and lines 106-114 should reference spawn locations grpc.rs:176 and http.rs:153
instead of grpc.rs:167 and http.rs:146.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fc189052-a328-4ddd-b540-9c411e828cb0
📒 Files selected for processing (7)
crates/ourios-ingester/src/receiver.rscrates/ourios-ingester/src/receiver/grpc.rscrates/ourios-ingester/src/receiver/http.rscrates/ourios-ingester/src/receiver/propagation.rscrates/ourios-ingester/tests/README.mdcrates/ourios-ingester/tests/rfc0039_3_ingest_propagation.rsdocs/rfcs/0039-inbound-trace-context-propagation.md
There was a problem hiding this comment.
Pull request overview
Implements RFC 0039 slice 2 by preserving inbound W3C trace context across the OTLP ingest tokio::spawn boundary, so spans created inside ingest_bound join the caller’s trace for both OTLP transports (gRPC + HTTP). This extends the inbound trace propagation work from the query path to the ingest path in ourios-ingester.
Changes:
- Extract inbound trace context before spawning the ingest task and re-attach it to the spawned future via
FutureExt::with_context(OTLP/HTTP and OTLP/gRPC). - Add a tonic
MetadataMapcarrier (MetadataExtractor) alongside the existingHeaderExtractor. - Add an integration test binary asserting the ingest batch span (and
commit wal) are parented to the caller across the spawn boundary; document the new harness-exempt test.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0039-inbound-trace-context-propagation.md | Updates RFC 0039 text to reflect gRPC extraction from tonic metadata and spawn-boundary parenting approach. |
| crates/ourios-ingester/src/receiver/propagation.rs | Adds MetadataExtractor + extract_context_from_metadata and unit tests for tonic metadata carrier behavior. |
| crates/ourios-ingester/src/receiver/http.rs | Extracts caller context from HTTP headers and attaches it to the spawned ingest task. |
| crates/ourios-ingester/src/receiver/grpc.rs | Extracts caller context from gRPC metadata and attaches it to the spawned ingest task. |
| crates/ourios-ingester/src/receiver.rs | Re-exports the new metadata propagation helpers alongside the existing header-based ones. |
| crates/ourios-ingester/tests/rfc0039_3_ingest_propagation.rs | New harness-exempt integration test verifying cross-spawn trace parenting for both transports. |
| crates/ourios-ingester/tests/README.md | Documents the new harness-exempt test binary and why it must be separate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Review follow-ups on #628: - CodeRabbit flagged three stale file:line references. Rather than correct the numbers a second time, §3.2's site table and §3.3 now name file + function only -- line numbers rot on every touch of surrounding code. - Copilot: §3.3's first bullet still described a tower PropagationLayer for the query and HTTP-ingest arms, which neither uses. Rewritten to the implemented per-handler extraction, pointing at §4 for why no layer. - Copilot: assert the gRPC carrier treats a malformed traceparent as absent (RFC0039.5, gRPC arm). The two carriers reach the propagator through different Extractor impls, so this was genuinely uncovered. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
What
RFC 0039 slice 2 — RFC0039.3: the caller's trace context survives the ingest
tokio::spawn, on both OTLP transports.Slice 1 (#627) gave the query span a parent. Both OTLP receivers still rooted their own traces: the
ingest logsspan is created insideingest_bound, past atokio::spawn, and ambient context does not cross that boundary. Each handler now extracts the caller's context from its own carrier before the spawn and attaches it to the whole spawned block:Wrapping the block rather than
ingest_bound's future is deliberate — it holds whether#[tracing::instrument]mints its span at call time or on first poll, so the span cannot be created outsideparent.The RFC amendment (§3.3 / §3.4)
§3.3 originally routed gRPC extraction through the tower auth layer into a request extension. Withdrawn during implementation, for two reasons:
LogsReceiver::exportdirectly, with no tower stack — so a layer-extracted context would have left the extraction itself uncovered, testing only the spawn hand-off.Consulted the OTel knowledge base before choosing: OTel prescribes that a service receiving upstream calls extracts in the receiving handler ("the one context on the wire becomes the parent of the new span the library creates"), and the OTel Demo's own C++ gRPC service does exactly this with a
GrpcServerCarrieroverclient_metadata(). The Gootelgrpc-interceptor pattern looks like the alternative but doesn't transfer — it extracts and creates the span in one place, which we cannot do across the spawn.Cost of the correction is the ~15-line
MetadataExtractorthat §3.4 had hoped to avoid. Both sections are amended in place, with the rationale recorded as an amendment note; §7's first open question is now resolved.Invariants (CLAUDE.md §3)
No invariant is touched. No new signal, no schema change, no on-disk change — this only sets the parent of spans that already exist. Both extractors are infallible by construction: absent, malformed, or unparsable context yields a context with no remote span, so the span roots itself exactly as before (RFC0039.2 / RFC0039.5). No
unwrap/expectin non-test code (§6.1).Tests
rfc0039_3_ingest_propagation.rs(new binary) — both arms assert the batch span joins the caller's trace, is parented to the caller's span, and thatcommit walstill nests under it inside that trace.d1c539…,11aa7e…) and the assertion fails at 0 spans in the caller's trace.rfc0038_3_spawn_boundary.rsowns the no-inbound-context case and is left untouched, so both cases stay covered.SpanContextas the HTTP one; binary (-bin) metadata keys are withheld fromkeys().Verification
cargo fmt --all --check✅cargo clippy --all-targets --all-features -- -D warnings✅cargo nextest run --all-features✅ 1248 passed, 38 skippedmdbook build✅Remaining in RFC 0039
Slice 3 (RFC0039.6, MCP tool spans) and slice 4 (RFC0039.4, the sampling assertion).
Summary by CodeRabbit
New Features
Documentation
Tests