phughk/ottyl
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Make elapsed-since-last an explicit param extractor, not a Duration-kind special case (ticket 038)

Browse Source 
Previously a Value-role param with kind: Duration silently meant "elapsed
time since the previous same-labeled event" instead of parsed text — the
same MetricPrimitiveKind::Duration variant meant two unrelated things
depending on which list (label_params vs value_param) referenced it.

Add ParamDeclaration::elapsed_since_last, a third extraction source
alongside capture_group/json_field_path: when set, the param is computed
(via a new resolve_param_value, used uniformly for label_params and
value_param) rather than text-extracted, and declare_param validates it's
paired with kind: Duration. apply_metric_to_log no longer branches on
(role, kind) — it dispatches purely on what each resolved param declares.

Migrate existing profiles: migrate_duration_value_params converts any
Duration-kind param referenced as a metric's value_param into the new
extractor, and resolve_or_create_legacy_param (ticket 037's pre-existing
migration) now carries the same meaning forward for pre-037 inline
Value-role Duration matchers.

Params.tsx offers "Elapsed since previous event" as its own Extraction
option (hides the now-irrelevant message-match-regex/capture-group
fields and forces kind to duration); Metrics.tsx labels such params
accordingly wherever they're listed as label/value param choices.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
Hugh Kaznowski
2026-09-05 23:01:32 +01:00
co-authored by Claude Sonnet 5
parent 375aa38ef8
commit 5e30e02a78
8 changed files with 352 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/engine.rs
+137 -65
View File
@@ -219,11 +219,19 @@ fn declare_param(
index: &mut BTreeMap<IndexKey, DataValue>,
request: DeclareParamRequest,
) -> Result<(), String> {
// An `elapsed_since_last` param is never text-extracted at all, so `message_match_regex` is
// irrelevant to it — but the field always compiles as a regex regardless of extractor (an empty
// pattern is valid and matches trivially), so it's harmless to still require it to parse.
let re = Regex::new(&request.message_match_regex)
.map_err(|err| format!("invalid regex: {err}"))?;
if request.elapsed_since_last && request.kind != MetricPrimitiveKind::Duration {
return Err("elapsed_since_last params must declare kind 'duration'".to_string());
}
// A Boolean-kind param never uses a capture group — its value is just whether the regex matched.
// Neither does an `elapsed_since_last` param — its value is computed, not captured.
let uses_json_field = request.json_field_path.as_deref().is_some_and(|path| !path.is_empty());
if request.kind != MetricPrimitiveKind::Boolean
&& !request.elapsed_since_last
&& !uses_json_field
&& !re.capture_names().flatten().any(|name| name == request.capture_group)
{
@@ -371,7 +379,9 @@ fn parse_metric_primitive(raw: &str, kind: MetricPrimitiveKind) -> Option<Metric
.ok()
.map(MetricPrimitive::Timestamp)
}
// A Value-role param never reaches this branch (see apply_metric_to_log); reached only for a Label-role param or standalone ParamDeclaration, parsed as a literal millisecond count.
// An `elapsed_since_last` param never reaches this branch (see `resolve_param_value`, which
// computes its value directly rather than extracting/parsing text); reached only for a
// Duration-kind param that parses a literal captured/field millisecond count.
MetricPrimitiveKind::Duration => raw.parse::<i64>().ok().map(MetricPrimitive::Duration),
// A standalone Boolean param never reaches this branch (see derive_log_params, which derives its
// value from whether the regex matched rather than parsing captured text); reached only if a metric
@@ -389,11 +399,15 @@ struct ResolvedParam {
capture_group: String,
json_field_path: Option<String>,
kind: MetricPrimitiveKind,
/// See `crate::data_value::ParamDeclaration::elapsed_since_last`. When set, `regex`/
/// `capture_group`/`json_field_path` above are unused — resolution goes through
/// `resolve_param_value`'s computed path instead of `extract_param_raw`.
elapsed_since_last: bool,
}
fn resolved_param_declarations(index: &BTreeMap<IndexKey, DataValue>) -> HashMap<String, ResolvedParam> {
param_declarations_range(index)
.filter_map(|(name, message_match_regex, capture_group, json_field_path, kind)| {
.filter_map(|(name, message_match_regex, capture_group, json_field_path, kind, elapsed_since_last)| {
Regex::new(message_match_regex).ok().map(|regex| {
(
name.to_string(),
@@ -402,6 +416,7 @@ fn resolved_param_declarations(index: &BTreeMap<IndexKey, DataValue>) -> HashMap
capture_group: capture_group.to_string(),
json_field_path: json_field_path.map(|path| path.to_string()),
kind,
elapsed_since_last,
},
)
})
@@ -426,14 +441,50 @@ fn extract_param_raw(
}
}
/// Resolves one param's value against `text`/`timestamp`, dispatching purely on what the param
/// itself declares (`ResolvedParam::elapsed_since_last`) rather than on whether it's being resolved
/// as a label or as the value — the same resolution is used for both in `apply_metric_to_log`, so
/// an `elapsed_since_last` param behaves identically regardless of which list references it. An
/// elapsed-since-last param's value is elapsed time since the previous event that produced
/// `labels_so_far` under `metric_name`, tracked via a `LastMetricEvent` index entry; its first
/// occurrence returns `None` (nothing to diff against yet) rather than a bogus zero. Every other
/// param falls through to ordinary text extraction (regex capture group or JSON field path) parsed
/// as its declared kind.
fn resolve_param_value(
index: &mut BTreeMap<IndexKey, DataValue>,
metric_name: &str,
resolved: &ResolvedParam,
labels_so_far: &BTreeMap<String, MetricPrimitive>,
text: &str,
timestamp: Timestamp,
) -> Option<MetricPrimitive> {
if resolved.elapsed_since_last {
// Tracked separately from Bucket: the first occurrence for a given label set pushes no
// Bucket entry, so the last-event timestamp can't live in Bucket.values.
let last_event_key = IndexKey::LastMetricEvent {
metric_name: metric_name.to_string(),
labels: labels_so_far.clone(),
};
let previous_timestamp = match index.get(&last_event_key) {
Some(DataValue::LastEventTimestamp { timestamp: previous }) => Some(*previous),
_ => None,
};
index.insert(last_event_key, DataValue::LastEventTimestamp { timestamp });
return previous_timestamp
.map(|previous| MetricPrimitive::Duration(timestamp.as_millis() - previous.as_millis()));
}
let raw = extract_param_raw(text, &resolved.regex, &resolved.capture_group, resolved.json_field_path.as_deref())?;
parse_metric_primitive(&raw, resolved.kind)
}
/// Resolves `label_params`/`value_param` by name against `resolved_params` — a dangling reference
/// (a param removed after this metric was declared) skips the whole match, same as any other
/// extraction failure. At most one param is the value param (its primitive becomes the event's
/// value, otherwise a fixed `1` marker is used); any param failing to match/parse skips the whole
/// match rather than storing a partial one. A `Duration`-kind value param is elapsed time since the
/// previous same-labeled event, so the first occurrence pushes no bucket (nothing to diff against)
/// rather than a bogus zero. When `regex` (the metric's own message-match regex) doesn't match and
/// `unmatched_labels` is `Some`, an occurrence is still registered with those fixed labels.
/// match rather than storing a partial one. When `regex` (the metric's own message-match regex)
/// doesn't match and `unmatched_labels` is `Some`, an occurrence is still registered with those
/// fixed labels.
#[allow(clippy::too_many_arguments)]
fn apply_metric_to_log(
index: &mut BTreeMap<IndexKey, DataValue>,
@@ -462,65 +513,27 @@ fn apply_metric_to_log(
let mut labels = BTreeMap::new();
for param_name in label_params {
let Some(resolved) = resolved_params.get(param_name) else { return };
let Some(raw) = extract_param_raw(
text,
&resolved.regex,
&resolved.capture_group,
resolved.json_field_path.as_deref(),
) else {
return;
};
let Some(parsed) = parse_metric_primitive(&raw, resolved.kind) else {
let Some(parsed) = resolve_param_value(index, metric_name, resolved, &labels, text, timestamp) else {
return;
};
labels.insert(param_name.clone(), parsed);
}
let mut value = None;
let mut is_duration = false;
if let Some(value_param) = value_param {
let Some(resolved) = resolved_params.get(value_param) else { return };
if resolved.kind == MetricPrimitiveKind::Duration {
is_duration = true;
} else {
let Some(raw) = extract_param_raw(
text,
&resolved.regex,
&resolved.capture_group,
resolved.json_field_path.as_deref(),
) else {
return;
};
let Some(parsed) = parse_metric_primitive(&raw, resolved.kind) else {
return;
};
value = Some(parsed);
}
}
if let Some(matched_labels) = matched_labels {
for (name, value) in matched_labels {
labels.insert(name.clone(), MetricPrimitive::String(value.clone()));
}
}
let value = if is_duration {
// Tracked separately from Bucket: a Duration's first occurrence pushes no Bucket entry, so the last-event timestamp can't live in Bucket.values.
let last_event_key =
IndexKey::LastMetricEvent { metric_name: metric_name.to_string(), labels: labels.clone() };
let previous_timestamp = match index.get(&last_event_key) {
Some(DataValue::LastEventTimestamp { timestamp: previous }) => Some(*previous),
_ => None,
};
index.insert(last_event_key, DataValue::LastEventTimestamp { timestamp });
match previous_timestamp {
Some(previous) => {
MetricPrimitive::Duration(timestamp.as_millis() - previous.as_millis())
let value = match value_param {
Some(value_param) => {
let Some(resolved) = resolved_params.get(value_param) else { return };
match resolve_param_value(index, metric_name, resolved, &labels, text, timestamp) {
Some(parsed) => parsed,
None => return,
}
None => return,
}
} else {
value.unwrap_or(MetricPrimitive::Number(Number::Int(1)))
None => MetricPrimitive::Number(Number::Int(1)),
};
push_bucket_value(index, metric_name, labels, sequence, timestamp, value);
@@ -943,7 +956,7 @@ fn metric_declarations_range(
fn param_declarations_range(
index: &BTreeMap<IndexKey, DataValue>,
) -> impl Iterator<Item = (&str, &str, &str, Option<&str>, MetricPrimitiveKind)> {
) -> impl Iterator<Item = (&str, &str, &str, Option<&str>, MetricPrimitiveKind, bool)> {
index
.range(IndexKey::LogParamSplit..IndexKey::ParamMetricSplit)
.filter_map(|(key, value)| match (key, value) {
@@ -954,6 +967,7 @@ fn param_declarations_range(
capture_group,
json_field_path,
kind,
elapsed_since_last,
..
}),
) => Some((
@@ -962,17 +976,23 @@ fn param_declarations_range(
capture_group.as_str(),
json_field_path.as_deref(),
*kind,
*elapsed_since_last,
)),
_ => None,
})
}
/// A param whose stored regex fails to compile (shouldn't happen — `declare_param` validates it) is skipped rather than panicking.
/// A param whose stored regex fails to compile (shouldn't happen — `declare_param` validates it) is
/// skipped rather than panicking. An `elapsed_since_last` param is skipped outright — it has no
/// meaningful reading as a standalone logs-table column (its value depends on which metric's label
/// set it's grouped under, which only exists once it's referenced by a metric's `label_params`/
/// `value_param`).
fn compiled_param_declarations(
index: &BTreeMap<IndexKey, DataValue>,
) -> Vec<(String, Regex, String, Option<String>, MetricPrimitiveKind)> {
param_declarations_range(index)
.filter_map(|(name, message_match_regex, capture_group, json_field_path, kind)| {
.filter(|(.., elapsed_since_last)| !elapsed_since_last)
.filter_map(|(name, message_match_regex, capture_group, json_field_path, kind, _)| {
Regex::new(message_match_regex).ok().map(|re| {
(
name.to_string(),
@@ -1178,12 +1198,15 @@ fn handle_query(
}
EngineQuery::ParamDeclarations => {
let declarations = param_declarations_range(index)
.map(|(name, message_match_regex, capture_group, json_field_path, kind)| ParamDeclarationDto {
name: name.to_string(),
message_match_regex: message_match_regex.to_string(),
capture_group: capture_group.to_string(),
json_field_path: json_field_path.map(|path| path.to_string()),
kind,
.map(|(name, message_match_regex, capture_group, json_field_path, kind, elapsed_since_last)| {
ParamDeclarationDto {
name: name.to_string(),
message_match_regex: message_match_regex.to_string(),
capture_group: capture_group.to_string(),
json_field_path: json_field_path.map(|path| path.to_string()),
kind,
elapsed_since_last,
}
})
.collect();
EngineResponse::ParamDeclarations(declarations)
@@ -1554,6 +1577,7 @@ mod tests {
capture_group: capture_group.to_string(),
json_field_path: None,
kind: MetricPrimitiveKind::String,
elapsed_since_last: false,
}
}
@@ -1589,6 +1613,34 @@ mod tests {
assert!(index.is_empty());
}
#[test]
fn declares_valid_elapsed_since_last_param_without_capture_group() {
let mut index = BTreeMap::new();
declare_elapsed_since_last_param(&mut index, "gap");
let declarations: Vec<_> = param_declarations_range(&index).collect();
assert_eq!(declarations.len(), 1);
assert!(declarations[0].5, "expected elapsed_since_last to be true");
}
#[test]
fn rejects_elapsed_since_last_param_with_non_duration_kind() {
let mut index = BTreeMap::new();
let result = declare_param(
&mut index,
DeclareParamRequest {
name: "gap".to_string(),
message_match_regex: ".".to_string(),
capture_group: String::new(),
json_field_path: None,
kind: MetricPrimitiveKind::String,
elapsed_since_last: true,
},
);
assert!(result.is_err());
assert!(index.is_empty());
}
#[test]
fn declares_valid_param_from_json_field_path() {
let mut index = BTreeMap::new();
@@ -1602,6 +1654,7 @@ mod tests {
capture_group: String::new(),
json_field_path: Some("foo.bar".to_string()),
kind: MetricPrimitiveKind::String,
elapsed_since_last: false,
},
);
assert!(result.is_ok());
@@ -1622,6 +1675,7 @@ mod tests {
capture_group: String::new(),
json_field_path: Some("foo.bar".to_string()),
kind: MetricPrimitiveKind::Number,
elapsed_since_last: false,
},
)
.unwrap();
@@ -1737,6 +1791,26 @@ mod tests {
capture_group: capture_group.to_string(),
json_field_path: None,
kind,
elapsed_since_last: false,
},
)
.unwrap();
}
/// Declares a standalone `elapsed_since_last` param (ticket 038) — computed as elapsed time
/// since the previous event sharing whatever label set it's grouped under, never text-extracted.
/// `message_match_regex`/`capture_group` are unused for this extractor but still need to satisfy
/// `declare_param`'s baseline "is this a valid regex" check, so an always-matching `"."` is fine.
fn declare_elapsed_since_last_param(index: &mut BTreeMap<IndexKey, DataValue>, name: &str) {
declare_param(
index,
DeclareParamRequest {
name: name.to_string(),
message_match_regex: ".".to_string(),
capture_group: String::new(),
json_field_path: None,
kind: MetricPrimitiveKind::Duration,
elapsed_since_last: true,
},
)
.unwrap();
@@ -1957,6 +2031,7 @@ mod tests {
capture_group: String::new(),
json_field_path: Some("request.path".to_string()),
kind: MetricPrimitiveKind::String,
elapsed_since_last: false,
},
)
.unwrap();
@@ -2579,10 +2654,7 @@ mod tests {
fn declare_login_duration_metric(index: &mut BTreeMap<IndexKey, DataValue>) {
let pattern = r"user=(?P<user>\S+) logged in";
declare_test_param(index, "user", pattern, "user", MetricPrimitiveKind::String);
// A Duration-kind value param never actually extracts via its own capture group (see
// `apply_metric_to_log`) — its regex/capture-group only need to satisfy `declare_param`'s
// validation, so reusing "user" here is harmless.
declare_test_param(index, "gap", pattern, "user", MetricPrimitiveKind::Duration);
declare_elapsed_since_last_param(index, "gap");
declare_metric(index, metric_request_with_value("login_gap", pattern, vec!["user"], Some("gap"))).unwrap();
}