Allow embedding a Dag run in an external trace via run conf#69633
Conversation
xBis7
left a comment
xBis7
left a comment
There was a problem hiding this comment.
@dstandish I tested it manually with a python script that starts a root span and then triggers a dag with the context passed in the --conf param. It works as expected!
xBis7
left a comment
xBis7
left a comment
There was a problem hiding this comment.
I added some small comments but generally it looks good and I tested it manually and I don't see any issues.
xBis7
left a comment
xBis7
left a comment
There was a problem hiding this comment.
@dstandish Thanks for addressing all of my comments. LGTM!
ashb
left a comment
ashb
left a comment
There was a problem hiding this comment.
Overall looks good, but a number of nits, chiefly the error handling one
A run triggered by an external system that already emits a trace (an upstream orchestrator, event pipeline, CI job, or another Airflow) currently always starts its own root trace, so the Airflow run cannot be seen as part of that larger workflow. A reserved run conf key, airflow/parent_trace_context, lets a run opt into embedding under a supplied W3C traceparent: the whole run rides the external trace and parent-based samplers inherit its sampling decision. Absent the key, the run stays a root trace as before. closes: apache#67210
Guards the isinstance filter that keeps a malformed tracestate value from raising inside OpenTelemetry's TraceState.from_header instead of being ignored, per the function's documented contract.
Reduces duplication across three tests that only varied the sampler, parent sampled-flag, and force_sampled input.
Distinguishes it from future non-dag-run traces (e.g. scheduler debug spans) that may need their own external-trace embedding key.
A malformed dagrun_parent_trace_context value must never break Dag run creation. Guard the propagator extraction so any parsing failure falls back to a root trace instead of surfacing, and keep only string carrier members so a non-string tracestate can't discard an otherwise-valid parent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
07c19ac to
4a5ad1a
Compare
…9633) Add a new reserved key to dagrun conf, `airflow/parent_trace_context`. When set with a W3C traceparent, the dag run span will be nested under this trace.
|
|
||
|
|
||
| def _carrier_span_context(carrier: dict[str, str]): | ||
| ctx = TraceContextTextMapPropagator().extract(carrier) |
There was a problem hiding this comment.
Should this instead be:
| ctx = TraceContextTextMapPropagator().extract(carrier) | |
| ctx = extract(carrier) |
or get_global_textmap().extract(carrier)
So that distros and configurations can leverage their own?
There was a problem hiding this comment.
This sounds like a valid comment.
So that distros and configurations can leverage their own?
@tailg8nj Can you explain the need for this? Is there a scenario where the user would need to use a custom propagator in general? For example, not just here but throughout the project, in all other places where we propagate the context. Attach and detach something to every context?
There was a problem hiding this comment.
I actually meant to comment on dagrun.py above, but it's the same concept. I have outlined rationale in #70388
5 participants
Lets a Dag run opt into being embedded inside an external trace instead of always starting its own root trace. A run triggered by an external system that already emits a trace — an upstream orchestrator, event pipeline, CI job, or another Airflow — can now appear as part of that larger workflow end-to-end.
A reserved run conf key,
airflow/dagrun_parent_trace_context(a W3Ctraceparentstring, or a{traceparent, tracestate}mapping), is read at run-creation time. When present, the run's trace carrier rides the externaltrace_id(so the whole run —dag_runspan plus all task/worker spans — lives inside the external trace) and parent-based samplers inherit the external sampling decision; the emitteddag_runspan nests under the external span. When the key is absent, the run remains a root trace exactly as before. Malformed/missing values are ignored rather than failing run creation.This joins the existing per-run trace controls
airflow/trace_sampledandairflow/task_span_detail_level, and is documented alongside them. Supersedes the auto-nesting approach in #67424 (see the redesigned spec on the issue).closes: #67210
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines