alex/manage
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(monitoring): decommission legacy SSH-scraping poller (slice 1)

Browse Source 
The 2026-06-16/17 observability update externalised metrics to
Prometheus + node_exporter + Grafana, but the legacy Manage-side
SSH-scraping monitor was never removed. It duplicated the new stack,
ran SSH df on every machine every 300s, and fed nothing (its UI was
deleted in e2ad731). This slice decommissions the duplication.

Removed (backend):
- services/monitoring_poller.py (MonitoringPoller) — entire file
- services/monitoring_actions.py (disk_space, run_machine_operation,
  poll_machine_snapshot, build_machine_client) — entire file;
  run_machine_operation had only 2 callers (the poller + /disk), both gone
- tests/test_monitoring_actions.py
- endpoints: POST /api/monitoring/poller, GET /machines/{id}/actions,
  GET /disk (and the now-dead _resolve_machine helper)
- lifespan wiring (main.py), dependency wrapper (dependencies.py),
  poller.start()/kick() from machine save (routers/settings.py)
- SettingsStore: monitoring_machine_actions table CREATE + 2 indexes +
  record/list/prune_machine_actions methods; DROP TABLE IF EXISTS on
  startup cleans existing DBs (user-approved)
- config knobs: monitoring_poll_interval_seconds,
  monitoring_poll_initial_delay_seconds, monitoring_action_retention_days
- test_api.py: TestMonitoring._ensure_machine + test_disk

Kept (fits the new model): /machines, /prometheus-targets, /alerts,
/alertmanager-status, /alertmanager-webhook; the disk_usage JOB template
(manual on-demand, not monitoring); node_exporter_* machine fields
(they point Prometheus at the right host).

Gate: backend pytest 173 passed; ruff clean.
This commit is contained in:
Developer
2026-06-17 20:48:56 +00:00
parent 1c29299e8c
commit 08a3b616f6
11 changed files with 113 additions and 691 deletions
Download Patch File Download Diff File Expand all files Collapse all files
openspec/changes/decommission-monitoring-poller/plan.md
+101
View File
@@ -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)?