From e51d969c4302fd3ca0614bbb3d9cc981435daf4b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 30 Oct 2026 19:03:29 -0400 Subject: [PATCH] feat(alerts): a rule can fire when a reading rises, "rose by N" (#6724) A rule compared a reading with a fixed number, so Synology Disk 1's bad sectors going from 3,160 to 3,392 could say nothing: a count that only grows is soon past any line. "rose by" compares each reading with the one before it, kept on the alert's state and only for such a rule. A fall (a replaced disk) doesn't fire. On the rule form, through MCP, and worded in emails, the alert sentence, history and the timeline. Co-Authored-By: Claude Opus 5.5 --- docs/core/alerting.md | 1 + steward/alerts/episodes.py | 2 +- steward/alerts/routes.py | 2 +- steward/alerts/service.py | 6 ++- steward/core/alerts.py | 13 ++++- steward/core/mail.py | 2 + steward/mcp/tools/alerts.py | 8 +++- steward/mcp/tools/hosts.py | 2 +- steward/mcp/tools/overview.py | 2 +- .../versions/0049_alert_on_a_rise.py | 28 +++++++++++ steward/models/alerts.py | 10 ++++ steward/templates/alerts/rules_form.html | 3 +- steward/timeline/record.py | 2 +- tests/core/test_alert_episodes.py | 16 +++++++ tests/integration/test_alert_states.py | 48 +++++++++++++++++++ 15 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 steward/migrations/versions/0049_alert_on_a_rise.py diff --git a/docs/core/alerting.md b/docs/core/alerting.md index 3948cdc..9c71a92 100644 --- a/docs/core/alerting.md +++ b/docs/core/alerting.md @@ -73,6 +73,7 @@ Alert rules are created in the UI at `/alerts/`. Each rule requires: - **Metric name** — the metric key (e.g. `response_time_ms`, `up`, `error_rate_5xx_pct`) - **Operator** — `>`, `<`, `>=`, `<=`, `==`, `!=` - **Threshold** — numeric value +- **rose by** — instead of a line to cross, fires when a reading is at least the threshold above the one before it, for a count that only grows (a disk's bad sectors). A fall doesn't fire. A reading that hasn't risen counts as OK, so the checks to clear set how long the alert stays up, and rises inside that are one alert - **Consecutive failures required** — how many consecutive breaches before FIRING (default 1) The form offers only what has reported, read from `plugin_metric_latest`. diff --git a/steward/alerts/episodes.py b/steward/alerts/episodes.py index 3d63522..a8eb3a2 100644 --- a/steward/alerts/episodes.py +++ b/steward/alerts/episodes.py @@ -15,7 +15,7 @@ QUIET = frozenset({"inactive", "resolved"}) # The worst thing an episode went through, most severe first. _SEVERITY = ("firing", "acknowledged", "no_data", "pending") _WORDS = {">": "above", ">=": "at or above", "<": "below", "<=": "at or below", - "==": "at", "!=": "other than"} + "==": "at", "!=": "other than", "rose": "on a rise of"} def fires_when(rule) -> str: diff --git a/steward/alerts/routes.py b/steward/alerts/routes.py index 9c52ea9..a108e68 100644 --- a/steward/alerts/routes.py +++ b/steward/alerts/routes.py @@ -15,7 +15,7 @@ from steward.models.users import UserRole alerts_bp = Blueprint("alerts", __name__, url_prefix="/alerts") -_OPERATORS = [(op.value, op.value) for op in AlertOperator] +_OPERATORS = [(op.value, op.said) for op in AlertOperator] def _form_condition(form) -> dict: """The operator and threshold the form asks for. An up/down metric's form diff --git a/steward/alerts/service.py b/steward/alerts/service.py index ebad864..995c210 100644 --- a/steward/alerts/service.py +++ b/steward/alerts/service.py @@ -154,7 +154,7 @@ def condition_text(metric: str, operator: AlertOperator | str, threshold: float) if _is_breached(v, operator, threshold)] if len(fires_on) == 1: return f"is {fires_on[0]}" - return f"{metric_label(metric)} {operator.value} {metric_value(metric, threshold)}" + return f"{metric_label(metric)} {operator.said} {metric_value(metric, threshold)}" def alert_sentence(rule: AlertRule, label: str, value: float | None, @@ -175,7 +175,7 @@ def alert_sentence(rule: AlertRule, label: str, value: float | None, # not how long ago someone acknowledged it (the page says that apart). word = "breaching" if label == "acknowledged" else label for_how_long = f", {word} for {held}" if held else f", {word}" - condition = f" ({rule.operator.value} {metric_value(metric, rule.threshold)})" + condition = f" ({rule.operator.said} {metric_value(metric, rule.threshold)})" counted = count_words(metric, value) if counted and rule.resource_name in WHOLE_RESOURCES: # The count is the whole of it, and links where the swarm is shown. @@ -599,6 +599,8 @@ async def rule_fields(db, *, name: str, source_module: str, resource_name: str, threshold = float(threshold) except (TypeError, ValueError): raise ValueError(f"threshold must be a number, got {threshold!r}.") from None + if op == AlertOperator.rose and threshold <= 0: + raise ValueError("A rule on a rise needs how much it rises by, above 0.") try: consecutive = (1 if consecutive_failures_required in (None, "") else int(consecutive_failures_required)) diff --git a/steward/core/alerts.py b/steward/core/alerts.py index 615da3f..d236ef4 100644 --- a/steward/core/alerts.py +++ b/steward/core/alerts.py @@ -506,8 +506,14 @@ def _track_samples(state: AlertState, samples: list[PluginMetric], now: datetime return note -def _is_breached(value: float, operator: AlertOperator, threshold: float) -> bool: +def _is_breached(value: float, operator: AlertOperator, threshold: float, + previous: float | None = None) -> bool: + """Whether a reading breaks a condition. "rose" needs the reading before + it (`previous`): the first has nothing to have risen from, and a fall, + such as a replaced disk's count starting over, is no rise.""" op = operator.value + if op == "rose": + return previous is not None and value - previous >= threshold if op == ">": return value > threshold if op == "<": @@ -581,8 +587,10 @@ async def _evaluate_rule( """ from_state = state.state step = next_step(from_state, state.consecutive_failure_count, state.consecutive_ok_count, - _is_breached(value, rule.operator, rule.threshold), + _is_breached(value, rule.operator, rule.threshold, state.last_value), rule.consecutive_failures_required, rule.consecutive_ok_required) + if rule.operator == AlertOperator.rose: + state.last_value = value state.consecutive_failure_count = step.fail_count state.consecutive_ok_count = step.ok_count notifications: list[tuple[str, str, str | None]] = [] @@ -627,6 +635,7 @@ async def reset_rule_state(session: AsyncSession, rule: AlertRule, note: str, state.last_sample_at = None state.sample_interval_s = None state.no_data_since = None + state.last_value = None state.state = AlertStateEnum.inactive state.consecutive_failure_count = 0 state.consecutive_ok_count = 0 diff --git a/steward/core/mail.py b/steward/core/mail.py index 65a9fed..f9810df 100644 --- a/steward/core/mail.py +++ b/steward/core/mail.py @@ -224,6 +224,8 @@ def alert_facts(alert: dict, tz: tzinfo, show_utc: bool = True) -> dict: else _upper_first(f"{counted}{on} again.") elif counted: headline = _upper_first(f"{counted}{on}.") + elif state == "resolved" and alert.get("operator") == "rose": + headline = f"{metric_label(metric)}{on} has stopped rising" + (f", at {reading}." if reading else ".") elif state == "resolved": headline = f"{metric_label(metric)}{on} is back within its limit" + ( f", at {reading}." if reading else ".") diff --git a/steward/mcp/tools/alerts.py b/steward/mcp/tools/alerts.py index 35f701a..331a910 100644 --- a/steward/mcp/tools/alerts.py +++ b/steward/mcp/tools/alerts.py @@ -23,7 +23,7 @@ def _condition(rule: AlertRule) -> str: ok = rule.consecutive_ok_required or 1 tail = f" for {n} consecutive checks" if n > 1 else "" clear = f"; clears after {ok} OK checks" if ok > 1 else "" - return f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}{tail}{clear}" + return f"{rule.metric_name} {rule.operator.said} {rule.threshold:g}{tail}{clear}" def _action(rule: AlertRule) -> dict | None: @@ -288,7 +288,11 @@ async def create_alert_rule(name: str, source_module: str, resource: str, metric `resource` as "". Steward then keeps one rule per host, named ": ", and adds one within a minute for a host that starts reporting or joins the group. Only for host_agent. - operator: ">", "<", ">=", "<=", "==" or "!=". + operator: ">", "<", ">=", "<=", "==" or "!=", or "rose" to fire when a + reading is at least `threshold` above the one before it (for a count + that only grows). A falling reading doesn't fire, and the next reading + that hasn't risen counts as OK: clear_after_checks sets how long the + alert stays up, so repeats inside it are one alert. 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 diff --git a/steward/mcp/tools/hosts.py b/steward/mcp/tools/hosts.py index 7b04701..1098219 100644 --- a/steward/mcp/tools/hosts.py +++ b/steward/mcp/tools/hosts.py @@ -190,7 +190,7 @@ async def get_host(host: str, parts_limit: int = 20) -> dict: { "rule_id": rule.id, "rule_name": rule.name, "state": alert_svc.alert_label(state), "resource": rule.resource_name, "metric": rule.metric_name, - "condition": f"{rule.metric_name} {rule.operator.value} {rule.threshold:g}", + "condition": f"{rule.metric_name} {rule.operator.said} {rule.threshold:g}", "since": iso(state.last_transition_at), "age_s": age_s(state.last_transition_at), } for state, rule in active diff --git a/steward/mcp/tools/overview.py b/steward/mcp/tools/overview.py index acb1443..8016e78 100644 --- a/steward/mcp/tools/overview.py +++ b/steward/mcp/tools/overview.py @@ -127,7 +127,7 @@ def alert_findings(active, silent: dict[str, list[str]] | None = None) -> list[d what += _also_silent(silent.get(rule.resource_name)) out.append(finding( sev, f"alert_{s}", rule.name, - f"{rule.metric_name} {rule.operator.value} {rule.threshold:g} on " + f"{rule.metric_name} {rule.operator.said} {rule.threshold:g} on " f"{rule.source_module}/{rule.resource_name} {what}", since=state.no_data_since if s == "no_data" else state.last_transition_at, ids={"rule_id": rule.id}, diff --git a/steward/migrations/versions/0049_alert_on_a_rise.py b/steward/migrations/versions/0049_alert_on_a_rise.py new file mode 100644 index 0000000..97c460f --- /dev/null +++ b/steward/migrations/versions/0049_alert_on_a_rise.py @@ -0,0 +1,28 @@ +"""Alert rules: a condition that fires on a rise, and the reading before this one. + +Revision ID: 0049_alert_on_a_rise +Revises: 0048_tickets +Create Date: 2026-10-30 +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "0049_alert_on_a_rise" +down_revision: Union[str, None] = "0048_tickets" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # ALTER TYPE ... ADD VALUE cannot run inside a transaction block. + with op.get_context().autocommit_block(): + op.execute("ALTER TYPE alertoperator ADD VALUE IF NOT EXISTS 'rose'") + op.add_column("alert_states", sa.Column("last_value", sa.Float, nullable=True)) + + +def downgrade() -> None: + # Postgres can't drop an enum value; 'rose' stays in the type, unused. + op.execute("DELETE FROM alert_rules WHERE operator = 'rose'") + op.drop_column("alert_states", "last_value") diff --git a/steward/models/alerts.py b/steward/models/alerts.py index 0b4e0ce..38a7d52 100644 --- a/steward/models/alerts.py +++ b/steward/models/alerts.py @@ -14,6 +14,14 @@ class AlertOperator(str, enum.Enum): lte = "<=" eq = "==" ne = "!=" + # The reading against the one before it, not against a fixed line: for a + # count that only grows, where any fixed line is soon behind it. + rose = "rose" + + @property + def said(self) -> str: + """The operator as a condition reads: "> 90", "rose by 1".""" + return "rose by" if self is AlertOperator.rose else self.value class AlertStateEnum(str, enum.Enum): @@ -96,6 +104,8 @@ class AlertState(Base): last_sample_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) sample_interval_s: Mapped[float | None] = mapped_column(Float, nullable=True) no_data_since: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True) + # The reading before this one, kept only for a rule that fires on a rise. + last_value: Mapped[float | None] = mapped_column(Float, nullable=True) class MaintenanceWindow(Base): diff --git a/steward/templates/alerts/rules_form.html b/steward/templates/alerts/rules_form.html index 104bdf2..b0b0839 100644 --- a/steward/templates/alerts/rules_form.html +++ b/steward/templates/alerts/rules_form.html @@ -102,12 +102,13 @@
- + +

"rose by" compares each reading with the one before it, for a count that only grows. A reading that hasn't risen counts as OK, so "Good checks in a row before it clears" sets how long the alert stays up.

diff --git a/steward/timeline/record.py b/steward/timeline/record.py index 5c65028..a2de9f7 100644 --- a/steward/timeline/record.py +++ b/steward/timeline/record.py @@ -112,7 +112,7 @@ def alert_summary(type_: str, rule, value: float | None) -> str: if metric in _BINARY: reading = breach = f"{res} is {metric_value(metric, value)}" else: - op = getattr(rule.operator, "value", rule.operator) + op = getattr(rule.operator, "said", rule.operator) reading = f"{said} on {res} is {metric_value(metric, value)}" breach = f"{reading} ({op} {metric_value(metric, rule.threshold)})" return { diff --git a/tests/core/test_alert_episodes.py b/tests/core/test_alert_episodes.py index 1d08632..5d42b69 100644 --- a/tests/core/test_alert_episodes.py +++ b/tests/core/test_alert_episodes.py @@ -100,3 +100,19 @@ def test_the_verdict_counts_what_fired_and_for_how_long(): def test_an_up_or_down_rule_says_the_word(): assert fires_when(_rule(metric="is_up", op=AlertOperator.lt, threshold=0.5)) == "fires when down" + + +def test_a_rule_on_a_rise_reads_as_one(): + rule = _rule(metric="disk_bad_sectors", op=AlertOperator.rose, threshold=1.0) + assert fires_when(rule).startswith("fires on a rise of ") + assert AlertOperator.rose.said == "rose by" and AlertOperator.gt.said == ">" + + +def test_a_rise_is_against_the_reading_before_it(): + from steward.core.alerts import _is_breached + rose = AlertOperator.rose + assert _is_breached(3392.0, rose, 1.0, previous=3160.0) + assert not _is_breached(3392.0, rose, 1.0, previous=3392.0) # flat + assert not _is_breached(0.0, rose, 1.0, previous=3392.0) # a replaced disk starts over + assert not _is_breached(3392.0, rose, 1.0) # nothing to have risen from + assert not _is_breached(3161.0, rose, 5.0, previous=3160.0) # less than asked diff --git a/tests/integration/test_alert_states.py b/tests/integration/test_alert_states.py index 4f032b7..614bc28 100644 --- a/tests/integration/test_alert_states.py +++ b/tests/integration/test_alert_states.py @@ -230,3 +230,51 @@ def test_a_rule_without_state_gets_one_and_is_watched_again(app): assert first == (1, 0) # the covering rule has no state of its own assert second == 1 # a second pass adds nothing assert state == AlertStateEnum.firing + + +def test_a_rule_on_a_rise_fires_on_the_rise_and_clears_after_its_ok_run(app): + from sqlalchemy import select + from steward.alerts import service as alert_svc + from steward.core.alerts import record_metric, record_metrics + from steward.models.alerts import AlertState, AlertStateEnum + from steward.models.metrics import PluginMetric + from steward.models.users import User, UserRole + + async def _go(): + tag = uuid.uuid4().hex[:8] + res = f"box-{tag}" + async with app.db_sessionmaker() as s: + async with s.begin(): + user = User(username=f"rs-{tag}", email=f"rs-{tag}@test", password_hash="x", + role=UserRole.admin) + s.add(user) + await record_metrics(s, [PluginMetric(source_module="host_agent", resource_name=res, + metric_name="cpu_pct", value=100.0)]) + await s.flush() + with pytest.raises(ValueError, match="above 0"): + await alert_svc.rule_fields( + s, name=f"flat {tag}", source_module="host_agent", resource_name=res, + metric_name="cpu_pct", operator="rose", threshold=0) + fields = await alert_svc.rule_fields( + s, name=f"rise {tag}", source_module="host_agent", resource_name=res, + metric_name="cpu_pct", operator="rose", threshold=1, + consecutive_failures_required=1, consecutive_ok_required=2) + rule = await alert_svc.add_rule(s, fields, created_by=user.id) + rule_id = rule.id + + async def after(v): + async with app.db_sessionmaker() as s: + async with s.begin(): + await record_metric(s, "host_agent", res, "cpu_pct", v) + async with app.db_sessionmaker() as s: + state = (await s.execute(select(AlertState).where(AlertState.rule_id == rule_id))).scalar_one() + return state.state + + assert await after(100.0) == AlertStateEnum.inactive # the first has nothing to have risen from + assert await after(100.0) == AlertStateEnum.inactive # flat + assert await after(105.0) == AlertStateEnum.firing + assert await after(105.0) == AlertStateEnum.firing # one good check of two + assert await after(105.0) == AlertStateEnum.inactive + assert await after(40.0) == AlertStateEnum.inactive # a fall is no rise + + asyncio.run(_go())