From 4fbe93bdf765f717b6a33c531206aaecc68b2098 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 18:12:50 -0400 Subject: [PATCH] feat(alerts): a sentence per alert, a link to what it watches, a count in the sidebar (#5124) The Alerts page's active list now says each alert in one line around the resource it watches, e.g. "cpu_pct on web-1 is 94% (> 90%), firing for 12m", with web-1 linking to its host, monitor or container page (or the plugin's page for Traefik, UniFi and SNMP). Values take their unit from the metric name through formatting.metric_value. A silent series reads "No cpu_pct from web-1 for 20m"; an up/down metric says the word. Actions follow the routes' roles: acknowledge, edit, toggle and New rule for operators and admins, Delete for admins only, nothing for viewers. The sidebar shows how many alerts need attention beside Alerts, from an async context processor that skips htmx fragments, logged-out pages and the database-down page, and renders without the count if the query fails. Co-Authored-By: Claude Opus 5.5 --- steward/alerts/routes.py | 2 +- steward/alerts/service.py | 95 ++++++++++++++++++++ steward/app.py | 20 ++++- steward/core/formatting.py | 27 ++++++ steward/static/steward.css | 6 ++ steward/templates/alerts/list.html | 38 ++++---- steward/templates/base.html | 2 +- tests/core/test_alerts_page.py | 134 +++++++++++++++++++++++++++++ 8 files changed, 304 insertions(+), 20 deletions(-) create mode 100644 tests/core/test_alerts_page.py diff --git a/steward/alerts/routes.py b/steward/alerts/routes.py index 9474601..178c8fb 100644 --- a/steward/alerts/routes.py +++ b/steward/alerts/routes.py @@ -96,7 +96,7 @@ def _parse_ansible_action(form) -> tuple[dict | None, str | None]: @require_role(UserRole.viewer) async def list_alerts(): async with current_app.db_sessionmaker() as db: - active = await alert_svc.active_alerts(db) + active = await alert_svc.active_rows(db, datetime.now(timezone.utc)) rules = await alert_svc.list_rules(db) return await render_template("alerts/list.html", active=active, rules=rules, operators=_OPERATORS) diff --git a/steward/alerts/service.py b/steward/alerts/service.py index b391756..0c1ae5c 100644 --- a/steward/alerts/service.py +++ b/steward/alerts/service.py @@ -85,6 +85,101 @@ def alert_label(state: AlertState) -> str: return state.state.value +async def latest_value(db, rule: AlertRule) -> float | None: + """The newest sample of the series a rule watches, or None before the first.""" + return (await db.execute( + select(PluginMetric.value) + .where(PluginMetric.source_module == rule.source_module, + PluginMetric.resource_name == rule.resource_name, + PluginMetric.metric_name == rule.metric_name) + .order_by(PluginMetric.recorded_at.desc()).limit(1) + )).scalar_one_or_none() + + +# Sources whose resources have no page of their own: the plugin's page lists them. +PLUGIN_PAGES = {"traefik": "/plugins/traefik/", "unifi": "/plugins/unifi/", + "snmp": "/plugins/snmp/"} + + +async def resource_url(db, rule: AlertRule) -> str | None: + """Where the thing a rule watches is shown, or None when nothing shows it. + + host_agent resources are the host's name, or ":"; + docker's are "/"; a monitor's is its name under its type. + """ + from steward.models.hosts import Host + source, resource = rule.source_module, rule.resource_name + if source == "host_agent": + name = resource.split(":", 1)[0] + host_id = (await db.execute( + select(Host.id).where(Host.name == name).limit(1))).scalar_one_or_none() + return f"/hosts/{host_id}" if host_id else None + if source in MONITOR_TYPE_VALUES: + monitor_id = (await db.execute( + select(Monitor.id).where(Monitor.type == source, Monitor.name == resource).limit(1) + )).scalar_one_or_none() + return f"/monitors/{monitor_id}" if monitor_id else None + if source == "docker" and "/" in resource: + host_name, container = resource.split("/", 1) + host_id = (await db.execute( + select(Host.id).where(Host.name == host_name).limit(1))).scalar_one_or_none() + return f"/plugins/docker/container/{host_id}/{container}" if host_id else None + return PLUGIN_PAGES.get(source) + + +# Metrics whose value is a word: the sentence says the word, not the condition. +_BINARY = ("is_up", "is_healthy") + + +def alert_sentence(rule: AlertRule, label: str, value: float | None, + since: datetime | None, now: datetime) -> dict: + """An alert as one sentence around its resource: {"lead", "resource", + "tail"}, read lead + resource + tail, e.g. "cpu_pct on " + "web-1" + + " is 94% (> 90%), firing for 12m". label is alert_label(state); since is + when that label began (no_data_since for no_data).""" + from steward.core.formatting import human_duration, metric_value + metric = rule.metric_name + held = human_duration((now - since).total_seconds()) if since else None + if label == "no_data": + return {"lead": f"No {metric} from ", "resource": rule.resource_name, + "tail": f" for {held}" if held else ""} + for_how_long = f", {label} for {held}" if held else f", {label}" + if value is None: + reading = " has no reading yet" + elif metric in _BINARY: + return {"lead": "", "resource": rule.resource_name, + "tail": f" is {metric_value(metric, value)}{for_how_long}"} + else: + reading = f" is {metric_value(metric, value)}" + condition = f" ({rule.operator.value} {metric_value(metric, rule.threshold)})" + return {"lead": f"{metric} on ", "resource": rule.resource_name, + "tail": f"{reading}{condition}{for_how_long}"} + + +async def active_rows(db, now: datetime) -> list[dict]: + """The alerts page's active list: each alert's state, rule, sentence and + the link to what it watches.""" + rows = [] + for state, rule in await active_alerts(db): + label = alert_label(state) + since = state.no_data_since if label == "no_data" else state.last_transition_at + rows.append({ + "state": state, "rule": rule, "label": label, + "says": alert_sentence(rule, label, await latest_value(db, rule), since, now), + "url": await resource_url(db, rule), + }) + return rows + + +async def active_count(db) -> int: + """How many alerts need attention: the sidebar's number.""" + from sqlalchemy import func + return (await db.execute( + select(func.count()).select_from(AlertState) + .where(or_(AlertState.state.in_(ACTIVE_STATES), AlertState.no_data_since.is_not(None))) + )).scalar_one() + + async def list_rules(db) -> list[AlertRule]: return list((await db.execute(select(AlertRule).order_by(AlertRule.name))).scalars()) diff --git a/steward/app.py b/steward/app.py index 2c5baf1..f9c7789 100644 --- a/steward/app.py +++ b/steward/app.py @@ -3,7 +3,7 @@ from __future__ import annotations import asyncio import logging from pathlib import Path -from quart import Quart, render_template, request +from quart import Quart, g, render_template, request from .config import load_bootstrap from .database import ( init_db, ensure_database_reachable, DatabaseUnavailable, @@ -269,6 +269,23 @@ def create_app( "undecryptable_secrets": get_undecryptable_secrets(), } + @app.context_processor + async def _inject_alert_count(): + """The sidebar's count of alerts needing attention. Full pages only: + an htmx fragment has no sidebar, and a logged-out page has no nav.""" + from quart import session + if (request.headers.get("HX-Request") or not session.get("user_id") + or g.get("database_down")): + return {} + try: + from .alerts.service import active_count + async with app.db_sessionmaker() as db: + return {"active_alert_count": await active_count(db)} + except Exception: + # A count is decoration; the page renders without it. + logging.getLogger(__name__).exception("Active alert count failed") + return {} + # ── 11. Share-token middleware ───────────────────────────────────────────── @app.before_request async def _validate_share_token(): @@ -312,6 +329,7 @@ def create_app( except DatabaseUnavailable as exc: logger.error("Database unreachable while serving %s: %s", request.path, exc) + g.database_down = True return await render_template( "errors/database_unavailable.html"), 503 return None diff --git a/steward/core/formatting.py b/steward/core/formatting.py index 80c638d..867bac2 100644 --- a/steward/core/formatting.py +++ b/steward/core/formatting.py @@ -149,3 +149,30 @@ def worst_reading(readings, thresholds: dict): if _SEVERITY.get(grade, 0) > rank: worst, rank = (label, value, grade), _SEVERITY[grade] return worst + + +def metric_value(name: str, value: Any) -> str: + """A metric reading in its unit, from the name's suffix: _pct → 94%, + _ms → 120 ms, _bytes → 1.2 GiB, _bps → its rate, _secs / seconds_ → a + span, _c → 71°C, _days → 12 days; is_up / is_healthy read as yes or no. + None (no reading) is "—".""" + if value is None: + return "—" + n = name or "" + if n in ("is_up", "is_healthy"): + return {"is_up": ("up", "down"), "is_healthy": ("healthy", "unhealthy")}[n][0 if value >= 0.5 else 1] + if n.endswith("_pct") or "_pct_" in n: + return f"{value:.1f}%" if abs(value) < 10 else f"{value:.0f}%" + if n.endswith("_ms"): + return f"{value:.0f} ms" + if n.endswith("_bytes"): + return human_bytes(value) + if n.endswith(("_bps", "bytes_rate")): + return human_rate(value) + if n.endswith("_secs") or n.startswith("seconds_"): + return human_duration(value) + if n.endswith("_c"): + return f"{value:.0f}°C" + if n.endswith("_days"): + return f"{value:.0f} days" + return f"{value:g}" diff --git a/steward/static/steward.css b/steward/static/steward.css index 5e21127..ad37bb1 100644 --- a/steward/static/steward.css +++ b/steward/static/steward.css @@ -86,6 +86,12 @@ code { font-family: var(--fs-font-mono); font-size: 0.875em; background: var(--b color: var(--gold); border-left-color: var(--gold); background: color-mix(in srgb, var(--gold) 8%, transparent); } +/* How many alerts need attention, beside the Alerts link. */ +.nav-count { + margin-left: auto; padding: 0 0.45em; border-radius: var(--fs-radius-sm); + background: var(--red-dim); color: var(--red); font-size: 0.75rem; font-weight: 600; + font-variant-numeric: tabular-nums; +} .side-user { border-top: 1px solid var(--border); padding: 0.7rem 1.25rem; } .side-user form { margin: 0; } .side-user button { diff --git a/steward/templates/alerts/list.html b/steward/templates/alerts/list.html index f02b0a5..20f6227 100644 --- a/steward/templates/alerts/list.html +++ b/steward/templates/alerts/list.html @@ -2,10 +2,11 @@ {% block title %}Alerts — Steward{% endblock %} {% from "_macros.html" import page_header, status_badge, empty_state %} {% block content %} -{% call page_header("Alerts") %} +{% set can_edit = session.user_role in ('operator', 'admin') %} +{% call page_header("Alerts", "What is breaching now, in a sentence each, then every rule.") %} History Maintenance - New rule + {% if can_edit %}New rule{% endif %} {% endcall %} {% if active %} @@ -15,30 +16,29 @@ State + What is wrong Rule - Resource - Metric - {% for state, rule in active %} + {% for row in active %} - {{ status_badge(state.state) }} - {% if state.no_data_since %} - {{ status_badge('no_data', title="No sample since " ~ (state.last_sample_at.strftime('%Y-%m-%d %H:%M UTC') if state.last_sample_at else 'the rule started watching')) }} -
- since {{ state.no_data_since|ago }} -
+ {{ status_badge(row.label, title=("No sample since " ~ (row.state.last_sample_at.strftime('%Y-%m-%d %H:%M UTC') if row.state.last_sample_at else 'the rule started watching')) if row.label == 'no_data' else none) }} + {% if row.state.no_data_since and row.label != 'no_data' %} + {{ status_badge('no_data', title="Silent since " ~ row.state.no_data_since.strftime('%Y-%m-%d %H:%M UTC')) }} {% endif %} - {{ rule.name }} - {{ rule.resource_name }} - {{ rule.metric_name }} {{ rule.operator.value }} {{ rule.threshold }} + + {{- row.says.lead -}} + {%- if row.url %}{{ row.says.resource }}{% else %}{{ row.says.resource }}{% endif -%} + {{- row.says.tail -}} + + {{ row.rule.name }} - {% if state.state.value == 'firing' %} -
{{ csrf_field() }} + {% if can_edit and row.state.state.value == 'firing' %} + {{ csrf_field() }}
{% endif %} @@ -82,16 +82,20 @@ {{ rule.consecutive_failures_required }} / {{ rule.consecutive_ok_required }} History + {% if can_edit %} Edit
{{ csrf_field() }}
+ {% endif %} + {% if session.user_role == 'admin' %}
{{ csrf_field() }}
+ {% endif %} {% endfor %} @@ -99,6 +103,6 @@ {% else %} -
{{ empty_state("No alert rules yet.", "New rule", "/alerts/rules/new") }}
+
{{ empty_state("No alert rules yet.", "New rule" if can_edit else none, "/alerts/rules/new" if can_edit else none) }}
{% endif %} {% endblock %} diff --git a/steward/templates/base.html b/steward/templates/base.html index c655012..275fe3f 100644 --- a/steward/templates/base.html +++ b/steward/templates/base.html @@ -62,7 +62,7 @@
Monitoring
Monitors - Alerts + Alerts{% if active_alert_count %} {{ active_alert_count }}{% endif %}
Automation
diff --git a/tests/core/test_alerts_page.py b/tests/core/test_alerts_page.py new file mode 100644 index 0000000..f3a256c --- /dev/null +++ b/tests/core/test_alerts_page.py @@ -0,0 +1,134 @@ +"""The Alerts page says each alert as a sentence around a link to what it +watches, offers only the actions the viewer's role can take, and the sidebar +counts what needs attention (#5124).""" +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace + +import pytest +from quart import render_template, session + +from steward.alerts import service as alert_svc +from steward.alerts.service import alert_sentence +from steward.models.alerts import AlertOperator, AlertStateEnum +from tests.test_style_tokens import hex_colours + + +def _rule(metric="cpu_pct", op=AlertOperator.gt, threshold=90.0, resource="web-1", rid="r1"): + return SimpleNamespace(id=rid, name=f"{resource} {metric}", source_module="host_agent", + resource_name=resource, metric_name=metric, operator=op, + threshold=threshold, enabled=True, + consecutive_failures_required=1, consecutive_ok_required=1) + + +def _said(says): + return says["lead"] + says["resource"] + says["tail"] + + +@pytest.mark.parametrize("op, threshold, value, reads", [ + (AlertOperator.gt, 90.0, 94.0, "cpu_pct on web-1 is 94% (> 90%), firing for 12m"), + (AlertOperator.gte, 90.0, 90.0, "cpu_pct on web-1 is 90% (>= 90%), firing for 12m"), + (AlertOperator.lt, 5.0, 2.5, "cpu_pct on web-1 is 2.5% (< 5.0%), firing for 12m"), + (AlertOperator.lte, 5.0, 5.0, "cpu_pct on web-1 is 5.0% (<= 5.0%), firing for 12m"), + (AlertOperator.eq, 100.0, 100.0, "cpu_pct on web-1 is 100% (== 100%), firing for 12m"), + (AlertOperator.ne, 0.0, 3.0, "cpu_pct on web-1 is 3.0% (!= 0.0%), firing for 12m"), +]) +def test_a_firing_alert_reads_as_one_sentence(op, threshold, value, reads): + now = datetime.now(timezone.utc) + says = alert_sentence(_rule(op=op, threshold=threshold), "firing", value, + now - timedelta(minutes=12), now) + assert says["resource"] == "web-1" + assert _said(says) == reads + + +def test_units_follow_the_metric_name(): + now = datetime.now(timezone.utc) + says = alert_sentence(_rule("response_ms", threshold=200.0, resource="api"), "pending", 431.0, + now - timedelta(seconds=45), now) + assert _said(says) == "response_ms on api is 431 ms (> 200 ms), pending for 45s" + + +def test_an_up_or_down_metric_says_the_word(): + now = datetime.now(timezone.utc) + says = alert_sentence(_rule("is_up", AlertOperator.lt, 1.0, "nas"), "acknowledged", 0.0, + now - timedelta(hours=2, minutes=5), now) + assert _said(says) == "nas is down, acknowledged for 2h 5m" + + +def test_no_reading_yet_is_said_not_shown_as_a_number(): + now = datetime.now(timezone.utc) + says = alert_sentence(_rule(), "firing", None, now - timedelta(minutes=3), now) + assert _said(says) == "cpu_pct on web-1 has no reading yet (> 90%), firing for 3m" + + +def test_a_silent_series_says_how_long_it_has_been_quiet(): + now = datetime.now(timezone.utc) + says = alert_sentence(_rule(), "no_data", 50.0, now - timedelta(minutes=20), now) + assert _said(says) == "No cpu_pct from web-1 for 20m" + + +def _row(rule, label, url, state=AlertStateEnum.firing): + now = datetime.now(timezone.utc) + st = SimpleNamespace(state=state, no_data_since=None, last_sample_at=None, + last_transition_at=now - timedelta(minutes=12)) + return {"state": st, "rule": rule, "label": label, "url": url, + "says": alert_sentence(rule, label, 94.0, st.last_transition_at, now)} + + +async def test_an_operator_sees_the_sentence_linked_and_can_acknowledge(app): + rule = _rule() + async with app.test_request_context("/alerts/"): + session["user_role"] = "operator" + page = await render_template("alerts/list.html", active=[_row(rule, "firing", "/hosts/h1")], + rules=[rule], operators=[]) + + assert 'cpu_pct on web-1 is 94% (> 90%), firing for 12m' in page + assert 'Firing' in page + assert 'action="/alerts/r1/acknowledge"' in page + assert 'href="/alerts/rules/new"' in page and 'href="/alerts/rules/r1/edit"' in page + assert 'action="/alerts/rules/r1/delete"' not in page # deleting a rule is the admin's + main = page[page.index("web-1" in page # nothing to link to: no bare href + for action in ("/acknowledge", "/alerts/rules/new", "/edit", "/toggle", "/delete"): + assert action not in page, action + assert 'href="/alerts/history?rule=r1"' in page + + +async def test_an_admin_can_delete_a_rule(app): + rule = _rule() + async with app.test_request_context("/alerts/"): + session["user_role"] = "admin" + page = await render_template("alerts/list.html", active=[], rules=[rule], operators=[]) + assert 'action="/alerts/rules/r1/delete"' in page + assert 'data-confirm="Delete rule web-1 cpu_pct?"' in page + + +async def _active_count(_db): + return 3 + + +async def test_the_sidebar_counts_alerts_needing_attention(app, monkeypatch): + monkeypatch.setattr(alert_svc, "active_count", _active_count) + async with app.test_request_context("/alerts/"): + session["user_id"], session["user_role"] = "u1", "viewer" + page = await render_template("alerts/list.html", active=[], rules=[], operators=[]) + assert '3' in page + + +async def test_no_count_for_a_fragment_or_a_logged_out_page(app, monkeypatch): + monkeypatch.setattr(alert_svc, "active_count", _active_count) + async with app.test_request_context("/alerts/", headers={"HX-Request": "true"}): + session["user_id"], session["user_role"] = "u1", "viewer" + fragment = await render_template("alerts/list.html", active=[], rules=[], operators=[]) + async with app.test_request_context("/alerts/"): + logged_out = await render_template("alerts/list.html", active=[], rules=[], operators=[]) + assert "nav-count" not in fragment and "nav-count" not in logged_out