Skip to content

fix(tracing): continue inbound W3C trace context at the ACP boundary - #490

Open
NiteshDhanpal wants to merge 2 commits into
nextfrom
fix/acp-w3c-ingress-trace-context
Open

fix(tracing): continue inbound W3C trace context at the ACP boundary#490
NiteshDhanpal wants to merge 2 commits into
nextfrom
fix/acp-w3c-ingress-trace-context

Conversation

@NiteshDhanpal

@NiteshDhanpal NiteshDhanpal commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Targets next, and builds on the merged #484 (business↔obs correlation) and #485 (Temporal interceptor) — this is a single logical change, no longer stacked.

Problem

After #484 (business↔obs correlation + Option A) and #485 (Temporal interceptor), an async turn's obs trace still detached from the ingress: the workflow + every activity started fresh traces, unlinked from the event/send API call.

Root cause (proven on infra-staging with per-hop traceparent probes): the ingress traceparent arrives at the agent's ACP server in the HTTP header, but FastACP never extracts it — the active OTel context stays empty. So start_workflow/signal (and the asyncio.create_task background dispatch) fire with no active span, the Temporal interceptor injects nothing, and the workflow/activities root fresh traces.

Fix

Extract + attach the inbound W3C context in the ASGI RequestIDMiddleware, which wraps the whole request (so the background task inherits it via create_task's context copy). Now the interceptor propagates the ingress trace across the Temporal boundary and the workflow/activity inherit it → one connected trace. Fail-open (obs never breaks a request).

Parsing is hardened: it uses the explicit TraceContextTextMapPropagator (immune to OTEL_PROPAGATORS=datadog silently disabling W3C), combines repeated tracestate header lines (W3C/RFC7230 MUST) instead of collapsing to the last, and excludes inbound baggage so attacker-controlled keys aren't pulled into downstream context. traceparent is single-valued, so trace linkage is unchanged.

Verified (infra-staging)

Trace 12c6290e…: 347 spans across all 4 services in one tree — gateway POST /agents/{id}/rpc → agent ACP → RunWorkflowHandleSignal:receive_eventRunActivity → egp/identity/DB. Per-hop probes confirmed traceparent went from <none> → the ingress trace at every hop (ACP + worker). (Requires the control-plane header-forward fix, scaleapi/scale-agentex#396, to deliver the header.)

Tests

tests/test_trace_context_extraction.py — extract-makes-inbound-active, fail-open on missing header, safe detach, repeated tracestate combined, and inbound baggage not extracted. ruff clean.

🤖 Generated with Claude Code

Greptile Summary

The PR continues inbound W3C trace context at the ACP request boundary so asynchronous Temporal dispatch inherits the ingress trace.

  • Adds fail-open extraction and lifecycle-safe detachment of inbound traceparent and tracestate.
  • Preserves repeated tracestate header values while excluding inbound baggage.
  • Adds focused tests for extraction, repeated headers, baggage exclusion, and safe cleanup.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/agentex/lib/sdk/fastacp/base/base_acp_server.py Adds request-scoped W3C context extraction, repeated-tracestate preservation, fail-open handling, and guaranteed context detachment.
tests/test_trace_context_extraction.py Adds focused regression coverage for active trace continuation, repeated tracestate fields, baggage exclusion, and safe detachment.

Sequence Diagram

sequenceDiagram
    participant Gateway
    participant ACP as ACP Middleware
    participant Handler
    participant Temporal
    Gateway->>ACP: HTTP request with traceparent/tracestate
    ACP->>ACP: Extract and attach W3C context
    ACP->>Handler: Run request under ingress context
    Handler->>Temporal: Start workflow or signal
    Note over Handler,Temporal: Async dispatch inherits attached context
    Temporal-->>Handler: Dispatch accepted
    Handler-->>ACP: Response
    ACP->>ACP: Detach context safely
    ACP-->>Gateway: HTTP response
Loading

Reviews (4): Last reviewed commit: "fix(tracing): parse ACP ingress headers ..." | Re-trigger Greptile

Comment thread src/agentex/lib/sdk/fastacp/base/base_acp_server.py Outdated
Base automatically changed from feat/obs-correlation-edge to next August 6, 2026 21:32
@NiteshDhanpal
NiteshDhanpal force-pushed the fix/acp-w3c-ingress-trace-context branch from f780475 to a10bd94 Compare August 6, 2026 21:44
Root cause of async trace detachment (proven via [TP-DEBUG] probes): the ingress
traceparent arrives in the HTTP header (inbound=00-<trace>...) but FastACP never
extracts it, so the app's active OTel context is <none>. Downstream the Temporal
start_workflow/signal (incl. the asyncio.create_task background dispatch) fires
with no active span, the interceptor injects nothing, and the workflow + every
activity start FRESH traces disconnected from the ingress.

Extract + attach the inbound W3C context in the ASGI RequestIDMiddleware (wraps
the whole request, so the bg task inherits it via create_task's context copy).
Now the interceptor propagates the ingress trace across the Temporal boundary and
the workflow/activity inherit it -> one connected trace. Fail-open. Unit-tested.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@NiteshDhanpal
NiteshDhanpal force-pushed the fix/acp-w3c-ingress-trace-context branch from a10bd94 to ddb745d Compare August 10, 2026 23:36
…+ repeated-header support

- Use TraceContextTextMapPropagator().extract() instead of the ambient global
  propagator, so OTEL_PROPAGATORS=datadog (plausible in a DD shop; dd_only is the
  default mode) can't silently disable W3C extraction.
- Build a dict-of-lists carrier so repeated `tracestate` header lines are combined
  (W3C/RFC7230 MUST) instead of collapsing to the last value.
- Parse only traceparent/tracestate, so arbitrary inbound `baggage` isn't pulled
  into the downstream context.

Addresses the review comments on repeated-header collapse, ambient-propagator
dependence, and the implicit trust boundary. `traceparent` is single-valued, so
trace linkage (this PR's purpose) is unchanged.

Tests: repeated tracestate combined; inbound baggage not extracted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant