Skip to content

Commit abc7e28

Browse files
committed
Keep the W3C traceparent shorthand working under any OTEL_PROPAGATORS
The bare-string form of airflow/dagrun_parent_trace_context is documented as a W3C traceparent -- its value syntax, not just its key name -- so routing it through the configured propagators made a documented input silently produce a root trace on any deployment whose propagator set omits tracecontext.
1 parent ebc3310 commit abc7e28

3 files changed

Lines changed: 66 additions & 7 deletions

File tree

‎airflow-core/docs/administration-and-deployment/logging-monitoring/traces.rst‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,12 @@ run — from the UI/API "Trigger with config" dialog, the ``airflow dags trigger
135135
136136
{"airflow/dagrun_parent_trace_context": "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"}
137137
138-
A carrier mapping is read by the configured propagators, so under the default
139-
``OTEL_PROPAGATORS=tracecontext,baggage`` it accepts ``traceparent``, ``tracestate`` and
140-
``baggage``, and a deployment running a vendor propagator can supply that propagator's own keys.
138+
The two forms are read differently, on purpose. The bare-string shorthand is *defined* as a W3C
139+
``traceparent`` — its value syntax, not just its key name — so it is always parsed as one,
140+
regardless of ``OTEL_PROPAGATORS``. A carrier mapping is read by the configured propagators
141+
instead, so under the default ``OTEL_PROPAGATORS=tracecontext,baggage`` it accepts
142+
``traceparent``, ``tracestate`` and ``baggage``, and a deployment running a vendor propagator can
143+
supply that propagator's own keys. Use the mapping form when you are not propagating W3C.
141144

142145
.. code-block:: json
143146

‎airflow-core/src/airflow/models/dagrun.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from opentelemetry import propagate, trace
3232
from opentelemetry.context import context
3333
from opentelemetry.trace import StatusCode
34+
from opentelemetry.trace.propagation.tracecontext import TraceContextTextMapPropagator
3435
from sqlalchemy import (
3536
JSON,
3637
Enum,
@@ -220,15 +221,20 @@ def parent_trace_context(conf) -> context.Context | None:
220221
return None
221222
match conf.get(DAGRUN_PARENT_TRACE_CONTEXT_KEY):
222223
case str() as traceparent:
224+
# The shorthand's *value* syntax is W3C, not just its key name, so it keeps
225+
# being parsed as W3C whatever OTEL_PROPAGATORS says -- handing it to a
226+
# propagator expecting another format would silently drop the parent.
227+
extract = TraceContextTextMapPropagator().extract
223228
carrier = {"traceparent": traceparent}
224229
case dict() as raw:
225230
# Keep only str members: a non-str tracestate reaches TraceState.from_header
226231
# unvalidated and raises TypeError, which would drop the otherwise-valid parent.
232+
extract = propagate.extract
227233
carrier = {k: v for k, v in raw.items() if isinstance(v, str)}
228234
case _:
229235
return None
230236
try:
231-
ctx = propagate.extract(carrier)
237+
ctx = extract(carrier)
232238
except Exception:
233239
# Never let a malformed conf value fail run creation; fall back to a root trace.
234240
return None

‎airflow-core/tests/unit/models/test_dagrun.py‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4265,15 +4265,65 @@ def test_parent_trace_context_carrier_read_by_configured_propagators():
42654265
assert baggage.get_all(ctx) == {"tenant": "acme"}
42664266

42674267

4268-
@mock.patch("airflow.models.dagrun.propagate.extract", side_effect=ValueError("boom"))
4269-
def test_parent_trace_context_swallows_propagator_error(mock_extract):
4270-
"""A propagator failure degrades to a root trace instead of failing run creation."""
4268+
@pytest.fixture
4269+
def global_propagator_without_tracecontext():
4270+
"""Stand in for a deployment whose OTEL_PROPAGATORS does not include ``tracecontext``."""
4271+
from opentelemetry import propagate
4272+
from opentelemetry.baggage.propagation import W3CBaggagePropagator
4273+
from opentelemetry.propagators.composite import CompositePropagator
4274+
4275+
original = propagate.get_global_textmap()
4276+
propagate.set_global_textmap(CompositePropagator([W3CBaggagePropagator()]))
4277+
yield
4278+
propagate.set_global_textmap(original)
4279+
4280+
4281+
def test_parent_trace_context_string_shorthand_ignores_configured_propagators(
4282+
global_propagator_without_tracecontext,
4283+
):
4284+
"""The bare-string shorthand is a W3C traceparent by definition, so OTEL_PROPAGATORS cannot break it."""
42714285
from airflow.models.dagrun import parent_trace_context
42724286

42734287
conf = {DAGRUN_PARENT_TRACE_CONTEXT_KEY: f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"}
4288+
span_ctx = otel_trace.get_current_span(parent_trace_context(conf)).get_span_context()
4289+
assert format(span_ctx.trace_id, "032x") == _EXTERNAL_TRACE_ID
4290+
4291+
4292+
def test_parent_trace_context_carrier_mapping_follows_configured_propagators(
4293+
global_propagator_without_tracecontext,
4294+
):
4295+
"""The mapping form is the propagator-agnostic path, so a W3C-less propagator set drops it."""
4296+
from airflow.models.dagrun import parent_trace_context
4297+
4298+
conf = {
4299+
DAGRUN_PARENT_TRACE_CONTEXT_KEY: {"traceparent": f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"}
4300+
}
42744301
assert parent_trace_context(conf) is None
42754302

42764303

4304+
@pytest.mark.parametrize(
4305+
("conf_value", "patch_target"),
4306+
[
4307+
pytest.param(
4308+
f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01",
4309+
"airflow.models.dagrun.TraceContextTextMapPropagator.extract",
4310+
id="string-shorthand",
4311+
),
4312+
pytest.param(
4313+
{"traceparent": f"00-{_EXTERNAL_TRACE_ID}-{_EXTERNAL_SPAN_ID}-01"},
4314+
"airflow.models.dagrun.propagate.extract",
4315+
id="carrier-mapping",
4316+
),
4317+
],
4318+
)
4319+
def test_parent_trace_context_swallows_propagator_error(conf_value, patch_target):
4320+
"""A propagator failure degrades to a root trace instead of failing run creation."""
4321+
from airflow.models.dagrun import parent_trace_context
4322+
4323+
with mock.patch(patch_target, side_effect=ValueError("boom")):
4324+
assert parent_trace_context({DAGRUN_PARENT_TRACE_CONTEXT_KEY: conf_value}) is None
4325+
4326+
42774327
class TestDagRunTracing:
42784328
"""Tests for DagRun OpenTelemetry span behavior."""
42794329

0 commit comments

Comments
 (0)