diff --git a/backend/src/media_library_viewer_api/config.py b/backend/src/media_library_viewer_api/config.py index ccb9f27..7056b28 100644 --- a/backend/src/media_library_viewer_api/config.py +++ b/backend/src/media_library_viewer_api/config.py @@ -52,11 +52,6 @@ class Settings(BaseSettings): ssh_password: str = "" ssh_known_hosts_path: str = "" - # Monitoring poller - monitoring_poll_interval_seconds: int = 300 - monitoring_poll_initial_delay_seconds: int = 20 - monitoring_action_retention_days: int = 30 - # Observability prometheus_enabled: bool = True prometheus_file_sd_dir: str = "/app/backend/.cache/prometheus-file-sd" diff --git a/backend/src/media_library_viewer_api/dependencies.py b/backend/src/media_library_viewer_api/dependencies.py index bddecd0..892ed44 100644 --- a/backend/src/media_library_viewer_api/dependencies.py +++ b/backend/src/media_library_viewer_api/dependencies.py @@ -21,12 +21,6 @@ from media_library_viewer_api.clients.ssh import RemoteSSHClient from media_library_viewer_api.config import get_settings from media_library_viewer_api.services.mail_queue import MailQueue from media_library_viewer_api.services.mail_queue import get_mail_queue as _get_mail_queue -from media_library_viewer_api.services.monitoring_poller import ( - MonitoringPoller, -) -from media_library_viewer_api.services.monitoring_poller import ( - get_monitoring_poller as _get_monitoring_poller, -) from media_library_viewer_api.services.settings_store import SettingsStore from media_library_viewer_api.services.settings_store import get_settings_store as _get_settings_store @@ -251,11 +245,6 @@ def get_mail_queue() -> MailQueue: return _get_mail_queue() -def get_monitoring_poller() -> MonitoringPoller: - """Return the singleton background monitoring poller.""" - return _get_monitoring_poller() - - def get_settings_store() -> SettingsStore: """Return the singleton persistent settings store.""" return _get_settings_store() diff --git a/backend/src/media_library_viewer_api/main.py b/backend/src/media_library_viewer_api/main.py index 7ff37d9..3b6d609 100644 --- a/backend/src/media_library_viewer_api/main.py +++ b/backend/src/media_library_viewer_api/main.py @@ -13,7 +13,7 @@ from fastapi.responses import Response as FastAPIResponse from media_library_viewer_api.auth import require_jwt_auth, validate_auth_settings from media_library_viewer_api.config import get_settings -from media_library_viewer_api.dependencies import get_mail_queue, get_monitoring_poller, get_settings_store +from media_library_viewer_api.dependencies import get_mail_queue, get_settings_store from media_library_viewer_api.logging_utils import configure_logging, describe_settings, sanitize_log_extra from media_library_viewer_api.observability import ( get_request_id, @@ -46,13 +46,10 @@ async def lifespan(app: FastAPI): except Exception: logger.exception("Failed to write Prometheus file-SD targets during startup") mail_queue = get_mail_queue() - monitoring_poller = get_monitoring_poller() backup_poller = get_backup_poller() mail_queue.start() - monitoring_poller.start() backup_poller.start() yield - monitoring_poller.stop() backup_poller.stop() mail_queue.stop() logger.info("Backend shutdown complete") diff --git a/backend/src/media_library_viewer_api/routers/monitoring.py b/backend/src/media_library_viewer_api/routers/monitoring.py index a96df3c..125a370 100644 --- a/backend/src/media_library_viewer_api/routers/monitoring.py +++ b/backend/src/media_library_viewer_api/routers/monitoring.py @@ -1,18 +1,14 @@ -"""Monitoring router — disk checks, action history, and observability stack status.""" +"""Monitoring router — observability stack status (Alertmanager + Prometheus).""" from __future__ import annotations import logging from typing import Any -from fastapi import APIRouter, Body, Depends, HTTPException, Query +from fastapi import APIRouter, Body, Depends from media_library_viewer_api.config import get_settings from media_library_viewer_api.dependencies import get_settings_store -from media_library_viewer_api.services.monitoring_actions import ( - disk_space, - run_machine_operation, -) from media_library_viewer_api.services.settings_store import SettingsStore from media_library_viewer_api.services.targets import build_node_exporter_targets @@ -68,80 +64,12 @@ def _summary_from_alerts(alerts: list[dict[str, Any]]) -> dict[str, Any]: router = APIRouter(prefix="/api/monitoring", tags=["monitoring"]) -def _resolve_machine(store: SettingsStore, machine_id: str | None) -> dict[str, Any]: - """Return the requested machine or the first enabled machine. - - Monitoring is treated as a machine-by-machine view. If a machine is - explicitly requested but disabled, we surface that as a user-facing error so - the Settings tab can be used to re-enable it. - """ - machines = store.list_machines() - if machine_id: - machine = next((item for item in machines if item["id"] == machine_id), None) - if not machine: - raise HTTPException(status_code=404, detail="Monitoring machine not found") - if not machine.get("enabled"): - raise HTTPException(status_code=409, detail=f"Monitoring machine '{machine['name']}' is disabled") - return machine - - for machine in machines: - if machine.get("enabled"): - return machine - raise HTTPException(status_code=404, detail="No enabled monitoring machines configured") - - @router.get("/machines") def get_machines(store: SettingsStore = Depends(get_settings_store)) -> list[dict[str, Any]]: """Return enabled monitoring machines for the UI.""" return [m for m in store.list_machines() if m.get("enabled")] -@router.get("/poller") -def get_poller_status() -> dict[str, Any]: - """Return the backend poller status and configuration.""" - from media_library_viewer_api.dependencies import get_monitoring_poller - - poller = get_monitoring_poller().snapshot() - logger.info( - "Monitoring poller status requested running=%s poll_count=%s", - poller.get("worker_running"), - poller.get("poll_count"), - ) - return poller - - -@router.get("/machines/{machine_id}/actions") -def get_machine_actions( - machine_id: str, - limit: int = 20, - action: str | None = None, - status: str | None = None, - store: SettingsStore = Depends(get_settings_store), -) -> dict[str, Any]: - """Return recent action history for a single machine.""" - machine = _resolve_machine(store, machine_id) - actions = store.list_machine_actions(machine["id"], limit=limit, action=action, status=status) - return {"items": actions, "total": len(actions)} - - -@router.get("/disk") -def get_disk_space( - machine_id: str | None = Query(default=None), - store: SettingsStore = Depends(get_settings_store), -) -> dict[str, Any]: - """Return disk space for the configured path of a given machine.""" - machine = _resolve_machine(store, machine_id) - app_settings = get_settings() - path = str(machine.get("media_root") or app_settings.media_root or "/") - logger.info("Monitoring disk requested machine_id=%s path=%s", machine["id"], path) - return run_machine_operation( - machine, - store, - f"disk lookup for {path}", - lambda client: disk_space(client, path), - ) - - @router.get("/prometheus-targets") def get_prometheus_targets(store: SettingsStore = Depends(get_settings_store)) -> list[dict[str, Any]]: """Return Prometheus file-SD targets for remote Node Exporters. diff --git a/backend/src/media_library_viewer_api/routers/settings.py b/backend/src/media_library_viewer_api/routers/settings.py index 8ba4e46..4b88025 100644 --- a/backend/src/media_library_viewer_api/routers/settings.py +++ b/backend/src/media_library_viewer_api/routers/settings.py @@ -12,7 +12,7 @@ from pydantic import BaseModel, Field from media_library_viewer_api.clients.ssh import RemoteSSHClient from media_library_viewer_api.config import get_settings -from media_library_viewer_api.dependencies import get_monitoring_poller, get_settings_store +from media_library_viewer_api.dependencies import get_settings_store from media_library_viewer_api.services.db_maintenance import remove_sqlite_database from media_library_viewer_api.services.known_hosts import has_known_host from media_library_viewer_api.services.media_index import MediaIndex @@ -195,13 +195,8 @@ def post_machine( ) -> dict[str, Any]: saved = store.upsert_machine(machine.model_dump(exclude_none=True), machine.id) _write_prometheus_targets(store) - poller = get_monitoring_poller() - try: - saved_machine = MonitoringMachineInput.model_validate(saved) - _validate_saved_machine_ssh(saved_machine, store) - finally: - poller.start() - poller.kick() + saved_machine = MonitoringMachineInput.model_validate(saved) + _validate_saved_machine_ssh(saved_machine, store) return saved @@ -215,13 +210,8 @@ def put_machine( raise HTTPException(status_code=404, detail="Machine not found") saved = store.upsert_machine(machine.model_dump(exclude_none=True), machine_id) _write_prometheus_targets(store) - poller = get_monitoring_poller() - try: - saved_machine = MonitoringMachineInput.model_validate(saved) - _validate_saved_machine_ssh(saved_machine, store) - finally: - poller.start() - poller.kick() + saved_machine = MonitoringMachineInput.model_validate(saved) + _validate_saved_machine_ssh(saved_machine, store) return saved diff --git a/backend/src/media_library_viewer_api/services/monitoring_actions.py b/backend/src/media_library_viewer_api/services/monitoring_actions.py deleted file mode 100644 index 3fe3c4a..0000000 --- a/backend/src/media_library_viewer_api/services/monitoring_actions.py +++ /dev/null @@ -1,208 +0,0 @@ -"""Shared monitoring action helpers. - -The router and the background poller both use these helpers so machine -operations are recorded consistently whether they were triggered by a user -request or by the backend's scheduled polling loop. -""" - -from __future__ import annotations - -import json -import logging -import shlex -import time -import uuid -from typing import Any, Callable - -from fastapi import HTTPException - -from media_library_viewer_api.clients.local import LocalCommandClient -from media_library_viewer_api.clients.ssh import RemoteSSHClient -from media_library_viewer_api.config import get_settings -from media_library_viewer_api.observability import record_ssh_command -from media_library_viewer_api.services.settings_store import SettingsStore - -logger = logging.getLogger(__name__) - - -def build_machine_client(machine: dict[str, Any], store: SettingsStore): - """Build the appropriate command client for a machine definition.""" - mode = str(machine.get("mode") or "local").strip().lower() - if mode == "local": - return LocalCommandClient() - - key_directory = str(machine.get("key_directory") or "").strip() - key_name = str(machine.get("key_name") or "").strip() - key_path = f"{key_directory}/{key_name}" if key_directory and key_name else None - private_key = str(machine.get("ssh_private_key") or "") - passphrase = str(machine.get("ssh_private_key_passphrase") or "") - ssh_key_id = str(machine.get("ssh_key_id") or "").strip() - if ssh_key_id: - ssh_key = store.get_ssh_key(ssh_key_id) - if ssh_key: - private_key = str(ssh_key.get("private_key") or private_key) - passphrase = str(ssh_key.get("passphrase") or passphrase) - - settings = get_settings() - return RemoteSSHClient( - host=str(machine.get("host") or ""), - username=str(machine.get("username") or ""), - port=int(machine.get("port") or 22), - key_filename=key_path, - private_key=private_key or None, - private_key_passphrase=passphrase or None, - password=str(machine.get("password") or "") or None, - known_hosts_path=str(settings.ssh_known_hosts_file), - ) - - -def disk_space(client: Any, path: str = "/") -> dict[str, Any]: - """Return df information for the filesystem containing ``path``. - - Works against any client with a ``run`` method (local shell or SSH). - """ - command = ( - "df -P -B1 -- " + shlex.quote(path or "/") + " | awk 'NR==2 {printf " - '"{\\"filesystem\\":\\"%s\\",\\"size\\":%s,"' - '"\\"used\\":%s,\\"available\\":%s,"' - '"\\"used_pct\\":\\"%s\\",\\"mount\\":\\"%s\\"}", "' - "$1,$2,$3,$4,$5,$6}'" - ) - logger.debug("Reading disk space for path=%s", path) - result = client.run(command, timeout=20) - if result.exit_status != 0 or not result.stdout.strip(): - logger.warning("Failed to read disk space for %s: %s", path, result.stderr or result.stdout) - raise RuntimeError(result.stderr or result.stdout or "failed to read disk space") - data = json.loads(result.stdout) - logger.info("Disk space path=%s mount=%s used_pct=%s", path, data.get("mount"), data.get("used_pct")) - return data - - -def summarize_operation_result(action: str, result: Any) -> str: - """Turn an operation result into a compact human-readable summary.""" - if result is None: - return action - if isinstance(result, str): - text = result.strip().splitlines()[0] if result.strip() else action - return text[:200] - if isinstance(result, list): - return f"{action}: {len(result)} item(s)" - if isinstance(result, dict): - if action.startswith("disk lookup"): - used_pct = result.get("used_pct") - mount = result.get("mount") or result.get("filesystem") - return f"disk {mount or ''} used {used_pct or '?'}".strip() - if "message" in result and isinstance(result["message"], str): - return result["message"][:200] - return json_compact(result) - return action - - -def json_compact(value: Any) -> str: - try: - text = json.dumps(value, sort_keys=True, default=str) - return text[:200] - except Exception: - return str(value)[:200] - - -def run_machine_operation( - machine: dict[str, Any], - store: SettingsStore, - action: str, - callback: Callable[[Any], Any], - *, - summarize: Callable[[Any], str] | None = None, - request_id: str = "", - raise_http: bool = True, - client: Any | None = None, -) -> Any: - """Run a machine operation, record history, and optionally raise on failure.""" - started = time.perf_counter() - if client is None: - client = build_machine_client(machine, store) - try: - result = callback(client) - duration_ms = int((time.perf_counter() - started) * 1000) - record_ssh_command( - machine_id=machine.get("id") or "unknown", - action=action, - status="ok", - duration_seconds=duration_ms / 1000.0, - ) - store.record_machine_action( - machine, - action, - "ok", - duration_ms=duration_ms, - request_id=request_id, - message=(summarize(result) if summarize else summarize_operation_result(action, result)), - ) - return result - except HTTPException: - raise - except Exception as exc: # pragma: no cover - transport/network fallback - duration_ms = int((time.perf_counter() - started) * 1000) - record_ssh_command( - machine_id=machine.get("id") or "unknown", - action=action, - status="error", - duration_seconds=duration_ms / 1000.0, - ) - logger.exception( - "Monitoring %s failed machine_id=%s machine_name=%s", - action, - machine["id"], - machine["name"], - ) - error_text = str(exc) - store.record_machine_action( - machine, - action, - "error", - duration_ms=duration_ms, - request_id=request_id, - error=error_text, - ) - if not raise_http: - return None - status_code = 503 if machine.get("mode") == "local" else 502 - raise HTTPException( - status_code=status_code, - detail=f"{machine['name']}: {action} failed: {exc}", - ) from exc - - -def poll_machine_snapshot( - machine: dict[str, Any], - store: SettingsStore, - *, - metrics_limit: int = 70_000, - request_id: str | None = None, -) -> dict[str, Any]: - """Collect a backend-scheduled snapshot for a machine. - - The legacy POSIX collector has been removed; this now records a lightweight - disk-space lookup on the same schedule so action history stays useful. - """ - request_id = request_id or f"poll:{machine.get('id') or uuid.uuid4().hex}" - results: dict[str, Any] = {"request_id": request_id, "machine_id": machine.get("id"), "actions": []} - - client = build_machine_client(machine, store) - settings = get_settings() - path = str(machine.get("media_root") or settings.media_root or "/") - disk = run_machine_operation( - machine, - store, - f"disk lookup for {path}", - lambda client: disk_space(client, path), - request_id=request_id, - raise_http=False, - client=client, - ) - results["disk_mount"] = (disk or {}).get("mount") if isinstance(disk, dict) else None - results["actions"].append("disk lookup") - - return results - - diff --git a/backend/src/media_library_viewer_api/services/monitoring_poller.py b/backend/src/media_library_viewer_api/services/monitoring_poller.py deleted file mode 100644 index f4bbe96..0000000 --- a/backend/src/media_library_viewer_api/services/monitoring_poller.py +++ /dev/null @@ -1,197 +0,0 @@ -"""Background poller for monitoring machine snapshots. - -The poller runs entirely inside the backend. It periodically reads the defined -machines, collects a small snapshot from each enabled machine over SSH or local -shell execution, and stores the resulting history rows in the settings DB. - -This keeps the Monitoring page populated without any daemon or agent running on -the remote machines. -""" - -from __future__ import annotations - -import logging -import threading -import time -from dataclasses import dataclass -from typing import Any - -from media_library_viewer_api.config import get_settings -from media_library_viewer_api.services.monitoring_actions import poll_machine_snapshot -from media_library_viewer_api.services.settings_store import SettingsStore, get_settings_store - -logger = logging.getLogger(__name__) - - -@dataclass(frozen=True) -class PollerConfig: - interval_seconds: int = 300 - initial_delay_seconds: int = 20 - metrics_limit: int = 70_000 - retention_days: int = 30 - - -class MonitoringPoller: - """Single-worker background poller for monitoring snapshots.""" - - def __init__(self) -> None: - self._thread: threading.Thread | None = None - self._stop_event = threading.Event() - self._lock = threading.Lock() - self._last_run_at: float | None = None - self._last_success_at: float | None = None - self._last_error: str = "" - self._last_cycle_ms: int | None = None - self._poll_count = 0 - self._error_count = 0 - - def _config(self) -> PollerConfig: - settings = get_settings() - return PollerConfig( - interval_seconds=max(30, int(getattr(settings, "monitoring_poll_interval_seconds", 300) or 300)), - initial_delay_seconds=max(0, int(getattr(settings, "monitoring_poll_initial_delay_seconds", 20) or 20)), - metrics_limit=70_000, - retention_days=max(1, int(getattr(settings, "monitoring_action_retention_days", 30) or 30)), - ) - - def start(self) -> None: - """Start the background worker if it is not already running.""" - with self._lock: - if self._thread and self._thread.is_alive(): - return - self._stop_event.clear() - self._thread = threading.Thread(target=self._run, name="monitoring-poller", daemon=True) - self._thread.start() - logger.info("Monitoring poller started") - - def kick(self) -> None: - """Run one immediate snapshot cycle in the background.""" - store = get_settings_store() - config = self._config() - threading.Thread( - target=self._run_cycle, - args=(store, config), - name="monitoring-poller-kick", - daemon=True, - ).start() - logger.info("Monitoring poller kick requested") - - def stop(self, timeout: float = 5.0) -> None: - """Stop the worker thread and wait briefly for shutdown.""" - with self._lock: - thread = self._thread - if not thread: - return - self._stop_event.set() - thread.join(timeout=timeout) - if thread.is_alive(): - logger.warning("Monitoring poller did not stop within %.1fs", timeout) - else: - logger.info("Monitoring poller stopped") - with self._lock: - if self._thread is thread: - self._thread = None - - def status(self) -> dict[str, Any]: - """Return a small status snapshot for diagnostics and tests.""" - with self._lock: - return { - "worker_running": bool(self._thread and self._thread.is_alive()), - "stop_requested": self._stop_event.is_set(), - "last_run_at": self._last_run_at, - "last_success_at": self._last_success_at, - "last_error": self._last_error, - "last_cycle_ms": self._last_cycle_ms, - "poll_count": self._poll_count, - "error_count": self._error_count, - } - - def snapshot(self) -> dict[str, Any]: - """Return status plus the active polling configuration.""" - data = self.status() - config = self._config() - data.update( - { - "interval_seconds": config.interval_seconds, - "initial_delay_seconds": config.initial_delay_seconds, - "retention_days": config.retention_days, - } - ) - return data - - def _run_cycle(self, store: SettingsStore, config: PollerConfig) -> None: - start = time.perf_counter() - machines = store.list_machines() - enabled = [machine for machine in machines if machine.get("enabled")] - logger.info("Monitoring poll cycle starting enabled_machines=%s", len(enabled)) - cycle_errors = 0 - for machine in enabled: - if self._stop_event.is_set(): - break - try: - snapshot = poll_machine_snapshot( - machine, - store, - metrics_limit=config.metrics_limit, - request_id=f"poll:{machine['id']}:{int(time.time())}", - ) - logger.info( - "Monitoring poll snapshot machine_id=%s request_id=%s disk_mount=%s actions=%s", - machine["id"], - snapshot.get("request_id"), - snapshot.get("disk_mount"), - snapshot.get("actions"), - ) - except Exception: - cycle_errors += 1 - logger.exception( - "Monitoring poll snapshot failed machine_id=%s machine_name=%s", machine["id"], machine["name"] - ) - retention_seconds = config.retention_days * 24 * 60 * 60 - cutoff_ts = int(time.time()) - retention_seconds - removed = store.prune_machine_actions(cutoff_ts) - if removed: - logger.info("Pruned %s old monitoring action rows older than %s", removed, cutoff_ts) - duration_ms = int((time.perf_counter() - start) * 1000) - with self._lock: - self._last_run_at = time.time() - self._last_cycle_ms = duration_ms - self._poll_count += 1 - if cycle_errors: - self._error_count += cycle_errors - self._last_error = f"{cycle_errors} machine(s) failed" - else: - self._last_success_at = self._last_run_at - self._last_error = "" - logger.info( - "Monitoring poll cycle complete enabled_machines=%s errors=%s duration_ms=%s removed_rows=%s", - len(enabled), - cycle_errors, - duration_ms, - removed, - ) - - def _run(self) -> None: - config = self._config() - if config.initial_delay_seconds: - logger.info("Monitoring poller initial delay=%ss", config.initial_delay_seconds) - if self._stop_event.wait(config.initial_delay_seconds): - return - store = get_settings_store() - while not self._stop_event.is_set(): - try: - self._run_cycle(store, config) - except Exception: - with self._lock: - self._last_error = "poller cycle failed" - self._error_count += 1 - logger.exception("Monitoring poller cycle failed") - if self._stop_event.wait(config.interval_seconds): - break - - -_MONITORING_POLLER = MonitoringPoller() - - -def get_monitoring_poller() -> MonitoringPoller: - return _MONITORING_POLLER diff --git a/backend/src/media_library_viewer_api/services/settings_store.py b/backend/src/media_library_viewer_api/services/settings_store.py index e7cfa34..50dbcad 100644 --- a/backend/src/media_library_viewer_api/services/settings_store.py +++ b/backend/src/media_library_viewer_api/services/settings_store.py @@ -85,25 +85,10 @@ class SettingsStore: """ ) conn.execute("CREATE INDEX IF NOT EXISTS idx_monitoring_machines_mode ON monitoring_machines(mode)") - conn.execute( - """ - CREATE TABLE IF NOT EXISTS monitoring_machine_actions ( - id TEXT PRIMARY KEY, - machine_id TEXT NOT NULL, - machine_name TEXT NOT NULL, - mode TEXT NOT NULL, - action TEXT NOT NULL, - status TEXT NOT NULL, - created_at INTEGER NOT NULL, - duration_ms INTEGER NOT NULL, - request_id TEXT NOT NULL, - message TEXT NOT NULL, - error TEXT NOT NULL, - stdout_tail TEXT NOT NULL, - stderr_tail TEXT NOT NULL - ) - """ - ) + # The legacy SSH-scraping monitor (MonitoringPoller) was decommissioned; + # metrics now live in Prometheus/node_exporter/Grafana. Drop the orphan + # table on startup so existing databases get a clean slate. + conn.execute("DROP TABLE IF EXISTS monitoring_machine_actions") conn.execute( """ CREATE TABLE IF NOT EXISTS ssh_keys ( @@ -178,18 +163,6 @@ class SettingsStore: conn.execute( "CREATE INDEX IF NOT EXISTS idx_dashboard_shortcuts_type ON dashboard_shortcuts(shortcut_type)" ) - conn.execute( - """ - CREATE INDEX IF NOT EXISTS idx_monitoring_machine_actions_machine_time - ON monitoring_machine_actions(machine_id, created_at DESC) - """ - ) - conn.execute( - """ - CREATE INDEX IF NOT EXISTS idx_monitoring_machine_actions_action_status - ON monitoring_machine_actions(action, status) - """ - ) conn.execute(""" CREATE TABLE IF NOT EXISTS backup_jobs ( id TEXT PRIMARY KEY, @@ -563,88 +536,6 @@ class SettingsStore: with self.connect() as conn: conn.execute("DELETE FROM monitoring_machines WHERE id = ?", (machine_id,)) - def record_machine_action( - self, - machine: dict[str, Any], - action: str, - status: str, - *, - duration_ms: int, - request_id: str = "", - message: str = "", - error: str = "", - stdout_tail: str = "", - stderr_tail: str = "", - ) -> None: - """Store a compact action history row for a machine operation.""" - self.init_schema() - now = int(time.time()) - with self.connect() as conn: - conn.execute( - """ - INSERT INTO monitoring_machine_actions - ( - id, machine_id, machine_name, mode, action, status, - created_at, duration_ms, request_id, message, error, - stdout_tail, stderr_tail - ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - """, - ( - uuid.uuid4().hex, - str(machine.get("id") or ""), - str(machine.get("name") or ""), - str(machine.get("mode") or "local"), - action, - status, - now, - duration_ms, - request_id, - message, - error, - stdout_tail, - stderr_tail, - ), - ) - - def list_machine_actions( - self, - machine_id: str, - *, - limit: int = 20, - action: str | None = None, - status: str | None = None, - ) -> list[dict[str, Any]]: - self.init_schema() - clauses = ["machine_id = ?"] - params: list[Any] = [machine_id] - if action: - clauses.append("action = ?") - params.append(action) - if status: - clauses.append("status = ?") - params.append(status) - sql = ( - "SELECT machine_id, machine_name, mode, action, status, " - "created_at, duration_ms, request_id, message, error, stdout_tail, stderr_tail " - f"FROM monitoring_machine_actions WHERE {' AND '.join(clauses)} " - "ORDER BY created_at DESC LIMIT ?" - ) - params.append(max(1, min(int(limit), 200))) - with self.connect() as conn: - rows = conn.execute(sql, params).fetchall() - return [dict(row) for row in rows] - - def prune_machine_actions(self, older_than_ts: int) -> int: - """Delete action history rows older than the given timestamp.""" - self.init_schema() - with self.connect() as conn: - cur = conn.execute( - "DELETE FROM monitoring_machine_actions WHERE created_at < ?", - (int(older_than_ts),), - ) - return int(cur.rowcount or 0) - @staticmethod def _private_key_summary(private_key: str) -> dict[str, str]: if not private_key: diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 9056236..bbc7ff9 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -598,34 +598,6 @@ class TestJobs: class TestMonitoring: - def _ensure_machine(self): - store = app.dependency_overrides[get_settings_store]() - if not store.list_machines(): - store.upsert_machine( - { - "name": "Test Machine", - "mode": "ssh", - "enabled": True, - "services": ["monitoring", "files", "jellyfin"], - "host": "test-host", - "username": "test-user", - } - ) - - def test_disk(self, test_client, mock_ssh): - self._ensure_machine() - mock_ssh.run.return_value = CommandResult( - command="df ...", - exit_status=0, - stdout='{"filesystem":"/dev/sda1","size":1000000000,"used":500000000,"available":500000000,"used_pct":"50%","mount":"/"}', - stderr="", - ) - with patch("media_library_viewer_api.services.monitoring_actions.build_machine_client", return_value=mock_ssh): - response = test_client.get("/api/monitoring/disk") - assert response.status_code == 200 - data = response.json() - assert data["used_pct"] == "50%" - def test_prometheus_targets_empty(self, test_client): response = test_client.get("/api/monitoring/prometheus-targets") assert response.status_code == 200 diff --git a/backend/tests/test_monitoring_actions.py b/backend/tests/test_monitoring_actions.py deleted file mode 100644 index 3043502..0000000 --- a/backend/tests/test_monitoring_actions.py +++ /dev/null @@ -1,36 +0,0 @@ -from unittest.mock import MagicMock, patch - -from media_library_viewer_api.services.monitoring_actions import poll_machine_snapshot - - -def test_poll_machine_snapshot_records_disk_lookup(): - store = MagicMock() - machine = { - "id": "local", - "name": "This machine", - "mode": "local", - "media_root": "/srv/media", - } - - with ( - patch( - "media_library_viewer_api.services.monitoring_actions.build_machine_client", - return_value=object(), - ) as build_client, - patch( - "media_library_viewer_api.services.monitoring_actions.disk_space", - return_value={"mount": "/srv/media", "used_pct": "12.5%"}, - ) as disk_fn, - ): - result = poll_machine_snapshot(machine, store, metrics_limit=123, request_id="poll:test") - - assert result["request_id"] == "poll:test" - assert result["disk_mount"] == "/srv/media" - assert result["actions"] == ["disk lookup"] - build_client.assert_called_once_with(machine, store) - disk_fn.assert_called_once_with(build_client.return_value, "/srv/media") - assert store.record_machine_action.call_count == 1 - recorded_action = store.record_machine_action.call_args - assert recorded_action.args[1] == "disk lookup for /srv/media" - assert recorded_action.kwargs["request_id"] == "poll:test" - assert recorded_action.args[2] == "ok" diff --git a/openspec/changes/decommission-monitoring-poller/plan.md b/openspec/changes/decommission-monitoring-poller/plan.md new file mode 100644 index 0000000..d3f7ea0 --- /dev/null +++ b/openspec/changes/decommission-monitoring-poller/plan.md @@ -0,0 +1,101 @@ +# Plan — decommission-monitoring-poller + +> Status: **DRAFT — awaiting user approval before implementation.** +> Scope: a focused backend+frontend decommission, not a full SDD change. Plan-then-implement (user-approved 2026-06-17). +> Root cause this addresses: the 2026-06-16/17 observability update externalised metrics to Prometheus+Grafana+Loki+Alertmanager, but the *legacy Manage-side SSH-scraping monitor* (the `MonitoringPoller`, `/monitoring/disk`, `/monitoring/machines/{id}/actions`, and the `monitoring_machine_actions` SQLite table) was never removed. It duplicates the new stack, drains SSH budget every 300s, and feeds nothing (its UI was deleted in `e2ad731`). + +## 1. Problem + +Manage runs a background thread (`MonitoringPoller`) that, every 300s, SSHes into every configured machine, runs `df`, and stores the result in its own SQLite table (`monitoring_machine_actions`, 30-day retention). After the observability update, **Prometheus already scrapes node_exporter on these machines and Grafana already owns the dashboards**. The poller is pure duplication: more SSH sessions, more stale state, a second source of truth for "disk usage," and a SQLite table that nothing reads. + +The alerting side (Alertmanager proxy + `/alerts` + `/alertmanager-status` + `/alertmanager-webhook` + `/prometheus-targets` + `/machines`) already fits the new model and is untouched by this change. + +## 2. Goals / non-goals + +**Goals** + +- Stop the duplicated SSH-scraping of system metrics. +- Remove the dead `/disk`, `/poller`, `/machines/{id}/actions` surface and the SQLite history that fed it. +- Remove the now-orphaned frontend `DiskSpaceCard` + `DiskSpace` type. +- Leave Manage a clean thin-dashboard: Alertmanager alerts + Prometheus target health + Grafana deep-links. + +**Non-goals** + +- Do NOT touch the Alertmanager proxy, `/prometheus-targets`, `/machines`, or `/alertmanager-webhook` — they fit the model. +- Do NOT remove the `disk_usage` **job template** in `jobs.py` (user-approved: it is a manual on-demand Actions job, not monitoring). +- Do NOT remove `node_exporter_*` fields on `MonitoringMachine` — they configure where Prometheus scrapes; that is correct and stays. +- Do NOT introduce a Prometheus query proxy / PromQL reader in this change (that was the alternative the user did not pick). +- Do NOT add new features. This is a removal. + +## 3. Exact removal map (verified against source) + +### Backend — delete entirely + +- `backend/src/media_library_viewer_api/services/monitoring_poller.py` (the `MonitoringPoller` class, `PollerConfig`, `_MONITORING_POLLER`, `get_monitoring_poller`). + - **Verified sole callers:** `main.py` lifespan, `dependencies.py` wrapper, `routers/monitoring.py:/poller`, `routers/settings.py` (machine save → `poller.start()/kick()`). +- `backend/src/media_library_viewer_api/services/monitoring_actions.py` (the whole file: `build_machine_client`, `disk_space`, `summarize_operation_result`, `json_compact`, `run_machine_operation`, `poll_machine_snapshot`). + - **Verified:** `run_machine_operation` has exactly 2 callers (`poll_machine_snapshot` here, and `/monitoring/disk`) — both going. `tasks.py` does NOT use it. Nothing else imports the module. +- `backend/tests/test_monitoring_actions.py` (36 lines, tests `poll_machine_snapshot`). + +### Backend — edit in place + +- `backend/src/media_library_viewer_api/main.py` lifespan (lines ~48–56): remove `monitoring_poller = get_monitoring_poller()`, `monitoring_poller.start()`, `monitoring_poller.stop()`, and the `get_monitoring_poller` import on line 16. Keep `backup_poller` and `mail_queue` intact. +- `backend/src/media_library_viewer_api/dependencies.py`: remove the `MonitoringPoller` import block (lines 24–29) and the `get_monitoring_poller` wrapper (lines 254–256). +- `backend/src/media_library_viewer_api/routers/monitoring.py`: remove imports of `disk_space`, `run_machine_operation`, `poll_machine_snapshot`; remove the three endpoints `/poller` (99), `/machines/{machine_id}/actions` (113), `/disk` (127). Keep `/machines`, `/prometheus-targets`, `/alerts`, `/alertmanager-status`, `/alertmanager-webhook`. Also drop the now-unused `_resolve_machine` helper if it becomes unreferenced after `/disk` and `/actions` removal (verify during impl — `/machines` does not use it). +- `backend/src/media_library_viewer_api/routers/settings.py` (lines 198–204 and 218–224): remove the `get_monitoring_poller()` + `poller.start()` + `poller.kick()` calls from `post_machine` and `put_machine`. Keep `write_prometheus_targets(store)` (that is the new-model target generation). +- `backend/src/media_library_viewer_api/services/settings_store.py`: + - Remove `CREATE TABLE IF NOT EXISTS monitoring_machine_actions` (lines ~90) and its two indexes (`idx_monitoring_machine_actions_machine_time`, `idx_monitoring_machine_actions_action_status`, lines ~183–190) from `init_schema`. + - Remove methods `record_machine_action` (566), `list_machine_actions` (610), `prune_machine_actions` (638). + - Note: existing databases will keep the orphaned `monitoring_machine_actions` table harmlessly (no migration framework here — `init_schema` is `CREATE TABLE IF NOT EXISTS` + ad-hoc `ALTER`). A one-line `DROP TABLE IF EXISTS` can be added to `init_schema` for cleanliness; decide at impl time. +- `backend/src/media_library_viewer_api/config.py`: remove `monitoring_poll_interval_seconds` (56), `monitoring_poll_initial_delay_seconds` (57), `monitoring_action_retention_days` (58). + +### Backend — tests to fix + +- `backend/tests/test_api.py`: + - `TestMonitoring.test_disk` (line 615) — **remove** (tests `/api/monitoring/disk`). + - `TestMonitoring.test_prometheus_targets_empty` and `..._returns_enabled_ssh_node_exporter` — **keep** (test the surviving `/prometheus-targets`). + - `TestSettingsMachines` — **keep** but verify they still pass after the `poller` calls are removed from `post/put_machine`. + - `TestAlertmanager` — **keep** (untouched). + - The `disk_usage` reference at line 576/586 is the **Jobs** test (`/api/jobs/run`), NOT the monitoring poller — **keep** (the job template stays). + +### Frontend — delete + +- `frontend/src/components/DiskSpaceCard.tsx` — **verified orphaned** (only `__tests__/DiskSpaceCard.test.tsx` imports it; no page uses it). +- `frontend/src/components/__tests__/DiskSpaceCard.test.tsx`. +- `frontend/src/types/index.ts` `DiskSpace` interface (line 279) — remove after confirming no importer (grep shows none outside the type file). + +### Docs + +- `AGENTS.md` line 25 ("starts the mail queue and monitoring poller") → "...starts the mail queue and backup alert poller." +- `docs/monitoring-logging-design.md` line 65 (describes the poller) → update or strike the poller paragraph. +- `docs/MIGRATION_PLAN.md` line 110 (`/api/monitoring/disk` row) → remove the row. +- `docs/REQUIREMENTS.md` → add a note that Manage-side system-metric scraping is retired in favour of the external observability stack. +- `docs/superpowers/specs/2026-05-11-backup-monitoring-design.md` is a historical spec; leave as-is (it is an archived design doc). + +## 4. Slice plan (≤400 lines each, build+pytest green per slice) + +1. **Slice 1 — Backend removal (endpoints + poller + actions + store + config).** Delete `monitoring_poller.py`, `monitoring_actions.py`, `test_monitoring_actions.py`; edit `main.py`, `dependencies.py`, `routers/monitoring.py`, `routers/settings.py`, `settings_store.py`, `config.py`; fix `test_api.py` (`test_disk` removed, `TestSettingsMachines` re-checked). Gate: `cd backend && PYTHONPATH=src pytest`. +2. **Slice 2 — Frontend orphan removal.** Delete `DiskSpaceCard.tsx` + its test + `DiskSpace` type. Gate: `cd frontend && npm run build && npm run lint && npm test`. +3. **Slice 3 — Docs.** `AGENTS.md`, `docs/monitoring-logging-design.md`, `docs/MIGRATION_PLAN.md`, `docs/REQUIREMENTS.md`. Gate: none (docs); commit standalone. + +Estimated total: ~500–700 lines deleted, ~50–100 added (edits). Each slice well under 400. + +## 5. Risks & verification + +- **Hidden caller of `run_machine_operation` / `poll_machine_snapshot`**: mitigated — grep shows exactly the callers listed; re-grep at slice-1 start. +- **`TestSettingsMachines` breakage** once `poller.start()/kick()` is removed from `post/put_machine`: those tests mock `write_prometheus_targets` and don't assert on the poller; should pass. If they reference `get_monitoring_poller`, fix by dropping the assertion. +- **Orphaned SQLite table on existing DBs**: harmless (empty, unused). Optional `DROP TABLE IF EXISTS monitoring_machine_actions` in `init_schema` for cleanliness. +- **No browser smoke**: same caveat as the UI rework; backend covered by pytest. +- **`_resolve_machine` in monitoring.py** may become unused after `/disk` + `/actions` removal; remove if so. + +## 6. Acceptance + +- `cd backend && PYTHONPATH=src pytest` green (with `test_disk` + `test_monitoring_actions.py` removed). +- `grep -rnE 'MonitoringPoller|poll_machine_snapshot|/monitoring/disk|monitoring_machine_actions|monitoring_poll_interval_seconds|DiskSpaceCard' backend/ frontend/src/` → only historical/docs hits (spec.md archive is fine). +- `cd frontend && npm run build && npm run lint && npm test` green. +- Docs updated to reflect Manage no longer scrapes its own metrics. + +## 7. Open questions for the user (none blocking, defaults shown) + +- Q1. Existing DBs' orphaned `monitoring_machine_actions` table — (a) add `DROP TABLE IF EXISTS` to `init_schema` for a clean slate [default], or (b) leave it harmless? +- Q2. Commit/PR mechanics — same as the UI rework (commit per slice, no push until you say)?