feat(collector): add durable collector-host health monitoring - #735
Conversation
Add a read-only on-host health monitor (monday-collector-health.sh + systemd service/timer) that guards against a silent recurrence of the 2026-08-05/06 disk-full incident, plus two alert channels: an Aliyun Cloud Monitor disk alarm (primary) and a scheduled GitHub Actions workflow (fallback) that opens a needs-triage issue on breach. Checks: /data mount and disk free (<25% warn, <10% crit), governed service active+enabled+Result==success with restart-rate delta, upload timer/lane status, watchdog timer/service, incident fill source stays disabled, health.json freshness/gaps, upload last_error/failure_count delta, and delay-gate trips. The monitor is read-only toward units and never modifies tape or status files.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a read-only collector host health monitor with systemd scheduling, fixture-based contract tests, Aliyun Cloud Assistant execution, and deduplicated GitHub issue reporting for health breaches and invocation failures. ChangesCollector Health Monitoring
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant AliyunCloudAssistant
participant CollectorHost
participant GitHubIssues
GitHubActions->>AliyunCloudAssistant: Invoke collector health script
AliyunCloudAssistant->>CollectorHost: Run monday-collector-health.sh
CollectorHost-->>AliyunCloudAssistant: Return health JSON and exit code
AliyunCloudAssistant-->>GitHubActions: Return invocation result
GitHubActions->>GitHubIssues: Create or update deduplicated triage issue
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/monitor-collector-host.yml:
- Around line 164-174: Add a workflow or job-level concurrency group with
cancel-in-progress set to false so issue handling runs are serialized; retain
the existing lookup in .github/workflows/monitor-collector-host.yml lines
164-174 and apply the same serialized issue path at lines 190-199.
- Around line 111-144: Update the terminal-state handling loop to process Cloud
Assistant Output for Failed, Stopped, PartialFailed, and Timeout states using
the same decoding and `.ok` JSON validation as successful states. Treat a
nonzero exit code as a valid command result, and set invocation_failed=1 only
when no valid health snapshot exists and no useful exit/result was produced;
preserve diagnostic logging for terminal failures.
- Around line 45-48: Update the Aliyun CLI installation steps before aliyun
version to use an exact, pinned release archive instead of the latest URL,
download the matching official SHA-256 checksum, verify the archive against it,
and only then extract and install it.
In `@deployment/aliyun/monday-collector-health.sh`:
- Around line 334-342: Update write_state to record a breach whenever state
persistence fails: state-directory creation, temporary-file creation, or the
final mv/rename must trigger the existing breach mechanism before returning or
cleaning up. Preserve the current dry-run behavior and successful atomic write
flow.
- Around line 323-325: Update the trip-counting logic near the journalctl
pipeline to capture journalctl’s exit status separately instead of masking
failures with || true. Preserve a successful no-match query as trips=0, but
treat a failed journalctl query as a breach so the monitor cannot report ok:true
without inspectable evidence.
- Around line 252-253: Update the health-file validation around health_file to
reject symbolic links by requiring the path to be a regular file and not a
symlink before accepting it; preserve record_breach for missing or invalid
files. Add a symbolic-link fixture covering this validation, following the
regular non-symbolic health-file contract used by host-rust-lob-cutover.sh.
In `@deployment/aliyun/test-monday-collector-health.sh`:
- Around line 280-281: Remove the BSD/GNU dual-command fallback and use one
supported sed scenario-rewrite implementation at
deployment/aliyun/test-monday-collector-health.sh lines 280-281, 293-294,
306-307, 321-322, 421-422, and 434-435. Apply the same direct replacement
consistently at every site without suppressing errors or silently retrying with
another sed variant.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 447b8b86-0e90-4694-aeaa-90a6b6399210
📒 Files selected for processing (6)
.github/workflows/monitor-collector-host.ymldeployment/aliyun/README.mddeployment/aliyun/monday-collector-health.servicedeployment/aliyun/monday-collector-health.shdeployment/aliyun/monday-collector-health.timerdeployment/aliyun/test-monday-collector-health.sh
Addresses CodeRabbit review threads blocking PR #735: - write_state: record a breach when the state directory, temp file, or rename fails, so a monitor that cannot persist restart/upload deltas cannot report healthy. - check_delay_gate: capture journalctl's own exit status; a failed query means delay-gate evidence was uninspectable and is itself a breach. - check_binance_health: reject health.json when it is a symbolic link. - Test harness: replace the BSD/GNU dual sed -i fallback with one portable temp-file rewrite helper; add fixtures for symlink health.json, journald query failure, and state-persistence failure (53 pass).
…ize runs Addresses CodeRabbit review threads blocking PR #735: - Install the Aliyun CLI from a pinned, SHA-verified v3.4.11 release instead of the mutable latest URL. - Decode the command Output in every terminal invocation state. The health script exits nonzero on a breach, so a Failed/Stopped/PartialFailed/ Timeout invocation still carries a valid ok:false snapshot that must be surfaced rather than treated as a monitor failure. - Serialize runs via a concurrency group so overlapping ticks cannot both open a duplicate needs-triage issue.
DELAY_GATE_WINDOW=15min is not a valid journalctl timestamp; the host rejects it with 'Failed to parse timestamp: 15min', so both LOB units reported a permanent 'journald query failed' breach after the #735 install. Use '15 min ago', verified against journalctl on the target host. The mock-journalctl harness never exercised timestamp parsing.
…739) DELAY_GATE_WINDOW=15min is not a valid journalctl timestamp; the host rejects it with 'Failed to parse timestamp: 15min', so both LOB units reported a permanent 'journald query failed' breach after the #735 install. Use '15 min ago', verified against journalctl on the target host. The mock-journalctl harness never exercised timestamp parsing. Co-authored-by: Sonic Shih <sonic.shih@mandonothing.com>
Change contract
Add durable on-host monitoring so a silent disk-full (all governed collectors
down, uploads failing, delay-gate trips accumulating) can never go unnoticed
again. New
monday-collector-health.sh(POSIX, read-only) emits one JSONsnapshot and exits nonzero on breach: /data mount, disk warn<25%/crit<10%,
governed units active+enabled+Result=success+restart-rate delta, incident fill
sources (
bybit-options-archiver,polymarket-raw-ops-gate@) must staydisabled, health.json freshness/sequence_gaps, upload-status last_error /
failure_count delta, and journald delay-gate trips. Runs from a hardened
oneshot service + 5-min timer (no ConditionPathIsMountPoint, so a missing /data
is detected). Primary alert channel is a Cloud Monitor disk alarm (JSON in
README); fallback is a GitHub Actions 15-min workflow invoking the script via
Cloud Assistant and opening a deduped
needs-triageissue.Issue relationship
None
Out of scope
named-controller runtime cutover with host readback).
branches).
Dependencies and merge order
None. Based on
a74ee6f4; no file overlap with the other remediation branches.Focused validation
bash -n monday-collector-health.sh test-monday-collector-health.sh— PASS./test-monday-collector-health.sh— PASS (47 assertions: healthy baseline,disk crit/warn, inactive unit, timer not enabled, Result failure, restart
delta, health stale/gap/missing, upload error/delta, delay-gate trips, mount
unmounted, bybit enabled, poly gate indirect, missing upload-status not a
breach, JSON shape healthy/breaching, dry-run writes no state)
git diff --check— clean; workflow YAML parses.Rollout and rollback
Production impact is
Noneuntil an operator installs the service/timer andalarm on the host. Rollback of the script: revert the commit. The README
documents the governed install/rollback template (named controller, source
identity, stop rules).
Scope exception
6 files / 1296 non-generated lines exceeds the 750-line default. This is one
inseparable monitoring contract (script + unit + timer + test + alarm + fallback
workflow). Named reviewer (repository owner) approved the full diff in
docs/reviews/2026-08-07-incident-remediation-diff-review.md.Summary by CodeRabbit
New Features
Documentation