Files
Developer 29650ca512 chore(per-instance-hook-scoping): archive verified+synced change
Move to openspec/changes/archive/2026-07-09-per-instance-hook-scoping/
(R100 renames preserved). 9 artifacts. Canonical openspec/specs/
service-instance-scoping/ remains. Resolves multi-instance wrong-data bug
(hooks now scope by instance.id; instance switcher re-scopes).
Carry-overs: fetchBackupDashboard untouched (design decision 5); subquery
scoping for runs/alerts (schema asymmetry).
2026-07-10 00:17:51 +00:00

8.4 KiB
Raw Permalink Blame History

SDD Proposal: Per-Instance Hook Scoping

Change: per-instance-hook-scoping Phase: proposal Date: 2026-07-09

1. Problem / Why Now

This is a correctness fix, not a feature.

When a user has multiple instances of Alertmanager, Prometheus, or the backups service and opens a specific instance's service page, the Observability/backup tabs show data for whichever instance resolves first globally — not the instance whose page they're viewing. The root cause: the frontend hooks (useAlertmanagerAlerts, useAlertmanagerStatus, usePrometheusStatus, useBackupJobs/Runs/Alerts) query without a serviceId, and the consuming tabs (AlertsTab, MetricsTab, JobsTab) don't pass instance.id to them.

This was documented as a carry-over risk in the services-as-hub-ia verify-report (2026-06-26): "Hooks query globally, not per-instance... JobsTab, AlertsTab, MetricsTab show data for whichever instance the hook resolves as first-configured." The tabs were deliberately built to accept an instance prop and wait for the hooks to catch up — each tab carries an explicit TODO comment to that effect. The fix is now due.

2. Target Users and Situations

  • Primary users: Operators with more than one Alertmanager, Prometheus, or backups instance (e.g., a staging + production pair, or per-tenant instances).
  • Workflow moment: open /{serviceType}/{instanceId} to inspect a specific instance → the tabs must reflect THAT instance, not a global/first-resolved one.
  • Urgency: Medium. Single-instance operators see no bug; multi-instance operators see silently-wrong data. The incorrect-behavior surface grows as more instances are added.

3. Product Outcome

After this change, an authenticated user viewing a specific service instance's page sees alerts, status, and backup data scoped to that instance only. Switching the instance switcher (already present on ServicePage when enabledSiblings.length > 1) re-scopes the data. The dashboard widgets (which resolve via widget-instance → service_id) are unaffected — they were already correct.

4. Scope Boundaries and Non-Goals

In scope

  • Frontend hooks gain a serviceId param: useAlertmanagerAlerts(serviceId), useAlertmanagerStatus(serviceId), usePrometheusStatus(serviceId), useBackupJobs(serviceId), useBackupRuns(jobId, status, serviceId), useBackupAlerts(jobId, acknowledged, severity, serviceId). Each thread it into queryKey (so caches are per-instance) and pass it to the fetch function.
  • Fetch functions pass service_id as a query param: the existing fetch* functions in api/client.ts and api/backups.ts gain an optional serviceId argument appended to the request URL.
  • Tabs pass instance.id: AlertsTab, MetricsTab, JobsTab call the hooks with their received instance.id.
  • Backend backup endpoints gain service_id: get_backup_jobs, get_backup_runs, get_backup_alerts, get_backup_dashboard accept an optional service_id: str | None query param and thread it into SettingsStore query methods (which already have service attribution per the services-as-hub-ia Slice 3 work).
  • get_prometheus_targets stays global — it returns Node Exporter scrape targets for external Prometheus instances via http_sd_configs, not instance-scoped UI data. Out of scope by design.

Non-goals (explicitly out of scope)

  • Widget-level data scoping. Dashboard widgets (PrometheusChartWidget, AlertmanagerAlertsWidget, BackupsWidget, etc.) already resolve their service via the widget instance's service_id binding — they are correct and untouched.
  • useMonitoringMachines. Machines are a global cross-service concept (not per-instance); stays as-is.
  • Per-user scoping. Scoping is by service_id only.
  • Prometheus scrape-target endpoint. Stays global (see above).
  • Refactoring the hook return shapes or polling intervals. Only the serviceId input is added; outputs and timing are unchanged.
  • Grafana. Removed by prometheus-direct-charting; not present.

