Repository navigation
Conversation
potiuk
added
the
ready for maintainer review
xBis7
left a comment
xBis7
left a comment
There was a problem hiding this comment.
Hello, I just came across this PR and I've added some questions.
| return leaf_tis | ||
|
|
||
| def _emit_dagrun_span(self, state: DagRunState): | ||
| def _should_force_root_dagrun_span(self) -> bool: |
There was a problem hiding this comment.
When is a dag run span supposed to not be root? Who should have triggered it?
What happens if we add a new span under the scheduler and the scheduler triggers a dag_run, whose span isn't root? The span is going to be a child of the scheduler's span. Is it something we want to do or not?
There was a problem hiding this comment.
I agree. I think the dag run should be the root span, not the web request which creates it.
A link between the dag run root span and the request span would make sense, but to me putting the dag run spans under this API just doesn't make sense.
dstandish
left a comment
dstandish
left a comment
There was a problem hiding this comment.
Thanks for the PR it's a little different than what i was thinking when i made the associated issue.
The point was that when you create a dag run you have the capability to embed it in another trace from an external system. But that by default in general in airflow it would be a root span.
You are discriminating based on the run type, which does not seem like the right logic here. Thanks though.
|
Alternative that allows embedding into a parent trace via dag run conf: #69633 |
|
Closing in favor of #69633 |
Why
Dag run spans are currently forced to root by always passing an empty context when the span is created.
That is useful for Dag runs created by Airflow entry points such as the API, scheduler, asset scheduling, UI trigger, and backfill, since those runs represent new workflow roots.
closes: #67210
Was generative AI tooling used to co-author this PR?
Codex GPT 5.5
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.