Skip to content

feat(engine): honor W3C tracestate on inbound HTTP and surface distributed traces#1921

Open
guibeira wants to merge 7 commits into
mainfrom
feat/otel-missing-http-params
Open

feat(engine): honor W3C tracestate on inbound HTTP and surface distributed traces#1921
guibeira wants to merge 7 commits into
mainfrom
feat/otel-missing-http-params

Conversation

@guibeira

@guibeira guibeira commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

What

Completes W3C trace-context propagation for inbound HTTP requests and makes the
resulting distributed traces visible in the console.

Builds on #1769 (propagate traceparent) with:

  • Honor tracestate on inbound HTTPtraceparent's W3C companion header.
    extract_context only seeded traceparent into the propagation carrier, so
    TraceContextPropagator silently dropped any incoming tracestate. The HTTP
    boundary (rest_api/views.rs) now reads tracestate and forwards it through
    SpanExt::with_parent_headers. Non-HTTP callers pass None (the engine
    protocol does not carry tracestate across hops — boundary-only by design).
  • Redact baggage in header logs — baggage often carries user/session
    metadata; it's now treated as sensitive in sanitize_headers_for_logging.
  • Expose span tracestate in the traces APIStoredSpan dropped the
    span's own tracestate (only links carried it), so the honored value was
    invisible to engine::traces::list and the console.
  • Surface distributed traces in traces::list and traces::tree — both
    treated a span as a trace root only when parent_span_id was None. A trace
    entering iii via an incoming traceparent has a server span whose parent is
    the remote caller's span (never stored here), so the whole trace was hidden
    from the list and the detail view rendered "No span data available". Both now
    treat a span as a root when its parent is absent from the store.

Why

Without tracestate, iii silently drops vendor trace state on the server span,
breaking distributed traces that rely on it. The observability gaps meant a
correctly-propagated trace was invisible in the console, so the feature was
effectively unusable for its main purpose (end-to-end observability of requests
entering iii from an upstream service).

Notes

  • Verified end-to-end: a curl carrying traceparent + tracestate + baggage
    produces an engine HTTP span under the caller's trace id, carrying the
    tracestate, propagated to the worker span in the same trace, and visible in
    the console trace list and detail tree.
  • Unit tests added: tracestate extraction, empty-state default, baggage
    redaction, StoredSpan tracestate, and dangling-remote-parent roots in both
    traces::list and build_span_tree.
  • Boundary-only scope: full multi-hop tracestate propagation (protocol +
    queue jobs + SDKs) is intentionally out of scope.

Summary by CodeRabbit

  • New Features

    • Improved trace-context propagation across requests and background jobs, including support for additional tracing metadata.
    • Trace listings now include spans with missing parents as root spans, making externally started traces easier to view.
  • Bug Fixes

    • Preserved trace context more reliably when linking spans.
    • Sensitive request headers now redact baggage values in logs.

mikkel-arturo and others added 5 commits June 29, 2026 13:20
…1769)

Co-authored-by: Guilherme Beira <guilherme.vieira.beira@gmail.com>
Follow-up to the merged traceparent propagation PR.

- Add extract_context_with_state(traceparent, tracestate, baggage); the W3C
  TraceContextPropagator only populates a SpanContext's TraceState when both
  traceparent and tracestate are present, so a bare traceparent silently
  dropped any incoming tracestate. extract_context now delegates with None.
- SpanExt::with_parent_headers takes tracestate as the W3C companion header;
  the HTTP boundary (views.rs) reads the tracestate header and forwards it.
  Non-HTTP callers (queue adapters, invocation) pass None — the engine
  protocol does not carry tracestate across hops.
- Treat the baggage header as sensitive in sanitize_headers_for_logging so
  user/session metadata is not emitted to logs.
- Tests for tracestate extraction, empty-state default, and baggage redaction.
StoredSpan dropped the span's own W3C tracestate (only links carried it), so
the inbound tracestate honored on the HTTP server span was invisible to
engine::traces::list and the console. Add a top-level trace_state field
populated from the span context (omitted when empty), making trace-context
propagation verifiable end-to-end. OTLP-ingested spans pass None for now.

Verified e2e: curl with traceparent+tracestate -> HTTP span carries the trace
id and tracestate, propagated to the worker span in the same trace.
list_traces treated a span as a trace root only when parent_span_id was None.
A trace entering iii from an external caller via an incoming traceparent has a
server span whose parent is the remote caller's span — never stored here — so
the whole trace was hidden from the root-only listing (and thus the console).

Treat a span as a root when its parent is absent from the store, mirroring
build_span_tree's dangling-parent handling. Verified e2e: a curl carrying
traceparent+tracestate now appears in the default (root-only, non-internal)
trace list.
build_span_tree only rooted spans with no parent, so the server span of a
trace entering iii via an incoming traceparent (parent = remote caller, not
stored) was orphaned into the children map and never emitted — the trace
detail view showed "No span data available".

Treat a span as a root when its parent is absent from the span set, matching
the traces::list fix. Updated the orphan-child test to assert the corrected
behavior. Verified e2e: traces::tree now returns the HTTP server span and its
worker child for a propagated trace.
@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
iii-website Ready Ready Preview, Comment Jun 29, 2026 10:45pm
tech-spec Error Error Jun 29, 2026 10:45pm

Request Review

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 215c482e-b4cb-4204-aec5-2fef8b74c95c

📥 Commits

Reviewing files that changed from the base of the PR and between 05c2394 and 790cd1d.

📒 Files selected for processing (11)
  • engine/src/invocation/mod.rs
  • engine/src/telemetry.rs
  • engine/src/workers/observability/mod.rs
  • engine/src/workers/observability/otel.rs
  • engine/src/workers/queue/adapters/bridge.rs
  • engine/src/workers/queue/adapters/builtin/adapter.rs
  • engine/src/workers/queue/adapters/rabbitmq/worker.rs
  • engine/src/workers/queue/adapters/redis_adapter.rs
  • engine/src/workers/queue/queue.rs
  • engine/src/workers/rest_api/views.rs
  • engine/tests/observability_configuration_e2e.rs

📝 Walkthrough

Walkthrough

Adds W3C tracestate support throughout the engine: StoredSpan gains a trace_state field, a new extract_context_with_state function includes tracestate in the propagation carrier, and SpanExt::with_parent_headers gains a tracestate parameter with explicit set_parent error handling and span events. All call sites are updated. Separately, build_span_tree and traces::list now promote spans whose remote parent is absent from local storage to roots instead of discarding them.

Changes

W3C Tracestate Propagation and Dangling-Parent Root Fix

Layer / File(s) Summary
StoredSpan tracestate field and extract_context_with_state
engine/src/workers/observability/otel.rs
StoredSpan gains an optional trace_state field populated from span context; extract_context delegates to a new extract_context_with_state that inserts the tracestate header into the W3C carrier; OTLP ingestion sets trace_state: None; unit tests cover field population and extraction round-trips.
SpanExt::with_parent_headers tracestate parameter
engine/src/telemetry.rs
with_parent_headers gains a tracestate parameter, uses extract_context_with_state, and explicitly handles set_parent success/failure with debug/warn logging and new span events (traceparent.propagated, traceparent.set_parent_failed).
Call-site updates across queue adapters, invocation, and REST API
engine/src/invocation/mod.rs, engine/src/workers/queue/adapters/..., engine/src/workers/queue/queue.rs, engine/src/workers/rest_api/views.rs
All with_parent_headers call sites updated to pass explicit None for tracestate; dynamic_handler additionally extracts and passes real tracestate from incoming HTTP headers; baggage added to SENSITIVE_HEADERS redaction.
Dangling-parent spans promoted to roots
engine/src/workers/observability/mod.rs, engine/tests/observability_configuration_e2e.rs
build_span_tree precomputes present span IDs and treats spans with absent parents as roots; traces::list widens root filter to include spans whose parent is not stored locally; tests assert new semantics for orphan children and remote-parent traces.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • iii-hq/iii#151: Introduced the original with_parent_headers(traceparent, baggage) two-argument form across queue adapter spans that this PR extends to three arguments.
  • iii-hq/iii#1769: Previously modified SpanExt::with_parent_headers and HTTP span wiring in rest_api/views.rs, the same two files changed here.
  • iii-hq/iii#1817: Modified observability/mod.rs span-tree parent handling, the same logic extended here to treat dangling remote parents as roots.

Suggested reviewers

  • ytallo
  • andersonleal
  • sergiofilhowz

🐇 A trace without a parent once wandered alone,
Now promoted to root—a root all its own!
tracestate hops through headers with care,
set_parent succeeds (or logs a warn there).
Every span finds its place, no orphan unknown! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: inbound tracestate handling and distributed trace visibility.
Description check ✅ Passed The description follows the template with complete What, Why, and Notes sections and covers the implemented changes and context.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/otel-missing-http-params

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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