myps6415 opened a new pull request, #67943:
URL: https://github.com/apache/airflow/pull/67943
## Summary
`task.duration` (and its registry-derived legacy name
`dag.<dag_id>.<task_id>.duration`) was emitted for tasks ending in `SUCCESS` or
`FAILED` but missing entirely for `SKIPPED` tasks, despite the metric being
documented as available for all terminal states. Reproducible with
`ShortCircuitOperator`, `BranchPythonOperator`, and `BashOperator` with
`skip_on_exit_code` (narrowed in #61849 by @shivaam).
## Fix
The metric is emitted from `finalize()` in the Task SDK `task_runner`, gated
on `if ti.start_date and ti.end_date:`. The `SUCCESS` and `FAILED` exception
handlers set `ti.end_date` on the local `RuntimeTaskInstance` before
constructing the outbound `TaskState` message, so the guard passes. The two
`SKIPPED` handlers (`AirflowSkipException`, and the `SKIPPED` branch of
`DagRunTriggerException` with `skip_when_already_exists=True`) set `end_date`
only on the outbound `TaskState` message, leaving `ti.end_date` as `None` — so
`finalize()`'s guard failed for skipped tasks.
Set `ti.end_date` on the local instance in both handlers, mirroring the
`AirflowFailException` and `_handle_current_task_success` patterns. The
`TaskState` message references `ti.end_date` to keep the local instance and the
outbound message in sync.
## Test plan
- [x] New parametrized
`test_task_duration_metric_emitted_for_terminal_states` covers `success` /
`skipped` / `failed` — verifies `stats.timing("task.duration", ...)` is called
with the correct tags and that the legacy dotted form is also emitted via the
`metrics_template.yaml` registry.
- [x] Self-verified by stashing the production change: the `[skipped]`
parametrize case fails without the fix and passes with it; `[success]` and
`[failed]` pass in both cases.
- [x] Existing SKIPPED-related tests (`test_run_basic_skipped`,
`test_task_runner_calls_listeners_skipped`, etc.) still pass — no regression.
- [x] `ruff format` / `ruff check` / `mypy-task-sdk` / `prek run --from-ref
upstream/main --stage pre-commit` all green.
Note: `test_handle_trigger_dag_run_conflict[True-skipped]` was already
failing on `upstream/main` due to a mock-assertion mismatch (the test uses
`mock.call.send(msg=TriggerDagRun(...))` as kwarg but `_handle_trigger_dag_run`
calls `SUPERVISOR_COMMS.send(TriggerDagRun(...))` positionally). Unrelated to
this PR.
closes: #61849
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 4.7)
Generated-by: Claude Code (Opus 4.7) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
