Files
manage/openspec/changes/archive/decommission-monitoring-poller/plan.md
T
Developer ca8927834e chore(openspec): archive completed changes
Move finished change directories to openspec/changes/archive/:
- configurable-dashboard-widgets
- decommission-monitoring-poller
- service-registry
- unify-tasks-on-services

All associated implementation has been merged to main.
2026-06-23 19:38:34 +00:00

9.4 KiB
Raw Blame History

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 ~4856): 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 2429) and the get_monitoring_poller wrapper (lines 254256).
  • 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 198204 and 218224): 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 ~183190) 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_exporterkeep (test the surviving /prometheus-targets).
    • TestSettingsMachineskeep but verify they still pass after the poller calls are removed from post/put_machine.
    • TestAlertmanagerkeep (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.tsxverified 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: ~500700 lines deleted, ~50100 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)?