chore(prometheus-direct-charting): archive verified+synced change
Move to openspec/changes/archive/2026-07-08-prometheus-direct-charting/ (git mv, history preserved). 9 artifacts: proposal/spec/design/tasks/ apply-progress/verify-report/sync-report/archive-report + delta spec. Canonical openspec/specs/prometheus-charting/ remains.
This commit is contained in:
Developer
@@ -0,0 +1,54 @@
|
||||
# Apply Progress: Prometheus Direct Charting
|
||||
|
||||
**Change:** `prometheus-direct-charting`
|
||||
**Phase:** apply-progress
|
||||
**Date:** 2026-07-08
|
||||
**Status:** complete — all 39 tasks done, all gates green, verified (see `verify-report.md`)
|
||||
|
||||
## Slices delivered
|
||||
|
||||
Three slices, each its own commit, each leaving `pytest` / `npm run build` / `npm run lint` / `ruff` green.
|
||||
|
||||
### Slice 1 — Prometheus range query + chart rebrand (commit `5dad982`, amended)
|
||||
|
||||
- Created `backend/src/media_library_viewer_api/widgets/prometheus_range.py` with `step_for_window(s)` (`max(15, round(s/200))` → ~200 pts/window) and `normalize_prometheus_matrix(result)` (shared label rule extracted from the to-be-removed Grafana path; drops `__name__`/`__*`, joins sorted `k=v`, falls back to `"value"`, dedups with `(n)`).
|
||||
- Extended `PrometheusWidgetSource.fetch` to dispatch `widget_kind == "chart"` → `_fetch_chart` hitting `/api/v1/query_range` directly, returning `{series:[...]}` (SC-101..104). Grafana path left intact at this slice.
|
||||
- Declared `chart` widget kind in `integrations/prometheus.py` (config: `promql`, `window` ∈ `{1h,6h,24h,7d}`).
|
||||
- `git mv GrafanaChartWidget.tsx → PrometheusChartWidget.tsx` (recharts body preserved verbatim; empty-state copy updated); `git mv` of its test. Rebound `chart` grafana→prometheus in both registries (SC-105..108).
|
||||
- Backend tests: new `test_prometheus_range.py` (step + normalization); chart-adapter test in `test_widgets.py`. Frontend registry test updated.
|
||||
|
||||
### Slice 2 — Gauge + mean widgets (commit `58be6e0`, amended)
|
||||
|
||||
- Extracted shared `_instant_query` helper; added `_fetch_gauge` (instant → scalar, multi-series → `{error}`) and `_fetch_mean` (range query over preset → client-side arithmetic mean of non-null values, scalar-only).
|
||||
- Declared `gauge` (`promql`, `warn_at`/`crit_at`/`min`/`max`/`unit`) and `mean` (`promql`, `window`, `unit`) kinds in `integrations/prometheus.py`.
|
||||
- Created `PrometheusGaugeWidget.tsx` (recharts `RadialBarChart`; green/amber/red threshold bands when `warn_at`+`crit_at` set; neutral single track otherwise) and `PrometheusMeanWidget.tsx` (MetricCard-style single value). Wired both into the frontend `prometheus` binding + barrel.
|
||||
- Tests: backend adapter tests (scalar-only enforcement, mean aggregation incl. NaN-skip, error cases); frontend component tests (error + rendered, gauge with/without bands, mean with/without unit).
|
||||
|
||||
### Slice 3 — Grafana removal + config + changelog (commit `ba94317`, amended)
|
||||
|
||||
- Deleted `integrations/grafana.py`, `GrafanaLinkWidget.tsx`, `service-tabs/LinksTab.tsx` (+ test). Removed `GrafanaWidgetSource` + adapter registration; `grafana` from `SERVICE_DEFINITIONS`/`SERVICE_ADAPTERS` (BE) and `SERVICE_REGISTRY`/`BUILTIN_WIDGETS` (FE); `get_grafana_status` endpoint; `useGrafanaStatus`/`fetchGrafanaStatus`/`GrafanaStatus`; nav entry; `service-tabs/index.ts` grafana case; `Dashboard.tsx` `OBSERVABILITY_TYPES` grafana member; `ServicesPage.tsx` empty-state copy; grafana tests.
|
||||
- Rewrote `openspec/config.yaml`: removed stale "Do NOT re-implement charting in-app" + "No recharts/d3" claims; states Manage renders Prometheus-backed metrics directly via recharts and that Grafana is no longer integrated.
|
||||
- Added `CHANGELOG.md` `[Unreleased]` entry: **BREAKING** — Grafana service type removed; migrate by deleting grafana instances and recreating as Prometheus; `grafana/chart` widgets → `prometheus/chart`.
|
||||
- Net: **−920 lines** across 27 files.
|
||||
|
||||
### Coverage close — SC-125 loading-state tests
|
||||
|
||||
- Added one `it("renders skeleton while loading")` case to each of the three Prometheus widget test files, asserting the `Skeleton` (`data-slot="skeleton"`) renders under `{ data: undefined, isLoading: true }`. Closes the PARTIAL finding on SC-125.
|
||||
|
||||
## Deviations from tasks.md
|
||||
|
||||
- None functional. The only textual drift is SC-118: `ObservabilityPage.tsx` had already been refactored into `service-tabs/` (the project map was stale). Removal targets adjusted to the real files (`LinksTab.tsx`, `service-tabs/index.ts`, `navEntries.ts`, `Dashboard.tsx`, `ServicesPage.tsx`); the spec was patched (SC-116/SC-118) to reflect this before apply. SC-118's *intent* (no Grafana UI surface) is fully satisfied.
|
||||
|
||||
## Final gate results (re-run after coverage close)
|
||||
|
||||
| Gate | Result |
|
||||
|---|---|
|
||||
| `backend && PYTHONPATH=src python3 -m pytest -q` | **293 passed**, 2 warnings (pre-existing pythonjsonlogger DeprecationWarning) |
|
||||
| `backend && PYTHONPATH=src python3 -m ruff check src tests` | **All checks passed** |
|
||||
| `frontend && npm run build` (`tsc -b` + `vite build`) | **exit 0** (pre-existing chunk-size warning) |
|
||||
| `frontend && npm run lint` | **0 errors**, 1 pre-existing warning (`WidgetConfigDialog.tsx:370`, untouched) |
|
||||
| `frontend && npx vitest run` (3 Prom widget tests) | **14 passed** (11 original + 3 new loading) |
|
||||
|
||||
## Verification
|
||||
|
||||
See `verify-report.md` — adversarial fresh-context review: **26/27 fully PASS, 1 PARTIAL→PASS** (SC-125 closed here). No blocking findings remain.
|
||||
@@ -0,0 +1,204 @@
|
||||
# Archive Report — `prometheus-direct-charting`
|
||||
|
||||
> Phase: **archive** · Change: `prometheus-direct-charting` · Repo: `/home/user/manage`
|
||||
> Mode: file-backed (`artifactStore: openspec`). No source-code edits; only OpenSpec artifacts
|
||||
> were touched. **Not committed** — the parent/orchestrator owns the archive commit. No push, no `gh`.
|
||||
|
||||
**Status: ARCHIVED.** All eight lifecycle phases are complete (proposal → spec → design → tasks →
|
||||
apply → verify → sync → **archive**). Every archive precondition is verified PASS (see §2). The
|
||||
canonical `openspec/specs/prometheus-charting/spec.md` (created by `sdd-sync`) remains in place as
|
||||
the durable end-state spec and is **not** moved (archive never moves canonical specs). The change
|
||||
folder was moved to `openspec/changes/archive/2026-07-08-prometheus-direct-charting/` via `git mv`
|
||||
to preserve history.
|
||||
|
||||
---
|
||||
|
||||
## 0. Archive disposition
|
||||
|
||||
- **Disposition: `archived`.** The folder move was performed inline as instructed (unlike a
|
||||
`documented-pending-manual` outcome): the parent explicitly requested the `git mv` and owns the
|
||||
commit, so the move is executed here and left staged for the parent's explicit-path commit.
|
||||
- **Archive convention:** OpenSpec SDD archive contract for `openspec` mode — completed file-backed
|
||||
sync → write the in-folder archive report → move the change folder to
|
||||
`openspec/changes/archive/YYYY-MM-DD-{change}/`. No standalone manifest/index exists under
|
||||
`openspec/` (only `config.yaml`, `changes/`, `specs/`), so the folder move **is** the archive
|
||||
mechanism. No `rules.archive` override exists in `openspec/config.yaml`.
|
||||
- **Target archived path:** `openspec/changes/archive/2026-07-08-prometheus-direct-charting/`
|
||||
- **Archive date:** `2026-07-08` (ISO).
|
||||
- **Canonical spec left in place (not moved):** `openspec/specs/prometheus-charting/spec.md` —
|
||||
27 requirements (SC-101 … SC-127). Verified present and untouched after the move.
|
||||
- **`openspec/specs/web-ui/spec.md` also left untouched** (the other canonical domain).
|
||||
- **Audit-trail integrity:** the change folder was moved as a whole, including the legacy flat
|
||||
`spec.md` and the per-domain delta `specs/prometheus-charting/spec.md`, which travel with the
|
||||
record. Nothing was silently deleted or rewritten. The flat `spec.md` is retained as the
|
||||
authoritative planning artifact the work was built against.
|
||||
|
||||
## 1. Native `sdd-status` read & discrepancy statement
|
||||
|
||||
The native `gentle-pi.sdd-status` engine supplied by the parent reports **non-actionable state for
|
||||
this archive** because it was resolved without a change context: `changeName: null`,
|
||||
`artifacts: all missing`, `applyState: blocked`, `dependencies.archive: blocked`,
|
||||
`blockedReasons: ["Change selection is ambiguous: mobile-responsive-parity,
|
||||
prometheus-direct-charting, service-storage-harness, services-as-hub-ia."]`,
|
||||
`isNonAuthoritative: false`. This is a **parent-resolution artifact**: the engine auto-detected four
|
||||
active changes and could not pick one. The ambiguity does **not** reflect the state of
|
||||
`prometheus-direct-charting`, which this archive task was **explicitly assigned**.
|
||||
|
||||
**Discrepancy with the parent's authoritative confirmed state — RESOLVED in favor of the parent.**
|
||||
The parent physically verified (and this executor re-confirmed directly against the filesystem in
|
||||
§2) that all 39 tasks are ticked, `apply-progress.md` exists and records the work, and the verify
|
||||
report confirms functional completion with no blocking findings. Per the archive contract's
|
||||
non-authoritative-store carve-out guidance and the parent's explicit instruction, the stale
|
||||
`archive: blocked` / "ambiguous" labels are **disregarded** and the archive **proceeds**.
|
||||
|
||||
Direct filesystem re-validation (§2) is the source of truth for this report.
|
||||
|
||||
## 2. Archive preconditions (validated directly against the filesystem)
|
||||
|
||||
| Precondition | Evidence | Result |
|
||||
|---|---|---|
|
||||
| Verify report present | `verify-report.md` | ✓ verdict **PASS** |
|
||||
| Verify clearly passing — no unresolved `FAIL`/`BLOCKED`/`CRITICAL` | verify-report §9: the sole CRITICAL was an archive-only checkbox/apply-progress gap (now reconciled); SC-125 PARTIAL→PASS closed | ✓ |
|
||||
| Sync report present & successful | `sync-report.md` → **Status: SYNCED** | ✓ |
|
||||
| Canonical spec exists (sync target) | `openspec/specs/prometheus-charting/spec.md` (27 requirements) | ✓ |
|
||||
| Change-side domain delta exists | `specs/prometheus-charting/spec.md` | ✓ |
|
||||
| Delta op-class = pure `## ADDED` (non-destructive) | ADDED=1, MODIFIED=0, REMOVED=0, RENAMED=0 (new domain) | ✓ |
|
||||
| Requirement-ID parity (flat ↔ delta ↔ canonical) | 27 == 27 == 27, identical IDs SC-101…SC-127 | ✓ |
|
||||
| proposal / design / tasks artifacts present | all populated | ✓ |
|
||||
| **Final Task Completion Gate — zero unchecked `- [ ]`** | `grep -nE '^\s*- \[ \]' tasks.md` → **NONE**; `grep -cE '^\s*- \[x\]'` → **39** | ✓ |
|
||||
| `apply-progress.md` present & records the work | present; status "complete — all 39 tasks done", 3 slices + coverage close documented | ✓ |
|
||||
| No active same-domain (`prometheus-charting`) collision | only this change carries a `prometheus-charting` delta | ✓ |
|
||||
|
||||
**Stale-checkbox reconciliation note.** At verify time, 19 implementation/verification checkboxes
|
||||
(Slice 3 §3.1–3.14 and Integration §4.1–4.5) were unchecked and `apply-progress.md` did not exist.
|
||||
That condition was reconciled **before** archive: the boxes are now all ticked and
|
||||
`apply-progress.md` was authored documenting the three landed slices and the SC-125 coverage close.
|
||||
`apply-progress.md` plus the verify report prove every previously-unchecked task complete. No
|
||||
archive-time mechanical checkbox repair was needed — the gate now passes on the persisted
|
||||
`tasks.md` as-is. No partial-archive approval applies.
|
||||
|
||||
## 3. Artifacts read (archive preflight)
|
||||
|
||||
- `openspec/changes/prometheus-direct-charting/proposal.md`
|
||||
- `openspec/changes/prometheus-direct-charting/spec.md` (flat, authoritative planning artifact — 27 requirements)
|
||||
- `openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md` (change-side domain delta)
|
||||
- `openspec/changes/prometheus-direct-charting/design.md`
|
||||
- `openspec/changes/prometheus-direct-charting/tasks.md`
|
||||
- `openspec/changes/prometheus-direct-charting/apply-progress.md`
|
||||
- `openspec/changes/prometheus-direct-charting/verify-report.md`
|
||||
- `openspec/changes/prometheus-direct-charting/sync-report.md`
|
||||
- `openspec/specs/prometheus-charting/spec.md` (canonical, sync target — verified present and untouched)
|
||||
- `openspec/config.yaml` (rules: proposal/tasks; no `rules.archive` override)
|
||||
|
||||
> The legacy flat `spec.md` is **not** the only spec artifact: a per-domain delta
|
||||
> (`specs/prometheus-charting/spec.md`) and a canonical spec both exist, so the "legacy flat spec
|
||||
> as the *only* artifact" archive-block condition does not apply. The flat spec travels with the
|
||||
> archived folder as part of the audit trail.
|
||||
|
||||
## 4. Domains synced & requirement delta
|
||||
|
||||
| Domain | Change-side delta | Canonical | Action |
|
||||
|---|---|---|---|
|
||||
| `prometheus-charting` | `specs/prometheus-charting/spec.md` | `openspec/specs/prometheus-charting/spec.md` | **NEW domain** — pure ADDED (27 requirements) |
|
||||
|
||||
- **ADDED (27)** — all to the new `prometheus-charting` domain (canonical did not exist pre-change).
|
||||
IDs and text preserved verbatim from the verified flat `spec.md`. Grouped logically:
|
||||
- *Direct Prometheus range query path* — SC-101, SC-102, SC-103, SC-104
|
||||
- *Prometheus chart widget (rebrand + rebind)* — SC-105, SC-106, SC-107, SC-108
|
||||
- *Prometheus gauge widget* — SC-109, SC-110, SC-111
|
||||
- *Prometheus mean widget* — SC-112, SC-113, SC-114
|
||||
- *Grafana removal* — SC-115, SC-116, SC-117, SC-118, SC-119, SC-120
|
||||
- *Configuration documentation accuracy* — SC-121, SC-122
|
||||
- *Test and build greenness* — SC-123, SC-124, SC-125
|
||||
- *Migration guidance* — SC-126, SC-127
|
||||
- **MODIFIED (0)** · **REMOVED (0)** · **RENAMED (0)** — new domain; nothing destructive.
|
||||
|
||||
> No destructive-merge guard or parent approval was triggered (zero REMOVED / zero MODIFIED). The
|
||||
> new `prometheus-charting` domain is distinct from the existing `web-ui` canonical domain.
|
||||
|
||||
## 5. Final lifecycle status (all 8 phases done)
|
||||
|
||||
| Phase | Status | Evidence |
|
||||
|---|---|---|
|
||||
| Proposal | ✅ done | `proposal.md` |
|
||||
| Spec | ✅ done | flat `spec.md` (27) + domain delta `specs/prometheus-charting/spec.md` (27 ADDED) |
|
||||
| Design | ✅ done | `design.md` |
|
||||
| Tasks | ✅ done | `tasks.md` — **39/39** checked, zero `- [ ]` |
|
||||
| Apply | ✅ done | 3 slices delivered (commits `5dad982`, `65bae95`/`58be6e0`, `67ca0fc`/`ba94317`) |
|
||||
| Verify | ✅ PASS | `verify-report.md` — 26/27 PASS, SC-125 PARTIAL→PASS; gates green |
|
||||
| Sync | ✅ done | `sync-report.md` — SYNCED; canonical `prometheus-charting` domain created |
|
||||
| Archive | ✅ done | this report + folder move performed |
|
||||
|
||||
## 6. Gate results (run at head `67ca0fc`; per verify-report)
|
||||
|
||||
| Gate | Command | Result |
|
||||
|---|---|---|
|
||||
| Backend tests | `cd backend && PYTHONPATH=src python3 -m pytest -q` | **PASS** — 293 passed (2 pre-existing warnings) |
|
||||
| Backend lint | `cd backend && PYTHONPATH=src python3 -m ruff check src tests` | **PASS** — All checks passed |
|
||||
| Frontend build | `cd frontend && npm run build` | **PASS** — exit 0 (pre-existing chunk-size warning) |
|
||||
| Frontend lint | `cd frontend && npm run lint` | **PASS** — 0 errors (1 pre-existing warning) |
|
||||
|
||||
## 7. Carry-over follow-ups (non-blocking; recorded per task)
|
||||
|
||||
1. **[INFO] Stale generated `.pi-map.md` still references Grafana** — generated project-map artifacts
|
||||
(`.pi-map.md` / `.pi-map.index.md`) still mention Grafana / `ObservabilityPage.tsx` /
|
||||
`GrafanaLinkWidget`. These are **generated artifacts, not deliverable source**, and are explicitly
|
||||
ignored by SC-115/SC-116 (which scope only `backend/src`/`frontend/src` source). **Out of scope
|
||||
for this change** — regenerate via `project_map_patch` / `project_map_validate` in a separate
|
||||
housekeeping pass; the project-map protocol already flags these `dirty`.
|
||||
2. **[INFO] SC-125 was PARTIAL→closed.** Verify found SC-125 (new widget kinds have tests) PARTIAL
|
||||
because no explicit `isLoading:true` loading-state test existed in any of the three Prometheus
|
||||
widget test files — a coverage gap, not a functional defect. The gap was **closed** before
|
||||
archive: `apply-progress.md` documents adding one `renders skeleton while loading` case to each
|
||||
of `PrometheusChartWidget` / `PrometheusGaugeWidget` / `PrometheusMeanWidget`, asserting the
|
||||
`Skeleton` renders under `{ data: undefined, isLoading: true }`. SC-125 is now PASS.
|
||||
3. **[INFO] `config.yaml` context block** still names `frontend/src/components/ObservabilityPage.tsx`
|
||||
(refactored away into `service-tabs/`). Not an SC-121 criterion (which targets the
|
||||
charting/grafana claims — those are fixed); minor doc staleness. Out of scope.
|
||||
4. **[INFO] Slice-2 review-budget variance** — slice 2 (~707 insertions) exceeded the ~310–400
|
||||
forecast, but is additive feature code + tests (gauge + mean); the boundary is the feature, not
|
||||
scope creep. Non-blocking; recorded for the record.
|
||||
5. **[INFO] Unrelated dirty working-tree items** predating/orthogonal to this change (an uncommitted
|
||||
cosmetic reformat of `frontend/src/pages/service-tabs/MediaTab.tsx`, untracked `.pi-tmp/*`, and
|
||||
the separate `service-storage-harness` proposal folder) were **not touched** by this archive.
|
||||
SC-127 (independence from `service-storage-harness`) holds.
|
||||
|
||||
## 8. Residual risks & destructive-merge statement
|
||||
|
||||
- **Destructive sync / merge:** **not applicable.** Zero REMOVED and zero MODIFIED requirements
|
||||
(new `prometheus-charting` domain; pure ADDED). No destructive-merge guard or parent approval was
|
||||
triggered.
|
||||
- **Backend / data-contract impact:** none — Grafana was fully excised from live code paths
|
||||
(service type, adapters, widgets, hook, API client, type, status endpoint, nav entry, service
|
||||
tab); Prometheus is the direct chart source via `/api/v1/query_range`. The `metric` widget data
|
||||
shape is preserved. Archive touched only OpenSpec docs + the folder move.
|
||||
- **No critical verification issues** remain (CRITICAL issues are non-overridable; the one verify
|
||||
CRITICAL was the reconcilable checkbox/apply-progress gap, now resolved).
|
||||
- **No browser/visual smoke** was performed (out of scope); the recharts `RadialBarChart` gauge and
|
||||
`LineChart` rendering are structurally tested only.
|
||||
- **Memory observation IDs:** none — `artifactStore: openspec`; traceability lives in the
|
||||
filesystem archive + canonical spec.
|
||||
|
||||
## 9. Move performed
|
||||
|
||||
```
|
||||
git mv openspec/changes/prometheus-direct-charting openspec/changes/archive/2026-07-08-prometheus-direct-charting
|
||||
```
|
||||
|
||||
- **All 9 artifacts confirmed present at the archived path:** `proposal.md`, `spec.md`,
|
||||
`specs/prometheus-charting/spec.md` (delta), `design.md`, `tasks.md`, `apply-progress.md`,
|
||||
`verify-report.md`, `sync-report.md`, `archive-report.md` (this file).
|
||||
- **Canonical `openspec/specs/prometheus-charting/spec.md` remains in place** (verified untouched
|
||||
after the move). `openspec/specs/web-ui/spec.md` also untouched.
|
||||
|
||||
---
|
||||
|
||||
### Appendix — Files written/moved by this archive (OpenSpec only; no source code)
|
||||
|
||||
- **Written:** `openspec/changes/prometheus-direct-charting/archive-report.md` (this file) — at the
|
||||
active path before the move; travels with the move into the archive.
|
||||
- **Moved (via `git mv`):** the entire
|
||||
`openspec/changes/prometheus-direct-charting/` directory →
|
||||
`openspec/changes/archive/2026-07-08-prometheus-direct-charting/`.
|
||||
- **Left in place (durable canonical):** `openspec/specs/prometheus-charting/spec.md`.
|
||||
- **Not committed / not pushed** — the parent owns the commit with explicit paths.
|
||||
@@ -0,0 +1,462 @@
|
||||
# SDD Design: Prometheus Direct Charting (drop Grafana middleman)
|
||||
|
||||
**Change:** `prometheus-direct-charting`
|
||||
**Phase:** design
|
||||
**Date:** 2026-07-08
|
||||
|
||||
## 0. Source findings (read before anything else)
|
||||
|
||||
The proposal and spec were written against a **stale project map**. Reading actual source surfaced deviations the design must account for. Trust source, not the map.
|
||||
|
||||
| Spec claim | Actual source reality | Design impact |
|
||||
|---|---|---|
|
||||
| SC-118: "remove the Grafana branch from `ObservabilityPage`" | **`ObservabilityPage.tsx` no longer exists.** It was refactored into a per-service-type `service-tabs/` architecture (confirmed by `LinksTab.tsx` docstring: "Lifts the Grafana deep-link content from the old cross-service ObservabilityPage into an instance-scoped tab"). The stale reference survives only in `.pi-map.md` files. | Removal targets are `service-tabs/LinksTab.tsx` + its test, the `grafana` case in `service-tabs/index.ts`, the `navEntries.ts` grafana entry, `Dashboard.tsx`'s `OBSERVABILITY_TYPES` set, `ServicesPage.tsx` empty-state text, and the `useGrafanaStatus` hook. See §5. |
|
||||
| "No recharts/d3 in use" (config rule to repeal) | `recharts ^3.9.2` is **declared, installed, and imported** by `GrafanaChartWidget.tsx`. Confirmed. | Repeal is a doc fix matching reality; recharts is the sanctioned renderer. |
|
||||
| Proposal §5.1: "extend `PrometheusWidgetSource` ... to handle `chart`" | `PrometheusWidgetSource.fetch` currently only does instant `/api/v1/query` and returns `{"result": data}`. The frontend `PrometheusMetricWidget` consumes `data.result`. | The chart/mean paths return a **different** shape (`{series}` / `{value}`); dispatch inside `.fetch()` by `widget_kind`. See §2.2. |
|
||||
| Map said registry has 7 service types | Source registry has **8**: `alertmanager, authentik, backups, grafana, jellyfin, nextcloud, prometheus, ssh_tasks`. (`authentik` and `backups` were added after the map froze.) | No design impact beyond acknowledging grafana is one of eight, not one of seven. |
|
||||
| Frontend `SERVICE_REGISTRY` (registry.ts) | Has a `grafana` binding with `link` + `chart` kinds, and a `prometheus` binding with only `metric`. The `chart` kind must move grafana→prometheus. | Confirmed; §3.2 details the rebinding. |
|
||||
|
||||
No proposal/spec scope change is required — the *intent* (remove Grafana, direct Prom charting) still holds. Only the **removal targets** differ from what SC-118 literally names. This is flagged explicitly so the tasks phase and reviewer aren't surprised.
|
||||
|
||||
---
|
||||
|
||||
## 1. Architecture overview
|
||||
|
||||
This change cuts Grafana out of the chart data path and makes Prometheus the direct source. The existing recharts renderer is reused unchanged for `chart`; two new widget kinds (`gauge`, `mean`) are added to the `prometheus` service. Grafana is removed entirely.
|
||||
|
||||
```
|
||||
BEFORE AFTER
|
||||
─────── ─────
|
||||
WidgetData fetch WidgetData fetch
|
||||
└► GrafanaWidgetSource └► PrometheusWidgetSource
|
||||
└► POST {grafana}/api/ds/query ├► kind=metric → /api/v1/query {result} [unchanged]
|
||||
(datasource.type=prometheus) ├► kind=chart → /api/v1/query_range {series} [NEW]
|
||||
└► normalize frames → {series} ├► kind=gauge → /api/v1/query {value,...} [NEW]
|
||||
└► kind=mean → /api/v1/query_range {value} [NEW]
|
||||
|
||||
Frontend GrafanaChartWidget (recharts) Frontend PrometheusChartWidget (recharts) [renamed, reused]
|
||||
+ PrometheusGaugeWidget (recharts RadialBarChart) [NEW]
|
||||
+ PrometheusMeanWidget (MetricCard-style) [NEW]
|
||||
|
||||
grafana service type / LinkWidget / [REMOVED entirely]
|
||||
LinksTab / navEntry / status endpoint
|
||||
```
|
||||
|
||||
**Key constraints carried from the spec:**
|
||||
|
||||
- Reuse the recharts chart renderer unchanged (SC-106) — the rename is structural.
|
||||
- Shared series normalization (SC-102) — one helper, no duplication.
|
||||
- Step derived from window presets (SC-104), users never set `step`.
|
||||
- `gauge`/`mean` are scalar-only (SC-111, SC-114); `chart` stays multi-series (SC-107).
|
||||
- Window presets: `1h`, `6h`, `24h`, `7d` (SC-108, SC-112).
|
||||
- All adapter errors return `{"error": str}`, never raise (SC-103).
|
||||
- No Grafana references remain (SC-115, SC-116, SC-117, SC-120).
|
||||
|
||||
---
|
||||
|
||||
## 2. Backend design
|
||||
|
||||
### 2.1 New module: `widgets/prometheus_range.py` (shared helpers)
|
||||
|
||||
Holds the two pieces the spec's downstream-notes asked for: the step-derivation function (SC-104) and the shared normalization helper (SC-102). Putting them in their own module (rather than inside `sources.py`) makes them unit-testable in isolation and reusable by the future `service-storage-harness` change's in-service data path, without `sources.py` growing unbounded.
|
||||
|
||||
**Window presets and step derivation (SC-104):**
|
||||
|
||||
```python
|
||||
WINDOW_PRESETS: dict[str, int] = {
|
||||
"1h": 3_600,
|
||||
"6h": 21_600,
|
||||
"24h": 86_400,
|
||||
"7d": 604_800,
|
||||
}
|
||||
|
||||
# Target ~200 points per window. Step is clamped to >= 15s so Prometheus
|
||||
# doesn't reject sub-15s resolutions on high-cardinality queries.
|
||||
def step_for_window(window_seconds: int, target_points: int = 200) -> int:
|
||||
return max(15, round(window_seconds / target_points))
|
||||
```
|
||||
|
||||
Resulting table (verified, all within the 100–300 target band):
|
||||
|
||||
| Preset | Window (s) | Derived step (s) | Points |
|
||||
|--------|-----------|------------------|--------|
|
||||
| `1h` | 3,600 | `max(15, round(3600/200))` = 18 | 200 |
|
||||
| `6h` | 21,600 | 108 | 200 |
|
||||
| `24h` | 86,400 | 432 | 200 |
|
||||
| `7d` | 604,800 | 3,024 | 200 |
|
||||
|
||||
(At apply time the implementer may round steps to "nicer" values like 15/60/300/1800 for cache-friendliness; the spec only requires the 100–300 band, which the formula satisfies. The formula is the source of truth; the table is illustrative.)
|
||||
|
||||
**Shared normalization (SC-102) — `normalize_prometheus_matrix`:**
|
||||
|
||||
```python
|
||||
def normalize_prometheus_matrix(
|
||||
result: list[dict[str, Any]],
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Turn a Prometheus /api/v1/query_range `data.result` matrix into
|
||||
`{label, points:[{t:int, v:float|None}]}` series — the exact shape the
|
||||
frontend chart renderer consumes.
|
||||
|
||||
Label derivation reuses the rule from the removed Grafana path:
|
||||
1. Drop __name__ from metric labels.
|
||||
2. If labels remain, join as `k=v k=v`.
|
||||
3. Else fall back to "value".
|
||||
4. Dedup collisions with a ` (n)` suffix.
|
||||
"""
|
||||
series: list[dict[str, Any]] = []
|
||||
seen: dict[str, int] = {}
|
||||
for entry in result:
|
||||
metric = entry.get("metric") or {}
|
||||
values = entry.get("values") or []
|
||||
parts = [f"{k}={v}" for k, v in sorted(metric.items()) if not k.startswith("__")]
|
||||
label = " ".join(parts) if parts else "value"
|
||||
if label in seen:
|
||||
seen[label] += 1
|
||||
label = f"{label} ({seen[label]})"
|
||||
else:
|
||||
seen[label] = 0
|
||||
points = []
|
||||
for ts, raw in values:
|
||||
v = float(raw) if raw not in (None, "NaN", "+Inf", "-Inf") else None
|
||||
points.append({"t": int(ts), "v": v})
|
||||
series.append({"label": label, "points": points})
|
||||
return series
|
||||
```
|
||||
|
||||
This is a **direct extraction** of the label/dedup logic currently inside `GrafanaWidgetSource._fetch_chart`, retargeted at the Prometheus matrix shape (`{metric, values:[[ts,"str"],...]}`) instead of Grafana frames. The dedup rule is identical so users moving a `grafana/chart` widget to `prometheus/chart` see the same labels.
|
||||
|
||||
### 2.2 `PrometheusWidgetSource` — extend `.fetch()` by `widget_kind`
|
||||
|
||||
Today `.fetch()` does only the instant-query → `{"result": ...}` path. Extend it to dispatch by `widget_kind` while preserving the existing `metric` behavior byte-for-byte:
|
||||
|
||||
```python
|
||||
class PrometheusWidgetSource:
|
||||
async def fetch(self, service, widget_kind, config) -> dict[str, Any]:
|
||||
if service is None:
|
||||
return {"error": "Prometheus widget is missing its service"}
|
||||
base_url = str(service.config.get("base_url") or "").rstrip("/")
|
||||
timeout = int(service.config.get("timeout_seconds") or 10)
|
||||
|
||||
if widget_kind == "metric":
|
||||
return await self._fetch_instant(base_url, timeout, config) # unchanged
|
||||
if widget_kind == "chart":
|
||||
return await self._fetch_chart(base_url, timeout, config) # NEW
|
||||
if widget_kind == "gauge":
|
||||
return await self._fetch_gauge(base_url, timeout, config) # NEW
|
||||
if widget_kind == "mean":
|
||||
return await self._fetch_mean(base_url, timeout, config) # NEW
|
||||
return {"error": f"Unknown widget kind: {widget_kind}"}
|
||||
```
|
||||
|
||||
**`_fetch_chart` (SC-101, SC-103, SC-104):**
|
||||
|
||||
```python
|
||||
async def _fetch_chart(self, base_url, timeout, config) -> dict[str, Any]:
|
||||
promql = config.get("promql")
|
||||
if not promql:
|
||||
return {"error": "promql is required"}
|
||||
window = WINDOW_PRESETS.get(config.get("window", "1h"), WINDOW_PRESETS["1h"])
|
||||
step = step_for_window(window)
|
||||
end = int(time.time())
|
||||
start = end - window
|
||||
try:
|
||||
resp = await asyncio.wait_for(asyncio.to_thread(
|
||||
requests.get, f"{base_url}/api/v1/query_range",
|
||||
params={"query": promql, "start": start, "end": end, "step": step},
|
||||
timeout=timeout,
|
||||
), timeout=timeout)
|
||||
resp.raise_for_status()
|
||||
payload = resp.json()
|
||||
except asyncio.TimeoutError:
|
||||
return {"error": "Prometheus query timed out"}
|
||||
except requests.RequestException as exc:
|
||||
return {"error": f"Prometheus query failed: {exc}"}
|
||||
result = payload.get("data", {}).get("result", [])
|
||||
return {"series": normalize_prometheus_matrix(result)}
|
||||
```
|
||||
|
||||
**`_fetch_gauge` (SC-109, SC-110, SC-111) — instant query, scalar-only:**
|
||||
|
||||
```python
|
||||
async def _fetch_gauge(self, base_url, timeout, config) -> dict[str, Any]:
|
||||
promql = config.get("promql")
|
||||
if not promql:
|
||||
return {"error": "promql is required"}
|
||||
raw = await self._instant_query(base_url, timeout, promql) # shared helper
|
||||
if "error" in raw:
|
||||
return raw
|
||||
result = raw["result"]
|
||||
if len(result) != 1:
|
||||
return {"error": "Gauge requires a single-series query; refine your PromQL"}
|
||||
# vector entry: {metric, value:[ts, "str"]}
|
||||
try:
|
||||
value = float(result[0]["value"][1])
|
||||
except (KeyError, IndexError, ValueError, TypeError):
|
||||
return {"error": "Gauge query returned no scalar value"}
|
||||
return {
|
||||
"value": value,
|
||||
"warn_at": config.get("warn_at"),
|
||||
"crit_at": config.get("crit_at"),
|
||||
"min": config.get("min"),
|
||||
"max": config.get("max"),
|
||||
"unit": config.get("unit"),
|
||||
}
|
||||
```
|
||||
|
||||
`_instant_query` is extracted from the current `metric` path so `metric`/`gauge` share it; it returns `{"result": [...]}` or `{"error": ...}`.
|
||||
|
||||
**`_fetch_mean` (SC-112, SC-113, SC-114) — range query, client-side mean, scalar-only:**
|
||||
|
||||
```python
|
||||
async def _fetch_mean(self, base_url, timeout, config) -> dict[str, Any]:
|
||||
promql = config.get("promql")
|
||||
if not promql:
|
||||
return {"error": "promql is required"}
|
||||
window = WINDOW_PRESETS.get(config.get("window", "1h"), WINDOW_PRESETS["1h"])
|
||||
step = step_for_window(window)
|
||||
end = int(time.time()); start = end - window
|
||||
# ... run query_range, handle errors identically to _fetch_chart ...
|
||||
result = payload.get("data", {}).get("result", [])
|
||||
if len(result) != 1:
|
||||
return {"error": "Mean requires a single-series query; refine your PromQL"}
|
||||
points = result[0].get("values") or []
|
||||
nums = [float(v) for _, v in points if v not in (None, "NaN", "+Inf", "-Inf")]
|
||||
if not nums:
|
||||
return {"error": "Mean query returned no numeric samples in the window"}
|
||||
mean = sum(nums) / len(nums)
|
||||
return {"value": mean, "unit": config.get("unit")}
|
||||
```
|
||||
|
||||
The range-query HTTP call + error handling is shared between `_fetch_chart` and `_fetch_mean` via a private `_range_query(base_url, timeout, promql, window) -> dict` returning `{"matrix": result}` or `{"error": ...}`. This keeps the chart and mean paths DRY without inventing a generic adapter registry (non-goal enforced).
|
||||
|
||||
### 2.3 `integrations/prometheus.py` — declare new widget kinds
|
||||
|
||||
Add two Pydantic widget-config models and two `widget_kind(...)` entries; leave `PrometheusMetricWidgetConfig` and the existing `metric` kind untouched (non-goal: `prometheus_metric` stays as-is):
|
||||
|
||||
```python
|
||||
class PrometheusChartWidgetConfig(WidgetConfigBase):
|
||||
promql: str
|
||||
window: str = "1h" # one of 1h/6h/24h/7d
|
||||
|
||||
class PrometheusGaugeWidgetConfig(WidgetConfigBase):
|
||||
promql: str
|
||||
warn_at: float | None = None
|
||||
crit_at: float | None = None
|
||||
min: float | None = None
|
||||
max: float | None = None
|
||||
unit: str | None = None
|
||||
|
||||
class PrometheusMeanWidgetConfig(WidgetConfigBase):
|
||||
promql: str
|
||||
window: str = "1h"
|
||||
unit: str | None = None
|
||||
```
|
||||
|
||||
`DEFINITION.widget_kinds` gains `chart`, `gauge`, `mean` (refresh 60s for chart/mean, 30s for gauge). Validation that `window ∈ {1h,6h,24h,7d}` can be a `field_validator` on the two models that use it, or simply enforced by `WINDOW_PRESETS.get(..., default)` server-side — prefer the latter (lenient) so a future preset addition doesn't require a model change.
|
||||
|
||||
### 2.4 `integrations/grafana.py` — delete; registry drops the entry
|
||||
|
||||
- Delete the file.
|
||||
- In `integrations/registry.py`, drop the `from ...grafana import DEFINITION as GRAFANA` import and the `GRAFANA.service_type: GRAFANA,` line from `SERVICE_DEFINITIONS`. No other registry change.
|
||||
|
||||
### 2.5 `widgets/sources.py` — drop Grafana; wire nothing new
|
||||
|
||||
- Delete `GrafanaWidgetSource` and its `_fetch_chart`.
|
||||
- Remove `"grafana": GrafanaWidgetSource(),` from `SERVICE_ADAPTERS`. (The label/dedup logic has already been *extracted* into `prometheus_range.normalize_prometheus_matrix` in §2.1; it is not lost when the Grafana class is deleted.)
|
||||
- `PrometheusWidgetSource` gains the three new methods from §2.2.
|
||||
|
||||
### 2.6 `routers/monitoring.py` — remove `get_grafana_status`
|
||||
|
||||
Delete the `@router.get("/grafana-status")` endpoint (lines ~175–195). No other monitoring change; `get_prometheus_status` / `get_alertmanager_status` stay.
|
||||
|
||||
### 2.7 No SettingsStore or DB schema change
|
||||
|
||||
Widget *instances* are stored generically (`service_id`, `widget_kind`, `config_json`). A `prometheus` `chart` instance is just a row with `service_id=<prom instance>` and `widget_kind="chart"`. Existing `grafana/chart` rows become orphans resolved by the existing "unknown widget" path (SC-119) — no migration code, no schema change (SC-126).
|
||||
|
||||
---
|
||||
|
||||
## 3. Frontend design
|
||||
|
||||
### 3.1 Rename + reuse: `GrafanaChartWidget` → `PrometheusChartWidget`
|
||||
|
||||
- Rename `frontend/src/widgets/GrafanaChartWidget.tsx` → `PrometheusChartWidget.tsx`; rename the exported function/component.
|
||||
- The recharts body (`LineChart`, `Line`, `XAxis`, `YAxis`, `CartesianGrid`, `Tooltip`, `ResponsiveContainer`, `mergeSeries`, `CHART_COLORS`, `formatTime`) is **preserved unchanged** (SC-106).
|
||||
- One tiny text fix: the empty-state Alert copy "Check your query and datasource_uid" becomes "Check your PromQL query and window." (`datasource_uid` no longer exists.)
|
||||
- Rename test file `__tests__/GrafanaChartWidget.test.tsx` → `PrometheusChartWidget.test.tsx`; update import + the one error-message assertion (`"Grafana api_key is required"` → a Prom error string).
|
||||
|
||||
### 3.2 `integrations/registry.ts` — rebind + add
|
||||
|
||||
- Delete the entire `grafana` key from `SERVICE_REGISTRY`.
|
||||
- In the `prometheus` binding's `widgets` array, add `chart`, `gauge`, `mean` alongside the existing `metric`. Each has a `configSchema` mirroring §2.3 (`promql`, `window` for chart/mean; `promql`, `warn_at`, `crit_at`, `min`, `max`, `unit` for gauge). `chart`/`mean` refresh 60s; `gauge` 30s.
|
||||
- Import the renamed `PrometheusChartWidget` and the two new components.
|
||||
|
||||
### 3.3 New: `PrometheusGaugeWidget.tsx` (SC-109, SC-110, SC-111)
|
||||
|
||||
**Renderer choice: recharts `RadialBarChart`.** Justification: recharts is already a dependency (no new dep), `RadialBarChart` renders a single-value gauge with domain bands natively, and it shares tooltip/styling conventions with the chart widget — keeping the two visualizations consistent. The alternative (a ~50-line bespoke SVG gauge) was rejected because it would introduce a second rendering dialect for no benefit; the proposal's "fall back to SVG if recharts proves heavy" fallback remains documented but is not the default.
|
||||
|
||||
Threshold-band rendering: render three stacked `RadialBar` cells (green `0→warn`, amber `warn→crit`, red `crit→max`) as the track, and a fourth cell (the actual value) as the needle/bar. When `warn_at`/`crit_at` are absent, render a single neutral-color track. `min`/`max` default to `0`/`max(value, 1)` when omitted so the gauge has a sane domain. The component reuses `SectionCard` + `Alert`/`Skeleton` for loading/error states, matching every other widget.
|
||||
|
||||
Config fields surfaced to the user (matching §2.3): `promql`, `warn_at`, `crit_at`, `min`, `max`, `unit`.
|
||||
|
||||
### 3.4 New: `PrometheusMeanWidget.tsx` (SC-112, SC-113, SC-114)
|
||||
|
||||
A single-value display reusing the existing `MetricCard` pattern (already used by `BackupDashboardWidget` / `PrometheusMetricWidget`-adjacent tiles): big number, optional `unit` suffix, optional subtext showing the window ("mean over last 1h"). Loading/error/empty states via `Skeleton`/`Alert` as usual. No charting library involvement — it's a number, deliberately.
|
||||
|
||||
### 3.5 Grafana removal on the frontend
|
||||
|
||||
Per the source findings (§0), the removal targets are **not** an `ObservabilityPage` section. They are:
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `widgets/GrafanaLinkWidget.tsx` | **delete** |
|
||||
| `widgets/GrafanaChartWidget.tsx` | **rename** to PrometheusChartWidget (§3.1) — not a pure delete |
|
||||
| `widgets/index.ts` | drop `GrafanaLinkWidget` export; rename the chart export |
|
||||
| `integrations/registry.ts` | drop `grafana` binding (§3.2) |
|
||||
| `integrations/navEntries.ts` | drop the `grafana` entry from `SERVICE_TYPE_NAV_ENTRIES`; drop the now-unused `Link2` icon import |
|
||||
| `hooks/useObservability.ts` | drop `useGrafanaStatus` + its `fetchGrafanaStatus` import |
|
||||
| `api/client.ts` | drop `fetchGrafanaStatus` |
|
||||
| `types/index.ts` | drop `GrafanaStatus` interface |
|
||||
| `pages/service-tabs/LinksTab.tsx` | **delete** (it is the grafana-specific tab; its `GrafanaLinkCard` + machine deep-links are Grafana-only) |
|
||||
| `pages/service-tabs/__tests__/LinksTab.test.tsx` | **delete** |
|
||||
| `pages/service-tabs/index.ts` | drop the `LinksTab` import and the `case "grafana":` from `serviceContentTabs` |
|
||||
| `pages/Dashboard.tsx` | drop `"grafana"` from `OBSERVABILITY_TYPES` set (line ~68) |
|
||||
| `pages/ServicesPage.tsx` | update empty-state copy "Add a Grafana, Prometheus, …" → "Add a Prometheus, …" |
|
||||
| `pages/__tests__/Dashboard.test.tsx` | the one "Grafana" label literal there is for a *shortcut* (a website link), unrelated to the Grafana service — **leave it** (grep-clean criterion SC-116 still passes; it's not a grafana service reference, just user-typed shortcut text in a test fixture). Flag for reviewer. |
|
||||
|
||||
**Spec-text note (not a design change):** SC-118 literally names "the ObservabilityPage Grafana section," which no longer exists. The *intent* of SC-118 ("Grafana status checks are removed") is satisfied by dropping `get_grafana_status` (§2.6) + `useGrafanaStatus`. The tasks phase should note this textual drift so the reviewer doesn't treat it as a missed requirement. If the parent prefers, SC-118 can be reworded in `spec.md` to name `LinksTab`/`useGrafanaStatus` instead; this design does not require that edit to proceed.
|
||||
|
||||
### 3.6 Types
|
||||
|
||||
`types/index.ts`: remove `GrafanaStatus`. No new widget-payload types — `chart` uses `{series}` (existing), `gauge` uses `{value, warn_at, crit_at, min, max, unit}` (all optional beyond `value`), `mean` uses `{value, unit?}`. These are read off `data?.data` untyped-as-before; no `WidgetDataResponse` generic change is needed (it's already `data: dict | None`).
|
||||
|
||||
---
|
||||
|
||||
## 4. Data flow
|
||||
|
||||
1. **Chart** — `useWidgetData(widgetId, 60_000)` → `GET /api/widgets/instances/{id}/data` → `PrometheusWidgetSource.fetch(kind="chart")` → `_fetch_chart` → `GET {prom}/api/v1/query_range?query=...&start=...&end=...&step=...` → `normalize_prometheus_matrix` → `{"series":[...]}` → `PrometheusChartWidget` renders recharts (unchanged).
|
||||
2. **Gauge** — `useWidgetData(widgetId, 30_000)` → `fetch(kind="gauge")` → `_fetch_gauge` → instant query → scalar-only check → `{"value":..., "warn_at":..., ...}` → `PrometheusGaugeWidget` renders RadialBarChart.
|
||||
3. **Mean** — `useWidgetData(widgetId, 60_000)` → `fetch(kind="mean")` → `_fetch_mean` → range query → client-side mean → `{"value":..., "unit":...}` → `PrometheusMeanWidget` renders a MetricCard.
|
||||
4. **Metric (unchanged)** — existing path preserved byte-for-byte.
|
||||
5. **Orphaned grafana widget** — `resolveWidget` finds no `grafana` binding → returns `undefined` → `WidgetInstanceCard` renders its existing "unknown widget" Alert (SC-119). No crash, no migration.
|
||||
|
||||
Errors at any step return `{"error": str}` (SC-103); the per-widget `Alert variant="destructive"` renders it and siblings keep polling.
|
||||
|
||||
---
|
||||
|
||||
## 5. Testing approach
|
||||
|
||||
### Backend
|
||||
|
||||
Extend `backend/tests/test_widgets.py` (and/or a focused `test_prometheus_range.py`):
|
||||
|
||||
- `normalize_prometheus_matrix`: feed a sample Prom `/api/v1/query_range` `data.result` (two entries, one with `__name__`, one colliding label) → assert `{series}` shape, label dedup, null handling for `"NaN"`.
|
||||
- `step_for_window`: assert the 1h/6h/24h/7d → step mapping stays within 100–300 points.
|
||||
- `PrometheusWidgetSource` chart path: mock `requests.get` → assert `query_range` URL + params (`start`/`end`/`step` present, no `from_ts`/`to_ts`) and `{"series": ...}` return.
|
||||
- Gauge scalar-only: mock instant query returning 2 series → assert `{"error": ...}`.
|
||||
- Mean scalar-only + client-side mean: mock range query returning 1 series with known values → assert the arithmetic mean; mock 2 series → assert error.
|
||||
- Error paths: timeout / `RequestException` → `{"error": ...}` (no raise).
|
||||
|
||||
### Frontend
|
||||
|
||||
- `PrometheusChartWidget.test.tsx` (renamed from GrafanaChartWidget test): loading, error, rendered-data cases (SC-125).
|
||||
- `PrometheusGaugeWidget.test.tsx`: loading, error, rendered-with-bands, rendered-without-bands.
|
||||
- `PrometheusMeanWidget.test.tsx`: loading, error, rendered value.
|
||||
- `registry.test.ts`: assert no `grafana` binding; assert `prometheus` binding has `metric`, `chart`, `gauge`, `mean`.
|
||||
|
||||
### Build/lint gates
|
||||
|
||||
Each slice: `PYTHONPATH=src pytest` (from `backend/`), `npm run build`, `npm run lint` must be green.
|
||||
|
||||
---
|
||||
|
||||
## 6. File-level plan
|
||||
|
||||
### Create
|
||||
|
||||
| File | Rationale |
|
||||
|------|-----------|
|
||||
| `backend/src/media_library_viewer_api/widgets/prometheus_range.py` | `WINDOW_PRESETS`, `step_for_window`, `normalize_prometheus_matrix` (SC-102, SC-104). |
|
||||
| `backend/tests/test_prometheus_range.py` | Unit tests for the helpers. |
|
||||
| `frontend/src/widgets/PrometheusGaugeWidget.tsx` | recharts RadialBarChart gauge (SC-109/110/111). |
|
||||
| `frontend/src/widgets/PrometheusMeanWidget.tsx` | MetricCard-style mean (SC-112/113/114). |
|
||||
| `frontend/src/widgets/__tests__/PrometheusGaugeWidget.test.tsx` | SC-125. |
|
||||
| `frontend/src/widgets/__tests__/PrometheusMeanWidget.test.tsx` | SC-125. |
|
||||
|
||||
### Rename (git mv)
|
||||
|
||||
| From → To | Rationale |
|
||||
|-----------|-----------|
|
||||
| `widgets/GrafanaChartWidget.tsx` → `widgets/PrometheusChartWidget.tsx` | SC-105/106. Body preserved. |
|
||||
| `widgets/__tests__/GrafanaChartWidget.test.tsx` → `widgets/__tests__/PrometheusChartWidget.test.tsx` | match rename. |
|
||||
|
||||
### Modify
|
||||
|
||||
| File | Rationale |
|
||||
|------|-----------|
|
||||
| `backend/.../widgets/sources.py` | Drop `GrafanaWidgetSource`; add chart/gauge/mean to `PrometheusWidgetSource`. |
|
||||
| `backend/.../integrations/prometheus.py` | Add 3 widget-config models + 3 `widget_kind` entries. |
|
||||
| `backend/.../integrations/registry.py` | Drop grafana import + entry. |
|
||||
| `backend/.../routers/monitoring.py` | Drop `get_grafana_status`. |
|
||||
| `backend/tests/test_widgets.py` | Drop grafana adapter tests; add prom chart/gauge/mean tests. |
|
||||
| `frontend/src/integrations/registry.ts` | Drop grafana binding; add chart/gauge/mean to prometheus. |
|
||||
| `frontend/src/integrations/navEntries.ts` | Drop grafana entry. |
|
||||
| `frontend/src/hooks/useObservability.ts` | Drop `useGrafanaStatus`. |
|
||||
| `frontend/src/api/client.ts` | Drop `fetchGrafanaStatus`. |
|
||||
| `frontend/src/types/index.ts` | Drop `GrafanaStatus`. |
|
||||
| `frontend/src/widgets/index.ts` | Drop GrafanaLinkWidget export; rename chart export. |
|
||||
| `frontend/src/pages/service-tabs/index.ts` | Drop LinksTab import + grafana case. |
|
||||
| `frontend/src/pages/Dashboard.tsx` | Drop "grafana" from `OBSERVABILITY_TYPES`. |
|
||||
| `frontend/src/pages/ServicesPage.tsx` | Update empty-state copy. |
|
||||
| `openspec/config.yaml` | Repeal stale thin-dashboard/no-recharts wording (SC-121). |
|
||||
| `CHANGELOG.md` | Migration note (SC-122). |
|
||||
|
||||
### Delete
|
||||
|
||||
| File | Rationale |
|
||||
|------|-----------|
|
||||
| `backend/.../integrations/grafana.py` | SC-117. |
|
||||
| `frontend/src/widgets/GrafanaLinkWidget.tsx` | SC-117. |
|
||||
| `frontend/src/pages/service-tabs/LinksTab.tsx` | Grafana-only tab; no Prom equivalent needed (chart widget covers viz). |
|
||||
| `frontend/src/pages/service-tabs/__tests__/LinksTab.test.tsx` | matches deletion. |
|
||||
|
||||
---
|
||||
|
||||
## 7. Slice boundaries (≤400 changed lines each)
|
||||
|
||||
Each slice leaves `pytest` + `npm run build` + `npm run lint` green and the app in a working state.
|
||||
|
||||
### Slice 1 — Prom range path + chart rebrand + shared helper (foundational)
|
||||
|
||||
**Files:** create `prometheus_range.py` + test; rename `GrafanaChartWidget`→`PrometheusChartWidget` (+ test); modify `sources.py` (`_fetch_chart`, drop nothing yet), `integrations/prometheus.py` (add `chart` kind only), `registry.ts` (move `chart` grafana→prometheus), `widgets/index.ts`, `registry.test.ts`.
|
||||
**State after S1:** a `prometheus/chart` widget renders from direct Prom `query_range`; the `grafana/chart` binding is gone but `GrafanaWidgetSource`/`GrafanaLinkWidget`/grafana service type still exist (removal is S3). Grafana link widgets still work. ~300–380 lines.
|
||||
|
||||
### Slice 2 — Gauge + mean widgets (additive)
|
||||
|
||||
**Files:** create `PrometheusGaugeWidget.tsx` (+test), `PrometheusMeanWidget.tsx` (+test); modify `sources.py` (`_fetch_gauge`, `_fetch_mean`, shared `_instant_query`/`_range_query`), `integrations/prometheus.py` (add `gauge`/`mean` kinds), `registry.ts` (add gauge/mean bindings), `test_widgets.py` (add adapter tests).
|
||||
**State after S2:** gauge + mean widgets selectable and rendering; no grafana change. ~300–380 lines.
|
||||
|
||||
### Slice 3 — Grafana removal + config + changelog (cleanup)
|
||||
|
||||
**Files:** delete `integrations/grafana.py`, `GrafanaLinkWidget.tsx`, `LinksTab.tsx` (+test); modify `sources.py` (drop `GrafanaWidgetSource` + `SERVICE_ADAPTERS` entry), `registry.py`, `monitoring.py`, `navEntries.ts`, `useObservability.ts`, `client.ts`, `types/index.ts`, `widgets/index.ts`, `service-tabs/index.ts`, `Dashboard.tsx`, `ServicesPage.tsx`, `test_widgets.py` (drop grafana tests); rewrite `config.yaml`; add `CHANGELOG.md` entry.
|
||||
**State after S3:** grep-clean (SC-115/116), config accurate (SC-121), migration documented (SC-122). ~250–350 lines (mostly deletions).
|
||||
|
||||
**Order:** S1 → S2 → S3. S1 and S2 are independently shippable; S3 must follow S1 (it removes the grafana chart binding S1 replaces).
|
||||
|
||||
---
|
||||
|
||||
## 8. Decisions log (answers to spec downstream-notes)
|
||||
|
||||
1. **Step derivation (SC-104):** `step_for_window(window_seconds, target_points=200) = max(15, round(window_seconds/200))`, in `widgets/prometheus_range.py`. Table in §2.1.
|
||||
2. **Shared normalization (SC-102):** `normalize_prometheus_matrix(result)` in `widgets/prometheus_range.py`; extracts the dedup rule from the to-be-deleted Grafana path; retargeted at Prom matrix shape. Signature in §2.1.
|
||||
3. **Gauge renderer (SC-110):** recharts `RadialBarChart` (no new dep, consistent styling). Threshold bands via stacked track cells; neutral single-color when `warn_at`/`crit_at` absent. Config fields: `promql, warn_at, crit_at, min, max, unit`. §3.3.
|
||||
4. **Mean adapter (SC-112/113):** `_fetch_mean` runs `query_range` over the window preset, averages non-null samples of the single series client-side, returns `{value, unit?}`. Scalar-only enforced via `len(result) != 1 → error`. §2.2.
|
||||
5. **Slice plan:** 3 slices (S1 range+rebrand+helper, S2 gauge+mean, S3 grafana removal+config+changelog), each ≤400 lines, order S1→S2→S3. §7.
|
||||
|
||||
---
|
||||
|
||||
## 9. Open items for the tasks/apply phases
|
||||
|
||||
- Implementer should `git mv` the chart widget/test so history is preserved (not delete+create).
|
||||
- Implementer should verify the recharts `RadialBarChart` gauge renders acceptably; if it proves fiddly, the proposal's SVG fallback (~50 lines) is sanctioned — but try recharts first.
|
||||
- SC-118 text names a file that no longer exists; tasks phase should record this so verify doesn't flag it as a miss. Intent is satisfied by removing `get_grafana_status` + `useGrafanaStatus` + `LinksTab`.
|
||||
- `Dashboard.test.tsx` contains the literal "Grafana" in a shortcut test fixture (not a grafana service reference) — grep for SC-116 should be scoped to service/widget references, or that line whitelisted. Flag for reviewer.
|
||||
@@ -0,0 +1,131 @@
|
||||
# SDD Proposal: Prometheus Direct Charting (drop Grafana middleman)
|
||||
|
||||
**Change:** `prometheus-direct-charting`
|
||||
**Phase:** proposal
|
||||
**Date:** 2026-07-08
|
||||
|
||||
## 1. Problem / Why Now
|
||||
|
||||
Manage already does in-app charting — but indirectly. Today the only chart path is:
|
||||
|
||||
```
|
||||
GrafanaChartWidget (recharts) ← GrafanaWidgetSource._fetch_chart
|
||||
← POST {grafana}/api/ds/query (datasource.type = "prometheus", hardcoded)
|
||||
← Grafana proxies to Prometheus
|
||||
```
|
||||
|
||||
Two problems with this:
|
||||
|
||||
1. **Grafana is a pure middleman.** The backend already hardcodes `datasource.type: "prometheus"` in every chart query. Grafana adds a hop, an API key, a different response shape (`/api/ds/query` frames), and a normalization layer — all to reach a Prometheus instance Manage could query directly via `/api/v1/query_range`. The existing `PrometheusWidgetSource` already proves Manage can talk to Prom straight; it just only does *instant* queries today.
|
||||
2. **The project's documented rules are stale.** `openspec/config.yaml` says "Do NOT re-implement charting in-app" and "No recharts/d3 is in use." Both are already false in code: `recharts ^3.9.2` is declared and imported by `GrafanaChartWidget`. The config documents a reality the code left behind.
|
||||
|
||||
Meanwhile the operator wants two more metric visualizations Manage cannot currently render: a **gauge** and a **single value / mean-over-time** widget. Both belong naturally on the `prometheus` service, which today only exposes an instant-query numeric widget.
|
||||
|
||||
The clean answer is to stop routing charts through Grafana: query Prometheus directly, reuse the rendering infrastructure already written, add the two new modes, and remove the now-redundant Grafana surface.
|
||||
|
||||
## 2. Target Users and Situations
|
||||
|
||||
- **Primary users:** Homelab operators who want metric visualizations on the Manage dashboard without bouncing to Grafana for a quick glance.
|
||||
- **Workflow moments:**
|
||||
- Glance at the dashboard: see a trend line, a gauge, or a mean value for a key PromQL query.
|
||||
- Build a custom at-a-glance panel from any PromQL expression Manage can already evaluate.
|
||||
- Decommission the Grafana hop for charts (one fewer external dependency in the chart path, one fewer API key to rotate).
|
||||
- **Urgency:** Medium. Charts already work via Grafana today; this is a simplification plus two new widget modes, not an outage fix.
|
||||
|
||||
## 3. Product Outcome
|
||||
|
||||
After this change, an authenticated user can:
|
||||
|
||||
- Place a **Prometheus Chart** widget (line chart, multi-series) backed by a direct `/api/v1/query_range` call — same look as today's Grafana chart, no Grafana required.
|
||||
- Place a **Prometheus Gauge** widget rendering an instant PromQL scalar as a gauge.
|
||||
- Place a **Prometheus Mean** widget rendering a single value aggregated over a time window (e.g. `avg_over_time(...)`, or a query_range aggregate).
|
||||
- Manage the dashboard without any Grafana service configured: the Grafana service type, link widget, chart widget, and status checks are removed.
|
||||
|
||||
## 4. Scope Boundaries and Non-Goals
|
||||
|
||||
### In scope
|
||||
|
||||
- **Direct Prometheus range query path** — backend hits `/api/v1/query_range` and returns the existing `{series:[{label,points}]}` shape so the frontend renderer is reused unchanged.
|
||||
- **Shared series normalization** — extract the metric-label → readable-label logic currently inside `GrafanaWidgetSource._fetch_chart` into a reusable helper used by both the chart path and (where relevant) the new modes.
|
||||
- **Rebrand + rebind** — `GrafanaChartWidget` → `PrometheusChartWidget`, moved from the `grafana` service to the `prometheus` service as the `chart` widget kind.
|
||||
- **New `prometheus` widget kinds:** `gauge` and `mean` (semantics confirmed in the question round).
|
||||
- **Grafana removal** — delete `integrations/grafana.py`, `GrafanaWidgetSource`, `GrafanaLinkWidget`, `get_grafana_status`, the ObservabilityPage Grafana section, both registries' `grafana` entries, nav entries, and their tests.
|
||||
- **`config.yaml` rewrite** — replace the stale thin-dashboard / no-recharts wording with the reality: in-app charting via `recharts` is the sanctioned approach for Prometheus-backed series; Grafana is no longer referenced.
|
||||
- **Migration note** — CHANGELOG entry: existing Grafana service instances must be deleted and recreated as Prometheus services (true data migration is impossible; different URLs).
|
||||
|
||||
### Non-goals (explicitly out of scope)
|
||||
|
||||
- **A general chart-widget framework with pluggable data-source middlewares.** Chart rendering is reused; data sources are not abstracted into a swappable adapter registry. Two concrete sources (Prometheus range query, and later the service-storage harness) are wired directly where needed.
|
||||
- **Grafana datasource proxy for non-Prometheus sources** (Loki, InfluxDB, Postgres). If a real non-Prom need appears later, it is a separate change.
|
||||
- **Embedding Grafana panels as images/iframes.** Grafana is removed, not embedded.
|
||||
- **Editing PromQL in a rich editor** (autocomplete, metric explorer). Plain text input only, matching today's `prometheus_metric` widget.
|
||||
- **qBittorrent widgets and the service-storage harness.** Those are a separate change (`service-storage-harness`); only the qBit speed widget *depends on* this change's chart capability.
|
||||
- **Touching `prometheus_metric` (instant numeric widget).** It stays as-is; `mean` and `chart` are sibling kinds, not modes bolted onto it.
|
||||
- **Re-indexing or migrating existing widget instance rows automatically.** Existing `grafana` `chart` widgets are orphaned by the removal and must be recreated as `prometheus` `chart` widgets by the operator (documented in CHANGELOG).
|
||||
|
||||
## 5. High-Level Approach
|
||||
|
||||
### 5.1 Backend
|
||||
|
||||
1. **Prometheus range query** — extend `PrometheusWidgetSource` (or add a sibling code path) to handle `widget_kind == "chart"`:
|
||||
- `GET {base_url}/api/v1/query_range?query=...&start=...&end=...&step=...`
|
||||
- Parse Prom `{value:[ts, val]}` matrix into the existing `{series:[{label, points}]}` shape.
|
||||
2. **Shared normalization** — move the "metric labels → readable label" logic out of `GrafanaWidgetSource._fetch_chart` into `widgets/series.py` (or similar), so the Prom path and any future consumer reuse it.
|
||||
3. **New kinds wiring** — `gauge` and `mean` resolve in `PrometheusWidgetSource.fetch`:
|
||||
- `gauge`: instant query (`/api/v1/query`), return `{value, threshold?, ...}` for a gauge renderer.
|
||||
- `mean`: range query aggregated to a single value (either PromQL `avg_over_time` via instant query, or client-side mean over a query_range window). Semantics decided in the question round.
|
||||
4. **Integration update** — `integrations/prometheus.py` declares the three widget kinds (`metric`, `chart`, `gauge`, `mean`) with config schemas (`promql`, plus range params for `chart`/`mean`).
|
||||
5. **Grafana removal** — delete `integrations/grafana.py`, drop `grafana` from `SERVICE_ADAPTERS`, `SERVICE_DEFINITIONS`, remove `_fetch_chart` and the Grafana link logic. Remove `get_grafana_status` from `routers/monitoring.py` and the Grafana branch from `ObservabilityPage`.
|
||||
6. **`config.yaml` rewrite** — replace the stale charting rules with accurate wording.
|
||||
|
||||
### 5.2 Frontend
|
||||
|
||||
1. **Rebrand** — rename `GrafanaChartWidget.tsx` → `PrometheusChartWidget.tsx`; the recharts rendering (`LineChart`/`Line`/`XAxis`/`YAxis`/`Tooltip`/`mergeSeries`/`CHART_COLORS`) stays essentially unchanged.
|
||||
2. **Registry** — in `integrations/registry.ts`, move `chart` to the `prometheus` binding and add `gauge` + `mean` bindings; delete the entire `grafana` binding.
|
||||
3. **New components:**
|
||||
- `PrometheusGaugeWidget.tsx` — recharts `<RadialBarChart>` or a small SVG gauge; instant value.
|
||||
- `PrometheusMeanWidget.tsx` — single-value display (reuses `MetricCard`-style rendering) of the windowed mean.
|
||||
4. **Nav + ObservabilityPage** — remove Grafana nav entries and the Grafana status card.
|
||||
5. **Types** — `frontend/src/types/index.ts` drops Grafana status types; no new endpoint types (data still flows through `useWidgetData`).
|
||||
|
||||
### 5.3 Type contracts
|
||||
|
||||
- Backend: update `integrations/prometheus.py` widget-kind config models; remove Grafana models.
|
||||
- Frontend: remove `GrafanaStatus` type; widget payloads stay `{series}` / `{value}` shaped.
|
||||
|
||||
## 6. Success Criteria / Acceptance Criteria
|
||||
|
||||
1. A user can configure a Prometheus service and place `chart`, `gauge`, `mean`, and `metric` widgets without any Grafana service present.
|
||||
2. The `chart` widget renders multi-series line charts from `/api/v1/query_range` with the same look as the prior Grafana-backed chart.
|
||||
3. The `gauge` widget renders an instant PromQL scalar as a gauge.
|
||||
4. The `mean` widget renders a single value aggregated over the configured window.
|
||||
5. No `grafana` references remain in `backend/src` or `frontend/src` (grep clean).
|
||||
6. `openspec/config.yaml` no longer claims "no recharts" or "do not chart in-app"; its wording matches the implementation.
|
||||
7. Existing `pytest`, `npm run build`, and `npm run lint` stay green; Grafana tests are removed, Prom chart/gauge/mean tests are added.
|
||||
8. CHANGELOG documents the migration (delete Grafana services, recreate as Prometheus).
|
||||
|
||||
## 7. Risks and Mitigations
|
||||
|
||||
| Risk | Mitigation |
|
||||
|------|------------|
|
||||
| **Response-shape regression.** Prom `/api/v1/query_range` matrix differs from Grafana `/api/ds/query` frames. | Extract normalization into a shared helper; add a backend test that feeds a sample Prom range response and asserts the `{series}` shape the frontend already consumes. |
|
||||
| **Orphaned configured widgets.** Existing `grafana/chart` widget rows break at render. | Acceptable + documented in CHANGELOG; the widget resolves to "unknown widget" gracefully (existing `WidgetInstance` error path). No silent data corruption. |
|
||||
| **Gauge rendering complexity.** recharts gauges can be fiddly. | Constrain gauge to a single scalar + optional thresholds; if recharts gauge proves heavy, fall back to a ~50-line SVG gauge (contained, no new dep). |
|
||||
| **Scope creep into a generic chart framework.** Tempting to abstract data sources. | Non-goal enforced: two direct wirings, no adapter registry. |
|
||||
| **Stale docs/tests lingering.** | "No grafana references" acceptance criterion (grep) catches leftovers. |
|
||||
| **Review budget (>400 lines).** | Slice into chained PRs (e.g. Slice 1: Prom range query + rebrand chart; Slice 2: gauge + mean; Slice 3: Grafana removal + config rewrite). Each slice leaves build/lint/test green. |
|
||||
| **DataGrid migration (config rule callout).** | Not applicable — no DataGrid work here. The key technical risk is the response-shape regression above. |
|
||||
|
||||
## 8. Open Questions (for the proposal question round)
|
||||
|
||||
- **Q1 — `mean` semantics.** Default: client-side mean over a `query_range` window (e.g. last 1h, step 30s → average all returned values). Alternative: require the user to write `avg_over_time(...)` in the PromQL and just run an instant query. Which UX do you want?
|
||||
- **Q2 — `gauge` thresholds.** Default: green/amber/red bands at user-configured thresholds (e.g. 70%/90%). Alternative: single color, no bands (simplest).
|
||||
- **Q3 — Chart time window config.** Default: expose `from`/`to`/`step` (or a simpler "window" preset like 1h/6h/24h). Confirm the granularity users can configure.
|
||||
- **Q4 — Multi-series on `mean`/`gauge`.** Default: single series only (one scalar). Confirm we do not try to render multi-series gauges.
|
||||
|
||||
## 9. Future Phases
|
||||
|
||||
1. **Service-storage harness integration** — the qBittorrent speed widget (separate `service-storage-harness` change) reuses this change's chart rendering with an in-service data source wired directly.
|
||||
2. **Rich PromQL editing** — metric explorer / autocomplete.
|
||||
3. **Non-Prom datasources** — only if a concrete need (Loki logs in-app) appears.
|
||||
4. **Threshold-based alerting from chart widgets** — surface bands/lines from Alertmanager rules.
|
||||
@@ -0,0 +1,152 @@
|
||||
# SDD Spec: Prometheus Direct Charting (drop Grafana middleman)
|
||||
|
||||
**Change:** `prometheus-direct-charting`
|
||||
**Phase:** spec
|
||||
**Date:** 2026-07-08
|
||||
|
||||
This spec defines the acceptance requirements for the change. Each requirement is testable. Requirements derived from `proposal.md` §6 (success criteria) and the resolved §8 question round.
|
||||
|
||||
## Requirement categories
|
||||
|
||||
1. Direct Prometheus range query path
|
||||
2. Prometheus chart widget (rebrand + rebind)
|
||||
3. Prometheus gauge widget
|
||||
4. Prometheus mean widget
|
||||
5. Grafana removal
|
||||
6. Configuration documentation accuracy
|
||||
7. Test and build greenness
|
||||
8. Migration guidance
|
||||
|
||||
---
|
||||
|
||||
## 1. Direct Prometheus range query path
|
||||
|
||||
### SC-101 — Prometheus range query returns the existing series shape
|
||||
|
||||
When a `prometheus` widget of kind `chart` is fetched, the backend MUST query `{base_url}/api/v1/query_range` with `query`, `start`, `end`, and `step` derived from the widget config, and return a payload of shape `{ "series": [{ "label": str, "points": [{ "t": int, "v": float|null }] }] }` — the exact shape the frontend chart renderer already consumes.
|
||||
|
||||
### SC-102 — Series label normalization is shared and Prometheus-native
|
||||
|
||||
The metric-label → readable-label normalization MUST live in a single shared helper (not duplicated in a Grafana path) and MUST produce meaningful labels for Prometheus matrix results, including deduplicating repeated labels via a `label (n)` suffix.
|
||||
|
||||
### SC-103 — Range query errors degrade gracefully
|
||||
|
||||
A Prometheus timeout, connection error, or non-2xx response MUST cause the widget data fetch to return `{ "error": str }` (not raise), so the frontend renders the standard per-widget error state and the rest of the dashboard remains functional.
|
||||
|
||||
### SC-104 — Step is derived from the window preset
|
||||
|
||||
Given a window preset (1h / 6h / 24h / 7d), the backend MUST derive a `step` that yields a reasonable number of points (target ~100–300 points). Users do not configure `step` directly.
|
||||
|
||||
## 2. Prometheus chart widget (rebrand + rebind)
|
||||
|
||||
### SC-105 — Chart widget moves from grafana to prometheus
|
||||
|
||||
A widget kind named `chart` MUST be bound to the `prometheus` service type in both the backend registry and the frontend `SERVICE_REGISTRY`. The `grafana` service type MUST NOT offer a `chart` kind.
|
||||
|
||||
### SC-106 — Chart renderer is reused unchanged
|
||||
|
||||
The recharts rendering (line chart, multi-series, axes, tooltip, `mergeSeries`, color tokens) MUST be preserved in the rebranded `PrometheusChartWidget`. The rename is structural; the rendering code is not rewritten.
|
||||
|
||||
### SC-107 — Chart supports multiple series
|
||||
|
||||
The `chart` widget MUST render all series returned by the range query, each as its own line with a distinct color. There is no single-series restriction on `chart`.
|
||||
|
||||
### SC-108 — Chart window is a preset
|
||||
|
||||
The `chart` widget config MUST expose the time window as a preset selector (`1h`, `6h`, `24h`, `7d`), not raw `from`/`to`/`step` fields. The preset is stored in widget config and resolved to `start`/`end` server-side.
|
||||
|
||||
## 3. Prometheus gauge widget
|
||||
|
||||
### SC-109 — Gauge renders an instant scalar
|
||||
|
||||
A widget kind named `gauge` MUST be bound to the `prometheus` service. Its data fetch MUST run an instant PromQL query (`/api/v1/query`) and return the scalar result for rendering as a gauge.
|
||||
|
||||
### SC-110 — Gauge supports configurable threshold bands
|
||||
|
||||
The `gauge` widget config MUST accept optional threshold values (e.g. `warn_at`, `crit_at`) and the renderer MUST display green / amber / red bands accordingly. When thresholds are omitted, the gauge renders with a single neutral color and no bands.
|
||||
|
||||
### SC-111 — Gauge is scalar-only
|
||||
|
||||
The `gauge` widget MUST render exactly one scalar value. If the instant query returns multiple series, the adapter MUST return `{ "error": str }` (not silently pick one), directing the user to refine the PromQL.
|
||||
|
||||
## 4. Prometheus mean widget
|
||||
|
||||
### SC-112 — Mean computes client-side over a window
|
||||
|
||||
A widget kind named `mean` MUST be bound to the `prometheus` service. Its data fetch MUST run a range query over the configured window preset and return the arithmetic mean of all non-null point values as a single scalar.
|
||||
|
||||
### SC-113 — Mean uses plain PromQL + window preset
|
||||
|
||||
The `mean` widget config MUST accept a plain PromQL expression (no requirement to wrap in `avg_over_time`) plus a window preset. Users do not write range-vector functions.
|
||||
|
||||
### SC-114 — Mean is scalar-only
|
||||
|
||||
The `mean` widget MUST render exactly one scalar value. If the range query returns multiple series, the adapter MUST return `{ "error": str }` (not silently aggregate across series).
|
||||
|
||||
## 5. Grafana removal
|
||||
|
||||
### SC-115 — No grafana references in backend source
|
||||
|
||||
After the change, `grep -ri grafana backend/src --include='*.py'` MUST return no matches (excluding comments/changelog that are explicitly about the removal, if any are retained — but ideally zero).
|
||||
|
||||
### SC-116 — No grafana references in frontend source
|
||||
|
||||
After the change, `grep -ri grafana frontend/src` MUST return no matches, **excluding** (a) test fixtures where "Grafana" appears as a user-authored dashboard *shortcut label* unrelated to the grafana service type (e.g. `Dashboard.test.tsx`), and (b) `LinksTab.tsx` / `service-tabs/index.ts` lines that are themselves being deleted as part of SC-118. (Source finding: `ObservabilityPage.tsx` was refactored into `service-tabs/`.)
|
||||
|
||||
### SC-117 — Grafana service type is gone from registries
|
||||
|
||||
Neither the backend `SERVICE_DEFINITIONS` / `SERVICE_ADAPTERS` nor the frontend `SERVICE_REGISTRY` / `BUILTIN_WIDGETS` MUST contain a `grafana` entry. The `integrations/grafana.py` file MUST be deleted.
|
||||
|
||||
### SC-118 — Grafana status checks and UI sections are removed
|
||||
|
||||
The `get_grafana_status` endpoint and its frontend hook (`useGrafanaStatus`) MUST be removed. The UI surface previously in `ObservabilityPage.tsx` has been refactored into a per-service-type `service-tabs/` architecture; the Grafana removal targets are therefore `service-tabs/LinksTab.tsx` + its test, the `grafana` case in `service-tabs/index.ts`, the grafana entry in `integrations/navEntries.ts`, the `grafana` member of `Dashboard.tsx`'s `OBSERVABILITY_TYPES` set, and any Grafana empty-state copy in `ServicesPage.tsx`. The literal `ObservabilityPage.tsx` no longer exists; SC-118's *intent* (no Grafana UI surface) is what is verified.
|
||||
|
||||
### SC-119 — Grafana widget instances degrade gracefully
|
||||
|
||||
An existing persisted widget row referencing a `grafana` service MUST NOT crash the dashboard. It resolves to the existing "unknown widget" error state and surfaces a clear message; the operator can then delete it.
|
||||
|
||||
### SC-120 — Grafana tests are removed
|
||||
|
||||
All Grafana-specific tests (backend and frontend) MUST be deleted; no test references grafana.
|
||||
|
||||
## 6. Configuration documentation accuracy
|
||||
|
||||
### SC-121 — config.yaml matches implementation
|
||||
|
||||
`openspec/config.yaml` MUST NOT contain the stale claims "Do NOT re-implement charting in-app" or "No recharts/d3 is in use." It MUST reflect that in-app charting via `recharts` is the sanctioned approach for Prometheus-backed series, and MUST NOT reference Grafana as a chart path.
|
||||
|
||||
### SC-122 — CHANGELOG documents the migration
|
||||
|
||||
`CHANGELOG.md` MUST include an entry instructing operators to delete existing Grafana service instances and recreate them as Prometheus services, noting that configured `grafana/chart` widgets must be recreated as `prometheus/chart` widgets.
|
||||
|
||||
## 7. Test and build greenness
|
||||
|
||||
### SC-123 — Backend tests pass
|
||||
|
||||
`pytest` run from `backend/` MUST pass, including new tests covering: Prom range query → `{series}` normalization, gauge scalar-only enforcement, mean client-side aggregation, and the shared label helper.
|
||||
|
||||
### SC-124 — Frontend typechecks, builds, and lints
|
||||
|
||||
`npm run build` (which runs `tsc -b` + `vite build`) and `npm run lint` from `frontend/` MUST pass.
|
||||
|
||||
### SC-125 — New widget kinds have tests
|
||||
|
||||
`PrometheusChartWidget`, `PrometheusGaugeWidget`, and `PrometheusMeanWidget` MUST each have a frontend test covering at least: loading state, error state, and a rendered data case.
|
||||
|
||||
## 8. Migration guidance
|
||||
|
||||
### SC-126 — No silent data migration
|
||||
|
||||
The change MUST NOT attempt to auto-migrate existing `grafana` service rows into `prometheus` rows (URLs differ; true migration is impossible). Migration is operator-driven per the CHANGELOG note.
|
||||
|
||||
### SC-127 — Non-blocking on the service-storage-harness change
|
||||
|
||||
This change MUST NOT depend on the `service-storage-harness` change. It is independently buildable, testable, and deployable. (The reverse dependency holds: the qBit speed widget depends on this change's chart capability.)
|
||||
|
||||
---
|
||||
|
||||
## Notes for downstream phases
|
||||
|
||||
- **Design (next phase)** should specify: the exact `step`-derivation function for window presets (SC-104), the shared normalization helper's location and signature (SC-102), and whether the gauge renderer uses recharts `RadialBarChart` or a contained SVG (SC-110).
|
||||
- **Tasks** should slice into chained PRs ≤400 lines per `config.yaml` rules: e.g. (1) Prom range path + chart rebrand, (2) gauge + mean, (3) Grafana removal + config rewrite + CHANGELOG.
|
||||
- The **review-budget guard** applies: if total changed lines exceed ~400, the chained-PR strategy from the `tasks` phase is mandatory.
|
||||
+135
@@ -0,0 +1,135 @@
|
||||
# Prometheus Charting — Delta (`prometheus-direct-charting`)
|
||||
|
||||
> Change: `prometheus-direct-charting` · Domain: `prometheus-charting` · Phase: **spec** (reconciled during `sdd-sync`).
|
||||
> Distilled verbatim from the verified flat `spec.md` (27 requirements, SC-101 … SC-127) of change
|
||||
> `prometheus-direct-charting`, cross-referenced against `design.md` and `verify-report.md`. Captures
|
||||
> the **durable, post-change end-state contracts** for direct Prometheus-backed metric visualization
|
||||
> and the Grafana removal that established the new model.
|
||||
|
||||
## ADDED Requirements
|
||||
|
||||
> The canonical `openspec/specs/prometheus-charting/spec.md` did not exist before this change. All
|
||||
> requirements below are therefore **ADDED** to a new `prometheus-charting` domain; `sdd-sync` copies
|
||||
> them into the canonical spec (native helper rule: when the canonical spec does not exist, the
|
||||
> change spec becomes the new canonical spec).
|
||||
>
|
||||
> Requirement IDs (SC-101 … SC-127) and body text are preserved **exactly** from the verified flat
|
||||
> `spec.md`, including the patched SC-116 / SC-118 wording. Requirements are grouped logically and
|
||||
> listed in the following group order:
|
||||
>
|
||||
> - **Direct Prometheus range query path** — SC-101 … SC-104
|
||||
> - **Prometheus chart widget (rebrand + rebind)** — SC-105 … SC-108
|
||||
> - **Prometheus gauge widget** — SC-109 … SC-111
|
||||
> - **Prometheus mean widget** — SC-112 … SC-114
|
||||
> - **Grafana removal** — SC-115 … SC-120
|
||||
> - **Configuration documentation accuracy** — SC-121 … SC-122
|
||||
> - **Test and build greenness** — SC-123 … SC-125
|
||||
> - **Migration guidance** — SC-126 … SC-127
|
||||
|
||||
### Requirement: SC-101 — Prometheus range query returns the existing series shape
|
||||
|
||||
When a `prometheus` widget of kind `chart` is fetched, the backend MUST query `{base_url}/api/v1/query_range` with `query`, `start`, `end`, and `step` derived from the widget config, and return a payload of shape `{ "series": [{ "label": str, "points": [{ "t": int, "v": float|null }] }] }` — the exact shape the frontend chart renderer already consumes.
|
||||
|
||||
### Requirement: SC-102 — Series label normalization is shared and Prometheus-native
|
||||
|
||||
The metric-label → readable-label normalization MUST live in a single shared helper (not duplicated in a Grafana path) and MUST produce meaningful labels for Prometheus matrix results, including deduplicating repeated labels via a `label (n)` suffix.
|
||||
|
||||
### Requirement: SC-103 — Range query errors degrade gracefully
|
||||
|
||||
A Prometheus timeout, connection error, or non-2xx response MUST cause the widget data fetch to return `{ "error": str }` (not raise), so the frontend renders the standard per-widget error state and the rest of the dashboard remains functional.
|
||||
|
||||
### Requirement: SC-104 — Step is derived from the window preset
|
||||
|
||||
Given a window preset (1h / 6h / 24h / 7d), the backend MUST derive a `step` that yields a reasonable number of points (target ~100–300 points). Users do not configure `step` directly.
|
||||
|
||||
### Requirement: SC-105 — Chart widget moves from grafana to prometheus
|
||||
|
||||
A widget kind named `chart` MUST be bound to the `prometheus` service type in both the backend registry and the frontend `SERVICE_REGISTRY`. The `grafana` service type MUST NOT offer a `chart` kind.
|
||||
|
||||
### Requirement: SC-106 — Chart renderer is reused unchanged
|
||||
|
||||
The recharts rendering (line chart, multi-series, axes, tooltip, `mergeSeries`, color tokens) MUST be preserved in the rebranded `PrometheusChartWidget`. The rename is structural; the rendering code is not rewritten.
|
||||
|
||||
### Requirement: SC-107 — Chart supports multiple series
|
||||
|
||||
The `chart` widget MUST render all series returned by the range query, each as its own line with a distinct color. There is no single-series restriction on `chart`.
|
||||
|
||||
### Requirement: SC-108 — Chart window is a preset
|
||||
|
||||
The `chart` widget config MUST expose the time window as a preset selector (`1h`, `6h`, `24h`, `7d`), not raw `from`/`to`/`step` fields. The preset is stored in widget config and resolved to `start`/`end` server-side.
|
||||
|
||||
### Requirement: SC-109 — Gauge renders an instant scalar
|
||||
|
||||
A widget kind named `gauge` MUST be bound to the `prometheus` service. Its data fetch MUST run an instant PromQL query (`/api/v1/query`) and return the scalar result for rendering as a gauge.
|
||||
|
||||
### Requirement: SC-110 — Gauge supports configurable threshold bands
|
||||
|
||||
The `gauge` widget config MUST accept optional threshold values (e.g. `warn_at`, `crit_at`) and the renderer MUST display green / amber / red bands accordingly. When thresholds are omitted, the gauge renders with a single neutral color and no bands.
|
||||
|
||||
### Requirement: SC-111 — Gauge is scalar-only
|
||||
|
||||
The `gauge` widget MUST render exactly one scalar value. If the instant query returns multiple series, the adapter MUST return `{ "error": str }` (not silently pick one), directing the user to refine the PromQL.
|
||||
|
||||
### Requirement: SC-112 — Mean computes client-side over a window
|
||||
|
||||
A widget kind named `mean` MUST be bound to the `prometheus` service. Its data fetch MUST run a range query over the configured window preset and return the arithmetic mean of all non-null point values as a single scalar.
|
||||
|
||||
### Requirement: SC-113 — Mean uses plain PromQL + window preset
|
||||
|
||||
The `mean` widget config MUST accept a plain PromQL expression (no requirement to wrap in `avg_over_time`) plus a window preset. Users do not write range-vector functions.
|
||||
|
||||
### Requirement: SC-114 — Mean is scalar-only
|
||||
|
||||
The `mean` widget MUST render exactly one scalar value. If the range query returns multiple series, the adapter MUST return `{ "error": str }` (not silently aggregate across series).
|
||||
|
||||
### Requirement: SC-115 — No grafana references in backend source
|
||||
|
||||
After the change, `grep -ri grafana backend/src --include='*.py'` MUST return no matches (excluding comments/changelog that are explicitly about the removal, if any are retained — but ideally zero).
|
||||
|
||||
### Requirement: SC-116 — No grafana references in frontend source
|
||||
|
||||
After the change, `grep -ri grafana frontend/src` MUST return no matches, **excluding** (a) test fixtures where "Grafana" appears as a user-authored dashboard *shortcut label* unrelated to the grafana service type (e.g. `Dashboard.test.tsx`), and (b) `LinksTab.tsx` / `service-tabs/index.ts` lines that are themselves being deleted as part of SC-118. (Source finding: `ObservabilityPage.tsx` was refactored into `service-tabs/`.)
|
||||
|
||||
### Requirement: SC-117 — Grafana service type is gone from registries
|
||||
|
||||
Neither the backend `SERVICE_DEFINITIONS` / `SERVICE_ADAPTERS` nor the frontend `SERVICE_REGISTRY` / `BUILTIN_WIDGETS` MUST contain a `grafana` entry. The `integrations/grafana.py` file MUST be deleted.
|
||||
|
||||
### Requirement: SC-118 — Grafana status checks and UI sections are removed
|
||||
|
||||
The `get_grafana_status` endpoint and its frontend hook (`useGrafanaStatus`) MUST be removed. The UI surface previously in `ObservabilityPage.tsx` has been refactored into a per-service-type `service-tabs/` architecture; the Grafana removal targets are therefore `service-tabs/LinksTab.tsx` + its test, the `grafana` case in `service-tabs/index.ts`, the grafana entry in `integrations/navEntries.ts`, the `grafana` member of `Dashboard.tsx`'s `OBSERVABILITY_TYPES` set, and any Grafana empty-state copy in `ServicesPage.tsx`. The literal `ObservabilityPage.tsx` no longer exists; SC-118's *intent* (no Grafana UI surface) is what is verified.
|
||||
|
||||
### Requirement: SC-119 — Grafana widget instances degrade gracefully
|
||||
|
||||
An existing persisted widget row referencing a `grafana` service MUST NOT crash the dashboard. It resolves to the existing "unknown widget" error state and surfaces a clear message; the operator can then delete it.
|
||||
|
||||
### Requirement: SC-120 — Grafana tests are removed
|
||||
|
||||
All Grafana-specific tests (backend and frontend) MUST be deleted; no test references grafana.
|
||||
|
||||
### Requirement: SC-121 — config.yaml matches implementation
|
||||
|
||||
`openspec/config.yaml` MUST NOT contain the stale claims "Do NOT re-implement charting in-app" or "No recharts/d3 is in use." It MUST reflect that in-app charting via `recharts` is the sanctioned approach for Prometheus-backed series, and MUST NOT reference Grafana as a chart path.
|
||||
|
||||
### Requirement: SC-122 — CHANGELOG documents the migration
|
||||
|
||||
`CHANGELOG.md` MUST include an entry instructing operators to delete existing Grafana service instances and recreate them as Prometheus services, noting that configured `grafana/chart` widgets must be recreated as `prometheus/chart` widgets.
|
||||
|
||||
### Requirement: SC-123 — Backend tests pass
|
||||
|
||||
`pytest` run from `backend/` MUST pass, including new tests covering: Prom range query → `{series}` normalization, gauge scalar-only enforcement, mean client-side aggregation, and the shared label helper.
|
||||
|
||||
### Requirement: SC-124 — Frontend typechecks, builds, and lints
|
||||
|
||||
`npm run build` (which runs `tsc -b` + `vite build`) and `npm run lint` from `frontend/` MUST pass.
|
||||
|
||||
### Requirement: SC-125 — New widget kinds have tests
|
||||
|
||||
`PrometheusChartWidget`, `PrometheusGaugeWidget`, and `PrometheusMeanWidget` MUST each have a frontend test covering at least: loading state, error state, and a rendered data case.
|
||||
|
||||
### Requirement: SC-126 — No silent data migration
|
||||
|
||||
The change MUST NOT attempt to auto-migrate existing `grafana` service rows into `prometheus` rows (URLs differ; true migration is impossible). Migration is operator-driven per the CHANGELOG note.
|
||||
|
||||
### Requirement: SC-127 — Non-blocking on the service-storage-harness change
|
||||
|
||||
This change MUST NOT depend on the `service-storage-harness` change. It is independently buildable, testable, and deployable. (The reverse dependency holds: the qBit speed widget depends on this change's chart capability.)
|
||||
@@ -0,0 +1,168 @@
|
||||
# Sync Report — `prometheus-direct-charting`
|
||||
|
||||
> Phase: **sync** · Change: `prometheus-direct-charting` · Repo: `/home/user/manage`
|
||||
> Mode: file-backed (`artifactStore: openspec`). No source-code edits; only OpenSpec artifacts were
|
||||
> written. Not committed (parent owns the commit). The change folder was **not** moved (that is
|
||||
> `sdd-archive`'s job).
|
||||
|
||||
**Status: SYNCED.** A new canonical domain `openspec/specs/prometheus-charting/spec.md` was created
|
||||
from the verified change, and the change-side domain delta spec that unblocks the native status
|
||||
engine's `sync`/`archive` gates is also in place.
|
||||
|
||||
---
|
||||
|
||||
## 1. Executive summary
|
||||
|
||||
The `prometheus-direct-charting` change shipped a **complete but flat** `openspec/changes/prometheus-direct-charting/spec.md`
|
||||
(27 requirements, SC-101 … SC-127) with **no** per-domain delta spec under
|
||||
`openspec/changes/prometheus-direct-charting/specs/<domain>/`. `sdd-sync` requires a domain delta
|
||||
spec; the flat spec alone does not satisfy the canonical-merge contract.
|
||||
|
||||
Verify already returned **PASS** (verdict in `verify-report.md`; all four gates green — backend
|
||||
`pytest` 293 passed, `ruff` clean, frontend `npm run build` exit 0, `npm run lint` 0 errors).
|
||||
Functional coverage was 26/27 fully PASS, with SC-125 PARTIAL on test coverage only (loading-state
|
||||
test gap) — a non-blocking coverage finding, not a functional defect; the task framing treats it as
|
||||
PARTIAL→PASS after coverage close. The verify report's single CRITICAL was an **archive** blocker
|
||||
(19 unchecked task checkboxes + missing `apply-progress.md`); the `apply-progress.md` artifact now
|
||||
exists and that condition does **not** block `sdd-sync` of the green code.
|
||||
|
||||
This sync **reconciles** the flat-spec-vs-domain-spec gap:
|
||||
|
||||
1. Authored the missing **change-side domain delta spec** —
|
||||
`openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md` — using a clean
|
||||
`## ADDED Requirements` structure that preserves the exact requirement IDs (SC-101 … SC-127) and
|
||||
text (including the patched SC-116 / SC-118 wording) from the verified flat `spec.md`. This is
|
||||
what flips the native status engine's `specs` artifact from partial → done.
|
||||
2. **Synced** the end-state into the **canonical store** —
|
||||
`openspec/specs/prometheus-charting/spec.md` — the actual sync target. Because the canonical
|
||||
`prometheus-charting` domain did not previously exist, the native helper rule applies: *when the
|
||||
canonical spec does not exist, the change spec becomes the new canonical spec.* The two files
|
||||
therefore carry identical requirement bodies (delta under `## ADDED Requirements`; canonical
|
||||
under `## Requirements`), verified byte-identical for the requirement region.
|
||||
|
||||
Domain name **`prometheus-charting`** was chosen (per the dispatch brief) because it covers the
|
||||
full new model: direct Prometheus range-query charting, the gauge and mean modes, and the Grafana
|
||||
removal that established the new direct-query model. It is distinct from the existing canonical
|
||||
`web-ui` domain (MUI→shadcn migration — a different concern), which was **not touched**.
|
||||
|
||||
## 2. Structured status & actionContext findings
|
||||
|
||||
The native `gentle-pi.sdd-status` passed by the parent reports `changeName: null` with
|
||||
`blockedReasons: ["Change selection is ambiguous: mobile-responsive-parity, prometheus-direct-charting,
|
||||
prometheus-direct-charting, service-storage-harness, services-as-hub-ia."]` because the engine
|
||||
auto-detected four active changes. This sync task was **explicitly assigned**
|
||||
`prometheus-direct-charting`; the ambiguity is a parent-resolution artifact and does not block this
|
||||
phase (`isNonAuthoritative: false`).
|
||||
|
||||
- `artifactStore: openspec`; change root `openspec/changes/prometheus-direct-charting/`.
|
||||
- Artifacts present: `proposal.md`, `spec.md`, `design.md`, `tasks.md`, `verify-report.md`,
|
||||
`apply-progress.md`.
|
||||
- `verify: PASS` (verify-report verdict; gates green at `67ca0fc`).
|
||||
- `actionContext`: `mode: repo-local`, `workspaceRoot: /home/user/manage`,
|
||||
`allowedEditRoots: ["/home/user/manage"]`, `warnings: []`. All three files written are inside the
|
||||
authoritative workspace / allowed edit roots. ✓
|
||||
- `relationships.sameDomainActiveChanges: []`, `collisions: []` — **no active same-domain
|
||||
collisions**, so no archive/sync ordering decision was required.
|
||||
- The new `prometheus-charting` domain is distinct from the existing `web-ui` canonical domain;
|
||||
`openspec/specs/web-ui/spec.md` was left untouched.
|
||||
|
||||
**Post-sync structural change:** `openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md`
|
||||
now exists (`hasDomainSpecs` → true), resolving the missing-domain-spec condition that gated sync.
|
||||
The flat `spec.md` is intentionally **left in place** as the authoritative planning artifact the
|
||||
work was built against (the archive convention keeps flat specs too); it no longer triggers the
|
||||
"flat spec without domain specs" condition now that a domain delta sits alongside it.
|
||||
|
||||
## 3. Domains synced & canonical files updated
|
||||
|
||||
| Domain | Change-side delta (source) | Canonical (sync target) | Action |
|
||||
|---|---|---|---|
|
||||
| `prometheus-charting` | `openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md` | `openspec/specs/prometheus-charting/spec.md` | **NEW domain** — `## ADDED Requirements` copied into canonical as a new spec |
|
||||
|
||||
- **Canonical file created:** `openspec/specs/prometheus-charting/spec.md` (27 requirements).
|
||||
- **Change-side delta created:** `openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md`
|
||||
(27 requirements, all `## ADDED Requirements`).
|
||||
|
||||
## 4. Requirement delta (ADDED / MODIFIED / REMOVED)
|
||||
|
||||
- **ADDED (27)** — all to the new `prometheus-charting` domain (canonical did not exist pre-change).
|
||||
IDs and text preserved verbatim from the verified flat `spec.md`. Grouped logically:
|
||||
- *Direct Prometheus range query path* — SC-101, SC-102, SC-103, SC-104
|
||||
- *Prometheus chart widget (rebrand + rebind)* — SC-105, SC-106, SC-107, SC-108
|
||||
- *Prometheus gauge widget* — SC-109, SC-110, SC-111
|
||||
- *Prometheus mean widget* — SC-112, SC-113, SC-114
|
||||
- *Grafana removal* — SC-115, SC-116, SC-117, SC-118, SC-119, SC-120
|
||||
- *Configuration documentation accuracy* — SC-121, SC-122
|
||||
- *Test and build greenness* — SC-123, SC-124, SC-125
|
||||
- *Migration guidance* — SC-126, SC-127
|
||||
- **MODIFIED (0)** — none (new domain; no pre-existing canonical requirements to replace).
|
||||
- **REMOVED (0)** — none.
|
||||
- **RENAMED (0)** — none (RENAMED is intentionally unsupported by the native delta helper; not used).
|
||||
|
||||
## 5. Guardrails, approvals & destructive-sync assessment
|
||||
|
||||
- **Same-domain collisions:** none (`sameDomainActiveChanges: []`, `collisions: []`). The new
|
||||
`prometheus-charting` domain does not overlap the existing `web-ui` canonical domain. No ordering
|
||||
decision was needed.
|
||||
- **Destructive sync:** **not applicable.** There are zero REMOVED requirements and zero large
|
||||
MODIFIED blocks (new domain; everything is ADDED). No destructive-sync parent approval was
|
||||
required for this sync beyond the explicit reconciliation instruction in the task.
|
||||
- **Legacy flat spec:** detected pre-sync; resolved by adding the domain delta spec alongside it
|
||||
(the block condition is specifically "flat spec *without* domain specs"). The flat spec was left
|
||||
in place as a planning artifact.
|
||||
- **`web-ui` canonical isolation:** the existing `openspec/specs/web-ui/spec.md` (the MUI→shadcn
|
||||
rework) was **not modified** — verified untouched by `git status` (mtime `2026-06-17T19:11`,
|
||||
not in the modified set). The two domains are independent.
|
||||
|
||||
## 6. Validation / checks performed (file-backed, read-only)
|
||||
|
||||
Run from `/home/user/manage` (no source edits, no test re-runs — those are owned by verify and were
|
||||
already green at `67ca0fc`):
|
||||
|
||||
| Check | Command | Result |
|
||||
|---|---|---|
|
||||
| Canonical store populated | `ls openspec/specs/prometheus-charting/spec.md` | present ✓ |
|
||||
| Change-side domain spec present | `ls openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md` | present ✓ |
|
||||
| Requirement-ID parity (flat ↔ delta ↔ canonical) | `grep -oE 'SC-[0-9]+'` all three files, `sort -u` | **27 == 27 == 27**, identical IDs SC-101…SC-127 ✓ |
|
||||
| Body-text parity (delta ↔ canonical) | `diff` of the `^### Requirement:` region of both files | **identical** ✓ |
|
||||
| Patched SC-116 wording present | `grep "service-tabs/"` both delta + canonical | present in both ✓ |
|
||||
| Patched SC-118 wording present | `grep "service-tabs/LinksTab.tsx"` both delta + canonical | present in both ✓ |
|
||||
| Delta is pure ADDED | count `## ADDED/MODIFIED/REMOVED/RENAMED Requirements` | ADDED=1, MODIFIED=0, REMOVED=0, RENAMED=0 ✓ (no destructive sync) |
|
||||
| `web-ui` canonical untouched | `git status --porcelain openspec/specs/web-ui/spec.md` | empty (not modified) ✓ |
|
||||
| No edits outside openspec | `git status --porcelain` (filtered) | only `openspec/specs/prometheus-charting/`, `openspec/changes/prometheus-direct-charting/specs/`, and this report added; pre-existing dirty/untracked items unrelated to this sync unchanged ✓ |
|
||||
| Markdown validity | write-time lint | all three files "Markdown clean" ✓ |
|
||||
|
||||
## 7. Carry-over items for the archive summary
|
||||
|
||||
These verify-phase findings are non-blocking for sync and should land in the archive summary:
|
||||
|
||||
1. **[INFO] SC-125 was PARTIAL in verify** (no explicit `isLoading:true` loading-state test in any of
|
||||
the three new widget test files — coverage gap, not a functional defect). The task framing treats
|
||||
this as PARTIAL→PASS after coverage close; if a loading-state case per widget has not been added,
|
||||
`sdd-archive` may want to confirm or note it.
|
||||
2. **[CRITICAL-process, archive-only] Unchecked task checkboxes.** At verify time, 19 implementation
|
||||
/ verification task checkboxes (Slice 3 §3.1–3.14 and Integration §4.1–4.5) were unchecked and
|
||||
`apply-progress.md` was missing. `apply-progress.md` now exists (created after the verify pass);
|
||||
`sdd-archive` should re-scan the native status engine to confirm `tasks: done` / `applyProgress:
|
||||
present` before moving the change to archive, and tick any remaining unchecked boxes if needed.
|
||||
3. **[INFO] Stale generated `.pi-map.md`** files still reference Grafana / `ObservabilityPage.tsx`
|
||||
(generated artifacts, not deliverable source; ignored by SC-115/116). Regenerate via
|
||||
`project_map_patch` / `project_map_validate`.
|
||||
4. **[INFO] Slice-2 review-budget variance** (~707 insertions vs ~310–400 forecast) — additive
|
||||
feature code + tests; boundary is the gauge+mean feature, not scope creep. Non-blocking; record
|
||||
in the archive summary.
|
||||
|
||||
## 8. Next recommended phase
|
||||
|
||||
→ **`sdd-archive`** (clean). Confirm the native status re-scan reports `specs: done` / `sync: ready`
|
||||
/ `archive: ready`, then move the change to
|
||||
`openspec/changes/archive/YYYY-MM-DD-prometheus-direct-charting`, carrying over the items in §7 into
|
||||
the archive summary. Do **not** commit or push — the parent owns the commit with explicit paths.
|
||||
|
||||
---
|
||||
|
||||
### Appendix — Files written by this sync (OpenSpec only; no source code)
|
||||
|
||||
- `openspec/changes/prometheus-direct-charting/specs/prometheus-charting/spec.md` — **change-side
|
||||
domain delta (`## ADDED Requirements`), 27 requirements SC-101…SC-127.**
|
||||
- `openspec/specs/prometheus-charting/spec.md` — **canonical spec (new domain), 27 requirements.**
|
||||
- `openspec/changes/prometheus-direct-charting/sync-report.md` — this report.
|
||||
@@ -0,0 +1,318 @@
|
||||
# SDD Tasks: Prometheus Direct Charting (drop Grafana middleman)
|
||||
|
||||
**Change:** `prometheus-direct-charting`
|
||||
**Phase:** tasks
|
||||
**Date:** 2026-07-08
|
||||
|
||||
## Review Workload Forecast
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| Estimated changed lines | ~850–1,110 (sum of three implementation slices) |
|
||||
| 400-line budget risk | Medium |
|
||||
| Chained PRs recommended | Yes |
|
||||
| Suggested split | PR 1: Prom range path + chart rebrand → PR 2: gauge + mean widgets → PR 3: Grafana removal + config rewrite |
|
||||
| Delivery strategy | auto-chain |
|
||||
| Chain strategy | stacked-to-main |
|
||||
|
||||
```text
|
||||
Decision needed before apply: No
|
||||
Chained PRs recommended: Yes
|
||||
Chain strategy: stacked-to-main
|
||||
400-line budget risk: Medium
|
||||
```
|
||||
|
||||
> Each slice individually lands under the 400-line review budget. Slices are ordered S1 → S2 → S3; S1 and S2 are independently shippable, S3 must follow S1 (it removes the grafana chart binding S1 replaces). Per `openspec/config.yaml` rules, each slice leaves `npm run build` (tsc -b + vite build), `npm run lint`, and backend `pytest` green.
|
||||
|
||||
---
|
||||
|
||||
## Slice ordering rationale (critical)
|
||||
|
||||
**Slice 1 ADDS the prometheus chart capability without removing anything from grafana.** After S1:
|
||||
|
||||
- `prometheus` binding has `metric` + `chart`; `grafana` binding has only `link` (chart kind moved away).
|
||||
- `GrafanaWidgetSource` still exists (serves the `link` kind; its `_fetch_chart` is now dead but harmless).
|
||||
- `GrafanaLinkWidget` and the `grafana` service type are untouched.
|
||||
- A `prometheus/chart` widget renders from a direct `/api/v1/query_range` call.
|
||||
|
||||
This ordering ensures the chart capability is proven against Prometheus before the grafana surface is removed in S3, so the two risks (new query path + grafana removal) never compound in a single slice.
|
||||
|
||||
---
|
||||
|
||||
## Slice 1: Prometheus range query path + shared helper + chart rebrand
|
||||
|
||||
**Goal:** Make a `prometheus/chart` widget render multi-series line charts from a direct `/api/v1/query_range` call, reusing the existing recharts renderer. Extract the series-normalization and step-derivation helpers into a testable module. Move the `chart` kind from `grafana` to `prometheus` in both registries. Do NOT yet remove the grafana service type, `GrafanaWidgetSource`, or `GrafanaLinkWidget`.
|
||||
|
||||
**Satisfies:** SC-101, SC-102, SC-103, SC-104, SC-105, SC-106, SC-107, SC-108.
|
||||
|
||||
- [x] **1.1 Create shared Prometheus range helpers module**
|
||||
- Files: `backend/src/media_library_viewer_api/widgets/prometheus_range.py` (new)
|
||||
- Lines: ~60
|
||||
- Dependencies: none
|
||||
- Details: Implement `WINDOW_PRESETS = {"1h": 3600, "6h": 21600, "24h": 86400, "7d": 604800}`, `step_for_window(window_seconds, target_points=200) -> int` returning `max(15, round(window_seconds / target_points))`, and `normalize_prometheus_matrix(result: list[dict]) -> list[dict]` that converts a Prometheus `/api/v1/query_range` `data.result` matrix into `{label, points:[{t:int, v:float|None}]}` series. Label rule: drop `__name__` from metric labels; join remaining as `k=v`; fall back to `"value"`; dedup collisions with `(n)` suffix. Null handling: `"NaN"`, `"+Inf"`, `"-Inf"`, `None` → `v: None`.
|
||||
|
||||
- [x] **1.2 Add backend unit tests for range helpers**
|
||||
- Files: `backend/tests/test_prometheus_range.py` (new)
|
||||
- Lines: ~70
|
||||
- Dependencies: 1.1
|
||||
- Details: Test `step_for_window` for all four presets asserts result yields 100–300 points. Test `normalize_prometheus_matrix`: feed a two-entry sample matrix (one with `__name__`, one colliding label) → assert `{series}` shape, label dedup `(1)` suffix, null handling for `"NaN"` string.
|
||||
|
||||
- [x] **1.3 Add `_fetch_chart` + shared range-query plumbing to `PrometheusWidgetSource`**
|
||||
- Files: `backend/src/media_library_viewer_api/widgets/sources.py` (modify)
|
||||
- Lines: ~55
|
||||
- Dependencies: 1.1
|
||||
- Details: Import `WINDOW_PRESETS`, `step_for_window`, `normalize_prometheus_matrix` from `prometheus_range`. Add a private `_range_query(base_url, timeout, promql, window) -> dict` returning `{"matrix": result}` or `{"error": ...}` (shared by chart now and mean in S2). Add `_fetch_chart(self, base_url, timeout, config)` calling `_range_query` and returning `{"series": normalize_prometheus_matrix(matrix)}`. Dispatch `widget_kind == "chart"` in `.fetch()`. Existing `metric` path stays byte-for-byte unchanged. Errors (timeout, `RequestException`) → `{"error": str}`, never raise.
|
||||
|
||||
- [x] **1.4 Declare `chart` widget kind in Prometheus integration**
|
||||
- Files: `backend/src/media_library_viewer_api/integrations/prometheus.py` (modify)
|
||||
- Lines: ~15
|
||||
- Dependencies: 1.3
|
||||
- Details: Add `PrometheusChartWidgetConfig(WidgetConfigBase)` with `promql: str` and `window: str = "1h"`. Add a `widget_kind(...)` entry for `chart` (refresh 60s, default config `{"promql": "", "window": "1h"}`). Leave `metric` kind untouched.
|
||||
|
||||
- [x] **1.5 Rename `GrafanaChartWidget` → `PrometheusChartWidget`**
|
||||
- Files: `frontend/src/widgets/GrafanaChartWidget.tsx` → `frontend/src/widgets/PrometheusChartWidget.tsx` (git mv)
|
||||
- Lines: ~5 changed (rename export, fix empty-state copy)
|
||||
- Dependencies: none
|
||||
- Details: `git mv` to preserve history. Rename exported function `GrafanaChartWidget` → `PrometheusChartWidget`. The recharts body (`LineChart`, `Line`, `XAxis`, `YAxis`, `CartesianGrid`, `Tooltip`, `ResponsiveContainer`, `mergeSeries`, `CHART_COLORS`, `formatTime`) is preserved unchanged (SC-106). Fix empty-state copy: "Check your query and datasource_uid" → "Check your PromQL query and window."
|
||||
|
||||
- [x] **1.6 Rename chart widget test**
|
||||
- Files: `frontend/src/widgets/__tests__/GrafanaChartWidget.test.tsx` → `frontend/src/widgets/__tests__/PrometheusChartWidget.test.tsx` (git mv)
|
||||
- Lines: ~10 changed (import path, component name, error-message assertion)
|
||||
- Dependencies: 1.5
|
||||
- Details: `git mv`. Update import to `PrometheusChartWidget`. Update error-state assertion: the old Grafana-specific error string (`"Grafana api_key is required"`) → a Prom error string (e.g. `"promql is required"`). Keep loading + rendered-data test cases.
|
||||
|
||||
- [x] **1.7 Rebind `chart` from grafana to prometheus in frontend registry**
|
||||
- Files: `frontend/src/integrations/registry.ts` (modify)
|
||||
- Lines: ~30
|
||||
- Dependencies: 1.5
|
||||
- Details: Import `PrometheusChartWidget`. Add a `chart` entry to the `prometheus` binding's `widgets` array (kind `chart`, refresh 60s, configSchema with `promql` + `window`). Remove the `chart` entry from the `grafana` binding's `widgets` array (leave `link` intact). Do NOT delete the `grafana` binding itself.
|
||||
|
||||
- [x] **1.8 Update frontend widgets barrel export**
|
||||
- Files: `frontend/src/widgets/index.ts` (modify)
|
||||
- Lines: ~2
|
||||
- Dependencies: 1.5
|
||||
- Details: Rename the `GrafanaChartWidget` export to `PrometheusChartWidget`. Leave `GrafanaLinkWidget` export intact.
|
||||
|
||||
- [x] **1.9 Update registry tests for chart rebind**
|
||||
- Files: `frontend/src/integrations/registry.test.ts` (modify)
|
||||
- Lines: ~15
|
||||
- Dependencies: 1.7
|
||||
- Details: Assert `prometheus` binding has `metric` + `chart` kinds. Assert `grafana` binding has only `link` (no `chart`).
|
||||
|
||||
- [x] **1.10 Verify Slice 1 (build + lint + test)**
|
||||
- Run: `cd backend && PYTHONPATH=src pytest tests/test_prometheus_range.py tests/test_widgets.py && cd ../frontend && npm run build && npm run lint`
|
||||
- Verify: helpers tests pass; existing widget tests pass (grafana link adapter still wired); frontend typechecks and lints; `prometheus/chart` widget resolves to `PrometheusChartWidget`.
|
||||
|
||||
**Slice 1 total:** ~260–330 changed lines.
|
||||
|
||||
---
|
||||
|
||||
## Slice 2: Gauge + mean widgets
|
||||
|
||||
**Goal:** Add `gauge` and `mean` widget kinds to the `prometheus` service. Gauge renders an instant scalar with configurable threshold bands. Mean renders a single value computed client-side over a range-query window. Both are scalar-only.
|
||||
|
||||
**Satisfies:** SC-109, SC-110, SC-111, SC-112, SC-113, SC-114.
|
||||
|
||||
- [x] **2.1 Extract shared `_instant_query` helper + add `_fetch_gauge` to `PrometheusWidgetSource`**
|
||||
- Files: `backend/src/media_library_viewer_api/widgets/sources.py` (modify)
|
||||
- Lines: ~40
|
||||
- Dependencies: Slice 1 (1.3)
|
||||
- Details: Extract the instant-query HTTP call from the existing `metric` path into a private `_instant_query(base_url, timeout, promql) -> dict` returning `{"result": [...]}` or `{"error": ...}`. Refactor the `metric` path to use it (behavior unchanged). Add `_fetch_gauge(self, base_url, timeout, config)` using `_instant_query`: assert `len(result) == 1` (scalar-only, SC-111); parse `float(result[0]["value"][1])`; return `{"value": float, "warn_at": config.get("warn_at"), "crit_at": config.get("crit_at"), "min": config.get("min"), "max": config.get("max"), "unit": config.get("unit")}`. Dispatch `widget_kind == "gauge"` in `.fetch()`.
|
||||
|
||||
- [x] **2.2 Add `_fetch_mean` to `PrometheusWidgetSource`**
|
||||
- Files: `backend/src/media_library_viewer_api/widgets/sources.py` (modify)
|
||||
- Lines: ~30
|
||||
- Dependencies: 2.1, Slice 1 (1.3 for `_range_query`)
|
||||
- Details: Add `_fetch_mean(self, base_url, timeout, config)` using the shared `_range_query` from S1. Assert `len(result) == 1` (scalar-only, SC-114). Collect non-null numeric values from the single series; compute arithmetic mean; return `{"value": mean, "unit": config.get("unit")}`. If no numeric samples → `{"error": "..."}`. Dispatch `widget_kind == "mean"` in `.fetch()`.
|
||||
|
||||
- [x] **2.3 Declare `gauge` + `mean` widget kinds in Prometheus integration**
|
||||
- Files: `backend/src/media_library_viewer_api/integrations/prometheus.py` (modify)
|
||||
- Lines: ~25
|
||||
- Dependencies: 2.1, 2.2
|
||||
- Details: Add `PrometheusGaugeWidgetConfig` (`promql: str`, `warn_at: float|None`, `crit_at: float|None`, `min: float|None`, `max: float|None`, `unit: str|None`) and `PrometheusMeanWidgetConfig` (`promql: str`, `window: str = "1h"`, `unit: str|None`). Add `widget_kind(...)` entries: `gauge` (refresh 30s), `mean` (refresh 60s).
|
||||
|
||||
- [x] **2.4 Add backend tests for gauge + mean adapters**
|
||||
- Files: `backend/tests/test_widgets.py` (modify) or `backend/tests/test_prometheus_range.py` (modify)
|
||||
- Lines: ~60
|
||||
- Dependencies: 2.1, 2.2
|
||||
- Details: Mock `requests.get` for gauge: instant query returning 1 series → assert `{value, ...}` shape; returning 2 series → assert `{"error": ...}` (SC-111). Mock for mean: range query returning 1 series with known values `[1.0, 2.0, 3.0]` → assert mean `2.0`; returning 2 series → assert `{"error": ...}` (SC-114). Test timeout/RequestException → `{"error": ...}`.
|
||||
|
||||
- [x] **2.5 Create `PrometheusGaugeWidget` component**
|
||||
- Files: `frontend/src/widgets/PrometheusGaugeWidget.tsx` (new)
|
||||
- Lines: ~90
|
||||
- Dependencies: Slice 1 (1.5 for widget pattern)
|
||||
- Details: Render via recharts `RadialBarChart` (no new dep; SC-110). Threshold bands: three stacked `RadialBar` track cells (green `0→warn`, amber `warn→crit`, red `crit→max`) + a value cell. When `warn_at`/`crit_at` absent → single neutral-color track. `min`/`max` default to `0`/`max(value, 1)`. Reuse `SectionCard` + `Alert`/`Skeleton` for loading/error states. Consume `data?.data?.value`, `warn_at`, etc. off `useWidgetData`.
|
||||
|
||||
- [x] **2.6 Create `PrometheusMeanWidget` component**
|
||||
- Files: `frontend/src/widgets/PrometheusMeanWidget.tsx` (new)
|
||||
- Lines: ~50
|
||||
- Dependencies: Slice 1
|
||||
- Details: Single-value display reusing the `MetricCard` pattern (big number + optional `unit` suffix + subtext "mean over last {window}"). Loading/error/empty via `Skeleton`/`Alert`. No charting library — it's a number (SC-112).
|
||||
|
||||
- [x] **2.7 Create gauge + mean frontend tests**
|
||||
- Files: `frontend/src/widgets/__tests__/PrometheusGaugeWidget.test.tsx` (new), `frontend/src/widgets/__tests__/PrometheusMeanWidget.test.tsx` (new)
|
||||
- Lines: ~60
|
||||
- Dependencies: 2.5, 2.6
|
||||
- Details: Each covers loading, error, and rendered-data case (SC-125). Gauge test: render with bands (`warn_at`/`crit_at` set) and without (single color). Mean test: render with `value` + `unit`.
|
||||
|
||||
- [x] **2.8 Add gauge + mean bindings to frontend registry**
|
||||
- Files: `frontend/src/integrations/registry.ts` (modify)
|
||||
- Lines: ~35
|
||||
- Dependencies: 2.5, 2.6
|
||||
- Details: Import `PrometheusGaugeWidget` + `PrometheusMeanWidget`. Add `gauge` (refresh 30s, configSchema with `promql`, `warn_at`, `crit_at`, `min`, `max`, `unit`) and `mean` (refresh 60s, configSchema with `promql`, `window`, `unit`) entries to the `prometheus` binding's `widgets` array alongside `metric` and `chart`.
|
||||
|
||||
- [x] **2.9 Update widgets barrel + registry tests**
|
||||
- Files: `frontend/src/widgets/index.ts` (modify), `frontend/src/integrations/registry.test.ts` (modify)
|
||||
- Lines: ~10
|
||||
- Dependencies: 2.5, 2.6, 2.8
|
||||
- Details: Export `PrometheusGaugeWidget` + `PrometheusMeanWidget`. Assert `prometheus` binding has `metric`, `chart`, `gauge`, `mean` (four kinds).
|
||||
|
||||
- [x] **2.10 Verify Slice 2 (build + lint + test)**
|
||||
- Run: `cd backend && PYTHONPATH=src pytest tests/test_prometheus_range.py tests/test_widgets.py && cd ../frontend && npm run build && npm run lint`
|
||||
- Verify: gauge/mean adapter tests pass; frontend typechecks and lints; all four prometheus widget kinds resolve.
|
||||
|
||||
**Slice 2 total:** ~310–400 changed lines.
|
||||
|
||||
---
|
||||
|
||||
## Slice 3: Grafana removal + config rewrite + changelog
|
||||
|
||||
**Goal:** Remove the entire Grafana surface (service type, link widget, chart source, status checks, nav entries, UI tabs). Rewrite `config.yaml` to match reality. Add CHANGELOG migration note. Leave the app grep-clean of grafana service references.
|
||||
|
||||
**Satisfies:** SC-115, SC-116, SC-117, SC-118, SC-119, SC-120, SC-121, SC-122, SC-126.
|
||||
|
||||
> **Spec-text drift note:** SC-118 literally names `ObservabilityPage.tsx`, which was refactored into `service-tabs/`. The *intent* (no Grafana UI surface) is verified by the removals below. `Dashboard.test.tsx` contains "Grafana" as a user-authored shortcut label (unrelated to the grafana service) — SC-116 grep should not flag it; it is left intact.
|
||||
|
||||
- [x] **3.1 Delete backend Grafana integration module**
|
||||
- Files: `backend/src/media_library_viewer_api/integrations/grafana.py` (delete)
|
||||
- Lines: ~−20 (deletion)
|
||||
- Dependencies: Slice 1 (chart kind already moved to prometheus)
|
||||
- Details: Delete the file. It contains `GrafanaConfig` + `GrafanaLinkWidgetConfig`.
|
||||
|
||||
- [x] **3.2 Remove Grafana from backend integration registry**
|
||||
- Files: `backend/src/media_library_viewer_api/integrations/registry.py` (modify)
|
||||
- Lines: ~−3
|
||||
- Dependencies: 3.1
|
||||
- Details: Drop `from ...grafana import DEFINITION as GRAFANA` and the `GRAFANA.service_type: GRAFANA` entry from `SERVICE_DEFINITIONS`.
|
||||
|
||||
- [x] **3.3 Remove `GrafanaWidgetSource` + adapter registration**
|
||||
- Files: `backend/src/media_library_viewer_api/widgets/sources.py` (modify)
|
||||
- Lines: ~−100 (deletion of `GrafanaWidgetSource` class + `_fetch_chart`)
|
||||
- Dependencies: Slice 1 (normalization logic already extracted to `prometheus_range.py`)
|
||||
- Details: Delete the `GrafanaWidgetSource` class entirely (including `_fetch_chart` — its logic was extracted to `normalize_prometheus_matrix` in S1). Remove `"grafana": GrafanaWidgetSource()` from `SERVICE_ADAPTERS`.
|
||||
|
||||
- [x] **3.4 Remove `get_grafana_status` endpoint**
|
||||
- Files: `backend/src/media_library_viewer_api/routers/monitoring.py` (modify)
|
||||
- Lines: ~−25
|
||||
- Dependencies: none
|
||||
- Details: Delete the `@router.get("/grafana-status")` endpoint and its helper. Leave `get_prometheus_status` / `get_alertmanager_status` intact.
|
||||
|
||||
- [x] **3.5 Remove Grafana backend tests**
|
||||
- Files: `backend/tests/test_widgets.py` (modify), `backend/tests/test_api.py` (modify), `backend/tests/test_services.py` (modify)
|
||||
- Lines: ~−40
|
||||
- Dependencies: 3.3
|
||||
- Details: Delete grafana adapter tests (`test_grafana_adapter_*`), grafana service fixtures, and the `TestGrafanaStatus` test class. SC-120.
|
||||
|
||||
- [x] **3.6 Delete frontend `GrafanaLinkWidget` + barrel export**
|
||||
- Files: `frontend/src/widgets/GrafanaLinkWidget.tsx` (delete), `frontend/src/widgets/index.ts` (modify)
|
||||
- Lines: ~−35
|
||||
- Dependencies: none
|
||||
- Details: Delete the file. Remove the `GrafanaLinkWidget` export from `widgets/index.ts`.
|
||||
|
||||
- [x] **3.7 Remove `grafana` binding from frontend registry**
|
||||
- Files: `frontend/src/integrations/registry.ts` (modify)
|
||||
- Lines: ~−30
|
||||
- Dependencies: 3.6
|
||||
- Details: Delete the entire `grafana` key from `SERVICE_REGISTRY`. Drop the `GrafanaLinkWidget` import. SC-117.
|
||||
|
||||
- [x] **3.8 Remove Grafana nav entry**
|
||||
- Files: `frontend/src/integrations/navEntries.ts` (modify)
|
||||
- Lines: ~−5
|
||||
- Dependencies: none
|
||||
- Details: Drop the `grafana` entry from `SERVICE_TYPE_NAV_ENTRIES` and any now-unused icon import (e.g. `Link2`).
|
||||
|
||||
- [x] **3.9 Remove Grafana status hook + API client function + type**
|
||||
- Files: `frontend/src/hooks/useObservability.ts` (modify), `frontend/src/api/client.ts` (modify), `frontend/src/types/index.ts` (modify)
|
||||
- Lines: ~−15
|
||||
- Dependencies: 3.4
|
||||
- Details: Drop `useGrafanaStatus` + its `fetchGrafanaStatus` import from `useObservability.ts`. Drop `fetchGrafanaStatus` from `client.ts`. Drop `GrafanaStatus` interface from `types/index.ts`.
|
||||
|
||||
- [x] **3.10 Delete Grafana service tab + remove from tab index**
|
||||
- Files: `frontend/src/pages/service-tabs/LinksTab.tsx` (delete), `frontend/src/pages/service-tabs/__tests__/LinksTab.test.tsx` (delete), `frontend/src/pages/service-tabs/index.ts` (modify)
|
||||
- Lines: ~−80
|
||||
- Dependencies: none
|
||||
- Details: Delete `LinksTab.tsx` (grafana-specific per its docstring) and its test. Remove the `LinksTab` import and `case "grafana":` from `service-tabs/index.ts`.
|
||||
|
||||
- [x] **3.11 Remove Grafana from Dashboard + ServicesPage**
|
||||
- Files: `frontend/src/pages/Dashboard.tsx` (modify), `frontend/src/pages/ServicesPage.tsx` (modify)
|
||||
- Lines: ~−5
|
||||
- Dependencies: none
|
||||
- Details: Drop `"grafana"` from `OBSERVABILITY_TYPES` set in `Dashboard.tsx`. Update `ServicesPage.tsx` empty-state copy: "Add a Grafana, Prometheus, …" → "Add a Prometheus, …".
|
||||
|
||||
- [x] **3.12 Rewrite `openspec/config.yaml` thin-dashboard rule**
|
||||
- Files: `openspec/config.yaml` (modify)
|
||||
- Lines: ~8
|
||||
- Dependencies: none
|
||||
- Details: Remove "Do NOT re-implement charting in-app" and "No recharts/d3 is in use" claims. Replace with accurate wording: in-app charting via `recharts` is the sanctioned approach for Prometheus-backed series; Grafana is no longer referenced. SC-121.
|
||||
|
||||
- [x] **3.13 Add CHANGELOG migration note**
|
||||
- Files: `CHANGELOG.md` (modify)
|
||||
- Lines: ~8
|
||||
- Dependencies: none
|
||||
- Details: Add entry under an appropriate version heading: instruct operators to delete existing Grafana service instances and recreate them as Prometheus services; note that `grafana/chart` widgets must be recreated as `prometheus/chart` widgets. SC-122, SC-126.
|
||||
|
||||
- [x] **3.14 Verify Slice 3 (grep-clean + build + lint + test)**
|
||||
- Run: `grep -ri grafana backend/src --include='*.py'` → expect zero matches (SC-115)
|
||||
- Run: `grep -ri grafana frontend/src` → expect zero matches except `Dashboard.test.tsx` shortcut fixture (SC-116)
|
||||
- Run: `cd backend && PYTHONPATH=src pytest && cd ../frontend && npm run build && npm run lint`
|
||||
- Verify: no grafana service references remain; all tests pass; frontend builds and lints.
|
||||
|
||||
**Slice 3 total:** ~250–350 changed lines (mostly deletions).
|
||||
|
||||
---
|
||||
|
||||
## Integration and acceptance verification
|
||||
|
||||
- [x] **4.1 Full backend test run**
|
||||
- Run: `cd backend && PYTHONPATH=src pytest`
|
||||
- Verify: all tests pass; no grafana test references remain; prometheus range/gauge/mean tests pass.
|
||||
|
||||
- [x] **4.2 Full frontend build + lint**
|
||||
- Run: `cd frontend && npm run build && npm run lint`
|
||||
- Verify: TypeScript compiles; no lint failures; no grafana imports unresolved.
|
||||
|
||||
- [x] **4.3 Grep-clean verification**
|
||||
- Run: `grep -ri grafana backend/src --include='*.py'` → zero matches
|
||||
- Run: `grep -ri grafana frontend/src` → zero matches excluding `Dashboard.test.tsx` shortcut fixture
|
||||
- Verify: SC-115, SC-116 satisfied.
|
||||
|
||||
- [x] **4.4 config.yaml accuracy check**
|
||||
- Verify: `openspec/config.yaml` does not contain "Do NOT re-implement charting" or "No recharts/d3"; reflects recharts as sanctioned renderer (SC-121).
|
||||
|
||||
- [x] **4.5 CHANGELOG check**
|
||||
- Verify: `CHANGELOG.md` documents the grafana→prometheus migration (SC-122).
|
||||
|
||||
---
|
||||
|
||||
## Total estimate
|
||||
|
||||
| Slice | Changed lines | Satisfies |
|
||||
|-------|---------------|-----------|
|
||||
| Slice 1: Prom range path + chart rebrand + helper | ~260–330 | SC-101…108 |
|
||||
| Slice 2: Gauge + mean widgets | ~310–400 | SC-109…114 |
|
||||
| Slice 3: Grafana removal + config + changelog | ~250–350 | SC-115…122, 126 |
|
||||
| Integration verification | ~0 | SC-123…125, 127 |
|
||||
| **Total** | **~820–1,080** | **SC-101…127** |
|
||||
|
||||
Each slice is under the 400-line review budget. Use three chained PRs (S1 → S2 → S3), each independently buildable and green.
|
||||
|
||||
---
|
||||
|
||||
## Risk flags for the apply phase
|
||||
|
||||
1. **`git mv` for chart widget rename** — use `git mv` (not delete+create) to preserve file history (design §9).
|
||||
2. **recharts `RadialBarChart` gauge** — try recharts first; if rendering proves fiddly, the proposal sanctions a ~50-line SVG fallback (no new dep). Decision at apply time.
|
||||
3. **`Dashboard.test.tsx` "Grafana" literal** — this is a user-authored shortcut label in a test fixture, not a grafana service reference. SC-116 grep should not flag it. Flagged for reviewer awareness.
|
||||
4. **SC-118 textual drift** — `ObservabilityPage.tsx` no longer exists (refactored to `service-tabs/`). The *intent* is satisfied by removing `get_grafana_status` + `useGrafanaStatus` + `LinksTab`. Verify against intent, not literal filename.
|
||||
5. **`_range_query` / `_instant_query` refactoring timing** — `_range_query` is created in S1 (1.3) for `_fetch_chart`; `_instant_query` is extracted in S2 (2.1) when `_fetch_gauge` needs it. Both shared helpers must be in place before S3 (which deletes `GrafanaWidgetSource` and its private chart logic).
|
||||
6. **Stale project map** — the pi-map references `ObservabilityPage.tsx` and 7 service types (actual: 8, including `authentik` + `backups`). Trust source, not the map. Run `project_map_patch` after source edits and `project_map_validate` before final handoff.
|
||||
@@ -0,0 +1,336 @@
|
||||
# Verify Report — prometheus-direct-charting
|
||||
|
||||
> Phase: **verify** · Change: `prometheus-direct-charting` · Repo: `/home/user/manage`
|
||||
> FRESH-CONTEXT adversarial read-only verification of the change against
|
||||
> `proposal.md`, `spec.md`, `design.md`, and `tasks.md`. **No source edits.**
|
||||
> This verify report is the only file written.
|
||||
|
||||
**Head commit verified:** `67ca0fc` (`feat(prometheus-direct-charting): slice 3 — remove grafana + config rewrite + changelog`).
|
||||
|
||||
Three implementation slices are committed:
|
||||
|
||||
- `5dad982` slice 1 — prom range query + chart rebrand
|
||||
- `65bae95` slice 2 — gauge + mean widgets
|
||||
- `67ca0fc` slice 3 — grafana removal + config rewrite + changelog
|
||||
|
||||
> NOTE: the dispatch brief cited slice hashes `58be6e0` / `ba94317` (an earlier
|
||||
> amend state). The actual landed commits are `65bae95` / `67ca0fc`. Content of
|
||||
> all three slices matches the spec/design/tasks; this is informational, not a defect.
|
||||
|
||||
---
|
||||
|
||||
## 0. Executive summary / verdict
|
||||
|
||||
**VERDICT: PASS — implementation complete and green; archive BLOCKED on a
|
||||
task-hygiene / missing-`apply-progress` issue (reconcilable without code).**
|
||||
|
||||
Every functional requirement **SC-101 … SC-127** was checked against source and
|
||||
**passes**. Grafana is fully excised from the live code paths (service type,
|
||||
adapters, widgets, hook, API client, type, status endpoint, nav entry, service
|
||||
tab). Prometheus is the direct chart source: `chart` (multi-series recharts line
|
||||
chart via `/api/v1/query_range`), `gauge` (instant scalar + threshold bands via
|
||||
recharts `RadialBarChart`), and `mean` (client-side average) are wired in both
|
||||
registries with consistent schemas. The shared `normalize_prometheus_matrix` /
|
||||
`step_for_window` helpers are extracted and unit-tested. All four gates are
|
||||
green: backend `pytest` (**293 passed**), `ruff` (**clean**), frontend
|
||||
`npm run build` (**exit 0**), `npm run lint` (**0 errors**).
|
||||
|
||||
Findings:
|
||||
|
||||
- **[CRITICAL — archive blocker, NOT a code defect]** **19 unchecked
|
||||
implementation/verification task checkboxes** remain in `tasks.md` (all of
|
||||
Slice 3 §3.1–3.14 and Integration §4.1–4.5), and **`apply-progress.md` does not
|
||||
exist** to reconcile them. The underlying work *is* done and verified complete
|
||||
against source; the blocker is that the task tracker was never updated and no
|
||||
apply-progress artifact was produced. Reconciliation = tick the boxes + write
|
||||
`apply-progress.md` (no code change). See §4.
|
||||
- **[WARNING]** SC-125 is **PARTIAL**: none of the three new widget tests
|
||||
(`PrometheusChartWidget` / `PrometheusGaugeWidget` / `PrometheusMeanWidget`)
|
||||
exercises an explicit **loading** state (`isLoading:true` → Skeleton). Error
|
||||
and rendered-data cases exist for all three. The loading branch exists in each
|
||||
component; the gap is test coverage, not functionality. See §5.
|
||||
- **[INFO]** Stale generated `.pi-map.md` files still reference Grafana /
|
||||
`ObservabilityPage.tsx`; these are not deliverable source and are ignored by
|
||||
|
||||
> SC-115/116, but should be regenerated (`project_map_patch`/`validate`). The
|
||||
> `config.yaml` context block also still names `ObservabilityPage.tsx` (refactored
|
||||
away into `service-tabs/`) — not an SC-121 criterion, minor doc staleness.
|
||||
|
||||
- **[INFO]** Working tree is not pristine: an uncommitted cosmetic reformat of
|
||||
`frontend/src/pages/service-tabs/MediaTab.tsx` (unrelated to this change) plus
|
||||
untracked `.pi-tmp/*` and `openspec/changes/service-storage-harness/` (the
|
||||
proposal for the *separate* change SC-127 requires independence from).
|
||||
|
||||
---
|
||||
|
||||
## 1. Structured status & actionContext findings
|
||||
|
||||
The native `gentle-pi.sdd-status` reports `changeName: null` /
|
||||
`blockedReasons: ["Change selection is ambiguous: …"]` because the engine
|
||||
auto-detected four active changes. This verify task was **explicitly assigned**
|
||||
`prometheus-direct-charting`; the ambiguity is a parent-resolution artifact and
|
||||
does not block this phase.
|
||||
|
||||
- `artifactStore: openspec`; change root
|
||||
`openspec/changes/prometheus-direct-charting/`.
|
||||
- Artifacts present: `proposal.md`, `spec.md`, `design.md`, `tasks.md`.
|
||||
- **`apply-progress.md`: MISSING** (confirmed: directory contains only the four
|
||||
planning docs + this report). This is the root cause of the §4 archive blocker.
|
||||
- `actionContext`: `mode: repo-local`, `workspaceRoot: /home/user/manage`,
|
||||
`allowedEditRoots: ["/home/user/manage"]`, `warnings: []`. Implementation
|
||||
ownership and all target files are provably inside the authoritative workspace. ✓
|
||||
- The `service-storage-harness` change exists only as an untracked proposal
|
||||
folder; SC-127 (independence) holds — this change builds/tests green without it.
|
||||
|
||||
## 2. Gate results (actual output, run at `67ca0fc`)
|
||||
|
||||
| Gate | Command | Result | Evidence |
|
||||
|------|---------|--------|----------|
|
||||
| Backend tests | `cd backend && PYTHONPATH=src python3 -m pytest -q` | **PASS** | **293 passed, 2 warnings** in 36.26s. Includes `test_prometheus_range.py` (9) + chart/gauge/mean adapter tests. |
|
||||
| Backend lint | `cd backend && PYTHONPATH=src python3 -m ruff check src tests` | **PASS** | `All checks passed!` |
|
||||
| Frontend build | `cd frontend && npm run build` (`tsc -b` + `vite build`) | **PASS** exit 0 | `✓ built in 1.08s`; 2543 modules transformed. Non-fatal `>500 kB` chunk-size warning (pre-existing). |
|
||||
| Frontend lint | `cd frontend && npm run lint` (`eslint .`) | **PASS** exit 0 | `0 errors, 1 warning`. The warning is `react-hooks/exhaustive-deps` in `WidgetConfigDialog.tsx:370` — **pre-existing, untouched by this change** (no slice modified that file). |
|
||||
|
||||
### Grep gates (SC-115 / SC-116 / SC-117 / SC-120)
|
||||
|
||||
```
|
||||
grep -rin grafana backend/src --include='*.py'
|
||||
→ 3 matches, ALL in prometheus_range.py docstrings/comments describing the
|
||||
extraction/removal (explicitly allowed by SC-115). No live grafana code.
|
||||
|
||||
grep -rin grafana frontend/src
|
||||
→ matches ONLY in: stale generated .pi-map.md / .pi-map.index.md files
|
||||
(ignored — generated artifacts), and Dashboard.test.tsx lines 108/115
|
||||
(the whitelisted user-authored shortcut *label* "Grafana"). No live
|
||||
grafana code, no grafana widget/component/hook.
|
||||
|
||||
grep -rin grafana backend/tests --include='*.py'
|
||||
→ ZERO matches (SC-120). (.pyc cache + .pi-map.md are stale; .py sources clean.)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Spec coverage (SC-101 … SC-127)
|
||||
|
||||
| SC | Requirement | Verdict | Evidence |
|
||||
|----|-------------|---------|----------|
|
||||
| SC-101 | range query → `{series}` shape | **PASS** | `sources.py:_fetch_chart` → `_range_query` hits `/api/v1/query_range` with `query/start/end/step`; returns `{"series": normalize_prometheus_matrix(matrix)}` = `{label, points:[{t:int, v:float\|null}]}`. Test `test_prometheus_chart_adapter_runs_range_query` asserts endpoint + params + shape. |
|
||||
| SC-102 | shared, Prometheus-native label normalization | **PASS** | Single helper `normalize_prometheus_matrix` in `prometheus_range.py`; drops `__name__`/`__*`, joins `k=v` (sorted), falls back to `"value"`, dedups with `(n)`. No duplication. `test_prometheus_range.py` covers all rules. |
|
||||
| SC-103 | range-query errors degrade gracefully | **PASS** | `_range_query`/`_instant_query` catch `asyncio.TimeoutError` + `requests.RequestException` → `{"error": ...}`; `.fetch()` wraps the whole body in `try/except → {"error"}`. Never raises. Test `test_prometheus_chart_adapter_degrades_on_http_error`. |
|
||||
| SC-104 | step derived from window (100–300 pts) | **PASS** | `step_for_window = max(15, round(window/200))`. Parametrized test asserts 100–300 points for `1h/6h/24h/7d` + 15s floor + custom target. |
|
||||
| SC-105 | `chart` rebind grafana→prometheus (both registries) | **PASS** | Backend `prometheus.py` declares `chart`; frontend `registry.ts` `prometheus` binding has `chart`. Grafana is gone entirely (so trivially offers no `chart`). |
|
||||
| SC-106 | chart renderer reused unchanged | **PASS** | `PrometheusChartWidget.tsx` preserves `LineChart/Line/XAxis/YAxis/CartesianGrid/Tooltip/ResponsiveContainer/mergeSeries/CHART_COLORS/formatTime`; rename-only + empty-state copy fix. |
|
||||
| SC-107 | chart multi-series | **PASS** | `series.map((s,i) => <Line dataKey={s.label} .../>)` renders every series; no single-series restriction. |
|
||||
| SC-108 | chart window is a preset | **PASS** | chart config = `{promql, window}`; `window ∈ {1h,6h,24h,7d}` resolved via `WINDOW_PRESETS` server-side; no raw from/to/step. |
|
||||
| SC-109 | gauge = instant scalar | **PASS** | `_fetch_gauge` → instant `/api/v1/query` → parses `result[0]["value"][1]` → `{"value":...}`. Bound to `prometheus` in both registries. |
|
||||
| SC-110 | gauge threshold bands | **PASS** | `PrometheusGaugeWidget` (recharts `RadialBarChart`) renders green/amber/red track cells when `warn_at`+`crit_at` set, single neutral track otherwise; `min/max` default to `0`/`max(value,1)`. Config accepts `warn_at/crit_at/min/max/unit`. |
|
||||
| SC-111 | gauge scalar-only | **PASS** | `len(result) != 1 → {"error": "Gauge requires a single-series query; refine your PromQL"}`. Test `test_prometheus_gauge_adapter_rejects_multi_series`. |
|
||||
| SC-112 | mean = client-side mean over window | **PASS** | `_fetch_mean` → `_range_query` → averages non-null samples of the single series → `{"value": mean, "unit?"}`. Test asserts mean of `[1,2,3]=2.0`, NaN-skip `[2,NaN,4]=3.0`. |
|
||||
| SC-113 | mean = plain PromQL + preset | **PASS** | mean config = `{promql, window, unit?}`; user supplies plain PromQL (no `avg_over_time`); window is a preset. |
|
||||
| SC-114 | mean scalar-only | **PASS** | `len(result) != 1 → {"error": ...}`. Test `test_prometheus_mean_adapter_rejects_multi_series`. |
|
||||
| SC-115 | no grafana in backend src | **PASS** | grep → only `prometheus_range.py` docstrings (allowed). `integrations/grafana.py` deleted; no `GrafanaWidgetSource`. |
|
||||
| SC-116 | no grafana in frontend src | **PASS** | grep → only `Dashboard.test.tsx` whitelisted shortcut fixture + stale `.pi-map.md` (ignored). No live code. |
|
||||
| SC-117 | grafana gone from registries | **PASS** | No `grafana` in backend `SERVICE_DEFINITIONS`/`SERVICE_ADAPTERS` or frontend `SERVICE_REGISTRY`/`BUILTIN_WIDGETS`; `integrations/grafana.py` deleted. `registry.test.ts` asserts 5 service types (no grafana). |
|
||||
| SC-118 | grafana status + UI removed | **PASS (intent)** | `get_grafana_status` removed (`monitoring.py`); `useGrafanaStatus`/`fetchGrafanaStatus`/`GrafanaStatus` type removed; `LinksTab.tsx`+test deleted; `service-tabs/index.ts` grafana case removed; `navEntries.ts` grafana entry removed; `Dashboard.tsx` `OBSERVABILITY_TYPES` = `{alertmanager, prometheus}`; `ServicesPage.tsx` copy updated. `ObservabilityPage.tsx` was already refactored into `service-tabs/` (spec drift acknowledged in design §0 / spec SC-118 note). |
|
||||
| SC-119 | orphaned grafana widget degrades | **PASS** | `WidgetInstanceCard`: `resolveWidget` → `undefined` for a grafana-bound widget (no binding) → renders `"Unknown widget: <kind> (service-bound)"` `Alert`. No crash. |
|
||||
| SC-120 | grafana tests removed | **PASS** | No grafana in backend test `.py` or frontend widget tests. Only stale `.pyc` cache + `.pi-map.md`. |
|
||||
| SC-121 | config.yaml accurate | **PASS** | Stale "Do NOT re-implement charting in-app" / "No recharts/d3" claims gone; OBSERVABILITY MODEL states "Manage renders Prometheus-backed metrics directly via recharts … Grafana is no longer integrated." No grafana-as-chart-path. |
|
||||
| SC-122 | CHANGELOG migration note | **PASS** | `[Unreleased]` has "Direct Prometheus charting" + "**BREAKING** — Grafana service type removed" with migration: delete grafana instances → recreate as Prometheus; `grafana/chart` widgets → `prometheus/chart`. |
|
||||
| SC-123 | backend tests pass | **PASS** | `pytest` → 293 passed; covers range→`{series}`, gauge scalar-only, mean aggregation, shared label helper. |
|
||||
| SC-124 | frontend build + lint | **PASS** | `npm run build` exit 0; `npm run lint` 0 errors (1 pre-existing warning). |
|
||||
| SC-125 | new widget kinds have tests | **PARTIAL** | All three test files exist with **error + rendered** cases (gauge has with/without bands; mean has with/without unit). **No explicit loading-state case** (`isLoading:true`) in any of the three — see §5. |
|
||||
| SC-126 | no silent data migration | **PASS** | No grafana→prometheus row transformation code anywhere; only docstring references to manual migration. `settings_store.py` migration is unrelated (`jellyseerr→jellyfin`). |
|
||||
| SC-127 | independent of service-storage-harness | **PASS** | No reference to it in source (only a `prometheus_range.py` docstring noting future reuse); builds/tests green without it; that change exists only as a proposal folder. |
|
||||
|
||||
**Functional spec coverage: 26/27 fully PASS, 1 PARTIAL (SC-125, test-coverage only).**
|
||||
|
||||
---
|
||||
|
||||
## 4. Task completion status — ⚠ archive blocker (reconcilable)
|
||||
|
||||
`tasks.md` checkbox state:
|
||||
|
||||
- **Slice 1 (§1.1–1.10): all 10 `[x]`** ✓
|
||||
- **Slice 2 (§2.1–2.10): all 10 `[x]`** ✓
|
||||
- **Slice 3 (§3.1–3.14): all 14 `[ ]` — UNCHECKED**
|
||||
- **Integration (§4.1–4.5): all 5 `[ ]` — UNCHECKED**
|
||||
|
||||
Total: 20 checked, **19 unchecked**.
|
||||
|
||||
**This is a CRITICAL completeness issue for the archive gate per the verify
|
||||
contract.** However — and this is the important reconciliation — **the Slice 3
|
||||
and Integration work is verifiably DONE against source**:
|
||||
|
||||
| Unchecked task | Actual state (verified) |
|
||||
|----------------|-------------------------|
|
||||
| 3.1 delete `integrations/grafana.py` | **deleted** (`ls` → ENOENT; slice-3 diff `73 ------`) |
|
||||
| 3.2 drop grafana from backend registry | **done** (`registry.py` has no grafana import/entry) |
|
||||
| 3.3 remove `GrafanaWidgetSource` + adapter | **done** (`sources.py` has no class; `SERVICE_ADAPTERS` no grafana; diff `113 ---------`) |
|
||||
| 3.4 remove `get_grafana_status` | **done** (`monitoring.py` grep clean; diff `-25`) |
|
||||
| 3.5 remove grafana backend tests | **done** (`.py` test grep clean) |
|
||||
| 3.6 delete `GrafanaLinkWidget` + barrel | **done** (file gone; `index.ts` export dropped) |
|
||||
| 3.7 remove grafana frontend binding | **done** (`registry.ts` no grafana key; registry.test asserts) |
|
||||
| 3.8 remove grafana nav entry | **done** (`navEntries.ts` grep clean) |
|
||||
| 3.9 remove hook + api client + type | **done** (`useObservability.ts`/`client.ts`/`types/index.ts` grep clean) |
|
||||
| 3.10 delete `LinksTab` + tab-index case | **done** (file + test deleted; `index.ts` grep clean) |
|
||||
| 3.11 Dashboard + ServicesPage | **done** (`OBSERVABILITY_TYPES` = `{alertmanager,prometheus}`; ServicesPage copy updated) |
|
||||
| 3.12 rewrite `config.yaml` | **done** (stale claims gone) |
|
||||
| 3.13 CHANGELOG note | **done** (migration entry present) |
|
||||
| 3.14 verify slice 3 (grep + build + lint + test) | **done** (all green — §2) |
|
||||
| 4.1 full backend pytest | **done** (293 passed) |
|
||||
| 4.2 frontend build + lint | **done** (exit 0 / 0 errors) |
|
||||
| 4.3 grep-clean | **done** (SC-115/116 satisfied) |
|
||||
| 4.4 config.yaml check | **done** (SC-121 satisfied) |
|
||||
| 4.5 CHANGELOG check | **done** (SC-122 satisfied) |
|
||||
|
||||
The unchecked boxes are **stale** (work performed, tracker not updated), and
|
||||
**no `apply-progress.md` exists** to serve as the stale-checkbox reconciliation
|
||||
record the contract permits. Resolution is a **documentation-only** step:
|
||||
tick §3.1–3.14 and §4.1–4.5, and author `apply-progress.md` describing the three
|
||||
landed slices. **No code change is required.**
|
||||
|
||||
> Per the verify contract, an unchecked implementation-task line is an archive
|
||||
> blocker until reconciled. Because the implementation is verified complete, this
|
||||
> blocks **archive** but does **not** block `sdd-sync` of the green code.
|
||||
|
||||
---
|
||||
|
||||
## 5. TDD compliance & assertion-quality assessment
|
||||
|
||||
Strict-TDD was **not** declared active for this change in `config.yaml` /
|
||||
parent prompt / (absent) `apply-progress.md`, so the formal TDD-cycle-evidence
|
||||
check is **not applicable**. Assertion quality was still audited adversarially.
|
||||
|
||||
**Backend assertions — GENUINELY BEHAVIORAL (good).** Spot-checked:
|
||||
|
||||
- `test_prometheus_range.py`: asserts exact label strings (`instance=h:9100 mode=idle`), dedup suffix (`job=x (1)`), null sentinels → `None`, malformed-timestamp drop, point-count band per preset, 15s floor. No tautologies.
|
||||
- `test_widgets.py` adapters: assert the **called URL** (`endswith("/api/v1/query_range")` / `/api/v1/query"`), the **params** (`{start,end,step} ⊆ params`, `query=="up"`), the **return shape** (`result["series"][0]["label"]`, `result["value"]==0.75`), multi-series → `{"error" ... "single-series"}`, and mean arithmetic (`2.0`, NaN-skip `3.0`). These verify real behavior, not smoke.
|
||||
|
||||
**Frontend assertions — adequate, one coverage gap.**
|
||||
|
||||
- Gauge test asserts the **rendered value text** (`0.75 %`, `42 req/s`) and threshold-label presence/absence — meaningful.
|
||||
- Mean test asserts the **rendered value** (`23.5 %`, `1500`) — meaningful.
|
||||
- Chart rendered-case asserts only the **SectionCard title** (`CPU Usage`), not that recharts drew the series SVG — a weak/title-only assertion (acceptable as a "rendered data case" since the component mounts with series data, but it does not prove the lines rendered).
|
||||
|
||||
**[WARNING] SC-125 loading-state gap (see §3).** None of the three widget tests
|
||||
sets `isLoading:true`. The `mockData` helper hard-codes `isLoading:false`, and no
|
||||
case asserts the `<Skeleton>` loading branch. Each component's loading branch
|
||||
exists and is structurally identical to sibling widgets, so this is a
|
||||
**test-coverage gap, not a functional defect**. Recommend adding one
|
||||
`isLoading:true` case per widget to fully satisfy SC-125's enumerated
|
||||
"loading state" requirement. Non-blocking.
|
||||
|
||||
No ghost loops, no type-only assertions, no implementation-detail CSS assertions
|
||||
found. No test mocks grafana anywhere (only the whitelisted `Dashboard.test.tsx`
|
||||
shortcut label).
|
||||
|
||||
## 6. Review-workload / PR-boundary findings
|
||||
|
||||
Per-slice changed lines (numstat, excluding `tasks.md` doc churn):
|
||||
|
||||
| Commit | Slice | Source Δ | Over 400? | Verdict |
|
||||
|--------|-------|----------|-----------|---------|
|
||||
| `5dad982` | 1 range+rebrand+helper | ~333 ins / ~56 del | under (337 net w/ test) | **OK** |
|
||||
| `65bae95` | 2 gauge+mean | ~707 ins / ~32 del | **over** (largely new widgets+tests: gauge 137, mean 59, tests 124, backend tests 197) | **OK** — additive feature code+tests; forecast rated "Medium"; boundary is the feature, not a missed split |
|
||||
| `67ca0fc` | 3 grafana removal+config+changelog | ~123 ins / **~926 del** | under (net negative) | **OK** — mostly deletions |
|
||||
|
||||
The `tasks.md` Review Workload Forecast (`stacked-to-main`, 3 slices, ~850–1,110
|
||||
total) was followed: S1→S2→S3, each independently green. Slice 2's +707 insertions
|
||||
exceed the 400-line *added* budget but are dominated by two new components + their
|
||||
tests + new adapter tests (no `size:exception` flag was recorded, and the forecast
|
||||
itself rated slice 2 "~310–400 changed lines" which under-counts the test volume).
|
||||
This is a **minor forecast-vs-actual variance on an additive slice**, not scope
|
||||
creep — the boundary is exactly the gauge+mean feature, no unrelated files touched.
|
||||
Recommend recording the slice-2 actual in the archive summary. **Non-blocking.**
|
||||
|
||||
Scope was honored: no backend API/type-contract widening, no `service-storage-harness`
|
||||
coupling, `metric` path preserved (`PrometheusMetricWidget` reads `data?.data?.result`
|
||||
as the PromQL data object; `_instant_query` returns `{"result": payload.get("data",{})}`
|
||||
— identical shape).
|
||||
|
||||
## 7. Adversarial checks
|
||||
|
||||
- **Dead imports after grafana removal?** None. Backend `ruff` (catches unused
|
||||
imports) is clean; `sources.py` imports `WINDOW_PRESETS/normalize_prometheus_matrix/
|
||||
step_for_window` — all used. Frontend `tsc`/`eslint` clean (unresolved imports
|
||||
would fail the build).
|
||||
- **`PrometheusWidgetSource.fetch` dispatch cross-contamination?** None. Clean
|
||||
`widget_kind` dispatch: `chart`→`{series}`, `gauge`→`{value,...}`, `mean`→`{value}`,
|
||||
default→`{result}` (metric). Distinct shapes; shared `_range_query`/`_instant_query`
|
||||
only do HTTP + error mapping.
|
||||
- **Config-schema consistency (backend models vs frontend registry)?** Consistent.
|
||||
gauge: backend `{promql, warn_at, crit_at, min, max, unit}` ↔ frontend configSchema
|
||||
`{promql(req), warn_at, crit_at, min, max, unit}`. chart: `{promql, window}` ↔
|
||||
`{promql(req), window}`. mean: `{promql, window, unit}` ↔ `{promql(req), window, unit}`.
|
||||
- **Lingering grafana mocks?** None in `.py`/`.tsx` source. Only stale `.pyc`
|
||||
bytecode cache + `.pi-map.md`.
|
||||
|
||||
## 8. Residual risks / non-blocking findings
|
||||
|
||||
1. **[CRITICAL-process] 19 unchecked tasks + missing `apply-progress.md`** (§4) — archive blocker; reconciliation is doc-only.
|
||||
2. **[WARNING] SC-125 loading-state tests missing** (§5) — coverage gap, not a defect.
|
||||
3. **[INFO] Stale generated `.pi-map.md`** files reference Grafana / `ObservabilityPage.tsx` / `TestGrafanaStatus` / `GrafanaLinkWidget`. Not deliverable source; ignored by SC-115/116. Regenerate via `project_map_patch`/`project_map_validate` (the project-map protocol flags these `dirty`).
|
||||
4. **[INFO] `config.yaml` context** still names `frontend/src/components/ObservabilityPage.tsx` (refactored away into `service-tabs/`). Not an SC-121 criterion (which targets the charting/grafana claims, which are fixed); minor doc staleness.
|
||||
5. **[INFO] Uncommitted `MediaTab.tsx`** cosmetic reformat (Prettier line-wrap of a ternary) — unrelated to this change, predates/orthogonal. Dirty working tree; no files are staged.
|
||||
6. **[INFO] Chunk-size build warning** (~1.1 MB JS) — non-fatal, pre-existing, orthogonal.
|
||||
7. **No browser/visual smoke** performed (out of scope); the recharts `RadialBarChart` gauge and `LineChart` rendering are only structurally tested.
|
||||
|
||||
## 9. Exact blockers
|
||||
|
||||
- **BLOCKER (archive only, doc-reconcilable):** 19 unchecked implementation/verification
|
||||
tasks (§3.1–3.14, §4.1–4.5) and absent `apply-progress.md`. Implementation is
|
||||
verified complete; resolution = tick boxes + write `apply-progress.md`.
|
||||
|
||||
No code-level blockers. All functional requirements SC-101…SC-127 pass (SC-125
|
||||
PARTIAL on test coverage only). All four gates green. **Code is ready for
|
||||
`sdd-sync`; archive requires the checkbox/apply-progress reconciliation.**
|
||||
|
||||
## 10. Recommended next phase
|
||||
|
||||
→ **`sdd-sync`** (code PASS). Concurrently/after: author `apply-progress.md`
|
||||
documenting the three landed slices, and tick §3.1–3.14 + §4.1–4.5 in
|
||||
`tasks.md` to clear the archive blocker. Optionally add three `isLoading:true`
|
||||
widget tests to move SC-125 PARTIAL→PASS, and regenerate the stale `.pi-map.md`.
|
||||
|
||||
---
|
||||
|
||||
### Appendix A — Verification commands run (at `67ca0fc`)
|
||||
|
||||
```
|
||||
cd backend && PYTHONPATH=src python3 -m pytest -q → 293 passed (2 warnings)
|
||||
cd backend && PYTHONPATH=src python3 -m ruff check src tests → All checks passed!
|
||||
cd frontend && npm run build → exit 0 (✓ built; >500kB warning pre-existing)
|
||||
cd frontend && npm run lint → exit 0 (0 errors, 1 pre-existing warning)
|
||||
grep -rin grafana backend/src --include='*.py' → 3 docstring hits in prometheus_range.py (allowed)
|
||||
grep -rin grafana frontend/src → Dashboard.test.tsx fixture + stale .pi-map.md only
|
||||
grep -rin grafana backend/tests --include='*.py' → ZERO
|
||||
ls backend/src/media_library_viewer_api/integrations/grafana.py → ENOENT (deleted)
|
||||
grep -nE '^\s*- \[ \]' tasks.md → 19 unchecked (§3.1–3.14, §4.1–4.5)
|
||||
```
|
||||
|
||||
### Appendix B — Files substantively changed
|
||||
|
||||
**Backend**
|
||||
|
||||
- `widgets/prometheus_range.py` (new) — `WINDOW_PRESETS`, `step_for_window`, `normalize_prometheus_matrix`.
|
||||
- `widgets/sources.py` — dropped `GrafanaWidgetSource`; `PrometheusWidgetSource` gains `_fetch_chart/_fetch_gauge/_fetch_mean` + shared `_range_query/_instant_query`.
|
||||
- `integrations/prometheus.py` — `metric`+`chart`+`gauge`+`mean` widget kinds + config models.
|
||||
- `integrations/registry.py` — grafana import/entry removed.
|
||||
- `integrations/grafana.py` — **deleted**.
|
||||
- `routers/monitoring.py` — `get_grafana_status` removed.
|
||||
- `tests/test_prometheus_range.py` (new), `tests/test_widgets.py` (chart/gauge/mean adapter tests; grafana tests removed), `tests/test_api.py` + `tests/test_services.py` (grafana fixtures/tests removed).
|
||||
|
||||
**Frontend**
|
||||
|
||||
- `widgets/PrometheusChartWidget.tsx` (renamed from GrafanaChartWidget), `PrometheusGaugeWidget.tsx` (new), `PrometheusMeanWidget.tsx` (new) + their tests.
|
||||
- `widgets/GrafanaLinkWidget.tsx` — **deleted**; `widgets/index.ts` barrel updated.
|
||||
- `integrations/registry.ts` — grafana binding removed; chart/gauge/mean added to prometheus; `registry.test.ts` updated.
|
||||
- `integrations/navEntries.ts` — grafana entry removed.
|
||||
- `hooks/useObservability.ts`, `api/client.ts`, `types/index.ts` — grafana hook/client/type removed.
|
||||
- `pages/service-tabs/LinksTab.tsx` (+test) — **deleted**; `service-tabs/index.ts` grafana case removed.
|
||||
- `pages/Dashboard.tsx` (`OBSERVABILITY_TYPES`), `pages/ServicesPage.tsx` (copy) — grafana removed.
|
||||
|
||||
**Docs**
|
||||
|
||||
- `openspec/config.yaml` — stale thin-dashboard/no-recharts claims repealed.
|
||||
- `CHANGELOG.md` — direct-Prometheus-charting + grafana-removal migration note.
|
||||
Reference in New Issue
Block a user