alertgate/fail fast by Tofel · Pull Request #2844 · smartcontractkit/chainlink-testing-framework

Tofel · 2026-09-28T10:11:43Z

Fail fast on a condition that cannot become a pass

check currently always holds the runner until to + transitionGrace + drainTimeout, even when the run has already failed. In production that's minutes of dead waiting on a broken release.

This makes check stop as soon as it observes a monotone terminal verdict:

a post-from bad onset — the full classifier calls it newly_bad/flapping, which fails whether or not it later clears; or
an inability that already happened — heartbeat gap, sustained health=error, stale evaluation, in-window pause, absent rule.

A preexisting bad instance is deliberately not terminal: it can still become recovered, which passes. unobservable still beats violation.

An early exit can never be 0 (enforced in earlyResult). It may report 1 where a full run would later discover an inability and report 2.

terminalVerdict is pure and reuses proveCoverage + classifyRule over the observed sub-window.
On termination the run is classified over [from, At] by the same decide (window clamped, grace zeroed); the requested window and real thresholds are restored and TerminatedEarly is set on the result.
Recorder mode tails the recorder's log (complete lines only) for the guard; the strict whole-file ReadLog after writer exit is still the only evidence classified.
--no-fail-fast disables the guard and reproduces the previous behavior exactly.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

github-actions · 2026-09-28T10:12:45Z

📊 API Diff Results

No changes detected for module github.com/smartcontractkit/chainlink-testing-framework/grafana-alertcheck

View full report

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved recorder cleanup, PID-safety, error-handling, and nodata fail-fast issues block approval.

Review effort: Lite
Findings: 1 · 2

Open (3)

Prevent PID reuse from killing unrelated processes · New
Propagate cleanup open errors except missing recorder · New
Fail fast on configured sustained nodata unobservability · New

What changed in this PR

Adds fail-fast alert checking, recorder log tailing, and a stop command for detached recorders.

Changes:

Detects terminal failures and unobservable conditions early.
Adds recorder cleanup, --no-fail-fast, tests, documentation, and release metadata.

Review findings:

Critical (1 vote): Cleanup-only SIGKILL can target a reused PID.
Moderate: Recorder cleanup is skipped on tailing/setup errors (1 vote each).
Moderate (3 votes): Non-ENOENT recorder open errors are incorrectly ignored.
Moderate (2 votes): Configured nodata unobservability is not fail-fast eligible.

File	Reviewed change
`grafana-alertcheck/README.md`	Documents recorder cleanup.
`grafana-alertcheck/internal/gate/terminal.go`	Implements terminal verdicts and early results.
`grafana-alertcheck/internal/gate/terminal_test.go`	Tests terminal classification.
`grafana-alertcheck/internal/gate/stop.go`	Adds recorder stopping and cleanup.
`grafana-alertcheck/internal/gate/stop_test.go`	Tests recorder termination.
`grafana-alertcheck/internal/gate/logtail.go`	Tails complete recorder log records.
`grafana-alertcheck/internal/gate/logtail_test.go`	Tests incremental log parsing.
`grafana-alertcheck/internal/gate/classify.go`	Adds early-termination metadata.
`grafana-alertcheck/internal/gate/check.go`	Integrates fail-fast collection.
`grafana-alertcheck/internal/gate/check_test.go`	Tests fail-fast behavior.
`grafana-alertcheck/internal/gate/check_process.go`	Adds SIGKILL cleanup support.
`grafana-alertcheck/docs/reference/cli.md`	Documents CLI options.
`grafana-alertcheck/docs/index.md`	Updates operational guidance.
`grafana-alertcheck/docs/how-alerts-are-evaluated.md`	Documents fail-fast semantics.
`grafana-alertcheck/docs/architecture.md`	Documents architecture changes.
`grafana-alertcheck/cmd/table.go`	Displays early-exit details.
`grafana-alertcheck/cmd/stop.go`	Adds the `stop` command.
`grafana-alertcheck/cmd/stop_test.go`	Tests stop CLI behavior.
`grafana-alertcheck/cmd/main.go`	Registers the stop command.
`grafana-alertcheck/cmd/check.go`	Adds `--no-fail-fast`.
`grafana-alertcheck/.changeset/v0.1.2.md`	Records release changes.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

Fix recorder cleanup and repeated guard scans, and remove the tracked 1 artifact.

Review effort: Lite
Findings: 2

Open (2)

Remove tracked commit-message template/status dump file · New
Stop recorder before returning on tail read errors · New

Resolved since last review (3)

Prevent PID reuse from killing unrelated processes
Fail fast on configured sustained nodata unobservability
Propagate cleanup open errors except missing recorder

Copilot

Copilot review overview

🔵 Needs a closer look

Four moderate issues remain in fail-fast evaluation, deadline handling, result restoration, and preexisting-failure policy handling.

Review effort: Lite
Findings: None

Resolved since last review (2)

Stop recorder before returning on tail read errors
Remove tracked commit-message template/status dump file

… a pass check now stops collecting as soon as it observes a monotone terminal verdict instead of always holding the runner to to + transitionGrace + drainTimeout: - a post-from bad onset, which the full classifier calls newly_bad or flapping and fails whether or not it later clears; or - an inability that has already happened: a heartbeat gap, a sustained health=error run, a stale evaluation, an in-window pause, an absent rule. terminalVerdict is pure and reuses proveCoverage and classifyRule over the observed sub-window with a synthetic sentinel. A preexisting bad instance is deliberately not terminal: it can still become recovered, which passes. unobservable still beats violation (H6). In recorder mode the evidence lives in another process, so check tails the recorder's log while it waits, consuming complete newline-terminated records only. That reading is used only for the guard; the strict whole-file ReadLog still runs after the writer exits and is the only evidence classified. On a terminal verdict the run is classified over [from, At] by the same decide, with the policy window clamped and the grace zeroed; the requested window and real thresholds are restored and the Result carries TerminatedEarly. An early exit can never be 0. --no-fail-fast leaves the guard unset and reproduces the previous full-window behavior exactly.

…es (#2850) check's output now speaks plain words, in both the JSON vocabulary and the human tables: - outcomes: clean -> healthy, newly_bad -> new_failure, persistently_bad -> still_failing, flapping -> unstable, skipped -> paused, unobservable -> not_verified, and terminated_early.kind follows the same rename. A --min-observed deficit that no resolved rule explains is now not_counted instead of being blamed on a paused rule. - RESULTS and VIOLATIONS columns are spelled out (ALERT, VERDICT, BROKEN FOR, CHECKED EVERY, WINDOW COVERED, DETAILS; GRAFANA STATE, GRAFANA HEALTH, INSTANCES). INSTANCES is one word: the old "INSTANCE COUNT" header read as two columns, one of them empty under the count. - THRESHOLDS became LIMITS USED, with each limit named in plain words and explained by a legend under the table. The footer spells out the extra observation time, the evaluation wait and the clock difference from Grafana. The rename reaches the JSON output, so consumers of violations[].outcome, outcomes and terminated_early must move to the new vocabulary.

* feat(grafana-alertcheck): fail fast on a condition that cannot become a pass check now stops collecting as soon as it observes a monotone terminal verdict instead of always holding the runner to to + transitionGrace + drainTimeout: - a post-from bad onset, which the full classifier calls newly_bad or flapping and fails whether or not it later clears; or - an inability that has already happened: a heartbeat gap, a sustained health=error run, a stale evaluation, an in-window pause, an absent rule. terminalVerdict is pure and reuses proveCoverage and classifyRule over the observed sub-window with a synthetic sentinel. A preexisting bad instance is deliberately not terminal: it can still become recovered, which passes. unobservable still beats violation (H6). In recorder mode the evidence lives in another process, so check tails the recorder's log while it waits, consuming complete newline-terminated records only. That reading is used only for the guard; the strict whole-file ReadLog still runs after the writer exits and is the only evidence classified. On a terminal verdict the run is classified over [from, At] by the same decide, with the policy window clamped and the grace zeroed; the requested window and real thresholds are restored and the Result carries TerminatedEarly. An early exit can never be 0. --no-fail-fast leaves the guard unset and reproduces the previous full-window behavior exactly. * feat(grafana-alertcheck): rename outcomes and print plain-worded tables (#2850) check's output now speaks plain words, in both the JSON vocabulary and the human tables: - outcomes: clean -> healthy, newly_bad -> new_failure, persistently_bad -> still_failing, flapping -> unstable, skipped -> paused, unobservable -> not_verified, and terminated_early.kind follows the same rename. A --min-observed deficit that no resolved rule explains is now not_counted instead of being blamed on a paused rule. - RESULTS and VIOLATIONS columns are spelled out (ALERT, VERDICT, BROKEN FOR, CHECKED EVERY, WINDOW COVERED, DETAILS; GRAFANA STATE, GRAFANA HEALTH, INSTANCES). INSTANCES is one word: the old "INSTANCE COUNT" header read as two columns, one of them empty under the count. - THRESHOLDS became LIMITS USED, with each limit named in plain words and explained by a legend under the table. The footer spells out the extra observation time, the evaluation wait and the clock difference from Grafana. The rename reaches the JSON output, so consumers of violations[].outcome, outcomes and terminated_early must move to the new vocabulary.

…rder (#2843) * feat(grafana-alertcheck): add stop subcommand to reap a detached recorder Extract the recorder-stop protocol out of check into an exported gate.StopRecorder, then expose it as `stop --out <file> [--pidfile F]`. stop reuses check's pidfile-plus-flock authority: the flock proves a writer exists right now, the pidfile names it. Unlike check it is a cleanup operation, so it SIGKILLs a recorder that ignores SIGTERM, removes the pidfile, and treats a missing pidfile as "nothing to stop". That makes it idempotent and safe as an `if: always()` step after a failed work step, where the recorder's Setsid session means neither check nor the runner will otherwise reap it. check keeps its semantics unchanged: a writer that will not exit is a could-not-check, never a silent kill, and it never removes the pidfile. * alertgate/fail fast (#2844) * feat(grafana-alertcheck): fail fast on a condition that cannot become a pass check now stops collecting as soon as it observes a monotone terminal verdict instead of always holding the runner to to + transitionGrace + drainTimeout: - a post-from bad onset, which the full classifier calls newly_bad or flapping and fails whether or not it later clears; or - an inability that has already happened: a heartbeat gap, a sustained health=error run, a stale evaluation, an in-window pause, an absent rule. terminalVerdict is pure and reuses proveCoverage and classifyRule over the observed sub-window with a synthetic sentinel. A preexisting bad instance is deliberately not terminal: it can still become recovered, which passes. unobservable still beats violation (H6). In recorder mode the evidence lives in another process, so check tails the recorder's log while it waits, consuming complete newline-terminated records only. That reading is used only for the guard; the strict whole-file ReadLog still runs after the writer exits and is the only evidence classified. On a terminal verdict the run is classified over [from, At] by the same decide, with the policy window clamped and the grace zeroed; the requested window and real thresholds are restored and the Result carries TerminatedEarly. An early exit can never be 0. --no-fail-fast leaves the guard unset and reproduces the previous full-window behavior exactly. * feat(grafana-alertcheck): rename outcomes and print plain-worded tables (#2850) check's output now speaks plain words, in both the JSON vocabulary and the human tables: - outcomes: clean -> healthy, newly_bad -> new_failure, persistently_bad -> still_failing, flapping -> unstable, skipped -> paused, unobservable -> not_verified, and terminated_early.kind follows the same rename. A --min-observed deficit that no resolved rule explains is now not_counted instead of being blamed on a paused rule. - RESULTS and VIOLATIONS columns are spelled out (ALERT, VERDICT, BROKEN FOR, CHECKED EVERY, WINDOW COVERED, DETAILS; GRAFANA STATE, GRAFANA HEALTH, INSTANCES). INSTANCES is one word: the old "INSTANCE COUNT" header read as two columns, one of them empty under the count. - THRESHOLDS became LIMITS USED, with each limit named in plain words and explained by a legend under the table. The footer spells out the extra observation time, the evaluation wait and the clock difference from Grafana. The rename reaches the JSON output, so consumers of violations[].outcome, outcomes and terminated_early must move to the new vocabulary.

Tofel added this pull request to stack #2845 September 28, 2026 10:11

Tofel requested a lite review from Copilot September 28, 2026 10:11

Copilot started reviewing on behalf of Tofel September 28, 2026 10:12 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Comment thread grafana-alertcheck/internal/gate/stop.go Outdated

Comment thread grafana-alertcheck/internal/gate/stop.go Outdated

Comment thread grafana-alertcheck/internal/gate/terminal.go

Tofel force-pushed the alertgate/stop branch from fa0ada0 to dddb17e Compare September 28, 2026 13:49

Tofel force-pushed the alertgate/fail-fast branch from fd8829e to 6c470cb Compare September 28, 2026 13:50

Tofel requested a lite review from Copilot September 28, 2026 13:53

Copilot started reviewing on behalf of Tofel September 28, 2026 13:54 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Comment thread 1 Outdated

Comment thread grafana-alertcheck/internal/gate/check.go

Tofel force-pushed the alertgate/fail-fast branch from 6c470cb to 2522b37 Compare September 28, 2026 17:29

Tofel force-pushed the alertgate/stop branch 2 times, most recently from 775b869 to 9bbfec2 Compare September 28, 2026 18:09

Tofel force-pushed the alertgate/fail-fast branch from 2522b37 to 7e090a7 Compare September 28, 2026 18:09

Tofel requested a lite review from Copilot September 28, 2026 18:10

Copilot started reviewing on behalf of Tofel September 28, 2026 18:11 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Tofel force-pushed the alertgate/fail-fast branch from 7e090a7 to 5efa497 Compare September 28, 2026 18:44

Tofel force-pushed the alertgate/stop branch from 9bbfec2 to 9865ad6 Compare September 28, 2026 18:44

Tofel marked this pull request as ready for review September 29, 2026 09:08

Tofel requested a review from a team as a code owner September 29, 2026 09:08

product-security-plaid-production Bot requested a review from kalverra September 29, 2026 09:08

Tofel removed this pull request from stack #2845 September 29, 2026 09:17

Tofel merged commit 1d9c3bc into alertgate/stop Sep 29, 2026
43 of 54 checks passed

Tofel deleted the alertgate/fail-fast branch September 29, 2026 09:18

Navigation Menu

Repository navigation

alertgate/fail fast - #2844

alertgate/fail fast#2844
Tofel merged 2 commits into
alertgate/stopfrom
alertgate/fail-fast

Tofel commented Sep 28, 2026

Uh oh!

github-actions Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Tofel commented Sep 28, 2026

Fail fast on a condition that cannot become a pass

Uh oh!

github-actions Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 API Diff Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Sep 28, 2026 •

edited

Loading