5. High-Level Approach

5.1 Backend (small)

  1. routers/backups.py — add service_id: str | None = None query param to get_backup_jobs, get_backup_runs, get_backup_alerts, get_backup_dashboard; thread into SettingsStore calls.
  2. services/settings_store.pylist_backup_jobs, list_backup_runs, list_backup_alerts accept an optional service_id filter and add a WHERE clause when non-null. (The backup_jobs/backup_runs tables already carry service_id attribution from the earlier change; confirm and add the filter.)
  3. The Alertmanager/Prometheus status endpoints already accept service_id — no backend change needed there.

5.2 Frontend (most of the work)

  1. api/client.ts + api/backups.ts — each fetch* function gains an optional serviceId?: string arg; when provided, append ?service_id=<id> (or &service_id=<id> if other params exist) to the URL.
  2. hooks/useObservability.ts + hooks/useBackups.ts — each hook gains a serviceId?: string arg; include it in the queryKey (e.g., ["observability", "alerts", serviceId ?? ""]) so caches don't collide across instances; pass it to the fetch function.
  3. pages/service-tabs/AlertsTab.tsx, MetricsTab.tsx, JobsTab.tsx — replace the TODO comments with useXxx(instance.id).
  4. Tests: hook tests cover the per-instance queryKey; tab tests cover passing instance.id.

5.3 Backward compatibility

  • All new params are optional (serviceId?: string / service_id: str | None = None). When omitted, behavior is identical to today (first-configured/global). This means the existing dashboard widget callers (which don't pass serviceId because they resolve via widget-instance binding) and any external callers keep working unchanged.

6. Success Criteria / Acceptance Criteria

  1. The three tabs (AlertsTab, MetricsTab, JobsTab) pass instance.id to their hooks; the TODO comments are gone.
  2. With two instances of a service type configured, viewing instance A's page shows A's data and viewing instance B's page shows B's data (no cross-contamination).
  3. The instance switcher on ServicePage re-scopes the tab data on switch.
  4. Hook queryKeys include serviceId so React Query caches are per-instance (no stale cross-instance cache hits).
  5. Backend backup endpoints accept service_id and filter correctly; omitting it returns all (backward-compat).
  6. Dashboard widgets render identically before/after (they don't use these hooks — verify no regression).
  7. Backend tests (pytest) and frontend npm run build + npm run lint + npm run test stay green.

7. Risks and Mitigations

Risk Mitigation
Cache-key collision if serviceId is forgotten in some hook. Mandatory: every modified hook includes serviceId in queryKey; covered by a hook test.
Backup store filter regresses existing callers. list_backup_* with service_id=None must return everything. Add a backend test: service_id=None → all rows; service_id="X" → only X's rows.
A fetch function drops the param into the URL incorrectly (encoding, ? vs &). Use URLSearchParams or the existing pattern; one small util if helpful.
Dashboard widget regression (they use different hooks, but defensive check). SC6 explicitly verifies widgets render identically; widget tests unchanged.
Review budget (>400 lines). Single slice, likely ~250350 lines (frontend-dominant). If it creeps, split backend-filter from frontend-wiring.

8. Resolved Questions (no question round needed — investigation settled these)

  • Q1 — Which hooks? useAlertmanagerAlerts, useAlertmanagerStatus, usePrometheusStatus, useBackupJobs, useBackupRuns, useBackupAlerts. NOT usePrometheusTargets (global scrape config) or useMonitoringMachines (global).
  • Q2 — Backend readiness? Alertmanager/Prometheus status endpoints already take service_id. Backup endpoints need it added (data already attributed).
  • Q3 — Backward-compat? All params optional; omitting = today's behavior.

9. Future Phases

  1. Per-instance scoping for usePrometheusTargets if it ever becomes a UI concern (today it's an external-scrape endpoint).
  2. A shared useServiceScopedQuery helper if the per-instance queryKey pattern repeats beyond these hooks.
  3. Rich PromQL editor / threshold alerting (separate deferred items, not this change).