Skip to content

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

Open
NiteshDhanpal wants to merge 1 commit 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 1 commit into
nextfrom
fix/acp-w3c-ingress-trace-context

Conversation

@NiteshDhanpal

@NiteshDhanpal NiteshDhanpal commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Stacked on #484.

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).

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. ruff clean.

🤖 Generated with Claude Code

Greptile Summary

The PR continues inbound W3C trace context at the ACP ASGI boundary and safely detaches it after request processing.

  • Extracts and attaches the incoming OpenTelemetry context before invoking the wrapped application.
  • Ensures context cleanup occurs in a finally block.
  • Adds regression coverage for trace activation, absent headers, and no-op detachment.

Confidence Score: 4/5

The PR is not yet safe to merge because the previously reported repeated-header propagation failure remains outstanding.

The current carrier construction still overwrites earlier repeated baggage or tracestate fields, causing valid inbound propagation metadata to be omitted from the context passed downstream.

Files Needing Attention: src/agentex/lib/sdk/fastacp/base/base_acp_server.py

Important Files Changed

Filename Overview
src/agentex/lib/sdk/fastacp/base/base_acp_server.py Adds fail-open OpenTelemetry context extraction and request-scoped attachment and cleanup in the ASGI middleware.
tests/test_trace_context_extraction.py Adds focused unit coverage for activating inbound trace context and safely handling missing context or detach tokens.

Sequence Diagram

sequenceDiagram
    participant Gateway
    participant Middleware as RequestIDMiddleware
    participant ACP as ACP Handler
    participant Temporal as Temporal Interceptor
    Gateway->>Middleware: HTTP request + traceparent
    Middleware->>Middleware: Extract and attach OTel context
    Middleware->>ACP: Invoke request handler
    ACP->>Temporal: Start or signal workflow
    Temporal->>Temporal: Inject active context
    ACP-->>Middleware: Request processing completes
    Middleware->>Middleware: Detach OTel context
Loading

Reviews (2): Last reviewed commit: "fix(tracing): continue inbound W3C trace..." | Re-trigger Greptile

from opentelemetry import context as _otel_context
from opentelemetry.propagate import extract

carrier = {k.decode("latin-1"): v.decode("latin-1") for k, v in scope_headers}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Repeated propagation headers collapse

Converting the raw ASGI header list to a dictionary retains only the final value of repeated baggage or tracestate fields, silently omitting earlier propagation metadata from downstream context.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/sdk/fastacp/base/base_acp_server.py
Line: 69

Comment:
**Repeated propagation headers collapse**

Converting the raw ASGI header list to a dictionary retains only the final value of repeated `baggage` or `tracestate` fields, silently omitting earlier propagation metadata from downstream context.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Cursor Fix in Claude Code Fix in Codex

Base automatically changed from feat/obs-correlation-edge to next August 6, 2026 21:32
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 f780475 to a10bd94 Compare August 6, 2026 21:44
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