Skip to content

Commit de2e66e

Browse files
GWealecopybara-github
authored andcommitted
fix: tolerate a malformed traceparent header in the span processor
A caller-supplied traceparent header was stored in baggage before it was validated, then parsed with an unguarded `split("-")[2]` inside a span processor that runs on every span. A header with fewer than three segments, or a non-hex third segment, therefore raised from `on_start` for every child span of the request; the SDK does not catch span-processor exceptions, so it surfaced out of the application's own `start_span` call. Validate before storing, and guard the parse. Both are needed: the baggage key is the unprefixed `traceparent`, which a remote caller can also set directly, so the parse must not trust the value regardless of who wrote it. Co-authored-by: George Weale <gweale@google.com> PiperOrigin-RevId: 954783125
1 parent f4979b5 commit de2e66e

2 files changed

Lines changed: 176 additions & 14 deletions

File tree

src/google/adk/telemetry/_agent_engine.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,17 @@ def get_propagated_context(request: fastapi.Request) -> context.Context:
4343
)
4444

4545
if _GOOGLE_AE_TRACEPARENT_HEADER in request.headers:
46-
carrier = {"traceparent": request.headers[_GOOGLE_AE_TRACEPARENT_HEADER]}
47-
ctx = baggage.set_baggage(
48-
_TRACEPARENT_BAGGAGE_KEY,
49-
request.headers[_GOOGLE_AE_TRACEPARENT_HEADER],
50-
context=ctx,
51-
)
52-
ctx = tracecontext.TraceContextTextMapPropagator().extract(
53-
carrier=carrier, context=ctx
46+
ae_traceparent = request.headers[_GOOGLE_AE_TRACEPARENT_HEADER]
47+
extracted_ctx = tracecontext.TraceContextTextMapPropagator().extract(
48+
carrier={"traceparent": ae_traceparent}, context=ctx
5449
)
50+
# extract() returns the context unchanged when it rejects the header;
51+
# testing the extracted span for validity instead would false-accept,
52+
# since ctx usually already carries a valid span.
53+
if extracted_ctx is not ctx:
54+
ctx = baggage.set_baggage(
55+
_TRACEPARENT_BAGGAGE_KEY, ae_traceparent, context=extracted_ctx
56+
)
5557

5658
return ctx
5759

@@ -98,9 +100,13 @@ def _is_top_span(
98100
"""
99101
if span.parent is None or span.parent.span_id == 0:
100102
return True
101-
if _TRACEPARENT_BAGGAGE_KEY in baggage_items:
102-
parent_id_hex = str(baggage_items[_TRACEPARENT_BAGGAGE_KEY]).split("-")[2]
103-
parent_id_int = int(parent_id_hex, 16)
104-
if span.parent.span_id == parent_id_int:
105-
return True
106-
return False
103+
if _TRACEPARENT_BAGGAGE_KEY not in baggage_items:
104+
return False
105+
traceparent_parts = str(baggage_items[_TRACEPARENT_BAGGAGE_KEY]).split("-")
106+
if len(traceparent_parts) < 3:
107+
return False
108+
try:
109+
parent_span_id = int(traceparent_parts[2], 16)
110+
except ValueError:
111+
return False
112+
return span.parent.span_id == parent_span_id
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests for trace context propagated from request headers."""
16+
17+
from __future__ import annotations
18+
19+
import fastapi
20+
from google.adk.telemetry._agent_engine import get_propagated_context
21+
from google.adk.telemetry._agent_engine import TopSpanProcessor
22+
from opentelemetry import baggage
23+
from opentelemetry import context
24+
from opentelemetry.sdk.trace import ReadableSpan
25+
from opentelemetry.sdk.trace import TracerProvider
26+
from opentelemetry.sdk.trace.export import SimpleSpanProcessor
27+
from opentelemetry.sdk.trace.export.in_memory_span_exporter import InMemorySpanExporter
28+
import pytest
29+
30+
_AE_TRACEPARENT_HEADER = 'Google-Agent-Engine-Traceparent'
31+
_TRACEPARENT_HEADER = 'traceparent'
32+
_SUPPORT_ID_ATTRIBUTE = 'supportID'
33+
_SUPPORT_ID_VALUE = 'support-id-value'
34+
_TOP_SPAN = 'invocation'
35+
_CHILD_SPAN = 'child'
36+
37+
_TRACE_ID_HEX = '4bf92f3577b34da6a3ce929d0e0e4736'
38+
_REMOTE_SPAN_ID_HEX = '00f067aa0ba902b7'
39+
_WELL_FORMED_TRACEPARENT = f'00-{_TRACE_ID_HEX}-{_REMOTE_SPAN_ID_HEX}-01'
40+
41+
# Values the trace context propagator refuses, either because they do not
42+
# match the wire format or because the ids they carry are not usable.
43+
_REJECTED_TRACEPARENT_VALUES = [
44+
'x',
45+
'00-abc-zz-01',
46+
'',
47+
'00',
48+
'-',
49+
f'00-{_TRACE_ID_HEX}-{_REMOTE_SPAN_ID_HEX}',
50+
f'00-{"0" * 32}-{_REMOTE_SPAN_ID_HEX}-01',
51+
f'ff-{_TRACE_ID_HEX}-{_REMOTE_SPAN_ID_HEX}-01',
52+
]
53+
54+
55+
def _request(**headers: str) -> fastapi.Request:
56+
"""Builds a minimal request carrying the given headers."""
57+
return fastapi.Request({
58+
'type': 'http',
59+
'method': 'POST',
60+
'path': '/',
61+
'headers': [
62+
(name.lower().encode(), value.encode())
63+
for name, value in headers.items()
64+
],
65+
})
66+
67+
68+
def _record_spans(ctx: context.Context) -> dict[str, ReadableSpan]:
69+
"""Traces a child span under a top span with ctx attached, keyed by name."""
70+
exporter = InMemorySpanExporter()
71+
provider = TracerProvider(shutdown_on_exit=False)
72+
provider.add_span_processor(TopSpanProcessor())
73+
provider.add_span_processor(SimpleSpanProcessor(exporter))
74+
tracer = provider.get_tracer(__name__)
75+
76+
token = context.attach(ctx)
77+
try:
78+
with tracer.start_as_current_span(_TOP_SPAN):
79+
with tracer.start_as_current_span(_CHILD_SPAN):
80+
pass
81+
finally:
82+
context.detach(token)
83+
84+
return {span.name: span for span in exporter.get_finished_spans()}
85+
86+
87+
@pytest.mark.parametrize('header_value', _REJECTED_TRACEPARENT_VALUES)
88+
def test_rejected_header_still_produces_child_spans(header_value):
89+
"""A caller-supplied header must not be able to break span creation."""
90+
spans = _record_spans(
91+
get_propagated_context(_request(**{_AE_TRACEPARENT_HEADER: header_value}))
92+
)
93+
94+
assert set(spans) == {_TOP_SPAN, _CHILD_SPAN}
95+
96+
97+
@pytest.mark.parametrize('header_value', _REJECTED_TRACEPARENT_VALUES)
98+
def test_rejected_header_is_not_stored_in_baggage(header_value):
99+
"""Only a header the propagator accepted is worth carrying in baggage."""
100+
ctx = get_propagated_context(
101+
_request(**{_AE_TRACEPARENT_HEADER: header_value})
102+
)
103+
104+
assert _TRACEPARENT_HEADER not in baggage.get_all(context=ctx)
105+
106+
107+
@pytest.mark.parametrize('baggage_value', _REJECTED_TRACEPARENT_VALUES)
108+
def test_rejected_value_in_baggage_still_produces_child_spans(baggage_value):
109+
"""The processor runs on every span, so it cannot trust baggage contents."""
110+
spans = _record_spans(baggage.set_baggage(_TRACEPARENT_HEADER, baggage_value))
111+
112+
assert set(spans) == {_TOP_SPAN, _CHILD_SPAN}
113+
114+
115+
def test_well_formed_header_is_stored_in_baggage():
116+
"""The top span check reads the accepted header back out of baggage."""
117+
ctx = get_propagated_context(
118+
_request(**{_AE_TRACEPARENT_HEADER: _WELL_FORMED_TRACEPARENT})
119+
)
120+
121+
assert (
122+
baggage.get_all(context=ctx)[_TRACEPARENT_HEADER]
123+
== _WELL_FORMED_TRACEPARENT
124+
)
125+
126+
127+
def test_well_formed_header_marks_first_span_as_top_span():
128+
"""This is the propagation the rejected-header guards must not break."""
129+
spans = _record_spans(
130+
get_propagated_context(
131+
_request(**{
132+
_AE_TRACEPARENT_HEADER: _WELL_FORMED_TRACEPARENT,
133+
_TRACEPARENT_HEADER: _SUPPORT_ID_VALUE,
134+
})
135+
)
136+
)
137+
138+
assert spans[_TOP_SPAN].parent.span_id == int(_REMOTE_SPAN_ID_HEX, 16)
139+
assert spans[_TOP_SPAN].attributes[_SUPPORT_ID_ATTRIBUTE] == _SUPPORT_ID_VALUE
140+
assert _SUPPORT_ID_ATTRIBUTE not in spans[_CHILD_SPAN].attributes
141+
142+
143+
def test_first_span_is_parentless_when_header_is_rejected():
144+
"""Rejecting the header leaves the first span parentless, still the top."""
145+
spans = _record_spans(
146+
get_propagated_context(
147+
_request(**{
148+
_AE_TRACEPARENT_HEADER: 'x',
149+
_TRACEPARENT_HEADER: _SUPPORT_ID_VALUE,
150+
})
151+
)
152+
)
153+
154+
assert spans[_TOP_SPAN].parent is None
155+
assert spans[_TOP_SPAN].attributes[_SUPPORT_ID_ATTRIBUTE] == _SUPPORT_ID_VALUE
156+
assert _SUPPORT_ID_ATTRIBUTE not in spans[_CHILD_SPAN].attributes

0 commit comments

Comments
 (0)