diff --git a/steward/alerts/routes.py b/steward/alerts/routes.py index 3090367..7c2fa62 100644 --- a/steward/alerts/routes.py +++ b/steward/alerts/routes.py @@ -23,6 +23,7 @@ def _rule_form(form) -> dict: "operator": form.get("operator", ""), "threshold": form.get("threshold"), "consecutive_failures_required": form.get("consecutive_failures_required"), + "consecutive_ok_required": form.get("consecutive_ok_required"), } @@ -204,8 +205,7 @@ async def update_rule(rule_id: str): fields = await alert_svc.rule_fields(db, **_rule_form(form)) except ValueError as exc: return str(exc), 400 - for k, v in fields.items(): - setattr(rule, k, v) + await alert_svc.apply_rule_change(db, rule, fields) if apply_action: rule.ansible_action = ansible_action await log_audit(current_app, session.get("user_id"), session.get("username", ""), @@ -222,7 +222,7 @@ async def toggle_rule(rule_id: str): rule = result.scalar_one_or_none() if rule is None: return "Not found", 404 - rule.enabled = not rule.enabled + await alert_svc.apply_rule_change(db, rule, {}, enabled=not rule.enabled) action = "alert_rule.enabled" if rule.enabled else "alert_rule.disabled" await log_audit(current_app, session.get("user_id"), session.get("username", ""), action, entity_type="alert_rule", entity_id=rule.name) diff --git a/steward/alerts/service.py b/steward/alerts/service.py index 7826cb5..567c3fd 100644 --- a/steward/alerts/service.py +++ b/steward/alerts/service.py @@ -109,7 +109,8 @@ def _choices(values: list[str], limit: int = 20) -> str: async def rule_fields(db, *, name: str, source_module: str, resource_name: str, metric_name: str, - operator: str, threshold, consecutive_failures_required=1) -> dict: + operator: str, threshold, consecutive_failures_required=1, + consecutive_ok_required=1) -> dict: """Validated column values for a new or edited alert rule. The same choices the rule form offers: a known source, a resource that @@ -148,9 +149,39 @@ async def rule_fields(db, *, name: str, source_module: str, resource_name: str, raise ValueError("consecutive checks must be a whole number.") from None if consecutive < 1: raise ValueError("consecutive checks must be at least 1.") + try: + ok_checks = (1 if consecutive_ok_required in (None, "") + else int(consecutive_ok_required)) + except (TypeError, ValueError): + raise ValueError("checks to clear must be a whole number.") from None + if ok_checks < 1: + raise ValueError("checks to clear must be at least 1.") return {"name": name[:128], "source_module": source_module, "resource_name": resource_name, "metric_name": metric_name, "operator": op, "threshold": threshold, - "consecutive_failures_required": consecutive} + "consecutive_failures_required": consecutive, + "consecutive_ok_required": ok_checks} + + +# Changing any of these changes what the rule is watching, so whatever state +# it had built up no longer means anything. +WATCHED_FIELDS = ("source_module", "resource_name", "metric_name", "operator", "threshold") + + +async def apply_rule_change(db, rule: AlertRule, fields: dict, *, enabled: bool | None = None) -> None: + """Apply validated fields (and optionally enabled) to a rule, in the + caller's transaction. A change to what it watches, or disabling it, puts + its alert back to inactive (core.alerts.reset_rule_state).""" + from steward.core.alerts import reset_rule_state + changed = [k for k in WATCHED_FIELDS if k in fields and getattr(rule, k) != fields[k]] + for k, v in fields.items(): + setattr(rule, k, v) + disabling = enabled is False and rule.enabled + if enabled is not None: + rule.enabled = bool(enabled) + if disabling: + await reset_rule_state(db, rule, "rule disabled") + elif changed: + await reset_rule_state(db, rule, "rule changed: " + ", ".join(changed)) async def add_rule(db, fields: dict, *, created_by: str, enabled: bool = True, diff --git a/steward/core/alerts.py b/steward/core/alerts.py index 40089cb..5f5c0f0 100644 --- a/steward/core/alerts.py +++ b/steward/core/alerts.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio import logging import uuid +from dataclasses import dataclass from datetime import datetime, timezone from typing import TYPE_CHECKING @@ -151,6 +152,44 @@ def _is_breached(value: float, operator: AlertOperator, threshold: float) -> boo return False +@dataclass(frozen=True) +class Step: + """One sample's effect on an alert: the state after it, the two run + counters, and the event to record ("firing" / "resolved") if any.""" + state: AlertStateEnum + fail_count: int + ok_count: int + event: str | None = None + + +def next_step(state: AlertStateEnum, fail_count: int, ok_count: int, breached: bool, + fail_required: int, ok_required: int) -> Step: + """The alert state machine, as a pure function of one sample. + + inactive → pending → firing when `fail_required` breaching checks run in a + row; a non-breaching check while pending goes back to inactive. + firing and acknowledged resolve (to inactive, with a resolved event) only + after `ok_required` non-breaching checks in a row; a breaching check + resets that run. An acknowledgement holds while the alert is still + breached: only a resolve clears it. + """ + fail_required, ok_required = max(1, fail_required), max(1, ok_required) + S = AlertStateEnum + if state in (S.firing, S.acknowledged): + if breached: + return Step(state, fail_count + 1, 0) + if ok_count + 1 >= ok_required: + return Step(S.inactive, 0, 0, "resolved") + return Step(state, fail_count, ok_count + 1) + # inactive, pending (and the transient resolved, read as inactive) + if not breached: + return Step(S.inactive, 0, 0) + fails = fail_count + 1 if state == S.pending else 1 + if fails >= fail_required: + return Step(S.firing, fails, 0, "firing") + return Step(S.pending, fails, 0) + + async def _evaluate_rule( session: AsyncSession, rule: AlertRule, @@ -158,73 +197,56 @@ async def _evaluate_rule( value: float, now: datetime, ) -> list[tuple[str, str]]: - """Evaluate a single rule against a metric value. Returns list of (notif_type, event_id).""" - breached = _is_breached(value, rule.operator, rule.threshold) + """Apply one sample to a rule's state. Returns [(notif_type, event_id)].""" from_state = state.state + step = next_step(from_state, state.consecutive_failure_count, state.consecutive_ok_count, + _is_breached(value, rule.operator, rule.threshold), + rule.consecutive_failures_required, rule.consecutive_ok_required) + state.consecutive_failure_count = step.fail_count + state.consecutive_ok_count = step.ok_count notifications: list[tuple[str, str]] = [] - - if state.state == AlertStateEnum.inactive: - if breached: - state.consecutive_failure_count = 1 - if state.consecutive_failure_count >= rule.consecutive_failures_required: - state.state = AlertStateEnum.firing - state.last_transition_at = now - event_id = _add_event(session, rule.id, from_state.value, "firing", value, now) - notifications.append(("firing", event_id)) - else: - state.state = AlertStateEnum.pending - state.last_transition_at = now - - elif state.state == AlertStateEnum.pending: - if breached: - state.consecutive_failure_count += 1 - if state.consecutive_failure_count >= rule.consecutive_failures_required: - state.state = AlertStateEnum.firing - state.last_transition_at = now - event_id = _add_event(session, rule.id, from_state.value, "firing", value, now) - notifications.append(("firing", event_id)) - else: - state.consecutive_failure_count = 0 - state.state = AlertStateEnum.inactive - state.last_transition_at = now - - elif state.state == AlertStateEnum.firing: - if not breached: - # RESOLVED is transient: record event, notify, then immediately go inactive - event_id = _add_event(session, rule.id, from_state.value, "resolved", value, now) - notifications.append(("resolved", event_id)) - state.consecutive_failure_count = 0 - state.state = AlertStateEnum.inactive - state.last_transition_at = now - - elif state.state == AlertStateEnum.acknowledged: - if not breached: - event_id = _add_event(session, rule.id, from_state.value, "resolved", value, now) - notifications.append(("resolved", event_id)) - state.consecutive_failure_count = 0 - state.state = AlertStateEnum.inactive - state.last_transition_at = now + if step.state != from_state: + state.state = step.state + state.last_transition_at = now + if step.state == AlertStateEnum.inactive: state.acknowledged_by = None state.acknowledged_at = None - else: - state.consecutive_failure_count += 1 - if state.consecutive_failure_count >= rule.consecutive_failures_required: - state.state = AlertStateEnum.firing - state.last_transition_at = now - event_id = _add_event(session, rule.id, from_state.value, "firing", value, now) - notifications.append(("firing", event_id)) - + if step.event: + event_id = _add_event(session, rule.id, from_state.value, step.event, value, now) + notifications.append((step.event, event_id)) await session.flush() return notifications +async def reset_rule_state(session: AsyncSession, rule: AlertRule, note: str) -> None: + """Put a rule's alert back to inactive because the rule itself changed + (what it watches, its threshold, or it was disabled). An alert that was + firing or acknowledged gets a resolved event carrying `note`; nobody is + notified, since nothing about the watched resource changed.""" + state = (await session.execute( + select(AlertState).where(AlertState.rule_id == rule.id))).scalar_one_or_none() + if state is None: + return + now = datetime.now(timezone.utc) + if state.state in (AlertStateEnum.firing, AlertStateEnum.acknowledged): + _add_event(session, rule.id, state.state.value, "resolved", None, now, note=note) + if state.state != AlertStateEnum.inactive: + state.last_transition_at = now + state.state = AlertStateEnum.inactive + state.consecutive_failure_count = 0 + state.consecutive_ok_count = 0 + state.acknowledged_by = None + state.acknowledged_at = None + + def _add_event( session: AsyncSession, rule_id: str, from_state: str, to_state: str, - value: float, + value: float | None, now: datetime, + note: str | None = None, ) -> str: """Add an AlertEvent row and return its ID.""" event_id = str(uuid.uuid4()) @@ -235,6 +257,7 @@ def _add_event( to_state=to_state, metric_value=value, transitioned_at=now, + note=note, ) session.add(event) return event_id diff --git a/steward/mcp/tools/alerts.py b/steward/mcp/tools/alerts.py index da677ba..d0c5823 100644 --- a/steward/mcp/tools/alerts.py +++ b/steward/mcp/tools/alerts.py @@ -18,8 +18,10 @@ from ._shape import ( def _condition(rule: AlertRule) -> str: n = rule.consecutive_failures_required or 1 + ok = rule.consecutive_ok_required or 1 tail = f" for {n} consecutive checks" if n > 1 else "" - return f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}{tail}" + clear = f"; clears after {ok} OK checks" if ok > 1 else "" + return f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}{tail}{clear}" def _action(rule: AlertRule) -> dict | None: @@ -104,6 +106,7 @@ async def get_alert_history(rule: str = "", range: str = "7d", limit: int = 50) "from_state": e.from_state, "to_state": e.to_state, "metric_value": rnd(e.metric_value), + "note": e.note, "at": iso(e.transitioned_at), "age_s": age_s(e.transitioned_at), "email_sent": e.email_sent, @@ -226,7 +229,7 @@ def _rule_row(r: AlertRule) -> dict: async def create_alert_rule(name: str, source_module: str, resource: str, metric: str, operator: str, threshold: float, consecutive_checks: int = 1, - enabled: bool = True) -> dict: + clear_after_checks: int = 1, enabled: bool = True) -> dict: """Add an alert rule: notify when `metric` of `resource` crosses `threshold`. source_module, resource, metric: get_alert_rule_options lists the valid @@ -234,6 +237,9 @@ async def create_alert_rule(name: str, source_module: str, resource: str, metric operator: ">", "<", ">=", "<=", "==" or "!=". consecutive_checks: how many checks in a row must breach before it fires (default 1); higher values ride out a single blip. + clear_after_checks: how many checks in a row must be OK before a firing or + acknowledged alert resolves (default 1). An acknowledgement holds until + then, however long the breach lasts. Notifications go to the channels set in Steward's Settings. The playbook a rule runs when it fires is set separately (set_alert_action, admin). @@ -244,7 +250,8 @@ async def create_alert_rule(name: str, source_module: str, resource: str, metric fields = await alert_svc.rule_fields( db, name=name, source_module=source_module, resource_name=resource, metric_name=metric, operator=operator, threshold=threshold, - consecutive_failures_required=consecutive_checks) + consecutive_failures_required=consecutive_checks, + consecutive_ok_required=clear_after_checks) r = await alert_svc.add_rule(db, fields, created_by=caller.user_id, enabled=enabled) await audit_mcp("alert_rule.created", "alert_rule", r.name, {"rule_id": r.id, "source": r.source_module, "resource": r.resource_name, @@ -256,12 +263,15 @@ async def update_alert_rule(rule: str, name: str | None = None, source_module: s resource: str | None = None, metric: str | None = None, operator: str | None = None, threshold: float | None = None, consecutive_checks: int | None = None, + clear_after_checks: int | None = None, enabled: bool | None = None) -> dict: """Change an alert rule. Only the arguments you pass change; the rest stay. rule: its name or id (list_alert_rules). Pass enabled=false to silence it without deleting it. The changed rule is validated as create_alert_rule - validates a new one. Recorded in the audit log. + validates a new one. Changing what it watches (source, resource, metric, + operator, threshold) or disabling it puts its alert back to inactive; a + firing alert records a resolved event saying why. Recorded in the audit log. """ async with db_session() as db, db.begin(): r = await _rule(db, rule) @@ -274,14 +284,14 @@ async def update_alert_rule(rule: str, name: str | None = None, source_module: s operator=r.operator.value if operator is None else operator, threshold=r.threshold if threshold is None else threshold, consecutive_failures_required=(r.consecutive_failures_required - if consecutive_checks is None else consecutive_checks)) - for k, v in fields.items(): - setattr(r, k, v) - if enabled is not None: - r.enabled = bool(enabled) + if consecutive_checks is None else consecutive_checks), + consecutive_ok_required=(r.consecutive_ok_required + if clear_after_checks is None else clear_after_checks)) + await alert_svc.apply_rule_change(db, r, fields, enabled=enabled) changed = sorted(k for k, v in {"name": name, "source_module": source_module, "resource": resource, "metric": metric, "operator": operator, "threshold": threshold, "consecutive_checks": consecutive_checks, + "clear_after_checks": clear_after_checks, "enabled": enabled}.items() if v is not None) await audit_mcp("alert_rule.updated", "alert_rule", r.name, {"rule_id": r.id, "changed": changed}) return _rule_row(r) diff --git a/steward/migrations/versions/0028_alert_resolve_and_reset.py b/steward/migrations/versions/0028_alert_resolve_and_reset.py new file mode 100644 index 0000000..bae8ff9 --- /dev/null +++ b/steward/migrations/versions/0028_alert_resolve_and_reset.py @@ -0,0 +1,31 @@ +"""Alerts resolve after N OK checks; rule changes reset state with a note + +Revision ID: 0028_alert_resolve_and_reset +Revises: 0027_user_auth_source +Create Date: 2026-10-06 +""" +from typing import Sequence, Union +from alembic import op +import sqlalchemy as sa + +revision: str = "0028_alert_resolve_and_reset" +down_revision: Union[str, None] = "0027_user_auth_source" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column("alert_rules", sa.Column("consecutive_ok_required", sa.Integer, + nullable=False, server_default="1")) + op.add_column("alert_states", sa.Column("consecutive_ok_count", sa.Integer, + nullable=False, server_default="0")) + op.alter_column("alert_events", "metric_value", existing_type=sa.Float, nullable=True) + op.add_column("alert_events", sa.Column("note", sa.Text, nullable=True)) + + +def downgrade() -> None: + op.drop_column("alert_events", "note") + op.execute("UPDATE alert_events SET metric_value = 0 WHERE metric_value IS NULL") + op.alter_column("alert_events", "metric_value", existing_type=sa.Float, nullable=False) + op.drop_column("alert_states", "consecutive_ok_count") + op.drop_column("alert_rules", "consecutive_ok_required") diff --git a/steward/models/alerts.py b/steward/models/alerts.py index 3e86b8e..d41111b 100644 --- a/steward/models/alerts.py +++ b/steward/models/alerts.py @@ -41,6 +41,9 @@ class AlertRule(Base): ) threshold: Mapped[float] = mapped_column(Float, nullable=False) consecutive_failures_required: Mapped[int] = mapped_column(Integer, nullable=False, default=1) + # Non-breaching checks in a row before a firing (or acknowledged) alert resolves. + consecutive_ok_required: Mapped[int] = mapped_column(Integer, nullable=False, default=1, + server_default="1") enabled: Mapped[bool] = mapped_column(Boolean, nullable=False, default=True) # Optional admin-only action: run an Ansible playbook on transition-to-firing. # Shape: {source, playbook, inventory, extra_vars, limit, tags, check}. NULL = none. @@ -62,6 +65,8 @@ class AlertState(Base): Enum(AlertStateEnum), nullable=False, default=AlertStateEnum.inactive ) consecutive_failure_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0) + consecutive_ok_count: Mapped[int] = mapped_column(Integer, nullable=False, default=0, + server_default="0") last_transition_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, default=lambda: datetime.now(timezone.utc) ) @@ -98,7 +103,10 @@ class AlertEvent(Base): ) from_state: Mapped[str] = mapped_column(String(32), nullable=False) to_state: Mapped[str] = mapped_column(String(32), nullable=False) - metric_value: Mapped[float] = mapped_column(Float, nullable=False) + # None when the transition came from a change to the rule, not a sample. + metric_value: Mapped[float | None] = mapped_column(Float, nullable=True) + # Why a transition happened when the value alone doesn't say (a rule edit). + note: Mapped[str | None] = mapped_column(Text, nullable=True) transitioned_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), nullable=False, default=lambda: datetime.now(timezone.utc) ) diff --git a/steward/templates/alerts/list.html b/steward/templates/alerts/list.html index f2090b8..a739cac 100644 --- a/steward/templates/alerts/list.html +++ b/steward/templates/alerts/list.html @@ -61,7 +61,7 @@