From 534dc52e05a353ab9b4b5c7420b3fc6d7bf40a2d Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sun, 6 Sep 2026 01:50:16 +0100 Subject: [PATCH] Rename reserved sequence/timestamp params to ottyl_seq/ottyl_ts; add ottyl_fmt Ticket 044: `sequence` and `timestamp` collided with ordinary user-declared param names and didn't signal they're ottyl-owned, so they're renamed (breaking change, no back-compat alias) to `ottyl_seq`/`ottyl_ts` in the filter grammar (src/filter_parser.pest, src/filter_parser.rs), alert query restriction/evaluation (src/alerts.rs), and the filter-bar docs/placeholder in the webapp. Also adds a new reserved param `ottyl_fmt` exposing which format a line was auto-detected as ("json"/"logfmt"/"none") via a new `crate::logfmt::detect_format`, wired into the same filter grammar and alert restriction list as `ottyl_seq`/`ottyl_ts`. A mixed-format fixture test in engine.rs verifies `ottyl_fmt` reflects the actual detected format per line. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o --- src/alerts.rs | 37 ++++++++++++-- src/engine.rs | 75 ++++++++++++++++++++++++----- src/filter_parser.pest | 5 +- src/filter_parser.rs | 42 ++++++++++------ src/logfmt.rs | 38 +++++++++++++++ webapp/src/components/Docs.tsx | 21 +++++--- webapp/src/components/FilterBar.tsx | 2 +- 7 files changed, 179 insertions(+), 41 deletions(-) diff --git a/src/alerts.rs b/src/alerts.rs index 65f403d..3071b51 100644 --- a/src/alerts.rs +++ b/src/alerts.rs @@ -1,6 +1,6 @@ //! Alert rules: definitions, trigger evaluation, and ntfy delivery. //! -//! Alert queries are restricted to regex/sequence/timestamp/json[field] — a metric[param] comparison would require a full bucket rescan per line, breaking the proportional-cost guarantee. json[field] is fine here since, like regex/sequence/timestamp, it only inspects the line's own text/fields, no index lookup. +//! Alert queries are restricted to regex/ottyl_seq/ottyl_ts/ottyl_fmt/json[field] — a metric[param] comparison would require a full bucket rescan per line, breaking the proportional-cost guarantee. json[field] is fine here since, like regex/ottyl_seq/ottyl_ts/ottyl_fmt, it only inspects the line's own text/fields, no index lookup. //! //! `Threshold`/`Ratio` windows are maintained via `crate::rate::RateWindow` — the same windowed-rate //! implementation PromQL's `rate()` uses (`crate::engine::evaluate_promql`) — rather than a @@ -196,16 +196,16 @@ fn eval_alert_expr(expr: &FilterExpr, sequence: u64, timestamp: Timestamp, text: let re = Regex::new(pattern).map_err(|err| format!("invalid regex in alert query: {err}"))?; Ok(re.is_match(text)) } - FilterExpr::Comparison { metric, param: None, op, value } if metric == "sequence" => { + FilterExpr::Comparison { metric, param: None, op, value } if metric == "ottyl_seq" => { let target = match value { FilterValue::Number(n) => *n, FilterValue::Raw(raw) => raw .parse::() - .map_err(|_| format!("'sequence' comparisons require a numeric value, got {raw:?}"))?, + .map_err(|_| format!("'ottyl_seq' comparisons require a numeric value, got {raw:?}"))?, }; Ok(apply_compare_op((sequence as f64).total_cmp(&target), *op)) } - FilterExpr::Comparison { metric, param: None, op, value } if metric == "timestamp" => { + FilterExpr::Comparison { metric, param: None, op, value } if metric == "ottyl_ts" => { let target_ms = match value { FilterValue::Number(n) => *n as i64, FilterValue::Raw(raw) => filter_datetime::parse_datetime(raw, Timestamp::now()) @@ -214,8 +214,15 @@ fn eval_alert_expr(expr: &FilterExpr, sequence: u64, timestamp: Timestamp, text: }; Ok(apply_compare_op(timestamp.as_millis().cmp(&target_ms), *op)) } + FilterExpr::Comparison { metric, param: None, op, value } if metric == "ottyl_fmt" => { + let target = match value { + FilterValue::Number(n) => n.to_string(), + FilterValue::Raw(raw) => raw.clone(), + }; + Ok(apply_compare_op(crate::logfmt::detect_format(text).cmp(target.as_str()), *op)) + } FilterExpr::Comparison { metric, .. } => Err(format!( - "alert queries only support regex()/sequence/timestamp (not metric comparisons like \ + "alert queries only support regex()/ottyl_seq/ottyl_ts/ottyl_fmt (not metric comparisons like \ '{metric}[param]'): metric-based alert conditions aren't supported yet" )), FilterExpr::JsonComparison { field, op, value } => { @@ -574,6 +581,26 @@ mod tests { assert!(fired.is_empty()); } + #[test] + fn ottyl_fmt_alert_query_matches_detected_format() { + let rule = immediate_rule("ottyl_fmt==json"); + let mut states = HashMap::new(); + let index: BTreeMap = BTreeMap::new(); + + let fired = evaluate_alerts( + &[rule.clone()], + &mut states, + &index, + 0, + Timestamp::from_millis(0), + r#"{"level":"error"}"#, + ); + assert_eq!(fired.len(), 1); + + let fired = evaluate_alerts(&[rule], &mut states, &index, 1, Timestamp::from_millis(1), "plain text"); + assert!(fired.is_empty()); + } + #[test] fn metric_comparison_in_alert_query_is_rejected_not_panicking() { let rule = immediate_rule("some_metric[param]==5"); diff --git a/src/engine.rs b/src/engine.rs index 5a01237..5ed83fd 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -738,6 +738,9 @@ enum CompiledFilter { Regex(Regex), Sequence { op: CompareOp, target: f64 }, Timestamp { op: CompareOp, target_ms: i64 }, + /// `ottyl_fmt value` — like `Json`, this is line-local and re-detected fresh per line in + /// `eval_compiled` (`crate::logfmt::detect_format`) rather than pre-resolved against the index. + Fmt { op: CompareOp, target: String }, MetricMatch(HashSet), /// `json[field] value` — unlike `MetricMatch`, this can't be pre-resolved against the /// index (the JSON lives in the log line itself, not a declared metric/param), so it's @@ -767,8 +770,9 @@ fn compile_filter( .map(CompiledFilter::Regex) .map_err(|err| format!("invalid regex in filter: {err}")), FilterExpr::Comparison { metric, param, op, value } => match metric.as_str() { - "sequence" => compile_sequence_comparison(*op, value), - "timestamp" => compile_timestamp_comparison(*op, value, now), + "ottyl_seq" => compile_ottyl_seq_comparison(*op, value), + "ottyl_ts" => compile_ottyl_ts_comparison(*op, value, now), + "ottyl_fmt" => Ok(compile_ottyl_fmt_comparison(*op, value)), other => compile_metric_comparison(index, other, param.as_deref(), *op, value, now), }, FilterExpr::JsonComparison { field, op, value } => Ok(CompiledFilter::Json { @@ -787,19 +791,19 @@ fn compile_filter( } } -/// A `Raw` value is accepted defensively (parsed as a number) even though the grammar shouldn't produce one for `sequence`. -fn compile_sequence_comparison(op: CompareOp, value: &FilterValue) -> Result { +/// A `Raw` value is accepted defensively (parsed as a number) even though the grammar shouldn't produce one for `ottyl_seq`. +fn compile_ottyl_seq_comparison(op: CompareOp, value: &FilterValue) -> Result { let target = match value { FilterValue::Number(n) => *n, FilterValue::Raw(raw) => raw.parse::().map_err(|_| { - format!("'sequence' comparisons require a numeric value, got {raw:?}") + format!("'ottyl_seq' comparisons require a numeric value, got {raw:?}") })?, }; Ok(CompiledFilter::Sequence { op, target }) } /// A `Number` is treated as raw milliseconds; a `Raw` token is resolved via `filter_datetime::parse_datetime` against `now`. -fn compile_timestamp_comparison( +fn compile_ottyl_ts_comparison( op: CompareOp, value: &FilterValue, now: Timestamp, @@ -813,7 +817,19 @@ fn compile_timestamp_comparison( Ok(CompiledFilter::Timestamp { op, target_ms }) } -/// `metric[param] value` for any metric name other than `sequence`/`timestamp` — see [`CompiledFilter`]'s doc comment. +/// A `Number` target is stringified since `ottyl_fmt`'s value ("json"/"logfmt"/"none", +/// `crate::logfmt::detect_format`) is always text; any op is accepted since the strings compare +/// lexicographically fine for `==`/`!=` (the only ops that make real sense here), same as +/// `compare_metric_primitive`'s `String` arm. +fn compile_ottyl_fmt_comparison(op: CompareOp, value: &FilterValue) -> CompiledFilter { + let target = match value { + FilterValue::Number(n) => n.to_string(), + FilterValue::Raw(raw) => raw.clone(), + }; + CompiledFilter::Fmt { op, target } +} + +/// `metric[param] value` for any metric name other than `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` — see [`CompiledFilter`]'s doc comment. fn compile_metric_comparison( index: &BTreeMap, metric: &str, @@ -985,6 +1001,9 @@ fn eval_compiled(compiled: &CompiledFilter, sequence: u64, timestamp: Timestamp, CompiledFilter::Timestamp { op, target_ms } => { apply_compare_op(timestamp.as_millis().cmp(target_ms), *op) } + CompiledFilter::Fmt { op, target } => { + apply_compare_op(crate::logfmt::detect_format(text).cmp(target.as_str()), *op) + } CompiledFilter::MetricMatch(sequences) => sequences.contains(&sequence), CompiledFilter::Json { field, op, value } => eval_json_comparison(text, field, *op, value), CompiledFilter::And(lhs, rhs) => { @@ -2983,13 +3002,13 @@ mod tests { } #[test] - fn logs_query_with_sequence_comparison_filters_by_sequence() { + fn logs_query_with_ottyl_seq_comparison_filters_by_sequence() { let mut index = BTreeMap::new(); for i in 0..5u64 { ingest_log(&mut index, i, &format!("line {i}")); } - match handle_query(&index, logs_query(Some("sequence<2")), 0) { + match handle_query(&index, logs_query(Some("ottyl_seq<2")), 0) { EngineResponse::Logs(entries) => { let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); assert_eq!(sequences, vec![0, 1]); @@ -2999,7 +3018,7 @@ mod tests { } #[test] - fn logs_query_with_timestamp_comparison_filters_by_timestamp() { + fn logs_query_with_ottyl_ts_comparison_filters_by_timestamp() { let mut index = BTreeMap::new(); for i in 0..5u64 { let timestamp = Timestamp::from_millis(i as i64 * 1000); @@ -3009,7 +3028,7 @@ mod tests { ); } - match handle_query(&index, logs_query(Some("timestamp>2000")), 0) { + match handle_query(&index, logs_query(Some("ottyl_ts>2000")), 0) { EngineResponse::Logs(entries) => { let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); assert_eq!(sequences, vec![3, 4]); @@ -3018,6 +3037,40 @@ mod tests { } } + #[test] + fn logs_query_with_ottyl_fmt_comparison_filters_by_detected_format_for_mixed_format_lines() { + let mut index = BTreeMap::new(); + // Seeded fixture mixing all three detectable formats on purpose (ticket 044). + ingest_log(&mut index, 0, r#"{"level":"error","code":500}"#); + ingest_log(&mut index, 1, r#"ts=1788648105773 msg="entity_moved" x1=-4"#); + ingest_log(&mut index, 2, "just some plain prose here"); + ingest_log(&mut index, 3, r#"{"level":"info","code":200}"#); + + match handle_query(&index, logs_query(Some("ottyl_fmt==json")), 0) { + EngineResponse::Logs(entries) => { + let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); + assert_eq!(sequences, vec![0, 3]); + } + _ => panic!("expected Logs response"), + } + + match handle_query(&index, logs_query(Some("ottyl_fmt==logfmt")), 0) { + EngineResponse::Logs(entries) => { + let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); + assert_eq!(sequences, vec![1]); + } + _ => panic!("expected Logs response"), + } + + match handle_query(&index, logs_query(Some("ottyl_fmt==none")), 0) { + EngineResponse::Logs(entries) => { + let sequences: Vec = entries.iter().map(|e| e.sequence).collect(); + assert_eq!(sequences, vec![2]); + } + _ => panic!("expected Logs response"), + } + } + #[test] fn logs_query_with_json_field_comparison_filters_by_json_field() { let mut index = BTreeMap::new(); diff --git a/src/filter_parser.pest b/src/filter_parser.pest index 3ccaeba..9f1dd18 100644 --- a/src/filter_parser.pest +++ b/src/filter_parser.pest @@ -4,8 +4,9 @@ // - regex("...") raw-text regex search over log lines // - metric_name[param] value metric comparisons // - json[field] value JSON field comparisons (line parsed as JSON) -// - sequence value reserved metric name (no [param]) -// - timestamp value reserved metric name (no [param]) +// - ottyl_seq value reserved metric name (no [param]) +// - ottyl_ts value reserved metric name (no [param]) +// - ottyl_fmt value reserved metric name (no [param]) // - `and` / `or` boolean combinators, with `and` binding tighter than `or` // - parenthesized sub-expressions diff --git a/src/filter_parser.rs b/src/filter_parser.rs index efdc6f3..430591f 100644 --- a/src/filter_parser.rs +++ b/src/filter_parser.rs @@ -284,12 +284,12 @@ mod tests { } #[test] - fn parses_reserved_metric_sequence() { - let expr = parse("sequence<5").expect("should parse"); + fn parses_reserved_metric_ottyl_seq() { + let expr = parse("ottyl_seq<5").expect("should parse"); assert_eq!( expr, FilterExpr::Comparison { - metric: "sequence".to_string(), + metric: "ottyl_seq".to_string(), param: None, op: CompareOp::Lt, value: FilterValue::Number(5.0), @@ -298,12 +298,12 @@ mod tests { } #[test] - fn parses_reserved_metric_timestamp_as_raw_value() { - let expr = parse("timestamp>12:00").expect("should parse"); + fn parses_reserved_metric_ottyl_ts_as_raw_value() { + let expr = parse("ottyl_ts>12:00").expect("should parse"); assert_eq!( expr, FilterExpr::Comparison { - metric: "timestamp".to_string(), + metric: "ottyl_ts".to_string(), param: None, op: CompareOp::Gt, value: FilterValue::Raw("12:00".to_string()), @@ -311,6 +311,20 @@ mod tests { ); } + #[test] + fn parses_reserved_metric_ottyl_fmt() { + let expr = parse("ottyl_fmt==json").expect("should parse"); + assert_eq!( + expr, + FilterExpr::Comparison { + metric: "ottyl_fmt".to_string(), + param: None, + op: CompareOp::Eq, + value: FilterValue::Raw("json".to_string()), + } + ); + } + #[test] fn parses_json_field_comparison() { let expr = parse("json[level]==error").expect("should parse"); @@ -339,7 +353,7 @@ mod tests { #[test] fn json_field_composes_with_and_or() { - let expr = parse(r#"json[level]==error and sequence<5"#).expect("should parse"); + let expr = parse(r#"json[level]==error and ottyl_seq<5"#).expect("should parse"); let expected = FilterExpr::And( Box::new(FilterExpr::JsonComparison { field: "level".to_string(), @@ -347,7 +361,7 @@ mod tests { value: FilterValue::Raw("error".to_string()), }), Box::new(FilterExpr::Comparison { - metric: "sequence".to_string(), + metric: "ottyl_seq".to_string(), param: None, op: CompareOp::Lt, value: FilterValue::Number(5.0), @@ -358,21 +372,21 @@ mod tests { #[test] fn parses_and_or_with_and_binding_tighter() { - let expr = parse(r#"regex("error") and sequence<5 or timestamp>12:00"#) + let expr = parse(r#"regex("error") and ottyl_seq<5 or ottyl_ts>12:00"#) .expect("should parse"); let expected = FilterExpr::Or( Box::new(FilterExpr::And( Box::new(FilterExpr::Regex("error".to_string())), Box::new(FilterExpr::Comparison { - metric: "sequence".to_string(), + metric: "ottyl_seq".to_string(), param: None, op: CompareOp::Lt, value: FilterValue::Number(5.0), }), )), Box::new(FilterExpr::Comparison { - metric: "timestamp".to_string(), + metric: "ottyl_ts".to_string(), param: None, op: CompareOp::Gt, value: FilterValue::Raw("12:00".to_string()), @@ -384,20 +398,20 @@ mod tests { #[test] fn parenthesized_expression_overrides_precedence() { - let expr = parse(r#"regex("error") and (sequence<5 or timestamp>12:00)"#) + let expr = parse(r#"regex("error") and (ottyl_seq<5 or ottyl_ts>12:00)"#) .expect("should parse"); let expected = FilterExpr::And( Box::new(FilterExpr::Regex("error".to_string())), Box::new(FilterExpr::Or( Box::new(FilterExpr::Comparison { - metric: "sequence".to_string(), + metric: "ottyl_seq".to_string(), param: None, op: CompareOp::Lt, value: FilterValue::Number(5.0), }), Box::new(FilterExpr::Comparison { - metric: "timestamp".to_string(), + metric: "ottyl_ts".to_string(), param: None, op: CompareOp::Gt, value: FilterValue::Raw("12:00".to_string()), diff --git a/src/logfmt.rs b/src/logfmt.rs index 7a44a23..540460e 100644 --- a/src/logfmt.rs +++ b/src/logfmt.rs @@ -61,6 +61,22 @@ pub fn extract_pairs(line: &str) -> Vec<(&str, &str, bool)> { results } +/// The value of the `ottyl_fmt` reserved param (`crate::engine::compile_filter`/`eval_alert_expr`): +/// which format a line was recognized as when parsed. JSON is checked first since a JSON object can +/// incidentally contain `key=value`-shaped substrings that `extract_pairs` would otherwise pick up; +/// only a top-level object counts (arrays/scalars aren't what `crate::engine::eval_json_comparison` +/// treats as JSON either). Anything else falls back to logfmt-style pair extraction, and a line with +/// neither is `"none"`. +pub fn detect_format(text: &str) -> &'static str { + if serde_json::from_str::(text).is_ok_and(|value| value.is_object()) { + "json" + } else if !extract_pairs(text).is_empty() { + "logfmt" + } else { + "none" + } +} + #[cfg(test)] mod tests { use super::*; @@ -108,4 +124,26 @@ mod tests { let pairs = extract_pairs(r#"msg="he said \"hi\"""#); assert_eq!(pairs, vec![("msg", r#"he said \"hi\""#, true)]); } + + #[test] + fn detects_json_object_lines_as_json() { + assert_eq!(detect_format(r#"{"level":"error","code":500}"#), "json"); + } + + #[test] + fn detects_key_value_lines_as_logfmt() { + assert_eq!(detect_format(r#"ts=1788648105773 msg="entity_moved" x1=-4"#), "logfmt"); + } + + #[test] + fn detects_plain_prose_as_none() { + assert_eq!(detect_format("just some plain prose here"), "none"); + } + + #[test] + fn a_bare_json_array_or_scalar_is_not_treated_as_json() { + // Not an object, so falls through to logfmt/none like `eval_json_comparison` would treat it. + assert_eq!(detect_format("[1,2,3]"), "none"); + assert_eq!(detect_format("42"), "none"); + } } diff --git a/webapp/src/components/Docs.tsx b/webapp/src/components/Docs.tsx index f9c03d9..d244e8d 100644 --- a/webapp/src/components/Docs.tsx +++ b/webapp/src/components/Docs.tsx @@ -87,19 +87,23 @@ const SECTIONS: DocSection[] = [ {'regex("error")'} — lines containing "error".
  • - {'regex("error") and sequence<5'} — lines containing "error" with sequence under 5. + {'regex("error") and ottyl_seq<5'} — lines containing "error" with sequence under 5.
  • {'requests[duration]>100'} — the requests metric's captured{' '} duration param over 100.
  • - {'timestamp>-5m'} — lines from the last 5 minutes. + {'ottyl_ts>-5m'} — lines from the last 5 minutes. +
    • +
  • + {'ottyl_fmt==json'} — lines auto-detected as JSON.

    • - sequence and timestamp are reserved metric names present on every log line, so - they're always filterable without declaring anything. + ottyl_seq, ottyl_ts, and ottyl_fmt are reserved metric names present + on every log line, so they're always filterable without declaring anything. ottyl_fmt holds the + auto-detected format of the line's text — json, logfmt, or none.

    Date/time formats

    Timestamp comparisons accept three forms:

    @@ -121,7 +125,7 @@ const SECTIONS: DocSection[] = [
    • Only lines mentioning "timeout" in the last 15 minutes:{' '} - {'regex("timeout") and timestamp>-15m'} + {'regex("timeout") and ottyl_ts>-15m'}
    • Requests slower than 200ms, once a requests metric has a value-role{' '} @@ -134,7 +138,7 @@ const SECTIONS: DocSection[] = [ {'requests[path]==/checkout and regex("5[0-9]{2}")'}
    • - Everything before noon on a specific date: {'timestamp<2024-06-15T12:00:00Z'} + Everything before noon on a specific date: {'ottyl_ts<2024-06-15T12:00:00Z'}
    @@ -367,8 +371,9 @@ const SECTIONS: DocSection[] = [

    Query-based conditions (Immediate/Threshold/Ratio) reuse the filter bar's syntax, restricted to{' '} - regex(...), sequence, timestamp, and and/or - . A metric_name[param] comparison isn't supported in an alert query. + regex(...), ottyl_seq, ottyl_ts, ottyl_fmt, and{' '} + and/or. A metric_name[param] comparison isn't supported in an alert + query.

    A window accepts a plain number of milliseconds or a humantime-style duration: 30s,{' '} diff --git a/webapp/src/components/FilterBar.tsx b/webapp/src/components/FilterBar.tsx index 55bae43..d7418dc 100644 --- a/webapp/src/components/FilterBar.tsx +++ b/webapp/src/components/FilterBar.tsx @@ -35,7 +35,7 @@ export function FilterBar({ {collapseToggle} setDraft(e.target.value)} />