diff --git a/src/api.rs b/src/api.rs index 2641851..fd24877 100644 --- a/src/api.rs +++ b/src/api.rs @@ -8,6 +8,7 @@ use crate::dto::{ ParamDeclarationDto, LogsParams, ProfileSummaryDto, PromqlParams, SaveDashboardRequest, SettingsDto, TestAlertNotificationRequest, TimelineParams, UpdateAlertRuleRequest, UpdateSettingsRequest, }; +use crate::data_value::MetricPrimitiveKind; use crate::engine::{ DeclareMetricRequest, DeclareParamRequest, EngineHandle, EngineMessage, EngineQuery, EngineResponse, @@ -312,12 +313,61 @@ async fn get_stats(State(state): State) -> impl IntoResponse { async fn list_params(State(state): State) -> impl IntoResponse { match ask(&state.engine, EngineQuery::ParamDeclarations).await { - Ok(EngineResponse::ParamDeclarations(declarations)) => Json(declarations).into_response(), + Ok(EngineResponse::ParamDeclarations(mut declarations)) => { + // `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` (ticket 055) are computed line-locally rather than + // stored as index `ParamDeclaration` entries (see `crate::engine::declare_param`'s + // rejection of these names), so they're merged in here purely for discoverability + // rather than via `param_declarations_range`. This is deliberately a merge only in this + // one HTTP-facing listing, not in `EngineQuery::ParamDeclarations` itself — other + // internal callers of that query (promoting a metric's referenced params, replaying + // declarations on profile switch) must keep seeing only real, persistable entries. + declarations.extend(reserved_param_declaration_dtos()); + Json(declarations).into_response() + } Ok(_) => StatusCode::INTERNAL_SERVER_ERROR.into_response(), Err(status) => status.into_response(), } } +/// `GET /api/params`-only synthesized entries for the three reserved names +/// (`crate::engine::OTTYL_RESERVED_PARAM_NAMES`) — see `list_params`. `message_match_regex`/ +/// `capture_group` are empty since these are never text-extracted; `reserved: true` marks them as +/// non-deletable/non-editable, distinct from `auto_derived`. +fn reserved_param_declaration_dtos() -> [ParamDeclarationDto; 3] { + [ + ParamDeclarationDto { + name: "ottyl_seq".to_string(), + message_match_regex: String::new(), + capture_group: String::new(), + json_field_path: None, + kind: MetricPrimitiveKind::Number, + elapsed_since_last: false, + auto_derived: false, + reserved: true, + }, + ParamDeclarationDto { + name: "ottyl_ts".to_string(), + message_match_regex: String::new(), + capture_group: String::new(), + json_field_path: None, + kind: MetricPrimitiveKind::Timestamp, + elapsed_since_last: false, + auto_derived: false, + reserved: true, + }, + ParamDeclarationDto { + name: "ottyl_fmt".to_string(), + message_match_regex: String::new(), + capture_group: String::new(), + json_field_path: None, + kind: MetricPrimitiveKind::String, + elapsed_since_last: false, + auto_derived: false, + reserved: true, + }, + ] +} + async fn declare_param( State(state): State, Json(body): Json, @@ -345,6 +395,7 @@ async fn declare_param( // A request that reached here was accepted by `declare_param`, which always stores // it as non-auto (see `engine::declare_param`). auto_derived: false, + reserved: false, }; (StatusCode::CREATED, Json(dto)).into_response() } diff --git a/src/dto.rs b/src/dto.rs index c359520..167841d 100644 --- a/src/dto.rs +++ b/src/dto.rs @@ -28,6 +28,14 @@ pub struct ParamDeclarationDto { /// See `crate::data_value::ParamDeclaration::auto_derived`. The frontend uses this to disable /// editing — `POST /api/params` also rejects a request naming an auto-derived param. pub auto_derived: bool, + /// Set only for the built-in `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` names + /// (`crate::engine::OTTYL_RESERVED_PARAM_NAMES`), synthesized into the `GET /api/params` + /// response by `crate::api::list_params` rather than backed by any index entry — distinct from + /// `auto_derived` (which *is* a real, if machine-discovered, `ParamDeclaration`). Always + /// `false` for every other entry. There is no removal path for these at all (not just a + /// disabled one), so the frontend should treat `reserved` as non-deletable/non-editable. + #[serde(default)] + pub reserved: bool, } #[derive(Debug, Clone, Serialize)] diff --git a/src/engine.rs b/src/engine.rs index d5b2799..02c1a83 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -249,6 +249,13 @@ fn declare_param( index: &mut BTreeMap, request: DeclareParamRequest, ) -> Result<(), String> { + if OTTYL_RESERVED_PARAM_NAMES.contains(&request.name.as_str()) { + return Err(format!( + "'{}' is a reserved name, present on every log line without declaring anything; \ + it can't be redeclared as a param", + request.name + )); + } if let Some(DataValue::ParamDeclaration(existing)) = index.get(&IndexKey::ParamDeclaration { name: request.name.clone() }) { @@ -1498,6 +1505,7 @@ fn handle_query( kind, elapsed_since_last, auto_derived, + reserved: false, } }) .collect(); @@ -1703,6 +1711,24 @@ fn declared_value_kind( } } +/// The three reserved param names usable without declaring anything — see `compile_ottyl_seq_comparison`/ +/// `compile_ottyl_ts_comparison`/`compile_ottyl_fmt_comparison` for the filter-bar equivalent, and +/// `webapp/src/components/Docs.tsx` for the user-facing docs. Shared by `evaluate_promql` (dashboard +/// panels, this file) and `crate::api::list_params` (`GET /api/params` discoverability). +pub const OTTYL_RESERVED_PARAM_NAMES: [&str; 3] = ["ottyl_seq", "ottyl_ts", "ottyl_fmt"]; + +/// For `ottyl_seq`/`ottyl_ts` only (not `ottyl_fmt`, which `evaluate_promql` rejects outright before +/// this is consulted): returns a `(sequence, timestamp_ms) -> value` mapping used to synthesize a +/// virtual PromQL series straight from the ingested-log index, since neither is backed by a +/// `MetricDeclaration`/bucket the way every other metric name is. +fn ottyl_reserved_numeric_value_fn(metric_name: &str) -> Option f64> { + match metric_name { + "ottyl_seq" => Some(|sequence, _timestamp_ms| sequence as f64), + "ottyl_ts" => Some(|_sequence, timestamp_ms| timestamp_ms as f64), + _ => None, + } +} + /// Evaluates a parsed PromQL query against `index`, producing chart-ready series: baseline is one series per label combination; `sum()`/`avg()` merge series into one series per `by_labels` group (the whole result collapses to a single label-less series when `by_labels` is empty); `rate()` — bare or wrapped in `sum()`/`avg()` — turns each series into a windowed occurrences-per-second reading via `windowed_rate_series`, so it plots as a real line rather than one trailing point; `histogram()` groups by `by_labels` the same way sum()/avg() do, then buckets each group's values independently, so `histogram(m) by (label)` yields one histogram (a family of `le`-labeled series carrying the group's own labels) per label group instead of collapsing all groups together. /// `sum()`/`avg()` over a `String`-kind metric are rejected up front — `coerce_to_f64` has no /// numeric reading for a string, so silently treating every occurrence as `1.0` would let a sum @@ -1715,13 +1741,30 @@ fn evaluate_promql( start_ms: Option, end_ms: Option, ) -> Result, String> { + // `ottyl_fmt` is categorical ("json"/"logfmt"/"none", `crate::logfmt::detect_format`) with no + // sensible numeric reading, unlike `ottyl_seq`/`ottyl_ts` below — rather than silently + // returning an empty series (the failure mode ticket 055 calls out), a dashboard query over it + // is rejected up front with a message pointing back at the filter/query bar, where it already + // works (see `compile_ottyl_fmt_comparison`). + if parsed.metric_name == "ottyl_fmt" { + return Err( + "'ottyl_fmt' is a categorical value (json/logfmt/none), not a number, so it can't be \ + used in a dashboard PromQL query; filter on it in the search bar instead, e.g. \ + ottyl_fmt==json" + .to_string(), + ); + } + // Ticket 051: a metric with neither a declaration nor any recorded bucket data is genuinely // unknown (as opposed to a declared-but-empty-so-far metric, or an undeclared scraped // Prometheus metric ingested via `--metrics-url`'s `IngestMetricSample`, which never gets a // `MetricDeclaration` but still has bucket data) — most commonly because a dashboard panel // still references a metric that has since been deleted. Failing here with a clear error beats - // a panic or a silently-empty result. - if !index.contains_key(&IndexKey::MetricDeclaration { name: parsed.metric_name.clone() }) + // a panic or a silently-empty result. `ottyl_seq`/`ottyl_ts` (ticket 055) are exempt: they're + // synthesized straight from the ingested-log index below and never have a `MetricDeclaration` + // or bucket entries of their own. + if ottyl_reserved_numeric_value_fn(&parsed.metric_name).is_none() + && !index.contains_key(&IndexKey::MetricDeclaration { name: parsed.metric_name.clone() }) && bucket_values(index, &parsed.metric_name, None).next().is_none() { return Err(format!( @@ -1740,19 +1783,41 @@ fn evaluate_promql( } let mut series_points: BTreeMap, Vec<(i64, f64)>> = BTreeMap::new(); - for (labels, (_, timestamp, value)) in bucket_values(index, &parsed.metric_name, None) - .filter(|(labels, _)| matches_label_matchers(labels, &parsed.matchers)) - .filter(|(_, (_, timestamp, _))| { - start_ms.is_none_or(|start| timestamp.as_millis() >= start) - && end_ms.is_none_or(|end| timestamp.as_millis() <= end) - }) - { - let display_labels: BTreeMap = - labels.iter().map(|(name, value)| (name.clone(), value.to_string())).collect(); - series_points - .entry(display_labels) - .or_default() - .push((timestamp.as_millis(), coerce_to_f64(value))); + if let Some(reserved_value) = ottyl_reserved_numeric_value_fn(&parsed.metric_name) { + // `ottyl_seq`/`ottyl_ts` are computed line-locally (see `compile_ottyl_seq_comparison`/ + // `compile_ottyl_ts_comparison`) rather than backed by a `MetricDeclaration` and bucket + // entries, so they're synthesized here straight from the ingested-log index instead of + // going through `bucket_values`. Neither carries any labels, so a non-empty matcher list + // simply matches nothing, same as an unmatched label filter would against a real metric. + let no_labels: BTreeMap = BTreeMap::new(); + if matches_label_matchers(&no_labels, &parsed.matchers) { + for (sequence, timestamp, _text) in logs_range(index) { + let timestamp_ms = timestamp.as_millis(); + if start_ms.is_none_or(|start| timestamp_ms >= start) + && end_ms.is_none_or(|end| timestamp_ms <= end) + { + series_points + .entry(BTreeMap::new()) + .or_default() + .push((timestamp_ms, reserved_value(sequence, timestamp_ms))); + } + } + } + } else { + for (labels, (_, timestamp, value)) in bucket_values(index, &parsed.metric_name, None) + .filter(|(labels, _)| matches_label_matchers(labels, &parsed.matchers)) + .filter(|(_, (_, timestamp, _))| { + start_ms.is_none_or(|start| timestamp.as_millis() >= start) + && end_ms.is_none_or(|end| timestamp.as_millis() <= end) + }) + { + let display_labels: BTreeMap = + labels.iter().map(|(name, value)| (name.clone(), value.to_string())).collect(); + series_points + .entry(display_labels) + .or_default() + .push((timestamp.as_millis(), coerce_to_f64(value))); + } } for points in series_points.values_mut() { points.sort_by_key(|(timestamp_ms, _)| *timestamp_ms); @@ -3382,6 +3447,106 @@ mod tests { } } + #[test] + fn promql_count_over_ottyl_seq_gives_one_point_per_ingested_log_with_no_metric_declaration() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, "line one"); + ingest_log(&mut index, 1, "line two"); + ingest_log(&mut index, 2, "line three"); + + let parsed = promql::parse_query("count(ottyl_seq)").unwrap(); + let series = evaluate_promql(&index, &parsed, None, None).unwrap(); + assert_eq!(series.len(), 1); + assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0), (2, 1.0)]); + } + + #[test] + fn promql_bare_ottyl_seq_returns_the_actual_sequence_numbers() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, "line zero"); + ingest_log(&mut index, 5, "line five"); + + let parsed = promql::parse_query("ottyl_seq").unwrap(); + let mut series = evaluate_promql(&index, &parsed, None, None).unwrap(); + assert_eq!(series.len(), 1); + series[0].points.sort_by_key(|(ts, _)| *ts); + assert_eq!(series[0].points, vec![(0, 0.0), (5, 5.0)]); + } + + #[test] + fn promql_bare_ottyl_ts_returns_ingest_timestamps() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, "a"); + ingest_log(&mut index, 1, "b"); + + let parsed = promql::parse_query("ottyl_ts").unwrap(); + let series = evaluate_promql(&index, &parsed, None, None).unwrap(); + assert_eq!(series.len(), 1); + assert_eq!(series[0].points, vec![(0, 0.0), (1, 1.0)]); + } + + #[test] + fn promql_rate_over_ottyl_ts_produces_a_windowed_series_not_an_empty_one() { + let mut index = BTreeMap::new(); + for sequence in 0..5u64 { + ingest_log(&mut index, sequence, "line"); + } + + let parsed = promql::parse_query("rate(ottyl_ts[5s])").unwrap(); + let series = evaluate_promql(&index, &parsed, None, None).unwrap(); + assert_eq!(series.len(), 1); + assert!(!series[0].points.is_empty(), "rate(ottyl_ts[...]) must not silently return no data"); + } + + #[test] + fn promql_ottyl_seq_with_label_matcher_matches_nothing_since_it_has_no_labels() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, "line"); + + let parsed = promql::parse_query(r#"ottyl_seq{path="/a"}"#).unwrap(); + let series = evaluate_promql(&index, &parsed, None, None).unwrap(); + assert!(series.is_empty()); + } + + #[test] + fn promql_over_ottyl_fmt_is_explicitly_rejected_not_silently_empty() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, r#"{"level":"error"}"#); + + let parsed = promql::parse_query("count(ottyl_fmt)").unwrap(); + let err = evaluate_promql(&index, &parsed, None, None).unwrap_err(); + assert!(err.contains("ottyl_fmt"), "error should name the rejected metric: {err}"); + assert!(err.contains("categorical") || err.contains("filter"), "error should explain why: {err}"); + } + + #[test] + fn handle_query_promql_returns_series_for_ottyl_seq_end_to_end() { + let mut index = BTreeMap::new(); + ingest_log(&mut index, 0, "a"); + ingest_log(&mut index, 1, "b"); + + let response = handle_query( + &index, + EngineQuery::Promql { query: "count(ottyl_seq)".to_string(), start_ms: None, end_ms: None }, + 0, + ); + match response { + EngineResponse::Promql(Ok(series)) => { + assert_eq!(series[0].points, vec![(0, 1.0), (1, 1.0)]); + } + _ => panic!("expected EngineResponse::Promql(Ok(_))"), + } + } + + #[test] + fn declare_param_rejects_reserved_ottyl_names() { + let mut index = BTreeMap::new(); + for name in OTTYL_RESERVED_PARAM_NAMES { + let err = declare_param(&mut index, request(name, "x", "x")).unwrap_err(); + assert!(err.contains("reserved"), "expected a 'reserved' error for '{name}', got: {err}"); + } + } + // -- FilterBar query integration -- fn logs_query(filter: Option<&str>) -> EngineQuery { diff --git a/tickets/055-ottyl-reserved-params-in-dashboards.md b/tickets/055-ottyl-reserved-params-in-dashboards.md deleted file mode 100644 index a1d1b2b..0000000 --- a/tickets/055-ottyl-reserved-params-in-dashboards.md +++ /dev/null @@ -1,77 +0,0 @@ -# 055 — Make `ottyl_*` reserved params usable in dashboard (PromQL) queries, not just filters - -## Summary - -Three reserved params exist, all hardcoded in the filter compiler: -`ottyl_seq` (ingest sequence number), `ottyl_ts` (ingest timestamp, -supports relative/absolute values), and `ottyl_fmt` (auto-detected -`json`/`logfmt`/`none`) — defined at `src/engine.rs:773-775, 795, 806, 824`, -grammar comments at `src/filter_parser.pest:7-9`, docs at -`webapp/src/components/Docs.tsx:105-106, 375` ("reserved metric names -present on every log line, so they're always filterable without -declaring anything"). Alert conditions restrict to the same three plus -`regex()`/`json[field]` (`src/alerts.rs:3, 199-225`). - -These already work end-to-end for the filter/query bar syntax used by -`GET /api/logs`, `GET /api/timeline`, and alert conditions — that part of -"accessible parameters for queries" is true today. What's missing is -dashboards: PromQL-style panel queries (`src/promql.rs`, -`evaluate_promql` in `src/engine.rs:1562-1601`) have no special case for -`ottyl_*` anywhere. A PromQL query resolves `metric_name` via -`bucket_values(index, &parsed.metric_name)` -(`src/engine.rs`, called from `evaluate_promql`), which only returns data -for names backed by a `DataValue::MetricDeclaration` and bucket entries -created through `POST /api/metrics`. `ottyl_*` values are computed -line-locally at filter-compile time and never written into declared- -metric buckets, so `sum(ottyl_seq)` or similar in a dashboard panel would -silently return an empty series rather than real data. - -Separately, `ottyl_*` never appear in `GET /api/params` -(`src/api.rs:281-287` → `EngineQuery::ParamDeclarations` → -`param_declarations_range`, `src/engine.rs:1340-1355`), since that only -lists explicit `ParamDeclaration` index entries and `ottyl_*` never -create one. `FilterBar.tsx` (`webapp/src/components/FilterBar.tsx:36-41`) -has no autocomplete/suggestion list at all today (plain text input), so -there's nothing currently excluding them from UI suggestions either — -but if param suggestions are ever added, this gap should be kept in mind. - -## Scope - -- Add a special case in `evaluate_promql`/`src/promql.rs` (analogous to - the filter compiler's handling at `src/engine.rs:773-775`) that - synthesizes a virtual bucket series for `ottyl_seq`/`ottyl_ts`/ - `ottyl_fmt` metric names from the ingested-log index, instead of - requiring a `MetricDeclaration`. Decide and document in the PR how - `ottyl_fmt`'s categorical string values map onto a numeric time - series (e.g. count-by-value, or reject it from PromQL numeric - aggregation with a clear error, since `ottyl_seq`/`ottyl_ts` are - naturally numeric but `ottyl_fmt` is not). -- Decide whether `ottyl_*` should appear in `GET /api/params` (or a - parallel listing) so dashboard panel builders in the webapp can - discover them without prior knowledge of the reserved names; if yes, - extend `param_declarations_range` or add a small hardcoded merge at the - API layer rather than forcing them into the index as real - `ParamDeclaration` entries (they aren't user-declared and shouldn't be - deletable via ticket 051's/052's params-removal paths). -- Update `README.md`/`Docs.tsx` to state whether `ottyl_*` is usable in - dashboard queries, once implemented (or explicitly document it as - filter-only, if the numeric/categorical mismatch above makes broad - PromQL support impractical for `ottyl_fmt`). - -## Acceptance criteria - -- A dashboard panel using a PromQL query over `ottyl_seq` or `ottyl_ts` - (e.g. `count(ottyl_seq)` or a rate over `ottyl_ts`) returns real data - from ingested logs, verified with a seeded fixture. -- The behavior for `ottyl_fmt` in a PromQL query is either implemented - (with a stated value-to-series mapping) or explicitly rejected with a - clear, documented error — not a silent empty result. -- `GET /api/params` behavior for `ottyl_*` is decided and documented; - if included, they're clearly marked as reserved/non-deletable rather - than indistinguishable from user-declared params. - -## Depends on - -None (independent of the params-removal tickets, but note the -non-deletable requirement above if `ottyl_*` end up listed alongside -regular params). diff --git a/webapp/src/components/Docs.tsx b/webapp/src/components/Docs.tsx index 6620d9f..cf1fd95 100644 --- a/webapp/src/components/Docs.tsx +++ b/webapp/src/components/Docs.tsx @@ -107,7 +107,9 @@ const SECTIONS: DocSection[] = [ auto-detected format of the line's text — log_stream (macOS log stream output),{' '} json, logfmt, or none. A log_stream line also gets{' '} log_type, process, and pid params derived automatically from its - fixed-column header. + fixed-column header. All three reserved names also appear (flagged reserved) in{' '} + GET /api/params for discoverability, though there's no UI to edit or remove them — see also{' '} + Dashboards below for using ottyl_seq/ottyl_ts in a panel query.

Date/time formats

Timestamp comparisons accept three forms:

@@ -338,6 +340,14 @@ const SECTIONS: DocSection[] = [ Dashboards are saved per profile, so they travel with it when you switch or copy it — an active profile is required to create or save one; pick or create one from the settings menu first.

+

+ The reserved ottyl_seq/ottyl_ts names (see Filtering above) work in panel + queries the same as any declared metric — e.g. count(ottyl_seq) for log volume, or{' '} + rate(ottyl_ts[1m]) for ingest rate — since every log line carries them without a metric + declaration. ottyl_fmt is categorical (json/logfmt/none + ), not a number, so it's rejected in a panel query with an explicit error rather than silently charting + nothing — filter on it in the filter bar instead. +

), }, diff --git a/webapp/src/components/Params.tsx b/webapp/src/components/Params.tsx index afc51fd..2765cc7 100644 --- a/webapp/src/components/Params.tsx +++ b/webapp/src/components/Params.tsx @@ -52,6 +52,12 @@ export function Params({ params, onDeclared }: { params: ParamDeclaration[]; onD const [clearResult, setClearResult] = useState(null) const [clearError, setClearError] = useState(null) + // `reserved` params (`ottyl_seq`/`ottyl_ts`/`ottyl_fmt`) are synthesized server-side purely for + // discoverability (see `GET /api/params`) — they aren't real declarations, so they're kept out of + // the editable/clickable list and shown separately, non-interactively. + const declaredParams = useMemo(() => params.filter((p) => !p.reserved), [params]) + const reservedParams = useMemo(() => params.filter((p) => p.reserved), [params]) + const isBoolean = kind === 'boolean' const isElapsedSinceLast = source === 'elapsed_since_last' // The message match regex doubles as a sample line here so a pasted JSON log line unlocks the @@ -307,8 +313,10 @@ export function Params({ params, onDeclared }: { params: ParamDeclaration[]; onD {clearResult &&
{clearResult}
} {clearError &&
{clearError}
}
- {params.length === 0 && No params declared yet.} - {params.map((p) => ( + {declaredParams.length === 0 && ( + No params declared yet. + )} + {declaredParams.map((p) => (
+ {reservedParams.length > 0 && ( + <> +
+ Reserved (built in — present on every line, not editable) +
+
+ {reservedParams.map((p) => ( + + {p.name} ({p.kind}) + + ))} +
+ + )} diff --git a/webapp/src/lib/api.ts b/webapp/src/lib/api.ts index aad2ba4..757caf8 100644 --- a/webapp/src/lib/api.ts +++ b/webapp/src/lib/api.ts @@ -32,6 +32,8 @@ export interface ParamDeclaration { kind: 'string' | 'number' | 'timestamp' | 'duration' | 'boolean' /** A third extraction source, distinct from `capture_group`/`json_field_path`: when true, this param's value is never parsed from text — it's computed as elapsed time since the previous event sharing the same label set. Only meaningful paired with `kind: 'duration'`. */ elapsed_since_last?: boolean + /** Set only for the built-in `ottyl_seq`/`ottyl_ts`/`ottyl_fmt` names, synthesized into `GET /api/params` for discoverability — they're usable in the filter bar (and `ottyl_seq`/`ottyl_ts` in dashboard PromQL queries) without ever being declared, and can't be edited or removed. */ + reserved?: boolean } export interface TimelineBucket {