Emit metric dag auto paused - #69078
Conversation
|
CI falling |
|
I fixed it. The commands below execute successfully. uv run python scripts/ci/prek/check_metrics_synced_with_the_registry.py airflow-core/src/airflow/models/dagrun.pyCould you please rerun the GitHub Actions? |
|
@henry3260 |
potiuk
left a comment
There was a problem hiding this comment.
Thanks — useful metric. Auto-pausing after consecutive failures is exactly the kind of thing operators want alerting on, and today it happens silently. Emitting at the point of the pause in _check_last_n_dagruns_failed is the right place, and the test asserting the stats.incr call rather than just the pause behaviour is what makes it meaningful.
One inconsistency in the metric declaration inline.
Heads-up unrelated to the change itself: #70013 also edits metrics_template.yaml, in a nearby part of the file. Whichever of the two merges second will need a trivial rebase — no action now, just so it isn't a surprise.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
| legacy_name: "-" | ||
| name_variables: [] | ||
|
|
||
| - name: "dag.auto_paused" |
There was a problem hiding this comment.
The declaration doesn't match what the code actually emits. stats.incr(..., tags=self.stats_tags) uses DagRun.stats_tags, which carries both dag_id and run_type — but this entry says name_variables: [] and the description mentions only "dag_id tagging".
For comparison, #70013 declares its tagged gauges as name_variables: ["dag_id"] with a matching legacy_name. Worth being consistent: either list both variables here with an appropriate legacy_name, or narrow the emission to {"dag_id": self.dag_id} if run_type isn't wanted on this counter. Given it counts a Dag-level event rather than a run-level one, dropping run_type may actually be the cleaner answer.
Also minor: the description's second line starts at the same indentation as the key, which makes it a slightly odd multi-line plain scalar. Indenting the continuation under description: reads better and matches the surrounding entries.
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
There was a problem hiding this comment.
Thanks for the detailed review! I've dropped run_type from the tags so dag.auto_paused only carries dag_id (matching the "Dag-level event" reasoning you laid out), and fixed the description indentation to match the surrounding entries. CI is green now — happy to take another look whenever you get a chance.
commit: 98128f6
Drafted-by: Claude Code (Sonnet 5); reviewed by @takayoshi-makabe before posting
The metric was emitted in code but missing from the registry, causing the check-metrics-synced-with-registry static check to fail.
732b68f to
98128f6
Compare
Emit
dag.auto_pausedmetric when the scheduler automatically pauses a DAG after exceedingmax_consecutive_failed_dag_runs. Platform teams can now alert on this event directly via StatsD/OpenTelemetry without polling the database or correlating failure metrics manually.The metric includes
dag_idandrun_typetags via the existingstats_tagsproperty onDagRun.closes: #69004
Was generative AI tooling used to co-author this PR?
Claude Code
{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.