Hugh KaznowskiandClaude Sonnet 5
2908cccd76
There is no DELETE route, no persisted-removal path, and no UI affordance for removing a metric once declared — the engine's RemoveMetricDeclaration message only ever fires as part of whole-profile deletion today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
65 lines
3.4 KiB
Markdown
65 lines
3.4 KiB
Markdown
# 051 — Allow deleting a declared metric
|
|
|
|
## Summary
|
|
|
|
There is currently no way to delete a metric once declared. `src/api.rs`
|
|
only routes `GET`/`POST /api/metrics` and `GET`/`PUT /api/metrics/{name}`
|
|
(`src/api.rs:132-135`) — there is no `DELETE`. The engine already has the
|
|
machinery for it internally: `EngineMessage::RemoveMetricDeclaration`
|
|
(`src/engine.rs:103`) removes a metric's declaration from the index, but
|
|
it is only ever sent as part of `delete_profile`'s cleanup
|
|
(`src/api.rs:689`) when an entire profile is deleted, not as a
|
|
user-facing action on a single metric. Persistence has the same gap:
|
|
`Settings`/profile storage (`src/profile.rs`, post ticket 049 now inline
|
|
in `settings.json`) has `record_metric_declaration`/
|
|
`record_metric_declaration_update` but no corresponding removal method, so
|
|
even if a metric were removed from the live engine index there'd be
|
|
nothing to stop it reappearing on next load. The webapp UI
|
|
(`webapp/src/components/Metrics.tsx`) has no delete affordance either —
|
|
compare to param declarations, which have the same underlying gap (no
|
|
`RemoveParamDeclaration`-triggering UI or persisted-removal path either),
|
|
though this ticket is scoped to metrics only.
|
|
|
|
## Scope
|
|
|
|
- Add `DELETE /api/metrics/{name}` in `src/api.rs`, sending
|
|
`EngineMessage::RemoveMetricDeclaration` to the engine (same message
|
|
the profile-delete cleanup path already uses) and returning 404 if the
|
|
metric doesn't exist.
|
|
- Add a persisted-removal method (e.g. `Settings::remove_metric_declaration`
|
|
or equivalent, alongside `record_metric_declaration`/
|
|
`record_metric_declaration_update` in `src/profile.rs`/`src/settings.rs`)
|
|
so a deleted metric doesn't reappear when declarations replay from
|
|
`settings.json` on the next restart, and call it from the new route the
|
|
same way `persist_metric_declaration` is called from `declare_metric`.
|
|
- Add a delete affordance in `webapp/src/components/Metrics.tsx` (icon
|
|
button or similar, following whatever confirm-before-destructive-action
|
|
pattern already exists elsewhere in the settings/profile UI — check
|
|
`NavMenu.tsx`'s profile deletion for the existing convention) that calls
|
|
the new endpoint and removes the metric from the list on success.
|
|
- Decide and document what happens to bucket-value history already
|
|
recorded under the deleted metric's name (e.g. `MetricPrimitive` samples
|
|
indexed by metric name in the engine) — either leave it orphaned
|
|
(queryable by name won't work without a declaration, but doesn't need
|
|
active cleanup) or purge it; state the choice in the PR.
|
|
- Decide and document what happens to dashboard panels or alert rules
|
|
whose PromQL query text/filter references the deleted metric by name —
|
|
at minimum they should fail gracefully (a clear "unknown metric" error
|
|
rather than a panic or silent empty result) rather than needing to be
|
|
actively kept in sync; a full dependent-reference check/warning before
|
|
delete is out of scope unless trivial.
|
|
|
|
## Acceptance criteria
|
|
|
|
- A metric can be deleted via the API and the UI; it disappears from
|
|
`GET /api/metrics` immediately and does not reappear after a restart.
|
|
- Deleting an unknown metric name returns a clear 404, not a 500 or panic.
|
|
- A dashboard panel or alert rule referencing the deleted metric by name
|
|
fails with a readable error rather than crashing the engine or API,
|
|
verified with a seeded fixture.
|
|
|
|
## Depends on
|
|
|
|
None (touches the same engine/persistence seam as ticket 049, but that
|
|
ticket is already merged).
|