Skip to content

Commit f13c941

Browse files
NiteshDhanpalclaude
andcommitted
fix(tracing): parse ACP ingress headers with explicit W3C propagator + 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>
1 parent ddb745d commit f13c941

2 files changed

Lines changed: 55 additions & 6 deletions

File tree

src/agentex/lib/sdk/fastacp/base/base_acp_server.py

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@
4747

4848

4949
def _attach_incoming_otel_context(scope_headers: list[tuple[bytes, bytes]]) -> object | None:
50-
"""Extract the inbound W3C trace context (traceparent/tracestate/baggage) from
51-
ASGI headers and make it the active OpenTelemetry context for the request.
50+
"""Extract the inbound W3C trace context (traceparent/tracestate) from ASGI
51+
headers and make it the active OpenTelemetry context for the request.
5252
5353
FastACP is not otherwise instrumented to *continue* an incoming trace: the
5454
gateway forwards the traceparent header, but nothing on the Python side
@@ -64,10 +64,22 @@ def _attach_incoming_otel_context(scope_headers: list[tuple[bytes, bytes]]) -> o
6464
"""
6565
try:
6666
from opentelemetry import context as _otel_context
67-
from opentelemetry.propagate import extract
68-
69-
carrier = {k.decode("latin-1"): v.decode("latin-1") for k, v in scope_headers}
70-
return _otel_context.attach(extract(carrier))
67+
from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator
68+
69+
# ASGI headers are a list that can repeat a name, and a dict comprehension
70+
# keeps only the last value -- which silently drops repeated `tracestate`
71+
# lines (W3C/RFC7230 say they MUST be combined). Build a dict-of-lists so
72+
# the propagator's getter sees every value and `TraceState.from_header`
73+
# combines them; `traceparent` is single-valued so it is unaffected.
74+
carrier: dict[str, list[str]] = {}
75+
for k, v in scope_headers:
76+
carrier.setdefault(k.decode("latin-1").lower(), []).append(v.decode("latin-1"))
77+
# Use the W3C propagator explicitly rather than the ambient global one:
78+
# this ingress is W3C by contract, and `OTEL_PROPAGATORS=datadog` (plausible
79+
# in a DD shop, and dd_only is the default mode) would otherwise silently
80+
# extract nothing. It also parses only traceparent/tracestate, so arbitrary
81+
# inbound `baggage` is not pulled into the downstream context.
82+
return _otel_context.attach(TraceContextTextMapPropagator().extract(carrier))
7183
except Exception: # pragma: no cover - obs must never break a request
7284
return None
7385

tests/test_trace_context_extraction.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,42 @@ def test_no_inbound_traceparent_is_fail_open() -> None:
4646
_detach_otel_context(token)
4747

4848

49+
def test_repeated_tracestate_headers_are_combined() -> None:
50+
# ASGI can deliver tracestate as multiple header lines; W3C/RFC7230 require
51+
# combining them. The old dict-comprehension carrier kept only the last.
52+
from opentelemetry import trace as _trace
53+
54+
trace_id = "0af7651916cd43dd8448eb211c80319c"
55+
headers = [
56+
(b"traceparent", f"00-{trace_id}-b7ad6b7169203331-01".encode()),
57+
(b"tracestate", b"vendora=1"),
58+
(b"tracestate", b"vendorb=2"),
59+
]
60+
token = _attach_incoming_otel_context(headers)
61+
try:
62+
ts = _trace.get_current_span().get_span_context().trace_state
63+
assert ts.get("vendora") == "1"
64+
assert ts.get("vendorb") == "2" # would be missing if repeats collapsed
65+
finally:
66+
_detach_otel_context(token)
67+
68+
69+
def test_inbound_baggage_is_not_extracted() -> None:
70+
# W3C tracecontext-only extraction: arbitrary inbound baggage (attacker-
71+
# controlled keys) must not be pulled into the downstream context.
72+
from opentelemetry.baggage import get_all
73+
74+
trace_id = "0af7651916cd43dd8448eb211c80319c"
75+
headers = [
76+
(b"traceparent", f"00-{trace_id}-b7ad6b7169203331-01".encode()),
77+
(b"baggage", b"user_id=secret,role=admin"),
78+
]
79+
token = _attach_incoming_otel_context(headers)
80+
try:
81+
assert get_all() == {}
82+
finally:
83+
_detach_otel_context(token)
84+
85+
4986
def test_detach_none_is_safe() -> None:
5087
_detach_otel_context(None)

0 commit comments

Comments
 (0)