Merge ticket 052: allow clearing all params and re-extracting from ingested logs
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
+82
-1
@@ -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
|
||||
@@ -143,7 +160,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}",
|
||||
@@ -336,6 +353,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<ApiState>) -> 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<ApiState>) -> impl IntoResponse {
|
||||
match ask(&state.engine, EngineQuery::MetricDeclarations).await {
|
||||
Ok(EngineResponse::MetricDeclarations(declarations)) => Json(declarations).into_response(),
|
||||
@@ -981,6 +1017,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<path>\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();
|
||||
|
||||
+131
@@ -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<bool>,
|
||||
},
|
||||
/// 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<ClearParamsResult>,
|
||||
},
|
||||
DeclareMetric {
|
||||
request: DeclareMetricRequest,
|
||||
respond_to: oneshot::Sender<Result<(), String>>,
|
||||
@@ -196,6 +219,11 @@ pub async fn run(mut rx: mpsc::Receiver<EngineMessage>, 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);
|
||||
@@ -400,6 +428,42 @@ fn auto_declare_log_stream_params(index: &mut BTreeMap<IndexKey, DataValue>, tex
|
||||
}
|
||||
}
|
||||
|
||||
/// 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<IndexKey, DataValue>) -> usize {
|
||||
let names: Vec<String> =
|
||||
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<IndexKey, DataValue>) -> (usize, usize) {
|
||||
let texts: Vec<String> = 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
|
||||
@@ -2116,6 +2180,73 @@ mod tests {
|
||||
assert_eq!(approx_bytes_total, 3);
|
||||
}
|
||||
|
||||
fn insert_ingested_log(index: &mut BTreeMap<IndexKey, DataValue>, 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<level>\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();
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user