From a47f7710877ad1ef8f5e5c0d9e04230b7ed4c0bc Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Mon, 7 Sep 2026 10:32:27 +0100 Subject: [PATCH] Extract shared declare/update-metric backfill and auto-param insertion helpers declare_metric and update_metric duplicated the resolve-params + backfill_metric + insert-MetricDeclaration sequence; extracted into resolve_backfill_and_store_metric so the declare-vs-rename difference is expressed only in caller-side logic. Also extracted auto_declare_params's per-candidate insertion body into try_insert_auto_param so the candidate-building and insertion steps read as separate concerns. Reimplemented directly against current main (ticket 085) after the original worktree agent's branch was based on a stale pre-consolidation snapshot module layout that no longer matches main. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01TeyhRfMYjMditek6geDRoQ --- src/engine/snapshot/mod.rs | 85 ++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/src/engine/snapshot/mod.rs b/src/engine/snapshot/mod.rs index 85d06c1..e9b4edc 100644 --- a/src/engine/snapshot/mod.rs +++ b/src/engine/snapshot/mod.rs @@ -330,11 +330,18 @@ impl EngineSnapshot { pub(crate) fn declare_metric(&mut self, request: DeclareMetricRequest) -> Result<(), String> { let re = self.validate_metric_request(&request)?; self.metric_regex_cache.insert(request.name.clone(), re.clone()); - let resolved_params = self.resolved_param_declarations(); + self.resolve_backfill_and_store_metric(request, &re); + Ok(()) + } + /// Shared by `declare_metric` and `update_metric`: resolves param declarations, backfills + /// the metric's buckets from existing log history, then writes the `MetricDeclaration` entry. + /// Callers differ only in what happens before this (rename bookkeeping for updates). + fn resolve_backfill_and_store_metric(&mut self, request: DeclareMetricRequest, re: &Regex) { + let resolved_params = self.resolved_param_declarations(); self.backfill_metric( &request.name, - &re, + re, &request.label_params, request.value_param.as_deref(), &resolved_params, @@ -342,7 +349,7 @@ impl EngineSnapshot { request.unmatched_labels.as_ref(), ); self.index.insert( - IndexKey::MetricDeclaration { name: request.name }, + IndexKey::MetricDeclaration { name: request.name.clone() }, DataValue::MetricDeclaration { message_match_regex: request.message_match_regex, label_params: request.label_params, @@ -351,7 +358,6 @@ impl EngineSnapshot { unmatched_labels: request.unmatched_labels, }, ); - Ok(()) } pub(crate) fn validate_update_metric( @@ -372,7 +378,6 @@ impl EngineSnapshot { pub(crate) fn update_metric(&mut self, existing_name: &str, request: DeclareMetricRequest) -> Result<(), String> { let re = self.validate_update_metric(existing_name, &request)?; - let resolved_params = self.resolved_param_declarations(); self.clear_metric_data(existing_name); if request.name != existing_name { @@ -381,25 +386,7 @@ impl EngineSnapshot { } self.metric_regex_cache.insert(request.name.clone(), re.clone()); - self.backfill_metric( - &request.name, - &re, - &request.label_params, - request.value_param.as_deref(), - &resolved_params, - request.matched_labels.as_ref(), - request.unmatched_labels.as_ref(), - ); - self.index.insert( - IndexKey::MetricDeclaration { name: request.name.clone() }, - DataValue::MetricDeclaration { - message_match_regex: request.message_match_regex, - label_params: request.label_params, - value_param: request.value_param, - matched_labels: request.matched_labels, - unmatched_labels: request.unmatched_labels, - }, - ); + self.resolve_backfill_and_store_metric(request, &re); Ok(()) } @@ -698,31 +685,39 @@ impl EngineSnapshot { if new_this_line >= MAX_NEW_AUTO_PARAMS_PER_LINE || total >= MAX_AUTO_DECLARED_PARAMS { break; } - let index_key = IndexKey::ParamDeclaration { name: candidate.name.clone() }; - if self.index.contains_key(&index_key) { - continue; + if self.try_insert_auto_param(candidate, &mut total) { + new_this_line += 1; } - if let Ok(re) = Regex::new(&candidate.message_match_regex) { - self.param_regex_cache.insert(candidate.name.clone(), re); - } - self.index.insert( - index_key, - DataValue::ParamDeclaration(ParamDeclaration { - name: candidate.name, - message_match_regex: candidate.message_match_regex, - capture_group: candidate.capture_group, - json_field_path: candidate.json_field_path, - kind: infer_kind(&candidate.value), - elapsed_since_last: false, - auto_derived: true, - log_stream_gated: false, - }), - ); - new_this_line += 1; - total += 1; } } + /// Inserts `candidate` as an auto-derived `ParamDeclaration` unless a param with that name + /// is already declared, bumping `*total` and returning whether it inserted. + fn try_insert_auto_param(&mut self, candidate: AutoParamCandidate, total: &mut usize) -> bool { + let index_key = IndexKey::ParamDeclaration { name: candidate.name.clone() }; + if self.index.contains_key(&index_key) { + return false; + } + if let Ok(re) = Regex::new(&candidate.message_match_regex) { + self.param_regex_cache.insert(candidate.name.clone(), re); + } + self.index.insert( + index_key, + DataValue::ParamDeclaration(ParamDeclaration { + name: candidate.name, + message_match_regex: candidate.message_match_regex, + capture_group: candidate.capture_group, + json_field_path: candidate.json_field_path, + kind: infer_kind(&candidate.value), + elapsed_since_last: false, + auto_derived: true, + log_stream_gated: false, + }), + ); + *total += 1; + true + } + pub(crate) fn derive_log_stream_params_from_row( &mut self, header: &crate::engine::logfmt::LogStreamHeader,