From fe4e0f2b718a455fe7b39553adc2e7e9cee5946d Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 2 Sep 2026 17:17:22 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20/api/system/health=20=E2=80=94=20one=20?= =?UTF-8?q?verdict=20for=20the=20whole=20stack=20(milestone=20365=20step?= =?UTF-8?q?=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The single endpoint the nav indicator and the System page will both read. Composing a verdict is this module's job, not the UI's. Two kinds of part, answered differently. LEARNED — celery roles and the GPU agent, out of service_seen, where the question is "how long since it checked in" and the answer can be "it has not". PROBED — Postgres and Redis, always expected, never learned, because a last-seen for them would be actively misleading: that Redis answered thirty seconds ago says nothing about now. **The endpoint must never fail because something it checks has failed.** That inversion is easy to write by accident and it destroys the feature exactly when it is needed — a 500 when Redis is down instead of `redis: down`. Every probe is wrapped, every wait carries a deadline (rule 156), and the roster refresh swallows its own errors. The worst case is a part reported `unknown`, which is a true statement about the system. Postgres is probed first and gates the rest, because if it is unreachable nothing else can be read — and "the database is down" is the most useful single thing this can ever say. The staleness thresholds are the design risk, not the code, and they are deliberately generous: 90s to doubt, 300s to disbelieve. The constraint is a deploy rather than a crash — `docker compose up -d` rolls start-first, so a role is briefly served by two containers and then by neither while the old one drains. Thresholds tight enough to catch a crash in seconds would paint the page red on every update, and an alarm that cries wolf on every deploy is one nobody reads. Tune down only after watching a real deploy pass through. The numbers ship in the response so the UI can explain a `stale` without keeping a second copy of them. States are described in sentences rather than left as chips: "Scheduler has not checked in for 6 min — treat it as stopped" is what someone needs at the moment they are deciding whether to go and open Portainer. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01TTjbZZ6JirCMSaJzQV1RhA --- backend/app/api/__init__.py | 2 + backend/app/api/system_health.py | 192 +++++++++++++++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 backend/app/api/system_health.py diff --git a/backend/app/api/__init__.py b/backend/app/api/__init__.py index f500acd..075e15d 100644 --- a/backend/app/api/__init__.py +++ b/backend/app/api/__init__.py @@ -38,6 +38,7 @@ def all_blueprints() -> list[Blueprint]: from .suggestions import suggestions_bp from .system_activity import system_activity_bp from .system_backup import system_backup_bp + from .system_health import system_health_bp from .tags import tags_bp from .thumbnails import thumbnails_bp return [ @@ -51,6 +52,7 @@ def all_blueprints() -> list[Blueprint]: showcase_bp, settings_bp, system_activity_bp, + system_health_bp, system_backup_bp, admin_bp, cleanup_bp, diff --git a/backend/app/api/system_health.py b/backend/app/api/system_health.py new file mode 100644 index 0000000..e767ede --- /dev/null +++ b/backend/app/api/system_health.py @@ -0,0 +1,192 @@ +"""Is every part of FabledCurator running? One verdict, one endpoint. + +Milestone 365. The nav indicator and the System page both read this and +nothing else — composing a verdict is this module's job, not the UI's. + +## Two kinds of part, answered two different ways + +**Learned** — celery roles and the GPU agent, from `service_seen`. The +question is "how long since it checked in", and these are the parts that can +be ABSENT, which is the whole point: `celery inspect` alone reports presence, +so a dead worker is a shorter list rather than a red light. + +**Probed live** — Postgres and Redis. Always expected, never learned, and a +last-seen for them would be actively misleading: that Redis answered thirty +seconds ago says nothing about now. + +## This endpoint must never fail because something it checks has failed + +The inversion is easy to write by accident and it destroys the feature exactly +when it is needed — a 500 when Redis is down, instead of `redis: down`. Every +probe is wrapped, every wait has a deadline (rule 156), and the roster refresh +swallows its own errors. The worst case is a part reported `unknown`, which is +a true statement. +""" + +from __future__ import annotations + +import asyncio +import logging +import time +from datetime import UTC, datetime + +from quart import Blueprint, jsonify +from sqlalchemy import select, text + +from ..config import get_config +from ..extensions import get_session +from ..models import ServiceSeen +from ..services.service_roster import refresh_if_stale + +log = logging.getLogger(__name__) + +system_health_bp = Blueprint("system_health", __name__, url_prefix="/api/system") + +# How long a learned part may go quiet before it is doubted, then disbelieved. +# +# These are deliberately generous, and the reason is a deploy rather than a +# worker: `docker compose up -d` rolls start-first, so a role is briefly served +# by two containers and then by neither while the old one drains. Thresholds +# tight enough to catch a crash in seconds would paint the page red every time +# the stack is updated, and an alarm that cries wolf on every deploy is one +# nobody reads. Tune down only after watching a real deploy pass through. +STALE_AFTER_SECONDS = 90 +DOWN_AFTER_SECONDS = 300 + +# Probes cross a process boundary, so they carry deadlines. A hung Postgres +# must make this endpoint say "postgres: down", not hang alongside it. +PROBE_TIMEOUT_SECONDS = 2.0 + +_OK, _STALE, _DOWN, _UNKNOWN = "ok", "stale", "down", "unknown" + +# Worst-first, so an overall verdict is just the max. +_SEVERITY = {_OK: 0, _UNKNOWN: 1, _STALE: 2, _DOWN: 3} + + +def _age_state(age_seconds: float) -> str: + if age_seconds >= DOWN_AFTER_SECONDS: + return _DOWN + if age_seconds >= STALE_AFTER_SECONDS: + return _STALE + return _OK + + +def _describe_learned(name: str, state: str, age: float, details: dict) -> str: + """Say what the state MEANS. A red chip tells an operator less than a + sentence does at the moment they are deciding whether to go and look.""" + if state == _OK: + replicas = details.get("replicas") + if replicas and replicas > 1: + return f"{name} is running ({replicas} replicas)" + return f"{name} is running" + mins = int(age // 60) + ago = f"{mins} min" if mins else f"{int(age)}s" + if state == _STALE: + return f"{name} has not checked in for {ago}" + return f"{name} has not checked in for {ago} — treat it as stopped" + + +async def _probe_postgres(session) -> dict: + started = time.monotonic() + try: + await asyncio.wait_for( + session.execute(text("SELECT 1")), timeout=PROBE_TIMEOUT_SECONDS + ) + except Exception as exc: # noqa: BLE001 — a probe reports, it never raises + return { + "key": "postgres", "kind": "datastore", "name": "PostgreSQL", + "state": _DOWN, "detail": f"not answering: {type(exc).__name__}", + } + return { + "key": "postgres", "kind": "datastore", "name": "PostgreSQL", "state": _OK, + "detail": "answering", "latency_ms": round((time.monotonic() - started) * 1000, 1), + } + + +def _ping_redis_sync() -> None: + import redis # local import; mirrors system_activity's pattern + + client = redis.Redis.from_url( + get_config().celery_broker_url, + socket_connect_timeout=PROBE_TIMEOUT_SECONDS, + socket_timeout=PROBE_TIMEOUT_SECONDS, + ) + client.ping() + + +async def _probe_redis() -> dict: + started = time.monotonic() + try: + await asyncio.wait_for( + asyncio.to_thread(_ping_redis_sync), timeout=PROBE_TIMEOUT_SECONDS * 2 + ) + except Exception as exc: # noqa: BLE001 + return { + "key": "redis", "kind": "datastore", "name": "Redis", + "state": _DOWN, + "detail": f"not answering: {type(exc).__name__} — queues and workers " + f"cannot be reached either", + } + return { + "key": "redis", "kind": "datastore", "name": "Redis", "state": _OK, + "detail": "answering", "latency_ms": round((time.monotonic() - started) * 1000, 1), + } + + +@system_health_bp.route("/health", methods=["GET"]) +async def system_health(): + """Every part, its state, and one overall verdict. + + Response: {overall, parts: [{key, kind, name, state, detail, last_seen_at, + …}], checked_at} + """ + parts: list[dict] = [] + now = datetime.now(UTC) + + async with get_session() as session: + # Postgres first, and if it is unreachable nothing else can be read — + # say so rather than failing, because "the database is down" is the + # single most useful thing this endpoint can ever report. + pg = await _probe_postgres(session) + parts.append(pg) + + if pg["state"] == _OK: + # Rate-limited inside; see service_roster on why the web process + # is the right observer. + try: + await refresh_if_stale(session) + await session.commit() + except Exception: # noqa: BLE001 + log.warning("system health: roster refresh failed", exc_info=True) + + rows = ( + await session.execute(select(ServiceSeen).order_by(ServiceSeen.display_name)) + ).scalars().all() + for row in rows: + age = (now - row.last_seen_at).total_seconds() + state = _age_state(age) + parts.append({ + "key": row.key, + "kind": row.kind, + "name": row.display_name, + "state": state, + "detail": _describe_learned(row.display_name, state, age, row.details or {}), + "last_seen_at": row.last_seen_at.isoformat(), + "first_seen_at": row.first_seen_at.isoformat(), + **{k: v for k, v in (row.details or {}).items() if k != "agent_id"}, + }) + + parts.append(await _probe_redis()) + + overall = max((p["state"] for p in parts), key=lambda s: _SEVERITY[s], default=_UNKNOWN) + return jsonify({ + "overall": overall, + "parts": sorted(parts, key=lambda p: (-_SEVERITY[p["state"]], p["name"])), + "checked_at": now.isoformat(), + # So the UI can explain a `stale` without hard-coding the same numbers + # in a second place. + "thresholds": { + "stale_after_seconds": STALE_AFTER_SECONDS, + "down_after_seconds": DOWN_AFTER_SECONDS, + }, + })