From 4e37e2a7ee810f9da98bf0f1044ccad9267a8a20 Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sun, 6 Sep 2026 09:39:22 +0100 Subject: [PATCH] Add bulk clear-params action that re-extracts from retained logs (ticket 052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DELETE /api/params clears the entire standalone-param set — manual and auto-derived alike — then immediately re-runs auto_declare_params against every IndexKey::IngestedLog entry still held in the engine's index (oldest to newest), so params come back fresh from currently-retained log text without requiring new lines to be ingested. This is a deliberate full reset, unlike reload_engine_declarations's profile-switch cleanup, which preserves auto-derived params since those are engine-discovered facts rather than profile state; here the immediate re-extraction step makes clearing them safe. Settings::clear_param_declarations empties the active profile's persisted params so they don't reappear on restart. The webapp's Params section gets a single "Clear all" button (with a confirm dialog and a status line reporting removed/re-derived/logs_considered) rather than a per-param delete loop; logs_considered surfaces that lines already evicted by max_logs can't regain their params. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o --- src/api.rs | 83 +++++++++++++- src/engine.rs | 131 ++++++++++++++++++++++ src/profile.rs | 20 ++++ tickets/052-clear-params-and-reextract.md | 79 ------------- webapp/src/components/Params.tsx | 47 +++++++- webapp/src/lib/api.ts | 15 +++ 6 files changed, 293 insertions(+), 82 deletions(-) delete mode 100644 tickets/052-clear-params-and-reextract.md diff --git a/src/api.rs b/src/api.rs index 3c3a9ae..2c4e05f 100644 --- a/src/api.rs +++ b/src/api.rs @@ -55,6 +55,23 @@ impl ApiState { } } + /// Counterpart to `persist_param_declaration` for `DELETE /api/params` (ticket 052): empties the + /// active profile's stored params so the just-cleared set doesn't reappear from `settings.json` + /// on next load. Same best-effort semantics — a no-op if no profile is active, I/O failures only + /// logged, since the engine-side clear (already applied by the time this runs) is the source of + /// truth for the running process. + fn persist_clear_params(&self) { + let mut settings = self.settings.lock().unwrap(); + let Some(profile_name) = settings.active_profile.clone() else { + return; + }; + if let Err(err) = settings.clear_param_declarations(&self.settings_path, &profile_name) { + eprintln!( + "warning: failed to clear persisted param declarations for profile '{profile_name}': {err}" + ); + } + } + /// Same as `persist_param_declaration`, for metric declarations. Also promotes any of the /// metric's referenced params that only exist as auto-derived engine state (see /// `promoted_param_records`) into persisted profile params, so a metric built against an @@ -128,7 +145,7 @@ pub fn router(engine: EngineHandle, settings: SettingsHandle, settings_path: Pat .route("/api/logs", get(get_logs)) .route("/api/timeline", get(get_timeline)) .route("/api/stats", get(get_stats)) - .route("/api/params", get(list_params).post(declare_param)) + .route("/api/params", get(list_params).post(declare_param).delete(clear_params)) .route("/api/metrics", get(list_metrics).post(declare_metric)) .route( "/api/metrics/{name}", @@ -321,6 +338,25 @@ async fn declare_param( } } +/// `DELETE /api/params` (ticket 052): clears every declared param (manual and auto-derived) and +/// immediately re-derives whatever `auto_declare_params` can still find in log text the engine +/// still retains. The response body's `logs_considered` tells the caller how many retained lines +/// were re-scanned, so the UI can make clear that lines evicted by `max_logs` before this ran +/// can't regain their params — this is expected behavior, not a partial failure. +async fn clear_params(State(state): State) -> impl IntoResponse { + let (respond_to, response) = oneshot::channel(); + if state.engine.send(EngineMessage::ClearParams { respond_to }).await.is_err() { + return StatusCode::SERVICE_UNAVAILABLE.into_response(); + } + match response.await { + Ok(result) => { + state.persist_clear_params(); + Json(result).into_response() + } + Err(_) => StatusCode::SERVICE_UNAVAILABLE.into_response(), + } +} + async fn list_metrics(State(state): State) -> impl IntoResponse { match ask(&state.engine, EngineQuery::MetricDeclarations).await { Ok(EngineResponse::MetricDeclarations(declarations)) => Json(declarations).into_response(), @@ -937,6 +973,51 @@ mod tests { assert_eq!(body["capture_group"], "level"); } + /// Ticket 052's core acceptance criteria: `DELETE /api/params` removes every declared param + /// (manual and auto-derived), that removal survives a reload of persisted settings, and — + /// without any new line being ingested — the retained `IngestedLog` entries immediately + /// re-derive a fresh (auto-derived) param set. + #[tokio::test] + async fn clearing_params_persists_and_reextracts_from_retained_logs() { + let (app, engine, settings_handle, dir) = test_app(); + + let settings_path = dir.path().join("settings.json"); + { + let mut settings = settings_handle.lock().unwrap(); + settings.create_profile(&settings_path, "web".to_string()).unwrap(); + settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); + } + + // A manual declaration plus an auto-derived one (from ingesting a logfmt line before "path" is declared). + declare_test_param(&app, "path", r"request path=(?P\S+)", "path", "string").await; + ingest_lines(&engine, 0, &["request path=/a", "level=info x=1"]).await; + + let (_, before) = get_json(&app, "/api/params").await; + let before_names: Vec<&str> = + before.as_array().unwrap().iter().map(|p| p["name"].as_str().unwrap()).collect(); + assert!(before_names.contains(&"path")); + assert!(before_names.contains(&"level")); + assert!(before_names.contains(&"x")); + + let (status, result) = send_json(&app, "DELETE", "/api/params", json!({})).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(result["removed"], before_names.len() as u64); + // Re-derived without any new ingest — same three keys come back, now all auto-derived. + assert_eq!(result["reextracted"], 3); + assert_eq!(result["logs_considered"], 2); + + let (_, after) = get_json(&app, "/api/params").await; + let after_params = after.as_array().unwrap(); + assert_eq!(after_params.len(), 3); + assert!(after_params.iter().all(|p| p["auto_derived"] == true)); + + // The clear persisted: "path" (the only manually-declared, profile-stored param) doesn't + // reappear from settings.json on a fresh load. + let reloaded_settings = Settings::load(&settings_path); + let profile = reloaded_settings.profile("web").unwrap(); + assert!(profile.params.is_empty()); + } + #[tokio::test] async fn rejects_invalid_param_regex_with_400_and_error_body() { let (app, _engine, _settings, _dir) = test_app(); diff --git a/src/engine.rs b/src/engine.rs index a6794ff..1ff519e 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -36,6 +36,19 @@ pub struct MetricSample { /// Also the request body for `POST /api/params` — `Deserialize` is derived directly here rather than adding a separate DTO, since the shape already matches `ParamDeclaration`. pub type DeclareParamRequest = ParamDeclaration; +/// Response to `EngineMessage::ClearParams` (ticket 052) — also the JSON body for `DELETE /api/params`. +#[derive(Debug, Clone, serde::Serialize)] +pub struct ClearParamsResult { + /// How many param declarations (manual + auto-derived) were removed. + pub removed: usize, + /// How many params `auto_declare_params` re-derived immediately afterward from retained log text. + pub reextracted: usize, + /// How many `IndexKey::IngestedLog` entries were still retained (not yet evicted by `max_logs`) + /// and so were re-scanned — surfaced so a caller can tell the user re-extraction only covers + /// logs still held in memory, not lines evicted before the clear. + pub logs_considered: usize, +} + /// A metric references its params by name (resolved against the engine's standalone /// `ParamDeclaration` store in `apply_metric_to_log`) rather than embedding its own /// regex/capture-group/kind per param — see ticket 037. Also the request body for @@ -90,6 +103,16 @@ pub enum EngineMessage { name: String, respond_to: oneshot::Sender, }, + /// Ticket 052: wipes the entire standalone-param set (manual *and* auto-derived — unlike + /// `reload_engine_declarations`'s profile-switch cleanup, this is an explicit user-driven reset, + /// not a "keep engine-discovered facts across profiles" cleanup), then immediately re-runs + /// `auto_declare_params` over every `IndexKey::IngestedLog` entry still retained in the index, + /// oldest to newest, so declaration-order semantics match a fresh ingest. Lines already evicted + /// by `max_logs` can't regain their params — `ClearParamsResult::logs_considered` reports how + /// many retained lines were re-scanned, so a caller can surface that bound to the user. + ClearParams { + respond_to: oneshot::Sender, + }, DeclareMetric { request: DeclareMetricRequest, respond_to: oneshot::Sender>, @@ -195,6 +218,11 @@ pub async fn run(mut rx: mpsc::Receiver, settings: SettingsHandle let existed = index.remove(&IndexKey::ParamDeclaration { name }).is_some(); let _ = respond_to.send(existed); } + EngineMessage::ClearParams { respond_to } => { + let removed = clear_all_param_declarations(&mut index); + let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index); + let _ = respond_to.send(ClearParamsResult { removed, reextracted, logs_considered }); + } EngineMessage::DeclareMetric { request, respond_to } => { let result = declare_metric(&mut index, request); let _ = respond_to.send(result); @@ -326,6 +354,42 @@ fn auto_declare_params(index: &mut BTreeMap, text: &str) { } } +/// Ticket 052: removes every `IndexKey::ParamDeclaration` entry — manual and auto-derived alike. +/// Returns how many were removed. Unlike `RemoveParamDeclaration`'s single-name removal (used by +/// `reload_engine_declarations` during a profile switch, which deliberately keeps auto-derived +/// params since they're engine-discovered facts rather than profile state), this is a full reset: +/// the caller (`EngineMessage::ClearParams`) immediately re-runs `auto_declare_params` afterward, so +/// wiping the auto-derived ones too just means they come back fresh instead of stale. +fn clear_all_param_declarations(index: &mut BTreeMap) -> usize { + let names: Vec = + param_declarations_range(index).map(|(name, ..)| name.to_string()).collect(); + let removed = names.len(); + for name in names { + index.remove(&IndexKey::ParamDeclaration { name }); + } + removed +} + +/// Ticket 052: re-runs `auto_declare_params` against every `IndexKey::IngestedLog` entry still held +/// in the index, oldest to newest (matching normal ingest order, so declaration-order semantics — +/// e.g. which key "wins" a naming collision — come out the same as if the lines were freshly +/// ingested). Only ever called right after `clear_all_param_declarations`, so every param produced +/// here is fresh, not a re-declaration. +/// +/// Returns `(reextracted, logs_considered)`: how many params came back, and how many retained log +/// lines were re-scanned. Lines already dropped by `evict_excess_logs` before this ran aren't in the +/// index anymore and so can't contribute — `logs_considered` is how the caller surfaces that bound +/// to the user, rather than the re-extraction silently looking complete when it isn't. +fn reextract_params_from_retained_logs(index: &mut BTreeMap) -> (usize, usize) { + let texts: Vec = logs_range(index).map(|(_, _, text)| text.to_string()).collect(); + let logs_considered = texts.len(); + for text in &texts { + auto_declare_params(index, text); + } + let reextracted = param_declarations_range(index).count(); + (reextracted, logs_considered) +} + /// Validates the metric's own regex plus that every name in `label_params`/`value_param` refers to /// an already-declared standalone param (not just the first bad one, so a rejection reports every /// problem at once). Shared by `declare_metric` and `update_metric` so both reject a bad @@ -1928,6 +1992,73 @@ mod tests { assert_eq!(approx_bytes_total, 3); } + fn insert_ingested_log(index: &mut BTreeMap, sequence: u64, text: &str) { + index.insert( + IndexKey::IngestedLog { sequence, timestamp: Timestamp::from_millis(sequence as i64) }, + DataValue::LogLiteral { text: text.to_string() }, + ); + } + + #[test] + fn clear_all_param_declarations_removes_manual_and_auto_derived() { + let mut index = BTreeMap::new(); + auto_declare_params(&mut index, "x1=-4"); + declare_param(&mut index, request("level", r"level=(?P\w+)", "level")).unwrap(); + assert_eq!(param_declarations_range(&index).count(), 2); + + let removed = clear_all_param_declarations(&mut index); + + assert_eq!(removed, 2); + assert_eq!(param_declarations_range(&index).count(), 0); + } + + #[test] + fn reextract_params_from_retained_logs_rebuilds_from_ingested_text_without_new_ingest() { + let mut index = BTreeMap::new(); + insert_ingested_log(&mut index, 0, r#"level=info msg="started""#); + insert_ingested_log(&mut index, 1, "x1=-4 y1=34"); + auto_declare_params(&mut index, r#"level=info msg="started""#); + auto_declare_params(&mut index, "x1=-4 y1=34"); + assert_eq!(param_declarations_range(&index).count(), 4); + + clear_all_param_declarations(&mut index); + assert_eq!(param_declarations_range(&index).count(), 0); + + // No new lines ingested — re-extraction works purely off the `IngestedLog` entries already + // retained in the index (ticket 052's core acceptance criterion). + let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index); + + assert_eq!(logs_considered, 2); + assert_eq!(reextracted, 4); + let declarations: HashMap<&str, _> = + param_declarations_range(&index).map(|d| (d.0, d)).collect(); + assert!(declarations.contains_key("level")); + assert!(declarations.contains_key("msg")); + assert!(declarations.contains_key("x1")); + assert!(declarations.contains_key("y1")); + assert!(declarations.values().all(|d| d.6), "re-extracted params must be flagged auto_derived"); + } + + #[test] + fn reextract_params_from_retained_logs_only_covers_lines_still_in_the_index() { + // Simulates a line evicted (by `max_logs`) before the clear+re-extract runs: its key/value + // pair is simply gone from the index, same as `evict_excess_logs` leaves things, so + // `auto_declare_params` never sees it again. + let mut index = BTreeMap::new(); + insert_ingested_log(&mut index, 1, "x1=-4"); + auto_declare_params(&mut index, "evicted_only_key=1"); + auto_declare_params(&mut index, "x1=-4"); + assert_eq!(param_declarations_range(&index).count(), 2); + + clear_all_param_declarations(&mut index); + let (reextracted, logs_considered) = reextract_params_from_retained_logs(&mut index); + + assert_eq!(logs_considered, 1, "only the retained log line should be re-scanned"); + assert_eq!(reextracted, 1, "the evicted line's key can't come back"); + let declarations: Vec<_> = param_declarations_range(&index).collect(); + assert_eq!(declarations[0].0, "x1"); + } + #[test] fn declarations_stay_within_their_range() { let mut index = BTreeMap::new(); diff --git a/src/profile.rs b/src/profile.rs index c17e648..e3abd50 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -600,6 +600,26 @@ impl Settings { Ok(()) } + /// Empties `profile_name`'s stored param declarations, persisting settings — the counterpart to + /// `record_param_declaration` for `DELETE /api/params` (ticket 052). A profile only ever stores + /// manually-declared params (auto-derived ones live only in the engine, never persisted here), so + /// this is enough to keep the whole set from reappearing on next load; the engine-side clear + /// re-derives whatever auto params the retained log text still supports. + pub fn clear_param_declarations( + &mut self, + settings_path: &Path, + profile_name: &str, + ) -> Result<(), ProfileError> { + let profile = self + .profiles + .iter_mut() + .find(|p| p.name == profile_name) + .ok_or(ProfileError::NotFound)?; + profile.params.clear(); + self.save(settings_path)?; + Ok(()) + } + /// Records/updates a declared param into `profile_name`'s stored declarations (matched by param /// name), persisting settings. Intended to be called by the HTTP API alongside declaring against /// the engine, so declarations survive a restart. diff --git a/tickets/052-clear-params-and-reextract.md b/tickets/052-clear-params-and-reextract.md deleted file mode 100644 index 58fc6db..0000000 --- a/tickets/052-clear-params-and-reextract.md +++ /dev/null @@ -1,79 +0,0 @@ -# 052 — Allow clearing all params, then re-extract them from already-ingested logs - -## Summary - -There is no bulk "clear params" action. The engine already supports -removing a single param declaration internally — -`EngineMessage::RemoveParamDeclaration { name, respond_to }` -(`src/engine.rs:85-92`), handled at `src/engine.rs:190-197` by removing -the `IndexKey::ParamDeclaration` entry — but it is only ever invoked from -`reload_engine_declarations` (`src/api.rs:664-682`), a profile-switch -cleanup path that filters out auto-derived params (comment at -`src/api.rs:665-668`) and removes the rest one at a time. There is no -route exposing this to the user, no persistence-side removal method -(`src/profile.rs` has `record_param_declaration` at `profile.rs:606` but -nothing to un-record it), and no webapp affordance -(`webapp/src/lib/api.ts:283-287` only exposes `getParams`/`declareParam`). - -The harder half of this request — "logs get re-iterated to extract -params from them" — is not just a rewording of `auto_declare_params`. -That function runs once, at ingest time, against each incoming line -(`src/engine.rs:154`, `auto_declare_params` defined at -`src/engine.rs:307-325`, backed by `extract_pairs` in `src/logfmt.rs`) and -is never replayed. Re-extraction after a clear means running that same -logic again, after the fact, against lines already sitting in the index. -This is structurally possible: raw ingested text is retained as -`IndexKey::IngestedLog { sequence, timestamp } -> DataValue::LogLiteral` -in the same `BTreeMap` (`src/engine.rs:148-152`), occupying the index -range between `IndexKey::Start` and `IndexKey::LogParamSplit` -(iterated at `src/engine.rs:1166`, `1181`) — but only for lines that -haven't already been evicted by `max_logs`/`evict_excess_logs` -(`src/engine.rs:151-156`). Lines evicted before a clear+re-extract cannot -regain their params; this bound should be surfaced to the user, not -silently ignored. - -Note `main.rs:75-96`'s `profile::load_into_engine` replay path is not -directly reusable here — it replays *declarations* into a fresh engine -before ingest starts, whereas this feature must re-run *extraction* -against logs the engine already holds. - -## Scope - -- Add a bulk "clear params" `EngineMessage` (or reuse - `RemoveParamDeclaration` in a loop, mirroring - `reload_engine_declarations`'s existing pattern at - `src/api.rs:664-682`) that removes all declared params — deciding - whether auto-derived and manually-declared params are both cleared or - only one category, and stating that choice in the PR (the existing - `reload_engine_declarations` precedent excludes auto-derived ones for a - reason worth re-reading before copying it here). -- Add a persisted-removal counterpart in `src/profile.rs`/`src/settings.rs` - so cleared params don't reappear from `settings.json` on next load. -- After clearing, re-run `auto_declare_params` against every retained - `IndexKey::IngestedLog` entry still in the index (oldest to newest, to - preserve declaration-order semantics), producing a fresh param set from - currently-available log text. -- Expose this as a single user-facing action (`DELETE /api/params` or - similar in `src/api.rs`, plus a button in whatever webapp component - hosts the params list) — not a per-param loop the user has to drive - from the UI. -- Surface to the user (via response body or UI copy) that re-extraction - only covers logs still retained in memory, not lines already evicted by - `max_logs`. - -## Acceptance criteria - -- Triggering "clear params" removes all param declarations from - `GET /api/params` and they do not reappear after a restart. -- Immediately after clearing, params re-appear derived from the log text - still held in the engine's index, without requiring new lines to be - ingested, verified with a seeded fixture where params are cleared and - re-extracted from a fixed set of already-ingested lines. -- If some lines have been evicted before the clear, the re-extracted - param set reflects only the retained subset, and this is not reported - as an error. - -## Depends on - -None (touches the same `RemoveParamDeclaration`/`reload_engine_declarations` -seam used by profile switching). diff --git a/webapp/src/components/Params.tsx b/webapp/src/components/Params.tsx index ebb5c6c..afc51fd 100644 --- a/webapp/src/components/Params.tsx +++ b/webapp/src/components/Params.tsx @@ -1,5 +1,5 @@ import { useMemo, useState } from 'react' -import { ApiError, declareParam, type ParamDeclaration } from '../lib/api' +import { ApiError, clearParams, declareParam, type ParamDeclaration } from '../lib/api' import { CollapseToggle, handleSectionHeaderKeyDown, @@ -48,6 +48,9 @@ export function Params({ params, onDeclared }: { params: ParamDeclaration[]; onD // in place by name, so changing it here would silently create a second param and orphan the // original (and anything referencing it by name) rather than renaming it. const [editingName, setEditingName] = useState(null) + const [clearing, setClearing] = useState(false) + const [clearResult, setClearResult] = useState(null) + const [clearError, setClearError] = useState(null) const isBoolean = kind === 'boolean' const isElapsedSinceLast = source === 'elapsed_since_last' @@ -117,6 +120,33 @@ export function Params({ params, onDeclared }: { params: ParamDeclaration[]; onD } } + // Clears every declared param (manual and auto-derived) in one action, then immediately + // re-derives whatever the engine can still find in retained log text (ticket 052) — a single + // confirmed click rather than removing params one at a time. `logs_considered` in the response + // is surfaced in the status line so it's clear the re-extraction only covers logs the engine + // still holds, not ones already evicted by a max-logs cap. + async function handleClearAll() { + if (params.length === 0) return + if (!window.confirm(`Clear all ${params.length} declared param(s)? Params will immediately be re-derived from log lines still held in memory; lines already evicted by a log cap can't regain their params.`)) { + return + } + setClearing(true) + setClearError(null) + setClearResult(null) + try { + const result = await clearParams() + setClearResult( + `Cleared ${result.removed}, re-derived ${result.reextracted} from ${result.logs_considered} retained log line(s).`, + ) + resetForm() + onDeclared() + } catch (err) { + setClearError(err instanceof ApiError ? err.message : 'Failed to clear params') + } finally { + setClearing(false) + } + } + return (
-
Declared params
+
+ Declared params + +
+ {clearResult &&
{clearResult}
} + {clearError &&
{clearError}
}
{params.length === 0 && No params declared yet.} {params.map((p) => ( diff --git a/webapp/src/lib/api.ts b/webapp/src/lib/api.ts index 7832faf..0fc3bb7 100644 --- a/webapp/src/lib/api.ts +++ b/webapp/src/lib/api.ts @@ -287,6 +287,21 @@ export function declareParam(request: ParamDeclaration): Promise { + return sendJson('/api/params', 'DELETE') +} + export function fetchMetricDeclarations(): Promise { return getJson('/api/metrics') }