kaxil commented on code in PR #67592:
URL: https://github.com/apache/airflow/pull/67592#discussion_r3677732965
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/task_instances.py:
##########
@@ -233,6 +236,22 @@ def ti_run(
extra=json.dumps({"host_name": ti_run_payload.hostname}) if
ti_run_payload.hostname else None,
)
)
+ # Emit task.queued_duration on a real QUEUED -> RUNNING transition.
The scheduler
+ # refreshes queued_dttm every time it queues a task, so utcnow() -
queued_dttm is a
+ # meaningful queue wait for first runs and retries alike (a retry is a
new try that
+ # genuinely waited in the queue) — mirroring the legacy emit that
fired on every
+ # transition to RUNNING. Only resumes from deferral are skipped,
identified by
+ # next_method (the trigger sets it on resume), to avoid re-emitting
within the same
+ # try — this matches how the endpoint already detects deferral resumes
via
+ # next_kwargs / next_method.
+ # The registry-based legacy name
dag.<dag_id>.<task_id>.queued_duration is
+ # emitted automatically by stats.timing via metrics_template.yaml.
+ if ti.queued_dttm is not None and ti.next_method is None:
+ stats.timing(
+ "task.queued_duration",
+ timezone.utcnow() - ti.queued_dttm,
+ tags={"task_id": ti.task_id, "dag_id": ti.dag_id, "queue":
ti.queue},
Review Comment:
These tags don't line up with `task.scheduled_duration`, which goes out as
`{**ti.stats_tags, "queue": ti.queue}`: `dag_id`, `task_id`, `queue`,
`run_type`, plus `team_name` under multi-team and Dag tags when that config is
enabled. In Airflow 2 both metrics shared that single emit path in
`emit_state_change_metric`, so `queued_duration` carried `run_type` as well. As
written the restored metric can't be sliced the same way as its sibling, which
is the comparison people actually want (scheduling delay vs queue wait, broken
down the same way).
`DR.run_type` is a free add to the select since DR is already joined, and
`dr.team_name` gets resolved through `get_team_name_for_ti` further down this
same handler, so moving the emit below that point would reuse the value rather
than pay for a second query.
Raised the same thing on #67668, which is the other open fix for this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
