feat(grafana-alertcheck): add stop subcommand to reap a detached recorder by Tofel · Pull Request #2843 · smartcontractkit/chainlink-testing-framework

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

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.

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

github-actions · 2026-09-28T10:14:03Z

📊 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 identity and cleanup error-handling issues can cause unsafe termination or incomplete cleanup.

Review effort: Lite
Findings: 1 · 1

Open (2)

Prevent killing a reused PID after the flock is released · New
Propagate pidfile removal errors during cleanup · New

What changed in this PR

Adds a stop subcommand and shared recorder-stop protocol for safely reaping detached Grafana recorders.

Changes:

Adds SIGKILL fallback and pidfile cleanup.
Exposes gate.StopRecorder and registers the CLI command.
Adds tests and updates documentation while preserving check semantics.

File	Summary
`grafana-alertcheck/README.md`	Updates workflow documentation.
`grafana-alertcheck/internal/gate/stop.go`	Implements recorder stopping and cleanup.
`grafana-alertcheck/internal/gate/stop_test.go`	Tests stop behavior.
`grafana-alertcheck/internal/gate/check.go`	Reuses the shared stop protocol.
`grafana-alertcheck/internal/gate/check_process.go`	Adds SIGKILL support.
`grafana-alertcheck/docs/reference/cli.md`	Documents stop usage and semantics.
`grafana-alertcheck/docs/index.md`	Adds cleanup guidance.
`grafana-alertcheck/docs/architecture.md`	Documents stop and check semantics.
`grafana-alertcheck/cmd/stop.go`	Implements the stop command.
`grafana-alertcheck/cmd/stop_test.go`	Tests CLI behavior.
`grafana-alertcheck/cmd/main.go`	Registers the stop subcommand.

💡 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

Unresolved stop-protocol cleanup issues affect idempotence and can remove a newer pidfile; the unrelated 1 artifact should also be removed.

Review effort: Lite
Findings: 1 · 1 · 1

Open (3)

Remove pidfile before releasing exclusive log lock · New
Handle ESRCH race during recorder cleanup · New
Remove unrelated editor buffer file · New

Resolved since last review (2)

Prevent killing a reused PID after the flock is released
Propagate pidfile removal errors during cleanup

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved stop-protocol authority and process-cleanup safety issues must be addressed.

Review effort: Lite
Findings: 1

Open (1)

Lock contention can cause signaling of an unrelated process · New

Resolved since last review (3)

Remove pidfile before releasing exclusive log lock
Handle ESRCH race during recorder cleanup
Remove unrelated editor buffer file

Copilot

Copilot review overview

🔵 Needs a closer look

Two moderate unresolved findings require pidfile cleanup and stronger idempotence coverage.

Review effort: Lite
Findings: None

Resolved since last review (1)

Lock contention can cause signaling of an unrelated process

…rder 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.

* 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

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

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 grafana-alertcheck/internal/gate/stop.go Outdated

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

Comment thread 1 Outdated

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

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

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

Copilot AI reviewed Sep 28, 2026

View reviewed changes

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

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

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

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

Copilot AI reviewed Sep 28, 2026

View reviewed changes

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

sebawo approved these changes Sep 29, 2026

View reviewed changes

Tofel enabled auto-merge (squash) September 29, 2026 09:20

Tofel and others added 2 commits September 29, 2026 11:26

Tofel force-pushed the alertgate/stop branch from 1d9c3bc to 203e3c9 Compare September 29, 2026 09:27

sebawo approved these changes Sep 29, 2026

View reviewed changes

Tofel merged commit 85f64ec into main Sep 29, 2026
62 checks passed

Tofel deleted the alertgate/stop branch September 29, 2026 09:33

Navigation Menu

feat(grafana-alertcheck): add stop subcommand to reap a detached recorder - #2843

feat(grafana-alertcheck): add stop subcommand to reap a detached recorder#2843
Tofel merged 2 commits into
mainfrom
alertgate/stop

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!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Tofel commented Sep 28, 2026

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!

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!

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!

3 participants

github-actions Bot commented Sep 28, 2026 •

edited

Loading