Auto derived format
This commit is contained in:
+107
-3
@@ -98,6 +98,8 @@ impl From<&ParamDeclarationRecord> for DeclareParamRequest {
|
||||
json_field_path: record.json_field_path.clone(),
|
||||
kind: record.kind.into(),
|
||||
elapsed_since_last: record.elapsed_since_last,
|
||||
// A profile only ever stores manually-declared params (see `record_param_declaration`).
|
||||
auto_derived: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -682,17 +684,32 @@ impl ProfileStore {
|
||||
}
|
||||
|
||||
/// Records/updates a declared metric into `profile_name`'s stored declarations (matched by metric name), persisting the store; same integration intent as `record_param_declaration`.
|
||||
///
|
||||
/// `promoted_params` is any of the metric's referenced `label_params`/`value_param` that live
|
||||
/// only as auto-derived engine state (see `crate::engine::auto_declare_params`) rather than a
|
||||
/// persisted profile param — the caller (`ApiState::persist_metric_declaration`) resolves these
|
||||
/// against the live engine before calling in. Without persisting them alongside the metric, a
|
||||
/// restart would replay the metric before any log line re-derives its auto params, and the
|
||||
/// engine would reject it as referencing an undeclared param. Each is added only if `profile
|
||||
/// .params` doesn't already have an entry of that name, so an existing (e.g. manually-declared)
|
||||
/// param is never clobbered.
|
||||
pub fn record_metric_declaration(
|
||||
&mut self,
|
||||
store_path: &Path,
|
||||
profile_name: &str,
|
||||
request: &DeclareMetricRequest,
|
||||
promoted_params: &[ParamDeclarationRecord],
|
||||
) -> Result<(), ProfileError> {
|
||||
let profile = self
|
||||
.profiles
|
||||
.iter_mut()
|
||||
.find(|p| p.name == profile_name)
|
||||
.ok_or(ProfileError::NotFound)?;
|
||||
for param in promoted_params {
|
||||
if !profile.params.iter().any(|p| p.name == param.name) {
|
||||
profile.params.push(param.clone());
|
||||
}
|
||||
}
|
||||
let record: MetricDeclarationRecord = request.into();
|
||||
match profile.metrics.iter_mut().find(|r| r.name == record.name) {
|
||||
Some(existing) => *existing = record,
|
||||
@@ -702,13 +719,14 @@ impl ProfileStore {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Same as `record_metric_declaration`, for an in-place edit that may rename the metric: drops any persisted record under `old_name` first (when it differs from the new name) so a rename doesn't leave a stale duplicate declaration behind, then upserts by the new name as usual.
|
||||
/// Same as `record_metric_declaration`, for an in-place edit that may rename the metric: drops any persisted record under `old_name` first (when it differs from the new name) so a rename doesn't leave a stale duplicate declaration behind, then upserts by the new name as usual. `promoted_params` has the same meaning as in `record_metric_declaration`.
|
||||
pub fn record_metric_declaration_update(
|
||||
&mut self,
|
||||
store_path: &Path,
|
||||
profile_name: &str,
|
||||
old_name: &str,
|
||||
request: &DeclareMetricRequest,
|
||||
promoted_params: &[ParamDeclarationRecord],
|
||||
) -> Result<(), ProfileError> {
|
||||
let profile = self
|
||||
.profiles
|
||||
@@ -718,6 +736,11 @@ impl ProfileStore {
|
||||
if old_name != request.name {
|
||||
profile.metrics.retain(|r| r.name != old_name);
|
||||
}
|
||||
for param in promoted_params {
|
||||
if !profile.params.iter().any(|p| p.name == param.name) {
|
||||
profile.params.push(param.clone());
|
||||
}
|
||||
}
|
||||
let record: MetricDeclarationRecord = request.into();
|
||||
match profile.metrics.iter_mut().find(|r| r.name == record.name) {
|
||||
Some(existing) => *existing = record,
|
||||
@@ -815,6 +838,7 @@ mod tests {
|
||||
json_field_path: None,
|
||||
kind: MetricPrimitiveKind::String,
|
||||
elapsed_since_last: false,
|
||||
auto_derived: false,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1080,7 +1104,7 @@ mod tests {
|
||||
store.record_param_declaration(&store_path, "web", ¶m_req).unwrap();
|
||||
|
||||
let metric_req = metric_request("requests", r"duration=(?P<duration>\d+)", "duration");
|
||||
store.record_metric_declaration(&store_path, "web", &metric_req).unwrap();
|
||||
store.record_metric_declaration(&store_path, "web", &metric_req, &[]).unwrap();
|
||||
|
||||
let profile = store.get("web").unwrap();
|
||||
assert_eq!(profile.params.len(), 1);
|
||||
@@ -1134,6 +1158,7 @@ mod tests {
|
||||
&store_path,
|
||||
"web",
|
||||
&metric_request("requests", r"request duration=(?P<duration>\d+)", "duration"),
|
||||
&[],
|
||||
)
|
||||
.unwrap();
|
||||
store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap();
|
||||
@@ -1465,7 +1490,7 @@ mod tests {
|
||||
)
|
||||
.unwrap();
|
||||
store
|
||||
.record_metric_declaration(&store_path, "web", &metric_request("requests", r"path=(?P<path>\S+)", "path"))
|
||||
.record_metric_declaration(&store_path, "web", &metric_request("requests", r"path=(?P<path>\S+)", "path"), &[])
|
||||
.unwrap();
|
||||
|
||||
let before = store.get("web").unwrap().clone();
|
||||
@@ -1473,4 +1498,83 @@ mod tests {
|
||||
let after = reloaded.get("web").unwrap().clone();
|
||||
assert_eq!(before, after);
|
||||
}
|
||||
|
||||
/// Acceptance criterion: a metric declared against a param that only exists as auto-derived
|
||||
/// engine state (not yet a persisted profile param) still ingests correctly after a simulated
|
||||
/// restart, because `record_metric_declaration`'s `promoted_params` persists that param into the
|
||||
/// profile alongside the metric. Without this, `load_into_engine` would reject the metric on
|
||||
/// restart since declarations replay before any log line re-derives the auto param.
|
||||
#[tokio::test]
|
||||
async fn metric_referencing_auto_derived_param_survives_restart_via_promotion() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let store_path = dir.path().join("profiles.json");
|
||||
let settings_path = dir.path().join("settings.json");
|
||||
let mut settings = Settings::default();
|
||||
|
||||
let mut store = ProfileStore::load(&store_path);
|
||||
store.create(&store_path, &mut settings, &settings_path, "web".to_string()).unwrap();
|
||||
store.set_active(&mut settings, &settings_path, Some("web".to_string())).unwrap();
|
||||
|
||||
// Simulate the auto-derive path: ingest a line carrying a logfmt key before any param is
|
||||
// manually declared, so `dx` only exists as engine-internal auto-derived state.
|
||||
let (engine_tx, engine_rx) = mpsc::channel(16);
|
||||
let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone()));
|
||||
tokio::spawn(crate::engine::run(engine_rx, settings_handle));
|
||||
engine_tx
|
||||
.send(EngineMessage::Ingest(IngestedLine {
|
||||
sequence: 0,
|
||||
timestamp: Timestamp::from_millis(0),
|
||||
text: "dx=1 moved".to_string(),
|
||||
}))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
// Declare a metric referencing the auto-derived "dx" param — this succeeds live (the param
|
||||
// exists in the engine), mirroring the interactive-declare flow that hit this bug.
|
||||
let metric_req = metric_request("direction_x", r"dx=(?P<dx>\S+) moved", "dx");
|
||||
let (respond_to, response) = oneshot::channel();
|
||||
engine_tx
|
||||
.send(EngineMessage::DeclareMetric { request: metric_req.clone(), respond_to })
|
||||
.await
|
||||
.unwrap();
|
||||
response.await.unwrap().unwrap();
|
||||
|
||||
// Promote "dx" from its live (auto-derived) engine state, as `ApiState::persist_metric_declaration` does.
|
||||
let EngineResponse::ParamDeclarations(live_params) =
|
||||
ask(&engine_tx, EngineQuery::ParamDeclarations).await
|
||||
else {
|
||||
panic!("unexpected response");
|
||||
};
|
||||
let dx = live_params.iter().find(|p| p.name == "dx").unwrap();
|
||||
assert!(dx.auto_derived);
|
||||
let promoted = ParamDeclarationRecord {
|
||||
name: dx.name.clone(),
|
||||
message_match_regex: dx.message_match_regex.clone(),
|
||||
capture_group: dx.capture_group.clone(),
|
||||
json_field_path: dx.json_field_path.clone(),
|
||||
kind: dx.kind.into(),
|
||||
elapsed_since_last: dx.elapsed_since_last,
|
||||
};
|
||||
store.record_metric_declaration(&store_path, "web", &metric_req, &[promoted]).unwrap();
|
||||
|
||||
// Simulate a restart: drop everything, reload from disk into a *fresh* engine with no prior ingestion.
|
||||
drop(store);
|
||||
let reloaded_store = ProfileStore::load(&store_path);
|
||||
let profile = reloaded_store.get("web").unwrap().clone();
|
||||
assert_eq!(profile.params.len(), 1);
|
||||
assert_eq!(profile.params[0].name, "dx");
|
||||
|
||||
let (fresh_engine_tx, fresh_engine_rx) = mpsc::channel(16);
|
||||
let fresh_settings_handle = std::sync::Arc::new(std::sync::Mutex::new(Settings::default()));
|
||||
tokio::spawn(crate::engine::run(fresh_engine_rx, fresh_settings_handle));
|
||||
load_into_engine(&fresh_engine_tx, &profile).await;
|
||||
|
||||
let EngineResponse::MetricDeclarations(metrics) =
|
||||
ask(&fresh_engine_tx, EngineQuery::MetricDeclarations).await
|
||||
else {
|
||||
panic!("unexpected response");
|
||||
};
|
||||
assert_eq!(metrics.len(), 1);
|
||||
assert_eq!(metrics[0].name, "direction_x");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user