Add run_type tag to dagrun.duration.failed timeout metric - #67765
Conversation
boring-cyborg
Bot
added
the
area:Scheduler
0b1b2d3 to
035b401
Compare
|
Gentle ping when you have bandwidth, @potiuk 🙏 One-line metrics fix for #64765: the Dag-run timeout path emitted You commented on the prior stalled attempt (#64768), which stalled on CI/static checks rather than design so this keeps the diff to a single value change and ships green static checks, including the metrics-registry sync check. Regression test included. |
potiuk
added
the
ready for maintainer review
035b401 to
4f9d0e8
Compare
4f9d0e8 to
d177e16
Compare
When a Dag run timed out, the scheduler emitted dagrun.duration.failed with only a dag_id tag, while every other dagrun.duration.* metric (emitted by DagRun._emit_duration_stats_for_finished_state) also carries run_type. This made the timeout path inconsistent and dropped run_type from dashboards and alerts for timed-out runs. Emit the metric with dag_run.stats_tags so it includes both dag_id and run_type, matching the canonical completion path. closes: apache#64765
d177e16 to
aacad0d
Compare
|
Rebased this PR onto current upstream/main and pushed a cleaned update. I resolved the scheduler_job_runner.py conflict by preserving the upstream team_name tag handling and using DagRun.stats_tags for dag_id/run_type parity with the normal completion metric path. Validation run:
PR is already marked ready for review and is mergeable; only required review/checks remain. Drafted-by: Codex (GPT-5) |
potiuk
left a comment
potiuk
left a comment
There was a problem hiding this comment.
Correct and nicely scoped — switching to dag_run.stats_tags makes the timeout path consistent with DagRun._emit_duration_stats_for_finished_state, and the test pins the tag set. The newsfragment is appropriate since the metric shape is user-visible.
One thing worth being aware of downstream: dashboards keyed on the old tag set will now see an extra dimension.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
2 participants
What / impact
When a Dag run timed out, the scheduler emitted the
dagrun.duration.failedtiming metric with only adag_idtag. Every otherdagrun.duration.*emission also carriesrun_type, so dashboards and alerts that segment byrun_typesilently dropped timed-out runs (or saw an inconsistent tag set). After this change the timeout path emits bothdag_idandrun_type, consistent with normal completion.Why
The timeout branch in
_schedule_dag_runhardcodedtags={"dag_id": dag_run.dag_id}, while the canonical emitterDagRun._emit_duration_stats_for_finished_stateusesdag_run.stats_tags(which includesrun_type). This replaces the hardcoded dict withdag_run.stats_tags.run_typeisNOT NULLand low-cardinality, so the new tag set is a safe strict superset of the old one and the metric name is unchanged.How it unblocks
Makes timeout-duration metrics consistent and usable in
run_type-segmented observability. Supersedes #64768, which stalled on CI/static checks rather than design — this keeps the diff to a single value change and ships green static checks, including the metrics-registry sync check that blocked the prior attempt.Tests
One new regression test in
test_scheduler_job.py, verified to fail on the unfixed code, asserting thedagrun.duration.failedtiming call carriestags={"dag_id": …, "run_type": …}. Full scheduler suite passes; mypy + pre-commit + manual static checks (incl. metrics-registry sync) are green.closes: #64765
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines
Important
🛠️ Maintainer triage note for @Vamsi-klu · by
@potiuk· 2026-07-02 17:46 UTCHelpful heads-up from the maintainers — please address before this PR can be reviewed (see the Pull Request quality criteria):
main. Please rebase onto the latestmainand resolve them.The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.
Automated triage — may be imperfect; a maintainer takes the next look.