fix(cron): a successful run resolves the job's open incidents; a repeat re-opens them
The incident ledger only ever grew. A one-off failure (a drift skip after a global model bump, a provider outage) stayed `detected`/`alerted` forever after the job recovered, so `hermes cron incidents` listed 32 "open" incidents on an install where all 32 jobs had since run OK, and the list stopped saying anything about current health. A successful run now marks that job's `detected`/`alerted` incidents `resolved` (new state). `resolved` is distinct from the operator's `closed` ack on purpose: `upsert_incident` re-opens a resolved incident as `detected` when the same error signature recurs, so the operator is alerted again for a job that broke a second time, while `closed` keeps the signature silent as before. Wired from `_compose_run_delivery` next to the failure-side upsert; best-effort, store errors never affect delivery. CLI: `--state resolved` filter and a green `resolved` colour; `closed` is now dim. Docs updated in the same change.
This commit is contained in:
+29
-5
@@ -25,7 +25,9 @@ from hermes_time import now as _hermes_now
|
||||
# Optional test override (mirrors ``cron.executions.EXECUTIONS_FILE``).
|
||||
EXECUTIONS_FILE: Optional[Path] = None
|
||||
|
||||
INCIDENT_STATES = ("detected", "alerted", "closed")
|
||||
# ``resolved``: the job ran OK after the failure (auto); ``closed``: the operator acked the signature
|
||||
# and wants it silent. Only ``closed`` is terminal; a repeat of a resolved error re-opens it.
|
||||
INCIDENT_STATES = ("detected", "alerted", "resolved", "closed")
|
||||
_FAILURE_TYPE_ORDER = (
|
||||
("rate_limit", (r"\b429\b", "rate limit", "usage limit", "quota")),
|
||||
("timeout", ("timeout", "timed out")),
|
||||
@@ -146,7 +148,8 @@ def upsert_incident(
|
||||
"""Record (or refresh) the incident for ``job_id`` + ``error``; returns ``(incident_id,
|
||||
is_new)``. An existing row for the signature refreshes
|
||||
``last_seen_at``/``error``/``output_file`` and keeps its state — a ``closed`` incident stays
|
||||
closed. A changed error text mints a new incident."""
|
||||
closed, while a ``resolved`` one (job recovered, then broke the same way again) re-opens as
|
||||
``detected`` so the operator is alerted once more. A changed error text mints a new incident."""
|
||||
job_id = str(job_id or "")
|
||||
sig = _error_signature(job_id, error)
|
||||
stored_error = _redact_error(error)
|
||||
@@ -157,16 +160,19 @@ def upsert_incident(
|
||||
|
||||
with _transaction() as conn:
|
||||
row = conn.execute(
|
||||
"SELECT id FROM cron_incidents WHERE id=?", (incident_id,)
|
||||
"SELECT id, state FROM cron_incidents WHERE id=?", (incident_id,)
|
||||
).fetchone()
|
||||
if row is not None:
|
||||
reopen = row["state"] == "resolved"
|
||||
conn.execute(
|
||||
"""UPDATE cron_incidents
|
||||
SET last_seen_at=?, error=?, output_file=?
|
||||
SET last_seen_at=?, error=?, output_file=?,
|
||||
state=CASE WHEN state='resolved' THEN 'detected' ELSE state END,
|
||||
closed_at=CASE WHEN state='resolved' THEN NULL ELSE closed_at END
|
||||
WHERE id=?""",
|
||||
(now, stored_error, output_file, incident_id),
|
||||
)
|
||||
return incident_id, False
|
||||
return incident_id, reopen
|
||||
conn.execute(
|
||||
"""INSERT INTO cron_incidents
|
||||
(id, job_id, error_sig, state, failure_type,
|
||||
@@ -211,6 +217,24 @@ def ack_incident(incident_id: str) -> bool:
|
||||
return set_incident_state(incident_id, "closed")
|
||||
|
||||
|
||||
def close_incidents_for_recovered_job(job_id: str) -> int:
|
||||
"""Mark every open incident for ``job_id`` ``resolved`` after a successful run; returns how many.
|
||||
Without this the ledger only ever grows: a one-off failure (a config drift skip, a provider
|
||||
outage) stayed ``detected``/``alerted`` forever after the job recovered, so ``hermes cron
|
||||
incidents`` showed dozens of "open" incidents for jobs that had been green for weeks (32 of 32 on
|
||||
one install). ``resolved`` is distinct from the operator's ``closed`` on purpose: a repeat of the
|
||||
same error re-opens a resolved incident and alerts again (see ``upsert_incident``), whereas
|
||||
``closed`` keeps that signature silent."""
|
||||
now = _hermes_now().isoformat()
|
||||
with _transaction() as conn:
|
||||
cursor = conn.execute(
|
||||
"""UPDATE cron_incidents SET state='resolved', closed_at=?
|
||||
WHERE job_id=? AND state IN ('detected', 'alerted')""",
|
||||
(now, str(job_id or "")),
|
||||
)
|
||||
return int(cursor.rowcount or 0)
|
||||
|
||||
|
||||
def _state_filter(state: Optional[str]) -> tuple[str, tuple]:
|
||||
return ("", ()) if state is None else (" WHERE state=?", (state,))
|
||||
|
||||
|
||||
@@ -347,6 +347,17 @@ def _upsert_incident_for_failure(
|
||||
return False, None
|
||||
|
||||
|
||||
def _resolve_incidents_for_recovered_job(job: dict) -> None:
|
||||
"""Best-effort: a successful run marks the job's open incidents ``resolved`` (never touches an
|
||||
operator ``closed`` ack). Store errors log at debug; delivery is unaffected."""
|
||||
try:
|
||||
from cron.incidents import close_incidents_for_recovered_job
|
||||
|
||||
close_incidents_for_recovered_job(job["id"])
|
||||
except Exception as exc:
|
||||
logger.debug("Incident store unavailable for job %s (delivery unaffected): %s", job["id"], exc)
|
||||
|
||||
|
||||
def _mark_incident_alerted(incident_id: Optional[str]) -> None:
|
||||
"""Best-effort: mark incident ``alerted`` (no-op for closed; never resurrects an acked one)."""
|
||||
if not incident_id:
|
||||
@@ -2590,6 +2601,7 @@ def _compose_run_delivery(
|
||||
)
|
||||
elif success:
|
||||
deliver_content = final_response
|
||||
_resolve_incidents_for_recovered_job(job)
|
||||
else:
|
||||
# Record the job+error signature once; if already acked by the operator, suppress the
|
||||
# per-run ping. Best-effort: a ledger failure never breaks delivery.
|
||||
|
||||
+2
-1
@@ -270,7 +270,8 @@ def cron_runs(job_id: Optional[str] = None, limit: int = 20):
|
||||
print(f" {record['error']}")
|
||||
|
||||
|
||||
_INCIDENT_STATE_COLORS = {"detected": Colors.RED, "alerted": Colors.YELLOW, "closed": Colors.GREEN}
|
||||
_INCIDENT_STATE_COLORS = {"detected": Colors.RED, "alerted": Colors.YELLOW, "resolved": Colors.GREEN,
|
||||
"closed": Colors.DIM}
|
||||
|
||||
|
||||
def cron_incidents(args) -> int:
|
||||
|
||||
@@ -182,7 +182,7 @@ def build_cron_parser(subparsers, *, cmd_cron: Callable) -> None:
|
||||
# cron incidents — durable failure incidents (list/ack)
|
||||
cron_incidents = cron_subparsers.add_parser(
|
||||
"incidents", help="List or acknowledge durable cron failure incidents")
|
||||
cron_incidents.add_argument("--state", choices=["detected", "alerted", "closed"],
|
||||
cron_incidents.add_argument("--state", choices=["detected", "alerted", "resolved", "closed"],
|
||||
help="Filter incidents by lifecycle state")
|
||||
cron_incidents.add_argument(
|
||||
"incident_action", nargs="?", default="list", choices=["list", "ack"],
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
"""A successful run resolves the job's open incidents; the same error afterwards re-opens them.
|
||||
|
||||
The ledger only ever grew: a one-off failure (drift skip, provider outage) stayed ``detected``/``alerted``
|
||||
after the job had been green for weeks, so ``hermes cron incidents`` listed 32 "open" incidents on an
|
||||
install whose every job was healthy. ``resolved`` is auto and re-openable; ``closed`` is the operator's
|
||||
ack and stays silent.
|
||||
"""
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent.parent.parent))
|
||||
|
||||
import cron.incidents as incidents
|
||||
import cron.scheduler as sched
|
||||
|
||||
|
||||
def _point_db(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(incidents, "EXECUTIONS_FILE", tmp_path / "cron" / "executions.db")
|
||||
return incidents
|
||||
|
||||
|
||||
def test_successful_run_resolves_open_incidents_and_a_repeat_reopens_them(monkeypatch, tmp_path):
|
||||
inc = _point_db(monkeypatch, tmp_path)
|
||||
inc_id, _ = inc.upsert_incident("job-1", "provider 503")
|
||||
sched._mark_incident_alerted(inc_id)
|
||||
other_id, _ = inc.upsert_incident("job-2", "provider 503")
|
||||
|
||||
content, *_ = sched._compose_run_delivery(
|
||||
{"id": "job-1", "name": "j1"}, success=True, error=None, final_response="all good", output_file=None)
|
||||
|
||||
assert content == "all good"
|
||||
assert inc.get_incident(inc_id)["state"] == "resolved"
|
||||
assert inc.get_incident(inc_id)["closed_at"]
|
||||
assert inc.get_incident(other_id)["state"] == "detected" # another job's incident is untouched
|
||||
assert inc.count_incidents("resolved") == 1
|
||||
|
||||
same_id, reopened = inc.upsert_incident("job-1", "provider 503")
|
||||
assert same_id == inc_id and reopened is True
|
||||
assert inc.get_incident(inc_id)["state"] == "detected"
|
||||
assert inc.get_incident(inc_id)["closed_at"] is None
|
||||
|
||||
|
||||
def test_operator_ack_survives_a_successful_run(monkeypatch, tmp_path):
|
||||
inc = _point_db(monkeypatch, tmp_path)
|
||||
inc_id, _ = inc.upsert_incident("job-1", "known flaky")
|
||||
inc.ack_incident(inc_id)
|
||||
|
||||
assert inc.close_incidents_for_recovered_job("job-1") == 0
|
||||
assert inc.get_incident(inc_id)["state"] == "closed"
|
||||
_, reopened = inc.upsert_incident("job-1", "known flaky")
|
||||
assert reopened is False and inc.get_incident(inc_id)["state"] == "closed"
|
||||
@@ -429,7 +429,7 @@ ledger database as the execution history.
|
||||
|
||||
```bash
|
||||
hermes cron incidents # list incidents (newest activity first)
|
||||
hermes cron incidents --state alerted # filter: detected | alerted | closed
|
||||
hermes cron incidents --state alerted # filter: detected | alerted | resolved | closed
|
||||
hermes cron incidents ack <id> # acknowledge — stop re-pinging
|
||||
```
|
||||
|
||||
@@ -437,11 +437,17 @@ Acknowledging an incident silences the per-run failure ping for that exact
|
||||
signature only. Nothing else changes: the run history still records every
|
||||
failure, the failure streak keeps counting, and the moment the job starts
|
||||
failing with a *different* error a new incident is minted and alerts fire
|
||||
again. A successful run doesn't touch incidents — they are per-signature, not
|
||||
per-job.
|
||||
again.
|
||||
|
||||
A successful run marks every open incident for that job `resolved`, so the
|
||||
list reflects current health rather than every failure the job ever had. If
|
||||
the job later fails with the *same* error, the resolved incident re-opens as
|
||||
`detected` and you are alerted again. Acknowledged (`closed`) incidents are
|
||||
the exception: a success leaves them alone, and a repeat stays silent.
|
||||
|
||||
Incident lifecycle: `detected` (failure recorded) → `alerted` (at least one
|
||||
failure ping reached delivery) → `closed` (acknowledged; terminal for that
|
||||
failure ping reached delivery) → `resolved` (the job ran OK afterwards;
|
||||
re-opens on a repeat) or `closed` (acknowledged; terminal for that
|
||||
signature). Stored error text is secret-redacted and truncated before it is
|
||||
written.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user