From 8faf12f819d72d9b5accb60c1faa2f09fd34beed Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sat, 5 Sep 2026 15:24:28 +0100 Subject: [PATCH] Add Duration-type metric param: elapsed time since previous same-label match (ticket 023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MetricPrimitiveKind::Duration/MetricPrimitive::Duration(i64 ms) is a new primitive kind. On a metric's Value-role param (ticket 022), kind Duration is special-cased in apply_metric_to_log: instead of parsing the captured text, the value is derived as elapsed time since the previous counter event with the same label set, using the ingestion timestamp. That "previous event" lookup is tracked via a new IndexKey::LastMetricEvent/DataValue::LastEventTimestamp bookkeeping entry rather than Bucket.values itself, because a label set's first occurrence has nothing to diff against and pushes no Bucket entry at all (per spec: omit, don't emit a bogus zero) — but it still needs to be recorded so the *next* occurrence has something to diff against. Duration values compose with the existing bucket/PromQL surface unchanged (coerce_to_f64, filter comparisons, histogram()/gauge()). On any other param (Label-role metric param, or a standalone ParamDeclaration), Duration kind instead parses captured text as a literal i64 millisecond count, same as Number. Profile round-trip (ProfileMetricPrimitiveKind) and the Metrics.tsx declaration form are updated to match. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o --- src/data_value.rs | 13 +++- src/engine.rs | 124 +++++++++++++++++++++++++++++- src/index_key.rs | 11 +++ src/metric_primitive.rs | 7 ++ src/profile.rs | 4 + webapp/src/components/Metrics.tsx | 3 +- webapp/src/lib/api.ts | 9 ++- 7 files changed, 165 insertions(+), 6 deletions(-) diff --git a/src/data_value.rs b/src/data_value.rs index 95ed219..ca7ffc5 100644 --- a/src/data_value.rs +++ b/src/data_value.rs @@ -33,12 +33,19 @@ pub enum ParamRole { /// `ProfileMetricPrimitiveKind`), so `POST /api/params`/`POST /api/metrics` /// bodies use `"string"`/`"number"`/`"timestamp"`. #[allow(dead_code)] -#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum MetricPrimitiveKind { String, Number, Timestamp, + /// Ticket 023: milliseconds. On a metric's `Value`-role param, this + /// doesn't parse the captured text at all — the value is instead derived + /// as the elapsed time since the previous counter event with the same + /// label set (see `crate::engine::apply_metric_to_log`). On any other + /// param (a `Label`-role metric param, or a standalone `ParamDeclaration`), + /// it parses the captured text as a literal `i64` millisecond count. + Duration, } /// Value type paired with [`crate::index_key::IndexKey`] in the engine's primary index. @@ -62,4 +69,8 @@ pub enum DataValue { BucketValue { values: Vec<(u64, Timestamp, MetricPrimitive)>, }, + /// Paired with `IndexKey::LastMetricEvent` (ticket 023). + LastEventTimestamp { + timestamp: Timestamp, + }, } diff --git a/src/engine.rs b/src/engine.rs index 701ea3e..652ce70 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -266,6 +266,12 @@ fn parse_metric_primitive(raw: &str, kind: MetricPrimitiveKind) -> Option raw.parse::().ok().map(MetricPrimitive::Duration), } } @@ -286,6 +292,18 @@ fn parse_metric_primitive(raw: &str, kind: MetricPrimitiveKind) -> Option, metric_name: &str, @@ -301,10 +319,15 @@ fn apply_metric_to_log( let mut labels = BTreeMap::new(); let mut value = None; + let mut is_duration = false; for matcher in params { let Some(raw) = captures.name(&matcher.capture_group) else { return; }; + if matcher.role == ParamRole::Value && matcher.kind == MetricPrimitiveKind::Duration { + is_duration = true; + continue; + } let Some(parsed) = parse_metric_primitive(raw.as_str(), matcher.kind) else { return; }; @@ -318,12 +341,37 @@ fn apply_metric_to_log( } } + let value = if is_duration { + // Bookkeeping key is separate from the `Bucket` key below: a + // `Duration` value's first occurrence for this label set pushes no + // `Bucket` entry (nothing to diff against), so the "last event" + // timestamp can't live inside `Bucket.values` — it has to be tracked + // independently so the *next* occurrence still has something to + // diff against. + 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()) + } + None => return, + } + } else { + value.unwrap_or(MetricPrimitive::Number(Number::Int(1))) + }; + let key = IndexKey::Bucket { metric_name: metric_name.to_string(), labels }; + let entry = index .entry(key) .or_insert_with(|| DataValue::BucketValue { values: Vec::new() }); if let DataValue::BucketValue { values } = entry { - values.push((sequence, timestamp, value.unwrap_or(MetricPrimitive::Number(Number::Int(1))))); + values.push((sequence, timestamp, value)); } } @@ -580,6 +628,13 @@ fn compare_metric_primitive( (MetricPrimitive::String(s), FilterValue::Number(n)) => { s.as_str().partial_cmp(n.to_string().as_str()) } + (MetricPrimitive::Duration(ms), FilterValue::Number(v)) => (*ms as f64).partial_cmp(v), + (MetricPrimitive::Duration(ms), FilterValue::Raw(raw)) => { + let v: f64 = raw + .parse() + .map_err(|_| format!("expected a numeric value, got {raw:?}"))?; + (*ms as f64).partial_cmp(&v) + } }; Ok(ordering.is_some_and(|ordering| apply_compare_op(ordering, op))) } @@ -899,6 +954,7 @@ fn coerce_to_f64(value: &MetricPrimitive) -> f64 { Number::Float(v) => *v, }, MetricPrimitive::Timestamp(ts) => ts.as_millis() as f64, + MetricPrimitive::Duration(ms) => *ms as f64, MetricPrimitive::String(_) => 1.0, } } @@ -1653,6 +1709,72 @@ mod tests { assert_eq!(series[2].points[0].1, 1.0); } + fn declare_login_duration_metric(index: &mut BTreeMap) { + declare_metric( + index, + metric_request_with_roles( + "login_gap", + r"user=(?P\S+) logged in", + vec![ + ("user", "user", MetricPrimitiveKind::String, ParamRole::Label), + ("gap", "user", MetricPrimitiveKind::Duration, ParamRole::Value), + ], + ), + ) + .unwrap(); + } + + #[test] + fn duration_value_param_computes_elapsed_time_since_previous_same_labels_event() { + let mut index = BTreeMap::new(); + declare_login_duration_metric(&mut index); + + ingest_log(&mut index, 0, "user=alice logged in"); + ingest_log(&mut index, 1, "user=bob logged in"); + ingest_log(&mut index, 2, "user=alice logged in"); + + let values: Vec<_> = bucket_values(&index, "login_gap", None).collect(); + // First occurrences (alice@0, bob@1) produce no duration value at all — + // only alice's second occurrence (sequence 2) has a previous event to + // diff against. + assert_eq!(values.len(), 1); + let (labels, (sequence, timestamp, value)) = &values[0]; + assert_eq!(labels.get("user").map(|v| v.to_string()), Some("alice".to_string())); + assert_eq!(*sequence, 2); + // `ingest_log`'s helper timestamps by `sequence as millis`. + assert_eq!(timestamp.as_millis(), 2); + assert_eq!(*value, MetricPrimitive::Duration(2)); + } + + #[test] + fn duration_diffs_against_the_immediately_preceding_event_not_the_first() { + let mut index = BTreeMap::new(); + declare_login_duration_metric(&mut index); + + ingest_log(&mut index, 0, "user=alice logged in"); + ingest_log(&mut index, 3, "user=alice logged in"); + ingest_log(&mut index, 9, "user=alice logged in"); + + let values: Vec<_> = bucket_values(&index, "login_gap", None).collect(); + assert_eq!(values.len(), 2); + assert_eq!(values[0].1.2, MetricPrimitive::Duration(3)); + assert_eq!(values[1].1.2, MetricPrimitive::Duration(6)); + } + + #[test] + fn duration_values_are_queryable_via_promql_like_any_other_metric() { + let mut index = BTreeMap::new(); + declare_login_duration_metric(&mut index); + ingest_log(&mut index, 0, "user=alice logged in"); + ingest_log(&mut index, 10, "user=alice logged in"); + + let parsed = promql::parse_query("login_gap").unwrap(); + let series = evaluate_promql(&index, &parsed, None, None); + + assert_eq!(series.len(), 1); + assert_eq!(series[0].points, vec![(10, 10.0)]); + } + #[test] fn handle_query_promql_rejects_malformed_query() { let index = BTreeMap::new(); diff --git a/src/index_key.rs b/src/index_key.rs index ffbdcc7..2215d13 100644 --- a/src/index_key.rs +++ b/src/index_key.rs @@ -33,6 +33,17 @@ pub enum IndexKey { metric_name: String, labels: BTreeMap, }, + /// Ticket 023: internal bookkeeping (never returned by a query) tracking + /// the timestamp of the most recent counter event for one metric+label + /// combination, independent of whether that event actually pushed a + /// `Bucket` value — a `Duration`-kind `Value` param's *first* occurrence + /// of a label set has no previous event to diff against and pushes no + /// `Bucket` entry, but must still be recorded here so the *next* + /// occurrence can compute its elapsed time. + LastMetricEvent { + metric_name: String, + labels: BTreeMap, + }, /// Marker for the end of the whole range. Nothing sorts after this. End, } diff --git a/src/metric_primitive.rs b/src/metric_primitive.rs index 27bb644..ab4b060 100644 --- a/src/metric_primitive.rs +++ b/src/metric_primitive.rs @@ -10,6 +10,10 @@ pub enum MetricPrimitive { String(String), Number(Number), Timestamp(Timestamp), + /// Milliseconds elapsed (ticket 023). Distinct from `Number` so a + /// duration reads unambiguously in JSON/label form, even though it's + /// stored the same way (a plain `i64`). + Duration(i64), } impl PartialEq for MetricPrimitive { @@ -32,12 +36,14 @@ impl Ord for MetricPrimitive { MetricPrimitive::String(_) => 0, MetricPrimitive::Number(_) => 1, MetricPrimitive::Timestamp(_) => 2, + MetricPrimitive::Duration(_) => 3, } } match (self, other) { (MetricPrimitive::String(a), MetricPrimitive::String(b)) => a.cmp(b), (MetricPrimitive::Number(a), MetricPrimitive::Number(b)) => a.cmp(b), (MetricPrimitive::Timestamp(a), MetricPrimitive::Timestamp(b)) => a.cmp(b), + (MetricPrimitive::Duration(a), MetricPrimitive::Duration(b)) => a.cmp(b), (a, b) => rank(a).cmp(&rank(b)), } } @@ -54,6 +60,7 @@ impl fmt::Display for MetricPrimitive { MetricPrimitive::Number(Number::Int(n)) => write!(f, "{n}"), MetricPrimitive::Number(Number::Float(n)) => write!(f, "{n}"), MetricPrimitive::Timestamp(ts) => write!(f, "{}", ts.as_millis()), + MetricPrimitive::Duration(ms) => write!(f, "{ms}"), } } } diff --git a/src/profile.rs b/src/profile.rs index 6439e92..e89bb4c 100644 --- a/src/profile.rs +++ b/src/profile.rs @@ -60,6 +60,8 @@ pub enum ProfileMetricPrimitiveKind { String, Number, Timestamp, + /// Mirrors `MetricPrimitiveKind::Duration` (ticket 023). + Duration, } impl From for ProfileMetricPrimitiveKind { @@ -68,6 +70,7 @@ impl From for ProfileMetricPrimitiveKind { MetricPrimitiveKind::String => ProfileMetricPrimitiveKind::String, MetricPrimitiveKind::Number => ProfileMetricPrimitiveKind::Number, MetricPrimitiveKind::Timestamp => ProfileMetricPrimitiveKind::Timestamp, + MetricPrimitiveKind::Duration => ProfileMetricPrimitiveKind::Duration, } } } @@ -78,6 +81,7 @@ impl From for MetricPrimitiveKind { ProfileMetricPrimitiveKind::String => MetricPrimitiveKind::String, ProfileMetricPrimitiveKind::Number => MetricPrimitiveKind::Number, ProfileMetricPrimitiveKind::Timestamp => MetricPrimitiveKind::Timestamp, + ProfileMetricPrimitiveKind::Duration => MetricPrimitiveKind::Duration, } } } diff --git a/webapp/src/components/Metrics.tsx b/webapp/src/components/Metrics.tsx index 02d48a8..2a39ff0 100644 --- a/webapp/src/components/Metrics.tsx +++ b/webapp/src/components/Metrics.tsx @@ -213,12 +213,13 @@ export function Metrics({ range }: { range: TimeRange | null }) { +