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

Strip all comments outside src/consts.rs

Browse Source 
Removes doc comments, module comments, and inline comments across
the backend so the only comments left in the codebase are the brief
ones on the constants they document.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A1ZJ3SrayzLzVndhVq9BmS
This commit is contained in:
Hugh Kaznowski
2026-09-06 16:34:42 +01:00
co-authored by Claude Sonnet 5
parent 284e3562a2
commit b6401c1c3c
20 changed files with 10 additions and 1622 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/alerts.rs
-40
View File
@@ -1,14 +1,3 @@
//! Alert rules: definitions, trigger evaluation, and ntfy delivery.
//!
//! 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
//! bespoke `VecDeque`+prune loop, so the two agree on what "windowed rate" means (ticket 040). A
//! window is also backfilled once, on first evaluation, from already-ingested logs within its own
//! trailing window (see `backfill_window`): a rule created mid-session (or a process that replays
//! existing log content, e.g. `--file` tailing) no longer starts with a silently empty window.
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
use std::collections::{BTreeMap, HashMap, HashSet}; use std::collections::{BTreeMap, HashMap, HashSet};
use std::fmt; use std::fmt;
@@ -41,19 +30,16 @@ pub struct AlertRule {
#[serde(tag = "kind", rename_all = "lowercase")] #[serde(tag = "kind", rename_all = "lowercase")]
pub enum AlertCondition { pub enum AlertCondition {
Immediate { query: String }, Immediate { query: String },
/// Fires when `query`'s match count within a rolling `window_ms` crosses `threshold` (via `op`).
Threshold { Threshold {
query: String, query: String,
op: CompareOp, op: CompareOp,
threshold: f64, threshold: f64,
window_ms: i64, window_ms: i64,
}, },
/// Seen values live only in memory — a restart forgets them, so previously-seen values can fire again; a deliberate simplicity tradeoff, not an oversight.
NewValue { NewValue {
field_regex: String, field_regex: String,
capture_group: String, capture_group: String,
}, },
/// A zero `count_b` skips evaluation for that line rather than treating the ratio as infinite.
Ratio { Ratio {
query_a: String, query_a: String,
query_b: String, query_b: String,
@@ -86,7 +72,6 @@ impl From<io::Error> for AlertError {
} }
} }
/// Hashes the name and current time rather than pulling in a `uuid` dependency.
fn generate_alert_rule_id(name: &str) -> String { fn generate_alert_rule_id(name: &str) -> String {
let mut hasher = DefaultHasher::new(); let mut hasher = DefaultHasher::new();
name.hash(&mut hasher); name.hash(&mut hasher);
@@ -145,13 +130,6 @@ impl Settings {
} }
} }
/// Per-rule in-memory evaluation state; lives only for the process's lifetime — not persisted
/// across a restart with no replayed log history. Each `RateWindow` is backfilled once (see
/// `backfill_window`) from whatever matching history is already in the index at first evaluation,
/// so — unlike before ticket 040 — a rule doesn't start blind to matches ingested before it was
/// declared, or before this evaluation loop last started, as long as that history is still in the
/// in-memory log index (e.g. `--file` tailing replays existing file content from the start on
/// every process start).
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct AlertRuntimeState { pub struct AlertRuntimeState {
threshold_window: RateWindow, threshold_window: RateWindow,
@@ -163,10 +141,6 @@ pub struct AlertRuntimeState {
seen_values: HashSet<String>, seen_values: HashSet<String>,
} }
/// Builds a `RateWindow` seeded from already-ingested logs matching `query`, bounded to
/// `window_ms` before `now` (not the full log history) so the cost stays proportional to the
/// window rather than everything ever ingested. `before_sequence` excludes the line currently
/// being evaluated, which the caller pushes separately if it matches.
fn backfill_window( fn backfill_window(
index: &BTreeMap<IndexKey, DataValue>, index: &BTreeMap<IndexKey, DataValue>,
query: &str, query: &str,
@@ -184,7 +158,6 @@ fn backfill_window(
RateWindow::backfill(matching_timestamps, now, window_ms) RateWindow::backfill(matching_timestamps, now, window_ms)
} }
/// Evaluates a FilterBar query against a single line's own fields only (metric[param] comparisons aren't supported — see module doc; json[field] is supported since it's also line-local).
fn eval_alert_query(query: &str, sequence: u64, timestamp: Timestamp, text: &str) -> Result<bool, String> { fn eval_alert_query(query: &str, sequence: u64, timestamp: Timestamp, text: &str) -> Result<bool, String> {
let expr = filter_parser::parse(query).map_err(|err| err.to_string())?; let expr = filter_parser::parse(query).map_err(|err| err.to_string())?;
eval_alert_expr(&expr, sequence, timestamp, text) eval_alert_expr(&expr, sequence, timestamp, text)
@@ -237,7 +210,6 @@ fn eval_alert_expr(expr: &FilterExpr, sequence: u64, timestamp: Timestamp, text:
} }
} }
/// A malformed query (bad regex, unsupported metric comparison) is logged and simply never matches, rather than panicking.
pub fn evaluate_alerts( pub fn evaluate_alerts(
rules: &[AlertRule], rules: &[AlertRule],
states: &mut HashMap<String, AlertRuntimeState>, states: &mut HashMap<String, AlertRuntimeState>,
@@ -344,7 +316,6 @@ fn evaluate_condition(
let count_a = state.ratio_a_window.count() as f64; let count_a = state.ratio_a_window.count() as f64;
let count_b = state.ratio_b_window.count() as f64; let count_b = state.ratio_b_window.count() as f64;
if count_b == 0.0 { if count_b == 0.0 {
// Zero denominator: skip rather than treat the ratio as infinite.
return None; return None;
} }
let observed = count_a / count_b; let observed = count_a / count_b;
@@ -357,9 +328,6 @@ fn evaluate_condition(
} }
} }
/// Delivers to the single, process-wide ntfy sink configured in Settings. Errors are only logged
/// (no retry queue); spawned off the ingest hot path so a slow/unreachable sink never blocks ingestion.
/// A silently no-op'd delivery (no topic configured) is expected — a rule can be defined before the sink is.
pub async fn deliver_for_rule(rule_name: String, message: String, topic: Option<String>, server: Option<String>) { pub async fn deliver_for_rule(rule_name: String, message: String, topic: Option<String>, server: Option<String>) {
let Some(topic) = topic else { let Some(topic) = topic else {
return; return;
@@ -370,7 +338,6 @@ pub async fn deliver_for_rule(rule_name: String, message: String, topic: Option<
} }
} }
/// Standalone and `AlertRule`-free so a "send test notification" endpoint can call it directly.
pub async fn deliver_ntfy(server: &str, topic: &str, message: &str) -> Result<(), String> { pub async fn deliver_ntfy(server: &str, topic: &str, message: &str) -> Result<(), String> {
let url = format!("{}/{}", server.trim_end_matches('/'), topic); let url = format!("{}/{}", server.trim_end_matches('/'), topic);
let response = reqwest::Client::new() let response = reqwest::Client::new()
@@ -506,7 +473,6 @@ mod tests {
} }
let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 2, Timestamp::from_millis(2000), "error"); let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 2, Timestamp::from_millis(2000), "error");
assert_eq!(fired.len(), 1); assert_eq!(fired.len(), 1);
// Stays quiet for a non-matching line even though the window is full.
let fired = evaluate_alerts(&[rule], &mut states, &index, 3, Timestamp::from_millis(2100), "fine"); let fired = evaluate_alerts(&[rule], &mut states, &index, 3, Timestamp::from_millis(2100), "fine");
assert!(fired.is_empty()); assert!(fired.is_empty());
} }
@@ -527,7 +493,6 @@ mod tests {
let mut states = HashMap::new(); let mut states = HashMap::new();
let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new(); let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new();
assert!(evaluate_alerts(&[rule.clone()], &mut states, &index, 0, Timestamp::from_millis(0), "error").is_empty()); assert!(evaluate_alerts(&[rule.clone()], &mut states, &index, 0, Timestamp::from_millis(0), "error").is_empty());
// Second match arrives well after the first has aged out of the window.
assert!(evaluate_alerts(&[rule], &mut states, &index, 1, Timestamp::from_millis(5_000), "error").is_empty()); assert!(evaluate_alerts(&[rule], &mut states, &index, 1, Timestamp::from_millis(5_000), "error").is_empty());
} }
@@ -568,15 +533,11 @@ mod tests {
}; };
let mut states = HashMap::new(); let mut states = HashMap::new();
let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new(); let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new();
// Only an "error" so far; no "request" yet, so denominator is 0 -> skip.
assert!(evaluate_alerts(&[rule.clone()], &mut states, &index, 0, Timestamp::from_millis(0), "error").is_empty()); assert!(evaluate_alerts(&[rule.clone()], &mut states, &index, 0, Timestamp::from_millis(0), "error").is_empty());
// One request: ratio 1/1 = 1.0 >= 0.5 -> fires.
let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 1, Timestamp::from_millis(1), "request"); let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 1, Timestamp::from_millis(1), "request");
assert_eq!(fired.len(), 1); assert_eq!(fired.len(), 1);
// A second request: ratio 1/2 = 0.5 still >= 0.5 -> fires again.
let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 2, Timestamp::from_millis(2), "request"); let fired = evaluate_alerts(&[rule.clone()], &mut states, &index, 2, Timestamp::from_millis(2), "request");
assert_eq!(fired.len(), 1); assert_eq!(fired.len(), 1);
// A third request with no more errors: ratio drops to 1/3 -> no longer fires.
let fired = evaluate_alerts(&[rule], &mut states, &index, 3, Timestamp::from_millis(3), "request"); let fired = evaluate_alerts(&[rule], &mut states, &index, 3, Timestamp::from_millis(3), "request");
assert!(fired.is_empty()); assert!(fired.is_empty());
} }
@@ -606,7 +567,6 @@ mod tests {
let rule = immediate_rule("some_metric[param]==5"); let rule = immediate_rule("some_metric[param]==5");
let mut states = HashMap::new(); let mut states = HashMap::new();
let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new(); let index: BTreeMap<IndexKey, DataValue> = BTreeMap::new();
// Should log a warning and simply not fire, never panic.
let fired = evaluate_alerts(&[rule], &mut states, &index, 0, Timestamp::from_millis(0), "anything"); let fired = evaluate_alerts(&[rule], &mut states, &index, 0, Timestamp::from_millis(0), "anything");
assert!(fired.is_empty()); assert!(fired.is_empty());
} }
src/api.rs
+4 -124
View File
@@ -26,15 +26,12 @@ use tokio::sync::oneshot;
static WEBAPP_DIST: EmbedDir = fs_embed::fs_embed!("webapp/dist"); static WEBAPP_DIST: EmbedDir = fs_embed::fs_embed!("webapp/dist");
/// Embedded so an agent that has only discovered a port (no filesystem access into the ottyl
/// repo/binary) can still fetch it — see `SKILL.md` at the repo root and ticket 069.
const SKILL_MD: &str = include_str!("../SKILL.md"); const SKILL_MD: &str = include_str!("../SKILL.md");
#[derive(Clone)] #[derive(Clone)]
struct ApiState { struct ApiState {
engine: EngineHandle, engine: EngineHandle,
settings: SettingsHandle, settings: SettingsHandle,
/// Imported CSS files are stored alongside this path's parent directory (see `ApiState::css_dir`).
settings_path: Arc<PathBuf>, settings_path: Arc<PathBuf>,
} }
@@ -46,7 +43,6 @@ impl ApiState {
.unwrap_or_else(|| PathBuf::from(".")) .unwrap_or_else(|| PathBuf::from("."))
} }
/// Best-effort: a no-op if no profile is active, and I/O failures are only logged, since the engine declaration is already the source of truth.
fn persist_param_declaration(&self, request: &DeclareParamRequest) { fn persist_param_declaration(&self, request: &DeclareParamRequest) {
let mut settings = self.settings.lock().unwrap(); let mut settings = self.settings.lock().unwrap();
let Some(profile_name) = settings.active_profile.clone() else { let Some(profile_name) = settings.active_profile.clone() else {
@@ -60,11 +56,6 @@ impl ApiState {
} }
} }
/// Counterpart to `persist_param_declaration` for `DELETE /api/params` (ticket 052): empties the
/// active profile's stored params so the just-cleared set doesn't reappear from `settings.json`
/// on next load. Same best-effort semantics — a no-op if no profile is active, I/O failures only
/// logged, since the engine-side clear (already applied by the time this runs) is the source of
/// truth for the running process.
fn persist_clear_params(&self) { fn persist_clear_params(&self) {
let mut settings = self.settings.lock().unwrap(); let mut settings = self.settings.lock().unwrap();
let Some(profile_name) = settings.active_profile.clone() else { let Some(profile_name) = settings.active_profile.clone() else {
@@ -77,11 +68,6 @@ impl ApiState {
} }
} }
/// Same as `persist_param_declaration`, for metric declarations. Also promotes any of the
/// metric's referenced params that only exist as auto-derived engine state (see
/// `promoted_param_records`) into persisted profile params, so a metric built against an
/// auto-detected logfmt key survives a restart instead of being rejected as "undeclared" once
/// declarations replay before any log line re-derives it.
async fn persist_metric_declaration(&self, request: &DeclareMetricRequest) { async fn persist_metric_declaration(&self, request: &DeclareMetricRequest) {
let profile_name = { let profile_name = {
let settings = self.settings.lock().unwrap(); let settings = self.settings.lock().unwrap();
@@ -109,7 +95,6 @@ impl ApiState {
} }
} }
/// Same as `persist_metric_declaration`, for an in-place edit (`PUT /api/metrics/{name}`) — also drops the persisted record under `old_name` when the edit renames the metric, so a rename doesn't leave a stale duplicate declaration in the profile file.
async fn persist_metric_declaration_update(&self, old_name: &str, request: &DeclareMetricRequest) { async fn persist_metric_declaration_update(&self, old_name: &str, request: &DeclareMetricRequest) {
let profile_name = { let profile_name = {
let settings = self.settings.lock().unwrap(); let settings = self.settings.lock().unwrap();
@@ -138,9 +123,6 @@ impl ApiState {
} }
} }
/// Best-effort, matching `persist_param_declaration`'s intent — a no-op if no profile is
/// active, and I/O failures are only logged, since the engine's removal is already the source
/// of truth for what the live process shows.
fn persist_metric_declaration_removal(&self, name: &str) { fn persist_metric_declaration_removal(&self, name: &str) {
let mut settings = self.settings.lock().unwrap(); let mut settings = self.settings.lock().unwrap();
let Some(profile_name) = settings.active_profile.clone() else { let Some(profile_name) = settings.active_profile.clone() else {
@@ -203,7 +185,6 @@ async fn get_skill_md() -> impl IntoResponse {
([(header::CONTENT_TYPE, "text/markdown; charset=utf-8")], SKILL_MD) ([(header::CONTENT_TYPE, "text/markdown; charset=utf-8")], SKILL_MD)
} }
/// Falls back to `index.html` for unmatched paths so client-side routing works.
async fn serve_webapp(uri: axum::http::Uri) -> Response { async fn serve_webapp(uri: axum::http::Uri) -> Response {
let path = uri.path().trim_start_matches('/'); let path = uri.path().trim_start_matches('/');
let file = WEBAPP_DIST let file = WEBAPP_DIST
@@ -234,13 +215,6 @@ fn mime_for_extension(extension: &str) -> &'static str {
} }
} }
/// Ticket 075: every read query is served straight off a `left_right::ReadHandle` obtained from
/// `engine.reader_factory` — wait-free, and never funneled through the actor's mpsc queue, so a slow
/// query here can no longer block an unrelated cheap one. The one exception is a sorted `Logs` query
/// the first time its `sort_by` is used: no helper index exists yet for it, and only the actor (the
/// sole owner of the write side) can build one, so this sends `EnsureSortHelperIndex`, awaits the
/// ack, then re-enters a fresh `ReadHandle` (now caught up) before proceeding — every other read,
/// and every sorted query once the helper already exists, never reaches the actor at all.
async fn ask(engine: &EngineHandle, query: EngineQuery) -> Result<EngineResponse, StatusCode> { async fn ask(engine: &EngineHandle, query: EngineQuery) -> Result<EngineResponse, StatusCode> {
let read_handle = engine.reader_factory.handle(); let read_handle = engine.reader_factory.handle();
if let Some(sort_by) = crate::engine::query_sort_by(&query) { if let Some(sort_by) = crate::engine::query_sort_by(&query) {
@@ -262,13 +236,6 @@ async fn ask(engine: &EngineHandle, query: EngineQuery) -> Result<EngineResponse
Ok(crate::engine::handle_query_ro(&guard, query)) Ok(crate::engine::handle_query_ro(&guard, query))
} }
/// Any of `request`'s referenced params (`label_params`/`value_param`) not already present in
/// `existing_params` are looked up live in the engine and returned as records ready to persist
/// alongside the metric (see `ProfileStore::record_metric_declaration`'s `promoted_params`). A
/// metric can validly reference a param that's only auto-derived engine state (see
/// `crate::engine::auto_declare_params`) — that param was never written into any profile, so without
/// this promotion the metric would be rejected as "undeclared" the next time declarations replay on
/// startup, before any log line has re-derived it.
async fn promoted_param_records( async fn promoted_param_records(
engine: &EngineHandle, engine: &EngineHandle,
existing_params: &[ParamDeclarationRecord], existing_params: &[ParamDeclarationRecord],
@@ -342,13 +309,6 @@ async fn get_stats(State(state): State<ApiState>) -> impl IntoResponse {
async fn list_params(State(state): State<ApiState>) -> impl IntoResponse { async fn list_params(State(state): State<ApiState>) -> impl IntoResponse {
match ask(&state.engine, EngineQuery::ParamDeclarations).await { match ask(&state.engine, EngineQuery::ParamDeclarations).await {
Ok(EngineResponse::ParamDeclarations(mut declarations)) => { 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()); declarations.extend(reserved_param_declaration_dtos());
Json(declarations).into_response() Json(declarations).into_response()
} }
@@ -357,10 +317,6 @@ async fn list_params(State(state): State<ApiState>) -> impl IntoResponse {
} }
} }
/// `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] { fn reserved_param_declaration_dtos() -> [ParamDeclarationDto; 3] {
[ [
ParamDeclarationDto { ParamDeclarationDto {
@@ -421,8 +377,6 @@ async fn declare_param(
json_field_path: request.json_field_path, json_field_path: request.json_field_path,
kind: request.kind, kind: request.kind,
elapsed_since_last: request.elapsed_since_last, elapsed_since_last: request.elapsed_since_last,
// A request that reached here was accepted by `declare_param`, which always stores
// it as non-auto (see `engine::declare_param`).
auto_derived: false, auto_derived: false,
reserved: false, reserved: false,
}; };
@@ -433,11 +387,6 @@ async fn declare_param(
} }
} }
/// `DELETE /api/params` (ticket 052): clears every declared param (manual and auto-derived) and
/// immediately re-derives whatever `auto_declare_params` can still find in log text the engine
/// still retains. The response body's `logs_considered` tells the caller how many retained lines
/// were re-scanned, so the UI can make clear that lines evicted by `max_logs` before this ran
/// can't regain their params — this is expected behavior, not a partial failure.
async fn clear_params(State(state): State<ApiState>) -> impl IntoResponse { async fn clear_params(State(state): State<ApiState>) -> impl IntoResponse {
let (respond_to, response) = oneshot::channel(); let (respond_to, response) = oneshot::channel();
if state.engine.tx.send(EngineMessage::ClearParams { respond_to }).await.is_err() { if state.engine.tx.send(EngineMessage::ClearParams { respond_to }).await.is_err() {
@@ -493,7 +442,6 @@ async fn declare_metric(
} }
} }
/// Updates the metric declaration named by the URL path in place, reusing `declare_metric`'s validation (via `engine::update_metric`) rather than creating a second declaration or erroring on a duplicate-name conflict. `body.name` may differ from the path segment to rename the metric, so long as no other declaration already uses the new name.
async fn update_metric( async fn update_metric(
State(state): State<ApiState>, State(state): State<ApiState>,
AxumPath(name): AxumPath<String>, AxumPath(name): AxumPath<String>,
@@ -528,12 +476,6 @@ async fn update_metric(
} }
} }
/// Deletes the metric declaration named by the URL path — both from the live engine index and,
/// if a profile is active, from persisted settings (ticket 051), so it doesn't reappear on
/// restart. Its recorded bucket-value history is purged along with it (see the comment on
/// `EngineMessage::RemoveMetricDeclaration`'s handler in `engine.rs`), rather than left orphaned,
/// so a dashboard panel or PromQL query still referencing the metric by name fails with a clear
/// "unknown metric" error instead of silently keeping stale data alive.
async fn delete_metric( async fn delete_metric(
State(state): State<ApiState>, State(state): State<ApiState>,
AxumPath(name): AxumPath<String>, AxumPath(name): AxumPath<String>,
@@ -558,7 +500,6 @@ async fn delete_metric(
} }
} }
/// Malformed pairs (no `=`) are skipped rather than rejected.
fn parse_labels_filter(raw: &str) -> BTreeMap<String, String> { fn parse_labels_filter(raw: &str) -> BTreeMap<String, String> {
raw.split(',') raw.split(',')
.filter_map(|pair| pair.split_once('=')) .filter_map(|pair| pair.split_once('='))
@@ -639,7 +580,6 @@ async fn import_css(
} }
} }
/// Content is never copied, and the entry can never be deleted via `DELETE /api/settings/css/:id`.
async fn link_css( async fn link_css(
State(state): State<ApiState>, State(state): State<ApiState>,
Json(body): Json<LinkCssRequest>, Json(body): Json<LinkCssRequest>,
@@ -669,7 +609,6 @@ async fn remove_css(
} }
} }
/// Rejects a linked entry with 400 — there's no local content; the frontend should use `path_or_url` directly.
async fn get_css_content( async fn get_css_content(
State(state): State<ApiState>, State(state): State<ApiState>,
AxumPath(id): AxumPath<String>, AxumPath(id): AxumPath<String>,
@@ -730,7 +669,6 @@ async fn delete_alert_rule(State(state): State<ApiState>, AxumPath(id): AxumPath
} }
} }
/// Delivers a test notification without a saved rule so the sink can be verified before saving.
async fn test_alert_notification(Json(body): Json<TestAlertNotificationRequest>) -> impl IntoResponse { async fn test_alert_notification(Json(body): Json<TestAlertNotificationRequest>) -> impl IntoResponse {
let server = body.server.unwrap_or_else(|| "https://ntfy.sh".to_string()); let server = body.server.unwrap_or_else(|| "https://ntfy.sh".to_string());
match deliver_ntfy(&server, &body.topic, "Test notification from ottyl").await { match deliver_ntfy(&server, &body.topic, "Test notification from ottyl").await {
@@ -739,13 +677,11 @@ async fn test_alert_notification(Json(body): Json<TestAlertNotificationRequest>)
} }
} }
/// CSS entries have their own endpoint (`GET /api/settings/css`).
async fn get_settings(State(state): State<ApiState>) -> impl IntoResponse { async fn get_settings(State(state): State<ApiState>) -> impl IntoResponse {
let settings = state.settings.lock().unwrap(); let settings = state.settings.lock().unwrap();
Json(SettingsDto::from(&*settings)).into_response() Json(SettingsDto::from(&*settings)).into_response()
} }
/// `max_logs: null` clears the cap; a changed value is pushed to the engine so it applies without a restart.
async fn update_settings( async fn update_settings(
State(state): State<ApiState>, State(state): State<ApiState>,
Json(body): Json<UpdateSettingsRequest>, Json(body): Json<UpdateSettingsRequest>,
@@ -780,9 +716,6 @@ async fn update_settings(
.tx .tx
.send(EngineMessage::SetMaxLogs(new_max_logs.1)) .send(EngineMessage::SetMaxLogs(new_max_logs.1))
.await; .await;
// `SetMaxLogs` has no response to await on its own (ticket 075: it's fire-and-forget, same as
// `Ingest`), but this endpoint's caller reasonably expects a subsequent read to already
// reflect the new cap's eviction — barrier so it does.
state.engine.barrier().await; state.engine.barrier().await;
} }
Json(new_max_logs.0).into_response() Json(new_max_logs.0).into_response()
@@ -817,7 +750,6 @@ async fn create_profile(
} }
} }
/// If it was the active profile, `active_profile` is cleared, but the engine's already-declared params/metrics keep running until the next restart.
async fn delete_profile( async fn delete_profile(
State(state): State<ApiState>, State(state): State<ApiState>,
AxumPath(name): AxumPath<String>, AxumPath(name): AxumPath<String>,
@@ -829,10 +761,6 @@ async fn delete_profile(
} }
} }
/// Removes every currently-declared param/metric from the engine, then replays `profile`'s declarations into it.
/// Auto-derived params (`ParamDeclarationDto::auto_derived`) are left alone — they aren't part of
/// any profile, just facts discovered from the log stream, so a profile switch shouldn't discard
/// them (they'd otherwise only reappear once a new line re-introduces that key).
async fn reload_engine_declarations(engine: &EngineHandle, profile: &Profile) { async fn reload_engine_declarations(engine: &EngineHandle, profile: &Profile) {
if let Ok(EngineResponse::ParamDeclarations(params)) = if let Ok(EngineResponse::ParamDeclarations(params)) =
ask(engine, EngineQuery::ParamDeclarations).await ask(engine, EngineQuery::ParamDeclarations).await
@@ -972,7 +900,6 @@ mod tests {
use std::sync::Mutex; use std::sync::Mutex;
use tower::ServiceExt; use tower::ServiceExt;
/// Spins up a fresh engine + router pair backed by a temp settings file.
fn test_app() -> (Router, EngineHandle, SettingsHandle, tempfile::TempDir) { fn test_app() -> (Router, EngineHandle, SettingsHandle, tempfile::TempDir) {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let settings_path = dir.path().join("settings.json"); let settings_path = dir.path().join("settings.json");
@@ -1066,7 +993,6 @@ mod tests {
let status = delete_request(&app, "/api/settings/alerts/does-not-exist").await; let status = delete_request(&app, "/api/settings/alerts/does-not-exist").await;
assert_eq!(status, StatusCode::NOT_FOUND); assert_eq!(status, StatusCode::NOT_FOUND);
// AlertError::NotFound maps to a plain-text (not JSON) 404 body, so check status directly instead of via send_json.
let request = Request::builder() let request = Request::builder()
.method("PUT") .method("PUT")
.uri("/api/settings/alerts/does-not-exist") .uri("/api/settings/alerts/does-not-exist")
@@ -1104,10 +1030,6 @@ mod tests {
assert_eq!(body["capture_group"], "level"); assert_eq!(body["capture_group"], "level");
} }
/// Ticket 052's core acceptance criteria: `DELETE /api/params` removes every declared param
/// (manual and auto-derived), that removal survives a reload of persisted settings, and —
/// without any new line being ingested — the retained `IngestedLog` entries immediately
/// re-derive a fresh (auto-derived) param set.
#[tokio::test] #[tokio::test]
async fn clearing_params_persists_and_reextracts_from_retained_logs() { async fn clearing_params_persists_and_reextracts_from_retained_logs() {
let (app, engine, settings_handle, dir) = test_app(); let (app, engine, settings_handle, dir) = test_app();
@@ -1119,14 +1041,10 @@ mod tests {
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap();
} }
// A manual declaration plus an auto-derived one (from ingesting a logfmt line before "path" is declared).
declare_test_param(&app, "path", r"request path=(?P<path>\S+)", "path", "string").await; declare_test_param(&app, "path", r"request path=(?P<path>\S+)", "path", "string").await;
ingest_lines(&engine, 0, &["request path=/a", "level=info x=1"]).await; ingest_lines(&engine, 0, &["request path=/a", "level=info x=1"]).await;
let (_, before) = get_json(&app, "/api/params").await; let (_, before) = get_json(&app, "/api/params").await;
// Ticket 055 merges the three non-deletable `ottyl_*` reserved names into this listing
// too; `DELETE /api/params` (ticket 052) only ever clears real declarations, so the
// reserved ones must be excluded from the "everything got removed" count below.
let before_declared: Vec<&str> = before let before_declared: Vec<&str> = before
.as_array() .as_array()
.unwrap() .unwrap()
@@ -1141,7 +1059,6 @@ mod tests {
let (status, result) = send_json(&app, "DELETE", "/api/params", json!({})).await; let (status, result) = send_json(&app, "DELETE", "/api/params", json!({})).await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
assert_eq!(result["removed"], before_declared.len() as u64); assert_eq!(result["removed"], before_declared.len() as u64);
// Re-derived without any new ingest — same three keys come back, now all auto-derived.
assert_eq!(result["reextracted"], 3); assert_eq!(result["reextracted"], 3);
assert_eq!(result["logs_considered"], 2); assert_eq!(result["logs_considered"], 2);
@@ -1151,8 +1068,6 @@ mod tests {
assert_eq!(after_declared.len(), 3); assert_eq!(after_declared.len(), 3);
assert!(after_declared.iter().all(|p| p["auto_derived"] == true)); assert!(after_declared.iter().all(|p| p["auto_derived"] == true));
// The clear persisted: "path" (the only manually-declared, profile-stored param) doesn't
// reappear from settings.json on a fresh load.
let reloaded_settings = Settings::load(&settings_path); let reloaded_settings = Settings::load(&settings_path);
let profile = reloaded_settings.profile("web").unwrap(); let profile = reloaded_settings.profile("web").unwrap();
assert!(profile.params.is_empty()); assert!(profile.params.is_empty());
@@ -1177,9 +1092,6 @@ mod tests {
assert!(body["error"].as_str().unwrap().contains("invalid regex")); assert!(body["error"].as_str().unwrap().contains("invalid regex"));
} }
/// Declares a standalone param via `POST /api/params` — most metric-declaration tests need one
/// or more already-declared params to reference by name (ticket 037), rather than embedding a
/// regex/capture-group/kind inline in the metric request.
async fn declare_test_param(app: &Router, name: &str, pattern: &str, capture_group: &str, kind: &str) { async fn declare_test_param(app: &Router, name: &str, pattern: &str, capture_group: &str, kind: &str) {
let (status, _body) = send_json( let (status, _body) = send_json(
app, app,
@@ -1271,7 +1183,6 @@ mod tests {
assert_eq!(body["name"], "requests"); assert_eq!(body["name"], "requests");
assert_eq!(body["label_params"].as_array().unwrap().len(), 2); assert_eq!(body["label_params"].as_array().unwrap().len(), 2);
// Still exactly one declaration named "requests", now with the updated regex/params — not a second declaration alongside the original.
let (status, body) = get_json(&app, "/api/metrics").await; let (status, body) = get_json(&app, "/api/metrics").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
let metrics = body.as_array().unwrap(); let metrics = body.as_array().unwrap();
@@ -1357,8 +1268,6 @@ mod tests {
assert_eq!(status, StatusCode::NOT_FOUND); assert_eq!(status, StatusCode::NOT_FOUND);
} }
/// Acceptance criterion: deleting a metric persists the removal into the active profile too, so
/// it doesn't reappear from `settings.json` on the next restart.
#[tokio::test] #[tokio::test]
async fn deleting_metric_persists_removal_and_it_does_not_reappear_after_restart() { async fn deleting_metric_persists_removal_and_it_does_not_reappear_after_restart() {
let (app, _engine, settings_handle, dir) = test_app(); let (app, _engine, settings_handle, dir) = test_app();
@@ -1389,10 +1298,6 @@ mod tests {
assert!(Settings::load(&settings_path).profile("web").unwrap().metrics.is_empty()); assert!(Settings::load(&settings_path).profile("web").unwrap().metrics.is_empty());
} }
/// Acceptance criterion: a dashboard panel (PromQL query) that still references a metric by
/// name after it's been deleted fails with a readable "unknown metric" error, rather than a
/// panic or a silently-empty series — seeded here by declaring, ingesting into, deleting, then
/// re-querying the same metric name.
#[tokio::test] #[tokio::test]
async fn promql_query_against_deleted_metric_fails_with_readable_error() { async fn promql_query_against_deleted_metric_fails_with_readable_error() {
let (app, engine, _settings, _dir) = test_app(); let (app, engine, _settings, _dir) = test_app();
@@ -1420,7 +1325,6 @@ mod tests {
.unwrap(); .unwrap();
engine.barrier().await; engine.barrier().await;
// Sanity check: the dashboard-panel-style query works before deletion.
let (status, _body) = get_json(&app, "/api/query?promql=requests").await; let (status, _body) = get_json(&app, "/api/query?promql=requests").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
@@ -1478,11 +1382,9 @@ mod tests {
let values = body.as_array().unwrap(); let values = body.as_array().unwrap();
assert_eq!(values.len(), 2); assert_eq!(values.len(), 2);
for value in values { for value in values {
// MetricPrimitive derives Serialize as an externally-tagged enum, so JSON is {"String": "/a"}.
assert_eq!(value["labels"]["path"]["String"], "/a"); assert_eq!(value["labels"]["path"]["String"], "/a");
} }
// Bucket values group by label (BTreeMap-ordered "/a" before "/b"), not by sequence, so limit=1 picks sequence 2 from "/a" ahead of sequence 1 from "/b".
let (status, body) = get_json(&app, "/api/metrics/requests?since_sequence=0&limit=1").await; let (status, body) = get_json(&app, "/api/metrics/requests?since_sequence=0&limit=1").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
let values = body.as_array().unwrap(); let values = body.as_array().unwrap();
@@ -1548,7 +1450,6 @@ mod tests {
async fn declaring_param_persists_into_active_profile() { async fn declaring_param_persists_into_active_profile() {
let (app, _engine, settings_handle, dir) = test_app(); let (app, _engine, settings_handle, dir) = test_app();
// Use the same shared settings handle the router was built with — a separately-loaded Settings wouldn't be visible to the running app.
let settings_path = dir.path().join("settings.json"); let settings_path = dir.path().join("settings.json");
{ {
let mut settings = settings_handle.lock().unwrap(); let mut settings = settings_handle.lock().unwrap();
@@ -1576,11 +1477,6 @@ mod tests {
assert_eq!(profile.params[0].name, "level"); assert_eq!(profile.params[0].name, "level");
} }
/// Regression test: declaring a metric against a param that only exists as auto-derived engine
/// state (discovered from an ingested logfmt key, never manually declared via `POST
/// /api/params`) must persist that param into the active profile too, not just the metric —
/// otherwise a restart replays the metric before any log line re-derives the auto param, and the
/// engine rejects it as referencing an undeclared param (see `promoted_param_records`).
#[tokio::test] #[tokio::test]
async fn declaring_metric_against_auto_derived_param_promotes_it_into_the_profile() { async fn declaring_metric_against_auto_derived_param_promotes_it_into_the_profile() {
let (app, engine, settings_handle, dir) = test_app(); let (app, engine, settings_handle, dir) = test_app();
@@ -1592,8 +1488,6 @@ mod tests {
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap();
} }
// Ingest a line carrying a logfmt key before any param is manually declared, so "dx" only
// exists as engine-internal auto-derived state (never persisted anywhere).
ingest_lines(&engine, 0, &["dx=1 moved"]).await; ingest_lines(&engine, 0, &["dx=1 moved"]).await;
let (status, _body) = send_json( let (status, _body) = send_json(
@@ -1613,11 +1507,9 @@ mod tests {
let profile = reloaded_settings.profile("web").unwrap(); let profile = reloaded_settings.profile("web").unwrap();
assert_eq!(profile.metrics.len(), 1); assert_eq!(profile.metrics.len(), 1);
assert_eq!(profile.metrics[0].name, "direction_x"); assert_eq!(profile.metrics[0].name, "direction_x");
// The auto-derived "dx" param got promoted into the profile alongside the metric.
assert_eq!(profile.params.len(), 1); assert_eq!(profile.params.len(), 1);
assert_eq!(profile.params[0].name, "dx"); assert_eq!(profile.params[0].name, "dx");
// Simulate a restart: replay the persisted profile into a fresh engine with no prior ingestion.
let fresh_settings_handle = std::sync::Arc::new(std::sync::Mutex::new(Settings::default())); let fresh_settings_handle = std::sync::Arc::new(std::sync::Mutex::new(Settings::default()));
let fresh_engine_tx = crate::engine::spawn(fresh_settings_handle, 16); let fresh_engine_tx = crate::engine::spawn(fresh_settings_handle, 16);
profile::load_into_engine(&fresh_engine_tx, profile).await; profile::load_into_engine(&fresh_engine_tx, profile).await;
@@ -1734,7 +1626,6 @@ mod tests {
) )
.await; .await;
// Simulate a restart by reloading settings fresh from disk, independent of the running app/engine.
let settings_path = dir.path().join("settings.json"); let settings_path = dir.path().join("settings.json");
let reloaded_settings = Settings::load(&settings_path); let reloaded_settings = Settings::load(&settings_path);
let dashboards = reloaded_settings.list_dashboards("web").unwrap(); let dashboards = reloaded_settings.list_dashboards("web").unwrap();
@@ -1744,9 +1635,6 @@ mod tests {
assert_eq!(dashboards[0].panels[0].promql, "requests"); assert_eq!(dashboards[0].panels[0].promql, "requests");
} }
/// Ticket 075: `Ingest` has no response to await on its own, and reads no longer share a queue
/// with it — every caller here immediately reads back what it just ingested, so this barriers
/// once at the end rather than making every call site remember to.
async fn ingest_lines(engine: &EngineHandle, start_sequence: u64, texts: &[&str]) { async fn ingest_lines(engine: &EngineHandle, start_sequence: u64, texts: &[&str]) {
for (offset, text) in texts.iter().enumerate() { for (offset, text) in texts.iter().enumerate() {
let sequence = start_sequence + offset as u64; let sequence = start_sequence + offset as u64;
@@ -1776,8 +1664,6 @@ mod tests {
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
assert_eq!(body["max_logs"], 2); assert_eq!(body["max_logs"], 2);
// Existing over-cap logs are evicted as soon as the setting change
// reaches the engine, not just on the next ingest.
let (_, body) = get_json(&app, "/api/logs").await; let (_, body) = get_json(&app, "/api/logs").await;
assert_eq!(body.as_array().unwrap().len(), 2); assert_eq!(body.as_array().unwrap().len(), 2);
@@ -1822,8 +1708,6 @@ mod tests {
assert_eq!(body.as_array().unwrap().len(), 3); assert_eq!(body.as_array().unwrap().len(), 3);
} }
/// Ticket 045: seeded fixture mixing lines that carry the sort param with lines that don't,
/// exercised through the actual HTTP endpoint rather than just `handle_query` directly.
#[tokio::test] #[tokio::test]
async fn get_logs_sort_by_orders_by_param_and_toggles_missing_param_handling() { async fn get_logs_sort_by_orders_by_param_and_toggles_missing_param_handling() {
let (app, engine, _settings, _dir) = test_app(); let (app, engine, _settings, _dir) = test_app();
@@ -1834,28 +1718,24 @@ mod tests {
&engine, &engine,
0, 0,
&[ &[
"priority=3 boom", // sequence 0, has param "priority=3 boom",
"no param here", // sequence 1, missing "no param here",
"priority=1 boom", // sequence 2, has param "priority=1 boom",
"also missing", // sequence 3, missing "also missing",
], ],
) )
.await; .await;
// Default order (no sort_by): unchanged, plain ingest/ottyl_seq order.
let (status, body) = get_json(&app, "/api/logs").await; let (status, body) = get_json(&app, "/api/logs").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect(); let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect();
assert_eq!(sequences, vec![0, 1, 2, 3]); assert_eq!(sequences, vec![0, 1, 2, 3]);
// sort_by=priority with the fallback toggle on (the default): entries carrying the param
// sorted ascending by its value, then the missing-param entries ordered by ottyl_seq.
let (status, body) = get_json(&app, "/api/logs?sort_by=priority").await; let (status, body) = get_json(&app, "/api/logs?sort_by=priority").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect(); let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect();
assert_eq!(sequences, vec![2, 0, 1, 3]); assert_eq!(sequences, vec![2, 0, 1, 3]);
// Same sort_by, but with the fallback toggle explicitly off: missing-param lines are dropped.
let (status, body) = get_json(&app, "/api/logs?sort_by=priority&sort_missing_fallback=false").await; let (status, body) = get_json(&app, "/api/logs?sort_by=priority&sort_missing_fallback=false").await;
assert_eq!(status, StatusCode::OK); assert_eq!(status, StatusCode::OK);
let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect(); let sequences: Vec<u64> = body.as_array().unwrap().iter().map(|e| e["sequence"].as_u64().unwrap()).collect();
src/data_value.rs
-68
View File
@@ -3,61 +3,22 @@ use crate::timestamp::Timestamp;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::collections::BTreeMap; use std::collections::BTreeMap;
/// A named, standalone parameter: how to pull a value out of a matching log line, and what kind it
/// parses as. Ticket 037 unified this with the metric-declaration-embedded matcher that used to
/// exist alongside it (`RegexToMetricPrimitiveMatcher`) — a metric now references params by name
/// (`MetricDeclaration::label_params`/`value_param`) instead of redeclaring its own extractor per
/// param, so a param declared once (e.g. via the Params panel) can be reused as a label by one
/// metric and as the value by another without re-entering its regex/capture-group/kind.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ParamDeclaration { pub struct ParamDeclaration {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
pub capture_group: String, pub capture_group: String,
/// Alternative extraction source to `capture_group`: a dot-separated JSON field path (e.g.
/// `foo.bar`), read from lines that parse as a JSON object instead of via the regex capture
/// group. When present and non-empty, this takes precedence over `capture_group`.
/// `#[serde(default)]` so profile files/requests saved before this field existed still load
/// as plain regex-capture-group params.
#[serde(default)] #[serde(default)]
pub json_field_path: Option<String>, pub json_field_path: Option<String>,
pub kind: MetricPrimitiveKind, pub kind: MetricPrimitiveKind,
/// A third extraction source, alongside `capture_group`/`json_field_path`: when set, this
/// param's value is never parsed from text at all — it's computed as elapsed time since the
/// previous event that produced the same label set (ticket 038). Takes precedence over
/// `json_field_path`/`capture_group` when set, and `message_match_regex`/`capture_group`/
/// `json_field_path` are simply unused in that case (`declare_param` doesn't validate them for
/// this shape). Only meaningful paired with `kind == Duration`, since elapsed time is the only
/// primitive this computation can produce — `declare_param` enforces that pairing.
/// `#[serde(default)]` so params declared before this field existed keep parsing as plain
/// regex/JSON-field extraction.
#[serde(default)] #[serde(default)]
pub elapsed_since_last: bool, pub elapsed_since_last: bool,
/// Set when this param was discovered by `crate::engine::auto_declare_params` (a nom-based
/// `key=value` scan run on every ingested line) rather than declared through `POST /api/params`.
/// `declare_param` refuses to overwrite an auto-derived param by name, and it's never persisted
/// to a profile — see `crate::engine::declare_param` and `crate::api::reload_engine_declarations`.
/// `#[serde(default)]` so profiles/requests saved before this field existed still load as
/// ordinary (non-auto) params.
#[serde(default)] #[serde(default)]
pub auto_derived: bool, pub auto_derived: bool,
/// Set for a `log stream` column/message param declared by
/// `crate::engine::auto_declare_log_stream_params`. Those params' `message_match_regex` is a
/// pure fixed character-offset slice with no content anchor of its own (ticket 076) — it'll
/// happily "match" and extract garbage from any line at least that long, including a wrapped
/// multi-line dump's continuation line or the header row itself. When set, extraction (
/// `extract_param_raw`/`derive_log_params`) additionally requires
/// `crate::logfmt::is_log_stream_line(text)` before trusting the regex at all.
/// `#[serde(default)]` so params declared before this field existed keep extracting as before.
#[serde(default)] #[serde(default)]
pub log_stream_gated: bool, pub log_stream_gated: bool,
} }
/// Reads a value out of a JSON object by dot-separated field path (e.g. `"foo.bar"`), for
/// [`ParamDeclaration::json_field_path`]. Returns `None` when `text` doesn't parse as a JSON
/// object, the path doesn't resolve to a value, or the resolved value isn't a scalar
/// (string/number/bool) — there's no sensible flat text form of a nested object/array to feed into
/// `parse_metric_primitive`.
pub fn extract_json_field(text: &str, field_path: &str) -> Option<String> { pub fn extract_json_field(text: &str, field_path: &str) -> Option<String> {
let root: serde_json::Value = serde_json::from_str(text).ok()?; let root: serde_json::Value = serde_json::from_str(text).ok()?;
let mut current = root.as_object()?; let mut current = root.as_object()?;
@@ -77,14 +38,6 @@ pub fn extract_json_field(text: &str, field_path: &str) -> Option<String> {
} }
} }
/// Flattens a JSON object into dot-separated field paths mapped to each leaf scalar's raw text form
/// (string/number/bool), for [`crate::engine::auto_declare_params`] when a line is JSON
/// (`crate::logfmt::detect_format`) rather than logfmt — the JSON equivalent of
/// `crate::logfmt::extract_pairs`'s `key=value` scan. Nested objects recurse, dot-joining the path
/// (`"foo": {"bar": 1}` yields `"foo.bar"`); arrays and null are skipped since there's no single leaf
/// scalar to declare a param against, matching `extract_json_field`'s own restriction to scalars.
/// Returns an empty vec for anything that isn't a JSON object, same as `extract_pairs` returns empty
/// for a line with no `key=value` pairs.
pub fn flatten_json_object(text: &str) -> Vec<(String, String)> { pub fn flatten_json_object(text: &str) -> Vec<(String, String)> {
let Ok(serde_json::Value::Object(root)) = serde_json::from_str::<serde_json::Value>(text) else { let Ok(serde_json::Value::Object(root)) = serde_json::from_str::<serde_json::Value>(text) else {
return Vec::new(); return Vec::new();
@@ -111,7 +64,6 @@ fn flatten_json_object_into(
} }
} }
/// `rename_all = "lowercase"` matches other `kind`-style enums, so JSON bodies use "string"/"number"/"timestamp".
#[allow(dead_code)] #[allow(dead_code)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
@@ -119,45 +71,25 @@ pub enum MetricPrimitiveKind {
String, String,
Number, Number,
Timestamp, Timestamp,
/// A literal i64 millisecond count. Note this is unrelated to
/// `ParamDeclaration::elapsed_since_last`, which is a separate, independent extraction source —
/// a param using that source always parses/produces `Duration`, but not every `Duration`-kind
/// param is computed that way (one can also just parse a literal duration out of captured text,
/// like any other kind).
Duration, Duration,
/// A standalone param declared with this kind doesn't require (or use) a capture group: its value is
/// simply whether `message_match_regex` matches the line, case-insensitively or per the regex's own
/// flags — see `crate::engine::derive_log_params`. Unlike other kinds it's present as `true`/`false`
/// on every line rather than omitted when unmatched.
Boolean, Boolean,
} }
/// Value type paired with [`crate::index_key::IndexKey`] in the engine's primary index.
#[allow(dead_code)] #[allow(dead_code)]
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub enum DataValue { pub enum DataValue {
/// Paired with `IndexKey::IngestedLog`.
LogLiteral { text: String }, LogLiteral { text: String },
/// Paired with `IndexKey::ParamDeclaration`.
ParamDeclaration(ParamDeclaration), ParamDeclaration(ParamDeclaration),
/// Paired with `IndexKey::MetricDeclaration`. `label_params`/`value_param` reference standalone
/// `ParamDeclaration`s by name (resolved against the engine's param store in
/// `crate::engine::apply_metric_to_log`) rather than embedding their own extractor.
MetricDeclaration { MetricDeclaration {
message_match_regex: String, message_match_regex: String,
label_params: Vec<String>, label_params: Vec<String>,
/// At most one value param per metric; its primitive becomes the event's bucket value instead of a label.
value_param: Option<String>, value_param: Option<String>,
/// See `crate::engine::DeclareMetricRequest::matched_labels`.
matched_labels: Option<BTreeMap<String, String>>, matched_labels: Option<BTreeMap<String, String>>,
/// See `crate::engine::DeclareMetricRequest::unmatched_labels`.
unmatched_labels: Option<BTreeMap<String, String>>, unmatched_labels: Option<BTreeMap<String, String>>,
}, },
/// Paired with `IndexKey::Bucket`.
BucketValue { BucketValue {
values: Vec<(u64, Timestamp, MetricPrimitive)>, values: Vec<(u64, Timestamp, MetricPrimitive)>,
}, },
/// Paired with `IndexKey::LastMetricEvent`.
LastEventTimestamp { LastEventTimestamp {
timestamp: Timestamp, timestamp: Timestamp,
}, },
src/dto.rs
-59
View File
@@ -11,7 +11,6 @@ pub struct LogEntryDto {
pub sequence: u64, pub sequence: u64,
pub timestamp_ms: i64, pub timestamp_ms: i64,
pub text: String, pub text: String,
/// Only params whose regex matched (and parsed as the declared kind) are present — omitted rather than sent as null, so the frontend can render a blank for anything missing.
pub params: BTreeMap<String, MetricPrimitive>, pub params: BTreeMap<String, MetricPrimitive>,
} }
@@ -20,20 +19,10 @@ pub struct ParamDeclarationDto {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
pub capture_group: String, pub capture_group: String,
/// See `crate::data_value::ParamDeclaration::json_field_path`.
pub json_field_path: Option<String>, pub json_field_path: Option<String>,
pub kind: MetricPrimitiveKind, pub kind: MetricPrimitiveKind,
/// See `crate::data_value::ParamDeclaration::elapsed_since_last`.
pub elapsed_since_last: bool, pub elapsed_since_last: bool,
/// 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, 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)] #[serde(default)]
pub reserved: bool, pub reserved: bool,
} }
@@ -42,13 +31,9 @@ pub struct ParamDeclarationDto {
pub struct MetricDeclarationDto { pub struct MetricDeclarationDto {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
/// Names of standalone `ParamDeclaration`s used as this metric's bucket labels.
pub label_params: Vec<String>, pub label_params: Vec<String>,
/// Name of the standalone `ParamDeclaration` (if any) whose primitive becomes this metric's per-event bucket value instead of a label.
pub value_param: Option<String>, pub value_param: Option<String>,
/// See `crate::engine::DeclareMetricRequest::matched_labels`.
pub matched_labels: Option<BTreeMap<String, String>>, pub matched_labels: Option<BTreeMap<String, String>>,
/// See `crate::engine::DeclareMetricRequest::unmatched_labels`.
pub unmatched_labels: Option<BTreeMap<String, String>>, pub unmatched_labels: Option<BTreeMap<String, String>>,
} }
@@ -65,24 +50,7 @@ pub struct StatsDto {
pub last_sequence: Option<u64>, pub last_sequence: Option<u64>,
pub first_timestamp_ms: Option<i64>, pub first_timestamp_ms: Option<i64>,
pub last_timestamp_ms: Option<i64>, pub last_timestamp_ms: Option<i64>,
/// Ticket 011: a running, *approximate* estimate of the in-memory heap
/// held by currently-ingested logs, in bytes. "Approximate" because it
/// only counts each `IngestedLine::text`'s UTF-8 byte length — it does
/// not account for `String`'s allocator overhead/capacity slack, the
/// `BTreeMap`/`IndexKey` node overhead the text is stored under, or any
/// derived data (bucket values, params) the line produces elsewhere in
/// the index. That's judged the best cheap proxy for "how much memory is
/// this log's text costing us": it's O(1) to compute per log (`text.len()`,
/// already an interned byte count for UTF-8 `String`), moves in lockstep
/// with the dominant cost (raw log text, which is usually much larger
/// than the fixed per-entry bookkeeping), and needs no per-entry
/// bookkeeping beyond what eviction already tracks.
pub approx_bytes_total: u64, pub approx_bytes_total: u64,
/// Ticket 073: the highest ingestion-to-engine-processing lag (`ottyl_ts` at read time vs. when
/// the engine actor actually got around to processing that line) seen within the trailing 5
/// seconds, in milliseconds — see `crate::engine::record_ingest_lag`'s rolling high-watermark.
/// Stays near zero under normal load; rises under actor backpressure (a slow query in flight, a
/// large backlog) and decays back out of the window once the stall clears.
pub ingest_lag_high_watermark_ms: i64, pub ingest_lag_high_watermark_ms: i64,
} }
@@ -92,15 +60,8 @@ pub struct LogsParams {
pub start_ms: Option<i64>, pub start_ms: Option<i64>,
pub end_ms: Option<i64>, pub end_ms: Option<i64>,
pub limit: Option<usize>, pub limit: Option<usize>,
/// Raw FilterBar query string, e.g. `regex("error") and ottyl_seq<5`.
pub filter: Option<String>, pub filter: Option<String>,
/// Ticket 045: an already-declared param name to order the returned entries by, in place of the
/// default ingest order (which tracks `ottyl_ts`). `None` (the default) leaves ordering
/// unchanged.
pub sort_by: Option<String>, pub sort_by: Option<String>,
/// Ticket 045: only meaningful alongside `sort_by`. `true` (the default) orders lines missing
/// that param among themselves by `ottyl_seq` and appends them after the sorted lines; `false`
/// drops those lines from the response entirely.
pub sort_missing_fallback: Option<bool>, pub sort_missing_fallback: Option<bool>,
} }
@@ -109,11 +70,9 @@ pub struct TimelineParams {
pub start_ms: Option<i64>, pub start_ms: Option<i64>,
pub end_ms: Option<i64>, pub end_ms: Option<i64>,
pub bucket_ms: Option<i64>, pub bucket_ms: Option<i64>,
/// Same FilterBar query string as `LogsParams::filter`; only matching logs are counted into timeline buckets.
pub filter: Option<String>, pub filter: Option<String>,
} }
/// One (sequence, timestamp, value) occurrence within a metric's bucket for a given label combination.
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub struct BucketValueDto { pub struct BucketValueDto {
pub labels: BTreeMap<String, MetricPrimitive>, pub labels: BTreeMap<String, MetricPrimitive>,
@@ -122,7 +81,6 @@ pub struct BucketValueDto {
pub value: MetricPrimitive, pub value: MetricPrimitive,
} }
/// `labels` is a comma-separated `key=value` list (not repeated params) since axum's `Query` extractor needs a fixed set of field names.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct BucketValuesParams { pub struct BucketValuesParams {
pub labels: Option<String>, pub labels: Option<String>,
@@ -132,7 +90,6 @@ pub struct BucketValuesParams {
pub limit: Option<usize>, pub limit: Option<usize>,
} }
/// `labels` uses plain `String` (already Display-formatted) since charts just need something to show; `value` is always `f64` for plottability.
#[derive(Debug, Clone, Serialize, PartialEq)] #[derive(Debug, Clone, Serialize, PartialEq)]
pub struct PromqlSeriesDto { pub struct PromqlSeriesDto {
pub labels: BTreeMap<String, String>, pub labels: BTreeMap<String, String>,
@@ -146,27 +103,22 @@ pub struct PromqlParams {
pub end_ms: Option<i64>, pub end_ms: Option<i64>,
} }
/// JSON error body returned by API handlers on a 4xx/5xx response.
#[derive(Debug, Serialize)] #[derive(Debug, Serialize)]
pub struct ErrorDto { pub struct ErrorDto {
pub error: String, pub error: String,
} }
/// Every imported CSS entry is a selectable theme (ticket 020) — there's no separate
/// always-on-overlay mode to opt into.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct ImportCssRequest { pub struct ImportCssRequest {
pub filename: String, pub filename: String,
pub content: String, pub content: String,
} }
/// A reference to an external CSS path/URL that ottyl doesn't copy or own.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct LinkCssRequest { pub struct LinkCssRequest {
pub path_or_url: String, pub path_or_url: String,
} }
/// `id` is the real id for `Imported` entries or a stable id derived from the path/URL for `Linked` ones, so the frontend can key both kinds uniformly (only `Imported` entries are deletable/content-fetchable).
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub struct CssEntryDto { pub struct CssEntryDto {
pub id: String, pub id: String,
@@ -201,18 +153,13 @@ impl From<&CssEntry> for CssEntryDto {
} }
} }
/// `css_entries` is omitted here since it has its own `GET /api/settings/css` endpoint.
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub struct SettingsDto { pub struct SettingsDto {
pub color_scheme: ColorScheme, pub color_scheme: ColorScheme,
pub active_profile: Option<String>, pub active_profile: Option<String>,
/// `None` means unlimited.
pub max_logs: Option<usize>, pub max_logs: Option<usize>,
/// Id of the selected custom theme entry, if any. `None` means `color_scheme` alone determines the active palette.
pub custom_theme_id: Option<String>, pub custom_theme_id: Option<String>,
/// Ntfy topic every alert rule delivers to; `None` until configured.
pub ntfy_topic: Option<String>, pub ntfy_topic: Option<String>,
/// Ntfy server override; `None` means `https://ntfy.sh`.
pub ntfy_server: Option<String>, pub ntfy_server: Option<String>,
} }
@@ -229,7 +176,6 @@ impl From<&Settings> for SettingsDto {
} }
} }
/// `max_logs`/`custom_theme_id`/`ntfy_topic`/`ntfy_server` distinguish "omitted" (leave unchanged) from "present" (null clears, value sets), via `deserialize_double_option`.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct UpdateSettingsRequest { pub struct UpdateSettingsRequest {
pub color_scheme: Option<ColorScheme>, pub color_scheme: Option<ColorScheme>,
@@ -243,7 +189,6 @@ pub struct UpdateSettingsRequest {
pub ntfy_server: Option<Option<String>>, pub ntfy_server: Option<Option<String>>,
} }
/// Distinguishes a missing field (outer `None`) from present `null` (`Some(None)`) or present value (`Some(Some(value))`), which plain `Option<Option<T>>` can't do alone.
fn deserialize_double_option<'de, D, T>(deserializer: D) -> Result<Option<Option<T>>, D::Error> fn deserialize_double_option<'de, D, T>(deserializer: D) -> Result<Option<Option<T>>, D::Error>
where where
D: serde::Deserializer<'de>, D: serde::Deserializer<'de>,
@@ -266,7 +211,6 @@ pub struct UpdateAlertRuleRequest {
pub condition: AlertCondition, pub condition: AlertCondition,
} }
/// `server` defaults to `https://ntfy.sh` when absent.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct TestAlertNotificationRequest { pub struct TestAlertNotificationRequest {
pub topic: String, pub topic: String,
@@ -297,7 +241,6 @@ pub struct CreateProfileRequest {
pub name: String, pub name: String,
} }
/// Mirrors `profile::Panel`.
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PanelDto { pub struct PanelDto {
pub id: String, pub id: String,
@@ -328,7 +271,6 @@ impl From<&PanelDto> for Panel {
} }
} }
/// Mirrors `profile::Dashboard`.
#[derive(Debug, Clone, Serialize, Deserialize)] #[derive(Debug, Clone, Serialize, Deserialize)]
pub struct DashboardDto { pub struct DashboardDto {
pub id: String, pub id: String,
@@ -361,7 +303,6 @@ pub struct CreateDashboardRequest {
pub name: String, pub name: String,
} }
/// The dashboard `id` in the URL is authoritative; any `id` in the body is ignored.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
pub struct SaveDashboardRequest { pub struct SaveDashboardRequest {
pub name: String, pub name: String,
src/engine.rs
+5 -643
View File
File diff suppressed because it is too large Load Diff
src/filter_datetime.rs
-18
View File
@@ -1,21 +1,12 @@
//! Datetime parsing for the filter query language's timestamp comparisons: relative
//! humantime, time-of-day (resolved against the UTC calendar day of `now`), or a
//! full date/datetime literal.
// `#![allow(dead_code)]` stays since most of this module's surface is still only
// exercised by its own unit tests below.
#![allow(dead_code)] #![allow(dead_code)]
use chrono::{DateTime, NaiveDate, NaiveTime, TimeZone, Utc}; use chrono::{DateTime, NaiveDate, NaiveTime, TimeZone, Utc};
use crate::timestamp::Timestamp; use crate::timestamp::Timestamp;
/// Error returned when a datetime filter literal can't be resolved to a [`Timestamp`].
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub enum DateTimeParseError { pub enum DateTimeParseError {
/// The input matched a supported format's shape but its value was invalid, e.g. `25:99`.
Invalid { input: String, reason: String }, Invalid { input: String, reason: String },
/// The input didn't match any supported format at all.
Unparseable(String), Unparseable(String),
} }
@@ -37,8 +28,6 @@ impl std::fmt::Display for DateTimeParseError {
impl std::error::Error for DateTimeParseError {} impl std::error::Error for DateTimeParseError {}
/// Parses a filter datetime literal into an absolute [`Timestamp`]; `now` is a parameter
/// (rather than calling `Timestamp::now()` internally) so callers can pass a fixed value in tests.
pub fn parse_datetime(input: &str, now: Timestamp) -> Result<Timestamp, DateTimeParseError> { pub fn parse_datetime(input: &str, now: Timestamp) -> Result<Timestamp, DateTimeParseError> {
let trimmed = input.trim(); let trimmed = input.trim();
if trimmed.is_empty() { if trimmed.is_empty() {
@@ -60,8 +49,6 @@ pub fn parse_datetime(input: &str, now: Timestamp) -> Result<Timestamp, DateTime
Err(DateTimeParseError::Unparseable(input.to_string())) Err(DateTimeParseError::Unparseable(input.to_string()))
} }
/// Tries to interpret `input` as a signed relative humantime duration (`5h`, `-3m`);
/// returns `None` if it doesn't look like one at all, so the caller can try other formats.
fn try_relative(input: &str, now: Timestamp) -> Option<Result<Timestamp, DateTimeParseError>> { fn try_relative(input: &str, now: Timestamp) -> Option<Result<Timestamp, DateTimeParseError>> {
let (sign, rest) = match input.strip_prefix('-') { let (sign, rest) = match input.strip_prefix('-') {
Some(rest) => (-1i128, rest), Some(rest) => (-1i128, rest),
@@ -71,7 +58,6 @@ fn try_relative(input: &str, now: Timestamp) -> Option<Result<Timestamp, DateTim
}, },
}; };
// Require a digit followed by an alphabetic unit so date-like inputs (e.g. "2024-01-15") fall through to the date parser instead of erroring here as an invalid duration.
let looks_like_duration = rest let looks_like_duration = rest
.chars() .chars()
.next() .next()
@@ -97,9 +83,7 @@ fn try_relative(input: &str, now: Timestamp) -> Option<Result<Timestamp, DateTim
} }
} }
/// Tries to interpret `input` as a full date or RFC3339-ish datetime literal.
fn try_full_datetime(input: &str) -> Option<Result<Timestamp, DateTimeParseError>> { fn try_full_datetime(input: &str) -> Option<Result<Timestamp, DateTimeParseError>> {
// Only attempt this path for inputs shaped like a date, e.g. "2024-01-15".
let looks_like_date = input.len() >= 8 && input.as_bytes()[4] == b'-'; let looks_like_date = input.len() >= 8 && input.as_bytes()[4] == b'-';
if !looks_like_date { if !looks_like_date {
return None; return None;
@@ -123,7 +107,6 @@ fn try_full_datetime(input: &str) -> Option<Result<Timestamp, DateTimeParseError
} }
} }
/// Tries to interpret `input` as a bare time-of-day, resolved against the UTC calendar day of `now`.
fn try_time_of_day( fn try_time_of_day(
input: &str, input: &str,
now: Timestamp, now: Timestamp,
@@ -147,7 +130,6 @@ fn try_time_of_day(
let now_millis = now.as_millis(); let now_millis = now.as_millis();
let now_dt = Utc.timestamp_millis_opt(now_millis).single().unwrap_or_else(|| { let now_dt = Utc.timestamp_millis_opt(now_millis).single().unwrap_or_else(|| {
// Fall back to the epoch date; shouldn't happen since `Timestamp` is always a valid instant.
Utc.timestamp_millis_opt(0).single().expect("epoch is valid") Utc.timestamp_millis_opt(0).single().expect("epoch is valid")
}); });
let today = now_dt.date_naive(); let today = now_dt.date_naive();
src/filter_parser.pest
-19
View File
@@ -1,15 +1,3 @@
// Grammar for the FilterBar query language.
//
// Supports:
// - regex("...") raw-text regex search over log lines
// - metric_name[param] <op> value metric comparisons
// - json[field] <op> value JSON field comparisons (line parsed as JSON)
// - ottyl_seq <op> value reserved metric name (no [param])
// - ottyl_ts <op> value reserved metric name (no [param])
// - ottyl_fmt <op> value reserved metric name (no [param])
// - `and` / `or` boolean combinators, with `and` binding tighter than `or`
// - parenthesized sub-expressions
WHITESPACE = _{ " " | "\t" | "\n" | "\r" } WHITESPACE = _{ " " | "\t" | "\n" | "\r" }
file = { SOI ~ or_expr ~ EOI } file = { SOI ~ or_expr ~ EOI }
@@ -19,11 +7,6 @@ and_expr = { term ~ (and_kw ~ term)* }
term = _{ regex_expr | comparison | "(" ~ or_expr ~ ")" } term = _{ regex_expr | comparison | "(" ~ or_expr ~ ")" }
// These must be atomic (`@`): a non-atomic rule would let pest's implicit
// whitespace insertion slip in between the keyword and its own boundary
// check below, which breaks the lookahead (it would see the first
// non-whitespace character *after* skipping the separating space, instead
// of the character immediately following the keyword).
and_kw = @{ "and" ~ !(ASCII_ALPHANUMERIC | "_") } and_kw = @{ "and" ~ !(ASCII_ALPHANUMERIC | "_") }
or_kw = @{ "or" ~ !(ASCII_ALPHANUMERIC | "_") } or_kw = @{ "or" ~ !(ASCII_ALPHANUMERIC | "_") }
@@ -35,8 +18,6 @@ escaped_char = @{ "\\" ~ ANY }
comparison = _{ json_comparison | metric_comparison } comparison = _{ json_comparison | metric_comparison }
// `json[field] <op> value` must be tried before `metric_comparison` since "json" is
// otherwise a syntactically valid (if never declared) metric name.
json_comparison = { "json" ~ "[" ~ param ~ "]" ~ op ~ value } json_comparison = { "json" ~ "[" ~ param ~ "]" ~ op ~ value }
metric_comparison = { metric ~ ("[" ~ param ~ "]")? ~ op ~ value } metric_comparison = { metric ~ ("[" ~ param ~ "]")? ~ op ~ value }
src/filter_parser.rs
-17
View File
@@ -1,5 +1,3 @@
//! FilterBar query language: grammar (see `src/filter_parser.pest`), AST and parser.
//! Evaluating the AST against real log data is left to `engine.rs`.
#![allow(dead_code)] #![allow(dead_code)]
use pest::Parser; use pest::Parser;
@@ -11,20 +9,15 @@ use serde::{Deserialize, Serialize};
#[grammar = "filter_parser.pest"] #[grammar = "filter_parser.pest"]
struct FilterGrammar; struct FilterGrammar;
/// The parsed AST for a FilterBar query.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub enum FilterExpr { pub enum FilterExpr {
/// `regex("...")` — raw-text regex search over log lines.
Regex(String), Regex(String),
/// `metric[param] <op> value` — a single metric comparison.
Comparison { Comparison {
metric: String, metric: String,
param: Option<String>, param: Option<String>,
op: CompareOp, op: CompareOp,
value: FilterValue, value: FilterValue,
}, },
/// `json[field] <op> value` — a comparison against a field of the line's parsed JSON.
/// Lines that aren't valid JSON (or lack the field) simply don't match.
JsonComparison { JsonComparison {
field: String, field: String,
op: CompareOp, op: CompareOp,
@@ -34,8 +27,6 @@ pub enum FilterExpr {
Or(Box<FilterExpr>, Box<FilterExpr>), Or(Box<FilterExpr>, Box<FilterExpr>),
} }
/// Comparison operators for metric comparisons; also reused by `crate::alerts::AlertCondition`
/// for threshold/ratio comparisons (hence `Serialize`/`Deserialize`).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
pub enum CompareOp { pub enum CompareOp {
@@ -47,15 +38,12 @@ pub enum CompareOp {
Ne, Ne,
} }
/// The right-hand side of a comparison: values that parse as a number become
/// [`FilterValue::Number`], everything else is kept as an opaque [`FilterValue::Raw`] string.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub enum FilterValue { pub enum FilterValue {
Number(f64), Number(f64),
Raw(String), Raw(String),
} }
/// A located, human-readable parse error.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub struct FilterParseError { pub struct FilterParseError {
message: String, message: String,
@@ -79,12 +67,10 @@ impl std::error::Error for FilterParseError {}
impl From<pest::error::Error<Rule>> for FilterParseError { impl From<pest::error::Error<Rule>> for FilterParseError {
fn from(err: pest::error::Error<Rule>) -> Self { fn from(err: pest::error::Error<Rule>) -> Self {
// pest's Display already renders a located message, so use it and add a short prefix.
FilterParseError::new(format!("failed to parse filter expression: {err}")) FilterParseError::new(format!("failed to parse filter expression: {err}"))
} }
} }
/// Parse a FilterBar query string into a [`FilterExpr`] AST.
pub fn parse(input: &str) -> Result<FilterExpr, FilterParseError> { pub fn parse(input: &str) -> Result<FilterExpr, FilterParseError> {
let mut pairs = FilterGrammar::parse(Rule::file, input)?; let mut pairs = FilterGrammar::parse(Rule::file, input)?;
@@ -102,7 +88,6 @@ pub fn parse(input: &str) -> Result<FilterExpr, FilterParseError> {
fn build_or_expr(pair: Pair<Rule>) -> Result<FilterExpr, FilterParseError> { fn build_or_expr(pair: Pair<Rule>) -> Result<FilterExpr, FilterParseError> {
debug_assert_eq!(pair.as_rule(), Rule::or_expr); debug_assert_eq!(pair.as_rule(), Rule::or_expr);
// `or_kw` is atomic so it's not silent, showing up as a sibling pair between `and_expr` operands — skip it rather than assuming a fixed alternating structure.
let mut and_exprs = pair let mut and_exprs = pair
.into_inner() .into_inner()
.filter(|p| p.as_rule() != Rule::or_kw); .filter(|p| p.as_rule() != Rule::or_kw);
@@ -119,7 +104,6 @@ fn build_or_expr(pair: Pair<Rule>) -> Result<FilterExpr, FilterParseError> {
fn build_and_expr(pair: Pair<Rule>) -> Result<FilterExpr, FilterParseError> { fn build_and_expr(pair: Pair<Rule>) -> Result<FilterExpr, FilterParseError> {
debug_assert_eq!(pair.as_rule(), Rule::and_expr); debug_assert_eq!(pair.as_rule(), Rule::and_expr);
// Same reasoning as `build_or_expr` for `and_kw`.
let mut terms = pair let mut terms = pair
.into_inner() .into_inner()
.filter(|p| p.as_rule() != Rule::and_kw); .filter(|p| p.as_rule() != Rule::and_kw);
@@ -237,7 +221,6 @@ fn parse_value(s: &str) -> FilterValue {
} }
} }
/// Unescape a regex string literal's inner contents (`\"` -> `"`, `\\` -> `\`, `\x` -> `x`).
fn unescape(s: &str) -> String { fn unescape(s: &str) -> String {
let mut out = String::with_capacity(s.len()); let mut out = String::with_capacity(s.len());
let mut chars = s.chars(); let mut chars = s.chars();
src/index_key.rs
-7
View File
@@ -3,37 +3,30 @@ use crate::timestamp::Timestamp;
use serde::Serialize; use serde::Serialize;
use std::collections::BTreeMap; use std::collections::BTreeMap;
/// Key type for the engine's primary index. Variant order matters: derived `Ord` compares the discriminant first, carving the map into contiguous ranges in declaration order (logs, params, metric declarations, buckets).
#[allow(dead_code)] #[allow(dead_code)]
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)] #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize)]
pub enum IndexKey { pub enum IndexKey {
/// Marker for the start of the whole range. Never inserted, only navigated to.
Start, Start,
IngestedLog { IngestedLog {
sequence: u64, sequence: u64,
timestamp: Timestamp, timestamp: Timestamp,
}, },
/// Marker between the log range and the param range.
LogParamSplit, LogParamSplit,
ParamDeclaration { ParamDeclaration {
name: String, name: String,
}, },
/// Marker between the param range and the metric declaration range.
ParamMetricSplit, ParamMetricSplit,
MetricDeclaration { MetricDeclaration {
name: String, name: String,
}, },
/// Marker between metric declarations and their buckets.
MetricBucketSplit, MetricBucketSplit,
Bucket { Bucket {
metric_name: String, metric_name: String,
labels: BTreeMap<String, MetricPrimitive>, labels: BTreeMap<String, MetricPrimitive>,
}, },
/// Internal bookkeeping tracking the last event timestamp per metric+label, so a `Duration` param's first occurrence (which pushes no `Bucket`) still lets the next occurrence compute elapsed time.
LastMetricEvent { LastMetricEvent {
metric_name: String, metric_name: String,
labels: BTreeMap<String, MetricPrimitive>, labels: BTreeMap<String, MetricPrimitive>,
}, },
/// Marker for the end of the whole range. Nothing sorts after this.
End, End,
} }
src/ingest.rs
+1 -20
View File
@@ -7,8 +7,6 @@ use crate::timestamp::Timestamp;
use tokio::fs::File; use tokio::fs::File;
use tokio::io::{AsyncBufReadExt, BufReader}; use tokio::io::{AsyncBufReadExt, BufReader};
/// Reads stdin line by line, sequences and timestamps each line, and forwards it
/// to the engine task.
pub async fn run(engine: EngineHandle) { pub async fn run(engine: EngineHandle) {
let stdin = tokio::io::stdin(); let stdin = tokio::io::stdin();
let mut lines = BufReader::new(stdin).lines(); let mut lines = BufReader::new(stdin).lines();
@@ -28,7 +26,6 @@ pub async fn run(engine: EngineHandle) {
} }
} }
/// Identifies a specific file instance on disk, so a recreated path can be told apart from the same file simply growing.
#[derive(PartialEq, Eq, Clone, Copy)] #[derive(PartialEq, Eq, Clone, Copy)]
struct FileId(#[cfg(unix)] u64, #[cfg(not(unix))] ()); struct FileId(#[cfg(unix)] u64, #[cfg(not(unix))] ());
@@ -51,9 +48,6 @@ async fn open(path: &Path) -> Option<(File, FileId)> {
Some((file, file_id(&meta))) Some((file, file_id(&meta)))
} }
/// Tails `path` like `tail -f`, resuming from the start if the file is deleted/recreated or
/// truncated in place rather than exiting or getting stuck; `sequence` keeps incrementing
/// across a recreation since it's ottyl's own line counter, not tied to the source file.
pub async fn run_file(engine: EngineHandle, path: PathBuf) { pub async fn run_file(engine: EngineHandle, path: PathBuf) {
let mut sequence: u64 = 0; let mut sequence: u64 = 0;
@@ -73,7 +67,6 @@ pub async fn run_file(engine: EngineHandle, path: PathBuf) {
let mut chunk = String::new(); let mut chunk = String::new();
match reader.read_line(&mut chunk).await { match reader.read_line(&mut chunk).await {
Ok(0) => { Ok(0) => {
// EOF: decide whether the file was truncated/recreated or just hasn't grown yet.
match tokio::fs::metadata(&path).await { match tokio::fs::metadata(&path).await {
Ok(meta) => { Ok(meta) => {
let new_id = file_id(&meta); let new_id = file_id(&meta);
@@ -82,9 +75,7 @@ pub async fn run_file(engine: EngineHandle, path: PathBuf) {
} }
id = new_id; id = new_id;
} }
Err(_) => { Err(_) => {}
// Path gone (mid-recreation); wait for it to reappear and reopen fresh.
}
} }
tokio::time::sleep(POLL_INTERVAL).await; tokio::time::sleep(POLL_INTERVAL).await;
} }
@@ -108,7 +99,6 @@ pub async fn run_file(engine: EngineHandle, path: PathBuf) {
return; return;
} }
} }
// else: partial line at EOF (write in progress) — keep buffering.
} }
Err(_) => continue 'reopen, Err(_) => continue 'reopen,
} }
@@ -116,9 +106,6 @@ pub async fn run_file(engine: EngineHandle, path: PathBuf) {
} }
} }
/// Polls `url` on a fixed interval for a Prometheus/OpenMetrics-format response, pushing every parsed
/// sample straight into the engine. Errors (unreachable endpoint, non-2xx response) are only logged —
/// polling keeps retrying on the next tick rather than exiting.
pub async fn run_metrics_poll(engine: EngineHandle, url: String) { pub async fn run_metrics_poll(engine: EngineHandle, url: String) {
let client = reqwest::Client::new(); let client = reqwest::Client::new();
loop { loop {
@@ -143,10 +130,6 @@ pub async fn run_metrics_poll(engine: EngineHandle, url: String) {
} }
} }
/// Parses the Prometheus text exposition / OpenMetrics format: one sample per non-comment,
/// non-empty line, as `metric_name{label="value",...} value [timestamp_ms]` (labels and the
/// timestamp are both optional). A line that doesn't parse is skipped rather than aborting the
/// whole scrape.
fn parse_openmetrics(body: &str, now: Timestamp) -> Vec<MetricSample> { fn parse_openmetrics(body: &str, now: Timestamp) -> Vec<MetricSample> {
body.lines().filter_map(|line| parse_openmetrics_line(line, now)).collect() body.lines().filter_map(|line| parse_openmetrics_line(line, now)).collect()
} }
@@ -180,8 +163,6 @@ fn parse_openmetrics_line(line: &str, now: Timestamp) -> Option<MetricSample> {
Some(MetricSample { metric_name, labels, value, timestamp }) Some(MetricSample { metric_name, labels, value, timestamp })
} }
/// Splits on commas without handling an escaped comma inside a quoted label value — fine for the
/// common case this targets, not a full OpenMetrics-grammar parser.
fn parse_openmetrics_labels(raw: &str) -> BTreeMap<String, String> { fn parse_openmetrics_labels(raw: &str) -> BTreeMap<String, String> {
raw.split(',') raw.split(',')
.filter_map(|pair| pair.split_once('=')) .filter_map(|pair| pair.split_once('='))
src/logfmt.rs
-153
View File
@@ -1,8 +1,3 @@
//! Fast, tolerant `key=value`/`key="value"` tokenizer (logfmt-ish), used by
//! `crate::engine::auto_declare_params` to discover param names as lines are ingested. Scans
//! anywhere in the line rather than requiring the whole line to be logfmt, so it works on lines
//! with an arbitrary non-logfmt prefix/suffix (e.g. `ts=... msg="..." x1=-4 y1=34`).
use nom::branch::alt; use nom::branch::alt;
use nom::bytes::complete::{escaped, tag, take_until, take_while1}; use nom::bytes::complete::{escaped, tag, take_until, take_while1};
use nom::character::complete::{char, none_of, one_of}; use nom::character::complete::{char, none_of, one_of};
@@ -30,20 +25,10 @@ fn bareword_value(input: &str) -> IResult<&str, &str> {
take_while1(|c: char| !c.is_whitespace())(input) take_while1(|c: char| !c.is_whitespace())(input)
} }
/// A bareword value starting with a bare `=` means the raw text actually had `key==...`/`key===...` —
/// never a real logfmt pair. This is exactly what `pair` would otherwise produce when it lands
/// mid-way through a base64/hash blob whose padding (`=`/`==`) happens to sit right after a run of
/// alnum chars that `key()` greedily consumed — e.g. the `huig_dump.txt` fixture's
/// `<mask.hash: 'rKTEtG7JmnThhoKPbZiVrA=='>,`: `key()` eats `rKTEtG7JmnThhoKPbZiVrA`, the first `=` is
/// consumed as the pair's `=`, leaving a bareword value of `='>,` (ticket 050). Rejected here rather
/// than post-filtered by a caller so `extract_pairs` never reports the false positive to anyone.
fn looks_like_base64_padding_artifact(value: &str) -> bool { fn looks_like_base64_padding_artifact(value: &str) -> bool {
value.starts_with('=') value.starts_with('=')
} }
/// One `key=value` pair found on a line: the key, the raw captured value (unescaped as-is, quotes
/// stripped but any inner `\"`/`\\` left literal since callers only need it for numeric-kind
/// inference), and whether the value was quoted.
fn pair(input: &str) -> IResult<&str, (&str, &str, bool)> { fn pair(input: &str) -> IResult<&str, (&str, &str, bool)> {
let (input, k) = key(input)?; let (input, k) = key(input)?;
let (input, _) = char('=')(input)?; let (input, _) = char('=')(input)?;
@@ -58,9 +43,6 @@ fn pair(input: &str) -> IResult<&str, (&str, &str, bool)> {
} }
} }
/// Finds every `key=value`/`key="value"` pair anywhere in `line`. A position that doesn't start a
/// pair is skipped one char at a time, so this never fails outright — a line with no pairs at all
/// (plain prose, a JSON blob) just returns an empty vec.
pub fn extract_pairs(line: &str) -> Vec<(&str, &str, bool)> { pub fn extract_pairs(line: &str) -> Vec<(&str, &str, bool)> {
let mut results = Vec::new(); let mut results = Vec::new();
let mut rest = line; let mut rest = line;
@@ -80,25 +62,6 @@ pub fn extract_pairs(line: &str) -> Vec<(&str, &str, bool)> {
results results
} }
/// Fixed-column shape of macOS `log stream` output (ticket 050/054): a line starts with
/// `Timestamp Thread Type Activity PID TTL` (date, time+tz, a hex thread id, a bareword type, a hex
/// activity id, a decimal pid, a decimal ttl) followed by a free-text `process: message` tail — see
/// `huig_dump.txt`'s header row and body. Anchored at the very start of the line and requires the
/// hex/decimal shape of each column, so it won't accidentally fire on ordinary logfmt/JSON/prose that
/// merely resembles it in one column. Checked ahead of JSON/logfmt in `detect_format` since it's the
/// most specific of the three (an all-columns-plus-hex-ids match is a much stronger signal than an
/// incidental `key=value` substring), and is deliberately a *format label* rather than a dedicated
/// `ottyl_fmt` value change to JSON/logfmt detection — a `log stream` line's message tail can still
/// separately contain real `key=value` pairs (see the WiFi/radio stat lines in the fixture), which
/// `crate::engine::auto_declare_params` keeps discovering via `extract_pairs` regardless of this
/// line's `ottyl_fmt` label.
/// `crate::data_value::ParamDeclaration::log_stream_gated` params (the columns/`message`
/// `auto_declare_log_stream_params` declares) require this to hold before their own fixed-offset
/// `message_match_regex` is trusted at all (ticket 076) — that regex is pure character-offset
/// arithmetic with no content anchor of its own, so it happily "matches" (and extracts garbage
/// from) any unrelated line at least that long, e.g. a wrapped multi-line dump's continuation line
/// or the header line itself. This can't be baked into `message_match_regex` as a leading lookahead
/// instead: the `regex` crate deliberately doesn't support look-around at all.
static LOG_STREAM_RE: LazyLock<Regex> = LazyLock::new(|| { static LOG_STREAM_RE: LazyLock<Regex> = LazyLock::new(|| {
Regex::new( Regex::new(
r"^\d{4}-\d{2}-\d{2}\s+\S+\s+0x[0-9a-fA-F]+\s+\S+\s+0x[0-9a-fA-F]+\s+\d+\s+\d+\s+[^\s:]+:\s", r"^\d{4}-\d{2}-\d{2}\s+\S+\s+0x[0-9a-fA-F]+\s+\S+\s+0x[0-9a-fA-F]+\s+\d+\s+\d+\s+[^\s:]+:\s",
@@ -110,21 +73,6 @@ pub fn is_log_stream_line(line: &str) -> bool {
LOG_STREAM_RE.is_match(line) LOG_STREAM_RE.is_match(line)
} }
/// Regex fragment for one column separator under the generic whitespace-columnar rule ticket 059
/// asks for: a run of 2+ whitespace chars, or a lone tab, separates columns; a single plain space does
/// not (so a value that legitimately contains one -- e.g. `log stream`'s `date time+tz` timestamp --
/// stays intact within its own column). `is_separator_run` is the same rule applied at split time;
/// this is that rule re-expressed as a regex fragment for the trailing message field's
/// `message_match_regex`, which re-derives its value the same way whenever a caller queries by it
/// later (declarations don't cache extracted values -- see `crate::engine::extract_param_raw`).
/// Same idea as `COLUMN_VALUE_PATTERN`, but bounded to a known column `width` (in chars) -- needed for
/// every column except the last, since two adjacent `log stream` values can legitimately be separated
/// by only a single space when the first one nearly fills its column's on-screen width (e.g. the
/// `Timestamp` column: its own `date time+tz` value already contains one internal space, so a
/// content-only pattern can't tell that boundary apart from the one-space gap before the next column
/// starts -- seed this from `LogStreamColumn::start` positions instead). Greedily matches up to
/// `width` chars while requiring the match to end on a non-whitespace char, which is what excludes the
/// column's own trailing padding spaces from the capture.
fn bounded_column_value_pattern(width: usize) -> String { fn bounded_column_value_pattern(width: usize) -> String {
if width <= 1 { if width <= 1 {
r"\S".to_string() r"\S".to_string()
@@ -139,12 +87,6 @@ fn is_separator_run(run: &str) -> bool {
static WHITESPACE_RUN: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"[ \t]+").expect("static regex must compile")); static WHITESPACE_RUN: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"[ \t]+").expect("static regex must compile"));
/// Splits `line` at whitespace runs that qualify as a column separator (`is_separator_run`), returning
/// each piece alongside its byte offset in `line`; a lone plain space never splits. Consumes at most
/// `max_columns.map(|n| n - 1)` separators -- the final piece returned is the untouched remainder of
/// the line, including any further whitespace runs inside it. That's what lets a trailing unlabelled
/// `log stream` message tail (whose own internal spacing can be arbitrary) come back as one field
/// instead of being chopped up by it.
fn split_columns_with_offsets(line: &str, max_columns: Option<usize>) -> Vec<(usize, &str)> { fn split_columns_with_offsets(line: &str, max_columns: Option<usize>) -> Vec<(usize, &str)> {
let mut columns = Vec::new(); let mut columns = Vec::new();
let mut start = 0usize; let mut start = 0usize;
@@ -168,41 +110,21 @@ fn split_columns(line: &str, max_columns: Option<usize>) -> Vec<&str> {
split_columns_with_offsets(line, max_columns).into_iter().map(|(_, s)| s).collect() split_columns_with_offsets(line, max_columns).into_iter().map(|(_, s)| s).collect()
} }
/// A column-name identifier a generated `message_match_regex`'s named capture group can use as-is
/// (the regex crate requires `[A-Za-z_][0-9A-Za-z_]*`).
static COLUMN_NAME: LazyLock<Regex> = static COLUMN_NAME: LazyLock<Regex> =
LazyLock::new(|| Regex::new(r"^[A-Za-z][A-Za-z0-9_]*$").expect("static regex must compile")); LazyLock::new(|| Regex::new(r"^[A-Za-z][A-Za-z0-9_]*$").expect("static regex must compile"));
/// One column named by a detected `log stream` header (ticket 059). `start` is that column's position
/// in `char`s (not bytes -- see `char_slice`) within the header line -- meaningful only when
/// `LogStreamHeader::tab_delimited` is `false` (see that field's doc comment).
/// `Clone` so the engine's ingest path (`crate::engine::run`) can snapshot the current header value
/// by-value before handing it to a left-right op closure — the closure must be replayable against
/// two separate `EngineSnapshot` copies without touching the run loop's own `log_stream_header` var.
#[derive(Clone)] #[derive(Clone)]
pub struct LogStreamColumn { pub struct LogStreamColumn {
pub name: String, pub name: String,
start: usize, start: usize,
} }
/// A detected `log stream` header: its named columns, plus whether they were tab- or space-separated.
/// `log stream`'s real output right-pads each header name to a fixed on-screen width using runs of
/// 2+ spaces, and every data line's values land in that same fixed alignment regardless of how much
/// of that width they individually fill -- so for that shape, a data line's column boundaries are
/// best found at the header's own fixed character positions, not by re-splitting the data line's own
/// whitespace (see `split_log_stream_row`'s doc comment for why that alone isn't reliable there).
/// A tab-separated header carries no such alignment guarantee (values aren't padded to a shared
/// width), but a lone tab is itself an unambiguous separator, so that case instead re-splits each data
/// line directly.
#[derive(Clone)] #[derive(Clone)]
pub struct LogStreamHeader { pub struct LogStreamHeader {
pub columns: Vec<LogStreamColumn>, pub columns: Vec<LogStreamColumn>,
tab_delimited: bool, tab_delimited: bool,
} }
/// Byte-indexed `str` slicing doesn't work directly with `LogStreamColumn::start` (a *char* offset),
/// since `log stream` output is expected to stay ASCII in practice but callers shouldn't panic if a
/// message contains multi-byte chars before the slice point. `end` of `None` slices to end of string.
fn char_slice(s: &str, start: usize, end: Option<usize>) -> &str { fn char_slice(s: &str, start: usize, end: Option<usize>) -> &str {
if end.is_some_and(|end| end <= start) { if end.is_some_and(|end| end <= start) {
return ""; return "";
@@ -216,13 +138,6 @@ fn char_slice(s: &str, start: usize, end: Option<usize>) -> &str {
&s[byte_start..byte_end.max(byte_start)] &s[byte_start..byte_end.max(byte_start)]
} }
/// Recognizes a `log stream`-style header row generically (ticket 059), instead of assuming the fixed
/// `Timestamp Thread Type Activity PID TTL` names: a short run of bareword column names separated by
/// qualifying whitespace runs (`split_columns_with_offsets`) that is, critically, *not itself* a
/// `log stream` data line (`is_log_stream_line`) -- so a header is never mistaken for the data line
/// immediately following it. `crate::engine::auto_declare_log_stream_params` caches the result across
/// ingested lines, since `log stream` prints its header once, before any data line, and this only ever
/// sees one line at a time.
pub fn detect_log_stream_header(line: &str) -> Option<LogStreamHeader> { pub fn detect_log_stream_header(line: &str) -> Option<LogStreamHeader> {
if is_log_stream_line(line) { if is_log_stream_line(line) {
return None; return None;
@@ -244,21 +159,6 @@ pub fn detect_log_stream_header(line: &str) -> Option<LogStreamHeader> {
Some(LogStreamHeader { columns, tab_delimited }) Some(LogStreamHeader { columns, tab_delimited })
} }
/// Splits a `log stream` data line into `header`'s named columns plus, when present, one trailing
/// unlabelled tail -- `crate::engine::auto_declare_log_stream_params` maps the first
/// `header.columns.len()` pieces onto the header's names and the optional last piece onto the
/// `message` param.
///
/// For a space-padded fixed-width header, every column but the last is sliced at the header's own
/// fixed character positions (`LogStreamColumn::start`) rather than by re-splitting the data line's
/// own whitespace, since two adjacent values can legitimately be joined by only a single space when
/// the first nearly fills its column's width (e.g. `log stream`'s `Timestamp` column, whose value
/// already contains one internal space). Only the last named column's end is whitespace-run-detected
/// (`split_columns`), since that's what distinguishes its own value from the free-text message tail
/// `log stream` appends after it.
///
/// For a tab-delimited header, there's no shared on-screen width to align against, but a lone tab is
/// itself unambiguous, so this instead re-splits the data line directly on qualifying separators.
pub fn split_log_stream_row<'a>(line: &'a str, header: &LogStreamHeader) -> Vec<&'a str> { pub fn split_log_stream_row<'a>(line: &'a str, header: &LogStreamHeader) -> Vec<&'a str> {
if header.tab_delimited { if header.tab_delimited {
return split_columns(line, Some(header.columns.len() + 1)); return split_columns(line, Some(header.columns.len() + 1));
@@ -280,17 +180,6 @@ pub fn split_log_stream_row<'a>(line: &'a str, header: &LogStreamHeader) -> Vec<
result result
} }
/// Builds the `message_match_regex` for the `index`-th (0-based) of `header`'s named columns.
///
/// Fixed-width case: skips exactly `header.columns[index].start` chars, then captures this column's
/// own value bounded to its known width (`bounded_column_value_pattern`) -- unbounded for the last
/// column, which has no next column to bound against.
///
/// Tab-delimited case: skips `index` preceding columns' own value+separator pairs instead, since
/// there's no shared fixed width to anchor a character-position skip against.
///
/// Anchored at the start of the line since a `log stream` line never carries this fixed-per-run
/// column prefix anywhere but the very start.
pub fn log_stream_column_pattern(header: &LogStreamHeader, index: usize, capture_group: &str) -> String { pub fn log_stream_column_pattern(header: &LogStreamHeader, index: usize, capture_group: &str) -> String {
if header.tab_delimited { if header.tab_delimited {
let mut pattern = String::from("^"); let mut pattern = String::from("^");
@@ -309,9 +198,6 @@ pub fn log_stream_column_pattern(header: &LogStreamHeader, index: usize, capture
format!("^.{{{start}}}(?P<{capture_group}>{value_pattern})") format!("^.{{{start}}}(?P<{capture_group}>{value_pattern})")
} }
/// Builds the `message_match_regex` for the trailing unlabelled tail after `header`'s last named
/// column: everything from the end of that column's own separator to end of line, verbatim -- its own
/// internal whitespace, however irregular, is never treated as a further separator.
pub fn log_stream_message_pattern(header: &LogStreamHeader, capture_group: &str) -> String { pub fn log_stream_message_pattern(header: &LogStreamHeader, capture_group: &str) -> String {
if header.tab_delimited { if header.tab_delimited {
let mut pattern = String::from("^"); let mut pattern = String::from("^");
@@ -326,14 +212,6 @@ pub fn log_stream_message_pattern(header: &LogStreamHeader, capture_group: &str)
format!("^.{{{start}}}{COLUMN_VALUE_PATTERN}{COLUMN_SEPARATOR_PATTERN}(?P<{capture_group}>.*)$") format!("^.{{{start}}}{COLUMN_VALUE_PATTERN}{COLUMN_SEPARATOR_PATTERN}(?P<{capture_group}>.*)$")
} }
/// Go `log/slog` `NewJSONHandler` output (the `go-log` label): a single-line JSON object whose first
/// three fields are always `time`, `level`, `msg` in that fixed order (slog's `JSONHandler` writes the
/// built-in attrs before any caller-supplied ones), followed by zero or more arbitrary attrs and a
/// closing `}`. Parsed with nom rather than a full JSON decode since only that fixed prefix shape
/// needs verifying; the variable tail is deliberately not parsed field-by-field the way
/// `crate::engine::auto_declare_params` will separately pick up JSON leaf fields from the line.
/// Checked ahead of the generic JSON object check in `detect_format` since it's the more specific of
/// the two (see that function's doc comment for the general ordering rationale).
fn go_log_prefix(input: &str) -> IResult<&str, (&str, &str, &str)> { fn go_log_prefix(input: &str) -> IResult<&str, (&str, &str, &str)> {
let (input, _) = tag("{\"time\":\"")(input)?; let (input, _) = tag("{\"time\":\"")(input)?;
let (input, time) = take_until("\"")(input)?; let (input, time) = take_until("\"")(input)?;
@@ -352,14 +230,6 @@ pub fn is_go_log_line(text: &str) -> bool {
} }
} }
/// Python `logging` module output in the shape `"%(asctime)s %(levelname)s %(name)s %(message)s"`
/// (the `py-log` label): a `YYYY-MM-DD HH:MM:SS,mmm` timestamp, an uppercase level word, a logger
/// name, then a message tail that (for `delete-py-log`, and any caller using `logger.info("event=%s
/// %s=%s", ...)`-style calls) is itself logfmt `key=value` pairs. Only the fixed prefix is parsed with
/// nom; the tail is handed to `extract_pairs` both to confirm the shape (a py-log line with no pairs
/// at all wouldn't be distinguishable from plain prose after this prefix) and because that's the same
/// extraction `crate::engine::auto_declare_params` needs anyway. Checked ahead of the generic logfmt
/// check in `detect_format` since it's the more specific of the two.
fn py_log_prefix(input: &str) -> IResult<&str, (&str, &str, &str, &str)> { fn py_log_prefix(input: &str) -> IResult<&str, (&str, &str, &str, &str)> {
let (input, date) = take_while1(|c: char| c.is_ascii_digit() || c == '-')(input)?; let (input, date) = take_while1(|c: char| c.is_ascii_digit() || c == '-')(input)?;
let (input, _) = char(' ')(input)?; let (input, _) = char(' ')(input)?;
@@ -380,16 +250,6 @@ pub fn is_py_log_line(text: &str) -> bool {
} }
} }
/// The value of the `ottyl_fmt` reserved param (`crate::engine::compile_filter`/`eval_alert_expr`):
/// which format a line was recognized as when parsed. Checked most-specific-first: `log stream`'s
/// fixed-column shape (see `LOG_STREAM_RE`'s doc comment for why it's the most specific), then the two
/// named-tool shapes `go-log` (a `time`/`level`/`msg`-prefixed JSON object) and `py-log` (a
/// timestamp/level/logger prefix over logfmt pairs) — each is a strict subset of the generic JSON/logfmt
/// case checked right after it, so they'd otherwise never be reached. JSON is checked next 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
/// none of the above is `"none"`.
pub fn detect_format(text: &str) -> &'static str { pub fn detect_format(text: &str) -> &'static str {
if is_log_stream_line(text) { if is_log_stream_line(text) {
"log_stream" "log_stream"
@@ -471,15 +331,12 @@ mod tests {
#[test] #[test]
fn a_bare_json_array_or_scalar_is_not_treated_as_json() { 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("[1,2,3]"), "none");
assert_eq!(detect_format("42"), "none"); assert_eq!(detect_format("42"), "none");
} }
#[test] #[test]
fn rejects_base64_padding_fragment_as_a_pair() { fn rejects_base64_padding_fragment_as_a_pair() {
// The exact false-positive shape from ticket 050's fixture: `key()` greedily consumes the
// tail of the base64 blob as if it were a key, landing right before the `==` padding.
let pairs = extract_pairs( let pairs = extract_pairs(
"qname: <mask.hash: 'rKTEtG7JmnThhoKPbZiVrA=='>, qtype: A, client pid: 12260 (firefox)", "qname: <mask.hash: 'rKTEtG7JmnThhoKPbZiVrA=='>, qtype: A, client pid: 12260 (firefox)",
); );
@@ -541,7 +398,6 @@ mod tests {
assert_eq!(row[3], "0x0"); assert_eq!(row[3], "0x0");
assert_eq!(row[4], "606"); assert_eq!(row[4], "606");
assert_eq!(row[5], "0"); assert_eq!(row[5], "0");
// The message tail is never chopped up by its own internal single/double spacing.
assert_eq!(row[6], "WindowServer: (HSTouchHIDService) something happened"); assert_eq!(row[6], "WindowServer: (HSTouchHIDService) something happened");
} }
@@ -551,8 +407,6 @@ mod tests {
assert_eq!(column_names(&header), vec!["Timestamp", "Thread", "Type"]); assert_eq!(column_names(&header), vec!["Timestamp", "Thread", "Type"]);
assert!(header.tab_delimited); assert!(header.tab_delimited);
// Two tabs in a row still qualify as a single separator (like a run of 2+ spaces), so this
// still cleanly separates Timestamp/Thread/Type despite the doubled tab before "Default".
let spaced = split_log_stream_row("2026-09-06 01:48:54\t0x4dfed2\t\tDefault trailing message", &header); let spaced = split_log_stream_row("2026-09-06 01:48:54\t0x4dfed2\t\tDefault trailing message", &header);
let tabbed = split_log_stream_row("2026-09-06 01:48:54\t0x4dfed2\tDefault trailing message", &header); let tabbed = split_log_stream_row("2026-09-06 01:48:54\t0x4dfed2\tDefault trailing message", &header);
assert_eq!(spaced[0], "2026-09-06 01:48:54"); assert_eq!(spaced[0], "2026-09-06 01:48:54");
@@ -581,8 +435,6 @@ mod tests {
"WindowServer: (HSTouchHIDService) something happened" "WindowServer: (HSTouchHIDService) something happened"
); );
// Also exercise the Timestamp column specifically -- its own value contains one internal
// space, so a naive content-only pattern would previously over-consume into the next column.
let timestamp_re = Regex::new(&log_stream_column_pattern(&header, 0, "Timestamp")).unwrap(); let timestamp_re = Regex::new(&log_stream_column_pattern(&header, 0, "Timestamp")).unwrap();
assert_eq!( assert_eq!(
timestamp_re.captures(line).unwrap().name("Timestamp").unwrap().as_str(), timestamp_re.captures(line).unwrap().name("Timestamp").unwrap().as_str(),
@@ -592,10 +444,6 @@ mod tests {
#[test] #[test]
fn is_log_stream_line_rejects_non_columnar_lines_that_a_fixed_offset_column_regex_would_still_match() { fn is_log_stream_line_rejects_non_columnar_lines_that_a_fixed_offset_column_regex_would_still_match() {
// Ticket 076: this is exactly why `ParamDeclaration::log_stream_gated` params must check
// `is_log_stream_line` before trusting their own offset-based capture at all -- the raw
// column regex has no content anchor and matches plenty on lines that clearly aren't
// columnar `log stream` output.
let header_line = LOG_STREAM_FIXTURE.lines().next().unwrap(); let header_line = LOG_STREAM_FIXTURE.lines().next().unwrap();
let header = detect_log_stream_header(header_line).unwrap(); let header = detect_log_stream_header(header_line).unwrap();
let timestamp_re = Regex::new(&log_stream_column_pattern(&header, 0, "Timestamp")).unwrap(); let timestamp_re = Regex::new(&log_stream_column_pattern(&header, 0, "Timestamp")).unwrap();
@@ -614,7 +462,6 @@ mod tests {
.find(|line| line.contains("txRTSFrm=")) .find(|line| line.contains("txRTSFrm="))
.expect("fixture must contain the WiFi stat dump line"); .expect("fixture must contain the WiFi stat dump line");
assert_eq!(detect_format(line), "log_stream"); assert_eq!(detect_format(line), "log_stream");
// The embedded key=value pairs are still there for auto_declare_params to see.
assert!(!extract_pairs(line).is_empty()); assert!(!extract_pairs(line).is_empty());
} }
src/main.rs
-21
View File
@@ -26,8 +26,6 @@ use tokio::net::TcpListener;
use settings::Settings; use settings::Settings;
/// The resolved default settings path, rendered into `--help` so `--settings-path`'s help text
/// names a real, concrete location instead of only describing where to look for one.
static SETTINGS_PATH_HELP: LazyLock<String> = LazyLock::new(|| { static SETTINGS_PATH_HELP: LazyLock<String> = LazyLock::new(|| {
format!( format!(
"Path to the settings file. Defaults to {}.", "Path to the settings file. Defaults to {}.",
@@ -37,50 +35,35 @@ static SETTINGS_PATH_HELP: LazyLock<String> = LazyLock::new(|| {
) )
}); });
/// Observability dashboard from terminal output.
#[derive(Parser, Debug)] #[derive(Parser, Debug)]
struct Args { struct Args {
#[command(subcommand)] #[command(subcommand)]
command: Option<Command>, command: Option<Command>,
/// Port to serve the API and webapp on. Defaults to 0, letting the OS assign
/// an ephemeral free port.
#[arg(long, default_value_t = 0)] #[arg(long, default_value_t = 0)]
port: u16, port: u16,
#[arg(long, help = SETTINGS_PATH_HELP.as_str())] #[arg(long, help = SETTINGS_PATH_HELP.as_str())]
settings_path: Option<PathBuf>, settings_path: Option<PathBuf>,
/// File to tail for log lines, instead of reading from stdin. Behaves like `tail -f`.
#[arg(long)] #[arg(long)]
file: Option<PathBuf>, file: Option<PathBuf>,
/// URL of a Prometheus/OpenMetrics-compatible endpoint to poll for metrics, in addition to log ingestion.
#[arg(long)] #[arg(long)]
metrics_url: Option<String>, metrics_url: Option<String>,
/// Name to register this session under, so `ottyl mcp --session <name>` or `ottyl sessions`
/// can find it later. Defaults to a random human-readable name (e.g. "comic-gazelle").
/// Starting a second instance under the same name as one still running overwrites its
/// registration (the older process keeps running, just without a discoverable session entry).
#[arg(long)] #[arg(long)]
name: Option<String>, name: Option<String>,
} }
#[derive(Subcommand, Debug)] #[derive(Subcommand, Debug)]
enum Command { enum Command {
/// Start an MCP (Model Context Protocol) server over stdio, proxying tool calls to a running
/// ottyl instance's HTTP API. Register the printed command with an MCP client (e.g. Claude
/// Code's `.mcp.json`).
Mcp { Mcp {
/// Name of a running ottyl session to connect to (see `ottyl sessions`).
#[arg(long)] #[arg(long)]
session: Option<String>, session: Option<String>,
/// Port of a running ottyl instance to connect to, instead of resolving `--session` by name.
#[arg(long)] #[arg(long)]
port: Option<u16>, port: Option<u16>,
}, },
/// List known ottyl sessions and the ports they're serving on.
Sessions, Sessions,
} }
@@ -113,8 +96,6 @@ async fn main() {
let _ = engine_tx.tx.send(engine::EngineMessage::SetMaxLogs(max_logs)).await; let _ = engine_tx.tx.send(engine::EngineMessage::SetMaxLogs(max_logs)).await;
} }
// Reload the active profile's declarations before ingest starts: there's no way to
// reprocess early lines later, so declaring after ingest starts could miss lines that raced ahead.
{ {
let (active_profile_name, active_profile) = { let (active_profile_name, active_profile) = {
let mut settings_guard = settings.lock().unwrap(); let mut settings_guard = settings.lock().unwrap();
@@ -184,8 +165,6 @@ async fn main() {
.expect("server error"); .expect("server error");
} }
/// Resolves `--session`/`--port` into a target base URL and runs the MCP server over stdio until
/// the client disconnects.
async fn run_mcp(settings_path: &std::path::Path, session: Option<String>, port: Option<u16>) { async fn run_mcp(settings_path: &std::path::Path, session: Option<String>, port: Option<u16>) {
use rmcp::ServiceExt; use rmcp::ServiceExt;
src/mcp_server.rs
-40
View File
@@ -1,13 +1,3 @@
//! MCP (Model Context Protocol) server for agents (ticket 067): a short-lived process, started
//! via `ottyl mcp --session <name>` (see `crate::sessions`), that speaks MCP over stdio and
//! proxies each tool call to a *running* ottyl instance's existing HTTP API. Every tool here is a
//! thin translation onto `api.rs` routes/DTOs — no query logic is duplicated outside `engine.rs`.
//!
//! Uses the official `rmcp` SDK (https://github.com/modelcontextprotocol/rust-sdk) over the
//! `transport-io` (stdio) transport rather than hand-rolling the JSON-RPC/MCP handshake, since a
//! maintained implementation of the protocol's framing and lifecycle is worth the extra
//! dependency.
use std::collections::BTreeMap; use std::collections::BTreeMap;
use rmcp::handler::server::router::tool::ToolRouter; use rmcp::handler::server::router::tool::ToolRouter;
@@ -21,13 +11,10 @@ use serde_json::Value;
#[derive(Debug, Deserialize, schemars::JsonSchema)] #[derive(Debug, Deserialize, schemars::JsonSchema)]
pub struct QueryLogsParams { pub struct QueryLogsParams {
/// FilterBar query string, e.g. `regex("error") and ottyl_seq<5`. Omitted matches every log.
#[serde(default)] #[serde(default)]
pub filter: Option<String>, pub filter: Option<String>,
/// If true, return only how many logs matched instead of the log lines themselves.
#[serde(default)] #[serde(default)]
pub count_only: bool, pub count_only: bool,
/// Maximum number of log lines to return. Ignored when `count_only` is set.
#[serde(default)] #[serde(default)]
pub limit: Option<usize>, pub limit: Option<usize>,
} }
@@ -35,15 +22,11 @@ pub struct QueryLogsParams {
#[derive(Debug, Serialize, Deserialize, schemars::JsonSchema)] #[derive(Debug, Serialize, Deserialize, schemars::JsonSchema)]
pub struct DeclareParamParams { pub struct DeclareParamParams {
pub name: String, pub name: String,
/// Regex matched against each log line; `capture_group` extracts this param's value from it.
pub message_match_regex: String, pub message_match_regex: String,
pub capture_group: String, pub capture_group: String,
/// Dotted path into a JSON-structured log line to extract from instead of `capture_group`.
#[serde(default)] #[serde(default)]
pub json_field_path: Option<String>, pub json_field_path: Option<String>,
/// One of: "string", "number", "timestamp", "duration", "boolean".
pub kind: String, pub kind: String,
/// Extract the elapsed time since this param was last seen, instead of parsing captured text.
#[serde(default)] #[serde(default)]
pub elapsed_since_last: bool, pub elapsed_since_last: bool,
} }
@@ -52,23 +35,18 @@ pub struct DeclareParamParams {
pub struct DeclareMetricParams { pub struct DeclareMetricParams {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
/// Names of already-declared params whose values become this metric's bucket labels.
#[serde(default)] #[serde(default)]
pub label_params: Vec<String>, pub label_params: Vec<String>,
/// Name of an already-declared param whose value becomes this metric's per-event bucket value.
#[serde(default)] #[serde(default)]
pub value_param: Option<String>, pub value_param: Option<String>,
/// Fixed labels layered on top of `label_params` for matching lines.
#[serde(default)] #[serde(default)]
pub matched_labels: Option<BTreeMap<String, String>>, pub matched_labels: Option<BTreeMap<String, String>>,
/// Fixed labels recorded once for a non-matching line, instead of it being invisible to this metric.
#[serde(default)] #[serde(default)]
pub unmatched_labels: Option<BTreeMap<String, String>>, pub unmatched_labels: Option<BTreeMap<String, String>>,
} }
#[derive(Debug, Deserialize, schemars::JsonSchema)] #[derive(Debug, Deserialize, schemars::JsonSchema)]
pub struct CreateDashboardParams { pub struct CreateDashboardParams {
/// Profile to create the dashboard in. Defaults to the currently active profile.
#[serde(default)] #[serde(default)]
pub profile: Option<String>, pub profile: Option<String>,
pub name: String, pub name: String,
@@ -76,17 +54,13 @@ pub struct CreateDashboardParams {
#[derive(Debug, Deserialize, schemars::JsonSchema)] #[derive(Debug, Deserialize, schemars::JsonSchema)]
pub struct AddPanelParams { pub struct AddPanelParams {
/// Profile the dashboard lives in. Defaults to the currently active profile.
#[serde(default)] #[serde(default)]
pub profile: Option<String>, pub profile: Option<String>,
pub dashboard_id: String, pub dashboard_id: String,
/// Id of an existing panel to update in place; omit to add a new panel.
#[serde(default)] #[serde(default)]
pub panel_id: Option<String>, pub panel_id: Option<String>,
pub title: String, pub title: String,
/// PromQL-style query backing this panel (see `crate::promql`).
pub promql: String, pub promql: String,
/// One of: "bar", "line", "pie". Defaults to "line".
#[serde(default = "default_chart_type")] #[serde(default = "default_chart_type")]
pub chart_type: String, pub chart_type: String,
} }
@@ -97,7 +71,6 @@ fn default_chart_type() -> String {
#[derive(Debug, Deserialize, schemars::JsonSchema)] #[derive(Debug, Deserialize, schemars::JsonSchema)]
pub struct GetDashboardParams { pub struct GetDashboardParams {
/// Profile the dashboard lives in. Defaults to the currently active profile.
#[serde(default)] #[serde(default)]
pub profile: Option<String>, pub profile: Option<String>,
pub dashboard_id: String, pub dashboard_id: String,
@@ -107,9 +80,6 @@ pub struct GetDashboardParams {
pub struct OttylMcp { pub struct OttylMcp {
client: reqwest::Client, client: reqwest::Client,
base_url: String, base_url: String,
// Constructed fresh per call by the `#[tool_handler]`-generated `ServerHandler` methods
// (`Self::tool_router()`), not read through this field — kept only because `#[tool_router]`
// requires a field of this name/type to exist on the struct.
#[allow(dead_code)] #[allow(dead_code)]
tool_router: ToolRouter<OttylMcp>, tool_router: ToolRouter<OttylMcp>,
} }
@@ -138,8 +108,6 @@ impl OttylMcp {
format!("{}{path}", self.base_url) format!("{}{path}", self.base_url)
} }
/// Resolves the profile a dashboard tool call should operate on: the explicit `profile`
/// argument if given, otherwise the target instance's currently active profile.
async fn resolve_profile(&self, explicit: Option<String>) -> Result<String, McpError> { async fn resolve_profile(&self, explicit: Option<String>) -> Result<String, McpError> {
if let Some(profile) = explicit { if let Some(profile) = explicit {
return Ok(profile); return Ok(profile);
@@ -282,10 +250,6 @@ impl OttylMcp {
json_result(body) json_result(body)
} }
// Read-modify-write over HTTP (no dashboard-level locking): two concurrent `add_panel` calls
// against the same dashboard can race and one's addition can be lost, same as two browser tabs
// saving the same dashboard through the webapp today. Not addressed here — out of scope for a
// thin proxy over the existing `save_dashboard` endpoint.
#[tool(description = "Add a new panel to a dashboard, or update an existing one by panel_id.")] #[tool(description = "Add a new panel to a dashboard, or update an existing one by panel_id.")]
async fn add_panel( async fn add_panel(
&self, &self,
@@ -359,8 +323,6 @@ impl OttylMcp {
} }
} }
/// Percent-encodes a single path segment (profile/dashboard names/ids can contain characters that
/// aren't valid unescaped in a URL path).
fn urlencoding_path(segment: &str) -> String { fn urlencoding_path(segment: &str) -> String {
segment segment
.bytes() .bytes()
@@ -373,8 +335,6 @@ fn urlencoding_path(segment: &str) -> String {
.collect() .collect()
} }
/// Generates a new panel id by hashing title/promql/time, avoiding a `uuid` dependency (matches
/// `settings::generate_imported_css_id`'s approach for the same reason).
fn generate_panel_id(title: &str, promql: &str) -> String { fn generate_panel_id(title: &str, promql: &str) -> String {
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher}; use std::hash::{Hash, Hasher};
src/metric_primitive.rs
-6
View File
@@ -3,16 +3,13 @@ use serde::Serialize;
use std::cmp::Ordering; use std::cmp::Ordering;
use std::fmt; use std::fmt;
/// A simple, self-describing value used by metrics/labels without pulling in JSON.
#[allow(dead_code)] #[allow(dead_code)]
#[derive(Debug, Clone, Serialize)] #[derive(Debug, Clone, Serialize)]
pub enum MetricPrimitive { pub enum MetricPrimitive {
String(String), String(String),
Number(Number), Number(Number),
Timestamp(Timestamp), Timestamp(Timestamp),
/// Milliseconds elapsed; distinct from `Number` so it reads unambiguously in JSON/label form.
Duration(i64), Duration(i64),
/// A `Boolean`-kind param's value: whether `message_match_regex` matched the line. See `MetricPrimitiveKind::Boolean`.
Boolean(bool), Boolean(bool),
} }
@@ -51,7 +48,6 @@ impl Ord for MetricPrimitive {
} }
} }
/// Textual form used to compare label filters (query-string values) against a formatted `MetricPrimitive` rather than parsing them into one.
impl fmt::Display for MetricPrimitive { impl fmt::Display for MetricPrimitive {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self { match self {
@@ -65,7 +61,6 @@ impl fmt::Display for MetricPrimitive {
} }
} }
/// Numbers are kept as a tagged int/float pair so exact integers stay exact.
#[allow(dead_code)] #[allow(dead_code)]
#[derive(Debug, Clone, Copy, Serialize)] #[derive(Debug, Clone, Copy, Serialize)]
pub enum Number { pub enum Number {
@@ -74,7 +69,6 @@ pub enum Number {
} }
impl Number { impl Number {
/// Widens to `f64` for cross-kind numeric comparisons; `pub(crate)` since the filter evaluator in `engine.rs` needs it.
pub(crate) fn as_f64(&self) -> f64 { pub(crate) fn as_f64(&self) -> f64 {
match self { match self {
Number::Int(v) => *v as f64, Number::Int(v) => *v as f64,
src/profile.rs
-205
View File
@@ -1,8 +1,3 @@
//! Named, persistable bundles of parsing/indexing rules and dashboards for a CLI app.
//! Ticket 049 folded profile storage inline into `Settings` (`settings.json`) instead of a separate
//! `profiles.json`; this module still owns the `Profile`/`Dashboard`/declaration data model, plus the
//! CRUD methods on `Settings` (`impl Settings` below `ProfileError`) that used to live on the
//! now-removed `ProfileStore`.
#![allow(dead_code)] #![allow(dead_code)]
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
@@ -22,16 +17,13 @@ use crate::engine::{DeclareMetricRequest, DeclareParamRequest, EngineHandle, Eng
use crate::settings::Settings; use crate::settings::Settings;
use tokio::sync::oneshot; use tokio::sync::oneshot;
/// Mirrors `data_value::MetricPrimitiveKind` with `Serialize`/`Deserialize` so profiles can round-trip through JSON, since the engine-internal type only derives `Serialize`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
pub enum ProfileMetricPrimitiveKind { pub enum ProfileMetricPrimitiveKind {
String, String,
Number, Number,
Timestamp, Timestamp,
/// Mirrors `MetricPrimitiveKind::Duration`.
Duration, Duration,
/// Mirrors `MetricPrimitiveKind::Boolean`.
Boolean, Boolean,
} }
@@ -59,20 +51,14 @@ impl From<ProfileMetricPrimitiveKind> for MetricPrimitiveKind {
} }
} }
/// A stored param declaration, matching `DeclareParamRequest`'s shape (== `data_value::ParamDeclaration`).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct ParamDeclarationRecord { pub struct ParamDeclarationRecord {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
pub capture_group: String, pub capture_group: String,
/// See `crate::data_value::ParamDeclaration::json_field_path`; `#[serde(default)]`
/// so profiles saved before this field existed still load as plain regex-capture-group params.
#[serde(default)] #[serde(default)]
pub json_field_path: Option<String>, pub json_field_path: Option<String>,
pub kind: ProfileMetricPrimitiveKind, pub kind: ProfileMetricPrimitiveKind,
/// See `crate::data_value::ParamDeclaration::elapsed_since_last`; `#[serde(default)]` so
/// profiles saved before ticket 038 (where a `Duration`-kind value param implicitly meant this)
/// still load — `migrate_duration_value_params` below converts those.
#[serde(default)] #[serde(default)]
pub elapsed_since_last: bool, pub elapsed_since_last: bool,
} }
@@ -99,17 +85,12 @@ impl From<&ParamDeclarationRecord> for DeclareParamRequest {
json_field_path: record.json_field_path.clone(), json_field_path: record.json_field_path.clone(),
kind: record.kind.into(), kind: record.kind.into(),
elapsed_since_last: record.elapsed_since_last, elapsed_since_last: record.elapsed_since_last,
// A profile only ever stores manually-declared params (see `record_param_declaration`).
auto_derived: false, auto_derived: false,
log_stream_gated: false, log_stream_gated: false,
} }
} }
} }
/// Pre-037 role of an inline metric param matcher — a label identifying a bucket, or the (at most
/// one) value that becomes the event's captured value. Kept only so `LegacyMetricParamRecord` can
/// still deserialize a profile saved before params were unified into standalone `ParamDeclaration`s
/// referenced by name; see `migrate_legacy_metric_params`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
enum LegacyParamRole { enum LegacyParamRole {
@@ -118,10 +99,6 @@ enum LegacyParamRole {
Value, Value,
} }
/// Pre-037 inline metric param matcher shape (formerly `data_value::RegexToMetricPrimitiveMatcher`),
/// kept only to support migrating profiles saved before ticket 037 unified params — see
/// `migrate_legacy_metric_params`. Never produced when serializing a record built from a
/// `DeclareMetricRequest`.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
struct LegacyMetricParamRecord { struct LegacyMetricParamRecord {
param_name: String, param_name: String,
@@ -133,25 +110,16 @@ struct LegacyMetricParamRecord {
role: LegacyParamRole, role: LegacyParamRole,
} }
/// A stored metric declaration, matching `DeclareMetricRequest`'s shape. `legacy_params` is only
/// ever populated by deserializing a pre-037 profile file (old key name `"params"`) — see
/// `migrate_legacy_metric_params`, which converts it into `label_params`/`value_param` plus any
/// implicitly-created standalone params, and empties it back out so a subsequent save never writes
/// the old shape again.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct MetricDeclarationRecord { pub struct MetricDeclarationRecord {
pub name: String, pub name: String,
pub message_match_regex: String, pub message_match_regex: String,
/// Names of standalone `ParamDeclarationRecord`s used as this metric's bucket labels.
#[serde(default)] #[serde(default)]
pub label_params: Vec<String>, pub label_params: Vec<String>,
/// Name of the standalone `ParamDeclarationRecord` (if any) whose value becomes this metric's bucket value.
#[serde(default)] #[serde(default)]
pub value_param: Option<String>, pub value_param: Option<String>,
/// See `crate::engine::DeclareMetricRequest::matched_labels`; `#[serde(default)]` so older profiles still load.
#[serde(default)] #[serde(default)]
pub matched_labels: Option<std::collections::BTreeMap<String, String>>, pub matched_labels: Option<std::collections::BTreeMap<String, String>>,
/// See `crate::engine::DeclareMetricRequest::unmatched_labels`; `#[serde(default)]` so older profiles still load.
#[serde(default)] #[serde(default)]
pub unmatched_labels: Option<std::collections::BTreeMap<String, String>>, pub unmatched_labels: Option<std::collections::BTreeMap<String, String>>,
#[serde(default, rename = "params", skip_serializing_if = "Vec::is_empty")] #[serde(default, rename = "params", skip_serializing_if = "Vec::is_empty")]
@@ -185,15 +153,6 @@ impl From<&MetricDeclarationRecord> for DeclareMetricRequest {
} }
} }
/// Migrates every metric in `profile` still carrying pre-037 inline matchers
/// (`MetricDeclarationRecord::legacy_params`, deserialized from an old profile file's `"params"`
/// key) into the unified shape: each matcher becomes a reference to a standalone
/// `ParamDeclarationRecord`, resolved by reusing an existing declared param of the exact same name
/// and shape (regex/capture-group-or-json-field-path/kind) when one exists, or else creating a new
/// one implicitly named `<metric_name>.<param_name>` (added to `profile.params`). Deterministic
/// naming means re-running this on an already-migrated profile is a no-op, and running it twice on
/// the same legacy data produces the same result. Returns whether anything changed, so the caller
/// can decide whether to persist the migration.
fn migrate_legacy_metric_params(profile: &mut Profile) -> bool { fn migrate_legacy_metric_params(profile: &mut Profile) -> bool {
let mut migrated = false; let mut migrated = false;
for metric_index in 0..profile.metrics.len() { for metric_index in 0..profile.metrics.len() {
@@ -221,8 +180,6 @@ fn migrate_legacy_metric_params(profile: &mut Profile) -> bool {
migrated migrated
} }
/// Resolves one legacy matcher to a standalone param name, reusing an existing declared param of
/// the same bare name and shape, or else creating `<metric_name>.<param_name>` on `profile.params`.
fn resolve_or_create_legacy_param( fn resolve_or_create_legacy_param(
profile: &mut Profile, profile: &mut Profile,
metric_name: &str, metric_name: &str,
@@ -247,9 +204,6 @@ fn resolve_or_create_legacy_param(
capture_group: matcher.capture_group.clone(), capture_group: matcher.capture_group.clone(),
json_field_path: matcher.json_field_path.clone(), json_field_path: matcher.json_field_path.clone(),
kind: matcher.kind, kind: matcher.kind,
// Pre-037, a Value-role Duration matcher implicitly meant "elapsed time since the
// previous same-labeled event" (ticket 038's now-explicit `elapsed_since_last`) — carry
// that meaning forward rather than dropping it into a meaningless literal-parse matcher.
elapsed_since_last: matcher.role == LegacyParamRole::Value elapsed_since_last: matcher.role == LegacyParamRole::Value
&& matcher.kind == ProfileMetricPrimitiveKind::Duration, && matcher.kind == ProfileMetricPrimitiveKind::Duration,
}); });
@@ -257,18 +211,6 @@ fn resolve_or_create_legacy_param(
implicit_name implicit_name
} }
/// Migrates every already-declared `Duration`-kind param that's referenced as some metric's
/// `value_param` into the explicit `elapsed_since_last` extractor (ticket 038). Before ticket 038, a
/// `Duration`-kind value param implicitly meant "elapsed time since the previous same-labeled
/// event" (see `crate::engine::apply_metric_to_log`'s pre-038 history) — this migration makes that
/// meaning explicit so `Duration` kind no longer has a role-dependent special case. Only params
/// actually referenced as *some* metric's value param are converted: a `Duration`-kind param used
/// purely as a label param already meant a literal parsed millisecond count, and keeps meaning that.
/// Runs after `migrate_legacy_metric_params` (which populates `value_param` from pre-037
/// `legacy_params` and already sets `elapsed_since_last` correctly for those via
/// `resolve_or_create_legacy_param`), so this only has new work to do for profiles saved between
/// ticket 037 and ticket 038 landing, where `value_param` already pointed at a bare `Duration`-kind
/// param straight from the unified shape. Returns whether anything changed.
fn migrate_duration_value_params(profile: &mut Profile) -> bool { fn migrate_duration_value_params(profile: &mut Profile) -> bool {
let value_param_names: std::collections::HashSet<&str> = let value_param_names: std::collections::HashSet<&str> =
profile.metrics.iter().filter_map(|metric| metric.value_param.as_deref()).collect(); profile.metrics.iter().filter_map(|metric| metric.value_param.as_deref()).collect();
@@ -285,7 +227,6 @@ fn migrate_duration_value_params(profile: &mut Profile) -> bool {
migrated migrated
} }
/// A chart type a dashboard panel can render; kept in sync with the frontend's `'bar' | 'line' | 'pie'` union (see `webapp/src/lib/api.ts`).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
pub enum ChartType { pub enum ChartType {
@@ -294,7 +235,6 @@ pub enum ChartType {
Pie, Pie,
} }
/// One panel within a dashboard: a PromQL query plus how to chart its result.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Panel { pub struct Panel {
pub id: String, pub id: String,
@@ -303,7 +243,6 @@ pub struct Panel {
pub chart_type: ChartType, pub chart_type: ChartType,
} }
/// A named set of panels, saved as part of a profile.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Dashboard { pub struct Dashboard {
pub id: String, pub id: String,
@@ -312,7 +251,6 @@ pub struct Dashboard {
pub panels: Vec<Panel>, pub panels: Vec<Panel>,
} }
/// Generates an id by hashing a seed (dashboard/panel name) with the current time, avoiding a `uuid` dependency for what's otherwise an opaque string.
fn generate_id(seed: &str) -> String { fn generate_id(seed: &str) -> String {
let mut hasher = DefaultHasher::new(); let mut hasher = DefaultHasher::new();
seed.hash(&mut hasher); seed.hash(&mut hasher);
@@ -331,7 +269,6 @@ impl Dashboard {
} }
} }
/// A counter that splits every line into `category="error"`/`"other"` via `matched_labels`/`unmatched_labels` rather than a captured param, so case variants like "ERROR" collapse into one series; the `regex` crate lacks the lookaround needed to express "didn't match" any other way.
fn seeded_error_split_metric() -> MetricDeclarationRecord { fn seeded_error_split_metric() -> MetricDeclarationRecord {
MetricDeclarationRecord { MetricDeclarationRecord {
name: SEEDED_METRIC_NAME.to_string(), name: SEEDED_METRIC_NAME.to_string(),
@@ -344,7 +281,6 @@ fn seeded_error_split_metric() -> MetricDeclarationRecord {
} }
} }
/// The starter dashboard seeded into a brand-new `Default` profile: a line chart and a pie chart, both querying the seeded error-split counter.
fn seeded_default_dashboard() -> Dashboard { fn seeded_default_dashboard() -> Dashboard {
let mut dashboard = Dashboard::new("Overview".to_string()); let mut dashboard = Dashboard::new("Overview".to_string());
dashboard.panels = vec![ dashboard.panels = vec![
@@ -364,7 +300,6 @@ fn seeded_default_dashboard() -> Dashboard {
dashboard dashboard
} }
/// A named bundle of parsing/indexing rules for a particular CLI app, plus any saved dashboards.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Profile { pub struct Profile {
pub name: String, pub name: String,
@@ -387,7 +322,6 @@ impl Profile {
} }
} }
/// Replays every param/metric declaration stored in `profile` into `engine` (used on startup and in tests); any rejected declaration or send/recv failure is logged to stderr and skipped rather than panicking.
pub async fn load_into_engine(engine: &EngineHandle, profile: &Profile) { pub async fn load_into_engine(engine: &EngineHandle, profile: &Profile) {
for record in &profile.params { for record in &profile.params {
let (respond_to, response) = oneshot::channel(); let (respond_to, response) = oneshot::channel();
@@ -442,16 +376,11 @@ pub async fn load_into_engine(engine: &EngineHandle, profile: &Profile) {
} }
} }
/// Error returned by `Settings`'s profile-related mutations and lookups.
#[derive(Debug)] #[derive(Debug)]
pub enum ProfileError { pub enum ProfileError {
/// No profile with the given name exists.
NotFound, NotFound,
/// A profile with the given name already exists.
DuplicateName, DuplicateName,
/// The `Default` profile can't be deleted — there must always be at least one profile to fall back to.
CannotDeleteDefault, CannotDeleteDefault,
/// Underlying I/O failure reading/writing the profile store or settings file.
Io(io::Error), Io(io::Error),
} }
@@ -474,10 +403,6 @@ impl From<io::Error> for ProfileError {
} }
} }
/// Reads a legacy pre-049 `profiles.json` file (back when profiles lived in a separate file rather
/// than inline in `settings.json`) for `Settings::migrate_legacy_profiles_file`. Returns `None` if
/// the file doesn't exist or can't be read/parsed — treated as "nothing to migrate" rather than
/// failing settings load.
pub(crate) fn load_legacy_profile_store(path: &Path) -> Option<Vec<Profile>> { pub(crate) fn load_legacy_profile_store(path: &Path) -> Option<Vec<Profile>> {
let contents = fs::read_to_string(path).ok()?; let contents = fs::read_to_string(path).ok()?;
match serde_json::from_str::<Vec<Profile>>(&contents) { match serde_json::from_str::<Vec<Profile>>(&contents) {
@@ -492,23 +417,6 @@ pub(crate) fn load_legacy_profile_store(path: &Path) -> Option<Vec<Profile>> {
} }
} }
/// Turns any pre-unification/pre-explicit-duration metric or param shapes still present in
/// `profiles` into their current form; used by `Settings::load` on every startup (previously done
/// by the now-removed `ProfileStore::load`, before ticket 049 folded profile storage into
/// `settings.json`). Returns whether anything changed, so the caller can decide whether to
/// re-persist.
///
/// Ticket 037: turn any pre-unification inline metric matchers into standalone param references.
/// Deterministic naming makes this idempotent, so it's safe to run on every load; persisting the
/// result isn't required for correctness (migration re-runs identically next time), but it's done
/// here anyway so the on-disk file only carries the legacy `"params"` shape once, on the very next
/// save.
///
/// Ticket 038: follows on immediately after — once `migrate_legacy_metric_params` has resolved
/// every metric's `value_param`, convert any `Duration`-kind param still referenced that way into
/// the explicit `elapsed_since_last` extractor, so a profile saved either before 037 (inline
/// matchers) or between 037 and 038 (bare `value_param` reference) ends up behaving identically
/// under the new, explicit shape.
pub(crate) fn migrate_legacy_profile_shapes(profiles: &mut [Profile]) -> bool { pub(crate) fn migrate_legacy_profile_shapes(profiles: &mut [Profile]) -> bool {
let mut any_migrated = false; let mut any_migrated = false;
for profile in profiles { for profile in profiles {
@@ -521,17 +429,11 @@ pub(crate) fn migrate_legacy_profile_shapes(profiles: &mut [Profile]) -> bool {
any_migrated any_migrated
} }
/// Profile CRUD, previously on the standalone `ProfileStore` (removed by ticket 049): every mutation
/// here reads/writes `self.profiles` directly and persists via `Settings::save`, so `settings.json` is
/// the single source of truth instead of a separate file kept in sync alongside it.
impl Settings { impl Settings {
pub fn profile(&self, name: &str) -> Option<&Profile> { pub fn profile(&self, name: &str) -> Option<&Profile> {
self.profiles.iter().find(|p| p.name == name) self.profiles.iter().find(|p| p.name == name)
} }
/// Creates a new profile named `name`, persisting settings, copying the active profile's
/// params/metrics/dashboards as a starting point rather than starting blank. Fails with
/// `DuplicateName` if the name exists.
pub fn create_profile(&mut self, settings_path: &Path, name: String) -> Result<(), ProfileError> { pub fn create_profile(&mut self, settings_path: &Path, name: String) -> Result<(), ProfileError> {
if self.profile(&name).is_some() { if self.profile(&name).is_some() {
return Err(ProfileError::DuplicateName); return Err(ProfileError::DuplicateName);
@@ -547,8 +449,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Deletes the profile named `name`, persisting settings, clearing `active_profile` if it was
/// active. Fails with `NotFound`, or `CannotDeleteDefault` for the `Default` profile.
pub fn delete_profile(&mut self, settings_path: &Path, name: &str) -> Result<(), ProfileError> { pub fn delete_profile(&mut self, settings_path: &Path, name: &str) -> Result<(), ProfileError> {
if name == DEFAULT_PROFILE_NAME { if name == DEFAULT_PROFILE_NAME {
return Err(ProfileError::CannotDeleteDefault); return Err(ProfileError::CannotDeleteDefault);
@@ -562,9 +462,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Ensures a `Default` profile exists (seeding a starter dashboard/metric only on first
/// creation), and falls back to active if nothing else is. Called once on startup before anything
/// else touches profiles.
pub fn ensure_default_profile(&mut self, settings_path: &Path) -> Result<(), ProfileError> { pub fn ensure_default_profile(&mut self, settings_path: &Path) -> Result<(), ProfileError> {
if self.profile(DEFAULT_PROFILE_NAME).is_none() { if self.profile(DEFAULT_PROFILE_NAME).is_none() {
let mut profile = Profile::new(DEFAULT_PROFILE_NAME.to_string()); let mut profile = Profile::new(DEFAULT_PROFILE_NAME.to_string());
@@ -579,9 +476,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Sets (or clears, with `None`) the active profile. Fails with `NotFound` if `name` is `Some`
/// and no such profile exists. Doesn't itself reload declarations into a running engine — that
/// happens on next startup via `load_into_engine`.
pub fn set_active_profile( pub fn set_active_profile(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -597,11 +491,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Empties `profile_name`'s stored param declarations, persisting settings — the counterpart to
/// `record_param_declaration` for `DELETE /api/params` (ticket 052). A profile only ever stores
/// manually-declared params (auto-derived ones live only in the engine, never persisted here), so
/// this is enough to keep the whole set from reappearing on next load; the engine-side clear
/// re-derives whatever auto params the retained log text still supports.
pub fn clear_param_declarations( pub fn clear_param_declarations(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -617,9 +506,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Records/updates a declared param into `profile_name`'s stored declarations (matched by param
/// name), persisting settings. Intended to be called by the HTTP API alongside declaring against
/// the engine, so declarations survive a restart.
pub fn record_param_declaration( pub fn record_param_declaration(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -640,17 +526,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Records/updates a declared metric into `profile_name`'s stored declarations (matched by metric
/// name), persisting settings; 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( pub fn record_metric_declaration(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -677,10 +552,6 @@ impl Settings {
Ok(()) 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.
/// `promoted_params` has the same meaning as in `record_metric_declaration`.
pub fn record_metric_declaration_update( pub fn record_metric_declaration_update(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -711,11 +582,6 @@ impl Settings {
Ok(()) Ok(())
} }
/// Removes `name`'s persisted metric declaration from `profile_name` (ticket 051), so it doesn't
/// reappear from `settings.json` on the next restart. A no-op (not an error) if the profile has
/// no record under that name — matches `Settings::remove_css`'s sibling calls in spirit, but
/// deleting an already-gone declaration is fine here since the caller (the API layer) has
/// already confirmed against the live engine index whether the metric existed at all.
pub fn remove_metric_declaration( pub fn remove_metric_declaration(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -732,14 +598,11 @@ impl Settings {
Ok(()) Ok(())
} }
/// Lists the dashboards saved on `profile_name`; fails with `NotFound` if it doesn't exist.
pub fn list_dashboards(&self, profile_name: &str) -> Result<&[Dashboard], ProfileError> { pub fn list_dashboards(&self, profile_name: &str) -> Result<&[Dashboard], ProfileError> {
let profile = self.profile(profile_name).ok_or(ProfileError::NotFound)?; let profile = self.profile(profile_name).ok_or(ProfileError::NotFound)?;
Ok(&profile.dashboards) Ok(&profile.dashboards)
} }
/// Creates a new, empty dashboard named `name` on `profile_name`, persisting settings; fails
/// with `NotFound` if the profile doesn't exist.
pub fn create_dashboard( pub fn create_dashboard(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -757,9 +620,6 @@ impl Settings {
Ok(dashboard) Ok(dashboard)
} }
/// Replaces the dashboard identified by `dashboard_id` with `dashboard`, persisting settings;
/// `dashboard.id` is forced to `dashboard_id` so a caller can't change a dashboard's identity
/// through this call.
pub fn save_dashboard( pub fn save_dashboard(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -783,8 +643,6 @@ impl Settings {
Ok(dashboard) Ok(dashboard)
} }
/// Deletes the dashboard identified by `dashboard_id` from `profile_name`, persisting settings;
/// fails with `NotFound` if either doesn't exist.
pub fn delete_dashboard( pub fn delete_dashboard(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -839,10 +697,6 @@ mod tests {
} }
} }
/// Ticket 075: reads no longer go through `EngineHandle::tx` — this reads straight off
/// `EngineHandle::reader_factory`, same as the real API layer. Only used here for
/// `ParamDeclarations`/`MetricDeclarations` lookups, neither of which ever needs the
/// `EnsureSortHelperIndex` round-trip a sorted `Logs` query would (see `api.rs`'s own `ask`).
async fn ask(engine: &EngineHandle, query: EngineQuery) -> EngineResponse { async fn ask(engine: &EngineHandle, query: EngineQuery) -> EngineResponse {
let read_handle = engine.reader_factory.handle(); let read_handle = engine.reader_factory.handle();
let guard = read_handle.enter().expect("engine unavailable"); let guard = read_handle.enter().expect("engine unavailable");
@@ -864,7 +718,6 @@ mod tests {
let dup = settings.create_profile(&settings_path, "web".to_string()); let dup = settings.create_profile(&settings_path, "web".to_string());
assert!(matches!(dup, Err(ProfileError::DuplicateName))); assert!(matches!(dup, Err(ProfileError::DuplicateName)));
// Reload from disk: profiles persisted.
let reloaded_settings = Settings::load(&settings_path); let reloaded_settings = Settings::load(&settings_path);
assert_eq!(reloaded_settings.profiles.len(), 2); assert_eq!(reloaded_settings.profiles.len(), 2);
assert!(reloaded_settings.profile("web").is_some()); assert!(reloaded_settings.profile("web").is_some());
@@ -889,7 +742,6 @@ mod tests {
assert!(settings.profile(DEFAULT_PROFILE_NAME).is_some()); assert!(settings.profile(DEFAULT_PROFILE_NAME).is_some());
assert_eq!(settings.active_profile, Some(DEFAULT_PROFILE_NAME.to_string())); assert_eq!(settings.active_profile, Some(DEFAULT_PROFILE_NAME.to_string()));
// Idempotent: doesn't duplicate the profile or clobber an already-active (non-Default) profile.
settings.create_profile(&settings_path, "web".to_string()).unwrap(); settings.create_profile(&settings_path, "web".to_string()).unwrap();
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap();
settings.ensure_default_profile(&settings_path).unwrap(); settings.ensure_default_profile(&settings_path).unwrap();
@@ -926,7 +778,6 @@ mod tests {
.any(|p| p.chart_type == ChartType::Line && p.promql.contains(SEEDED_METRIC_NAME))); .any(|p| p.chart_type == ChartType::Line && p.promql.contains(SEEDED_METRIC_NAME)));
assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Pie && p.promql == SEEDED_METRIC_NAME)); assert!(dashboard.panels.iter().any(|p| p.chart_type == ChartType::Pie && p.promql == SEEDED_METRIC_NAME));
// Reload (simulating a restart) and ensure_default again: must not re-seed or duplicate.
let mut reloaded = Settings::load(&settings_path); let mut reloaded = Settings::load(&settings_path);
reloaded.ensure_default_profile(&settings_path).unwrap(); reloaded.ensure_default_profile(&settings_path).unwrap();
let default_profile = reloaded.profile(DEFAULT_PROFILE_NAME).unwrap(); let default_profile = reloaded.profile(DEFAULT_PROFILE_NAME).unwrap();
@@ -1009,7 +860,6 @@ mod tests {
assert_eq!(copy.params, settings.profile("web").unwrap().params); assert_eq!(copy.params, settings.profile("web").unwrap().params);
assert_eq!(copy.dashboards, settings.profile("web").unwrap().dashboards); assert_eq!(copy.dashboards, settings.profile("web").unwrap().dashboards);
// With no active profile, new profiles start empty as before.
settings.set_active_profile(&settings_path, None).unwrap(); settings.set_active_profile(&settings_path, None).unwrap();
settings.create_profile(&settings_path, "blank".to_string()).unwrap(); settings.create_profile(&settings_path, "blank".to_string()).unwrap();
let blank = settings.profile("blank").unwrap(); let blank = settings.profile("blank").unwrap();
@@ -1029,17 +879,14 @@ mod tests {
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap();
assert_eq!(settings.active_profile, Some("web".to_string())); assert_eq!(settings.active_profile, Some("web".to_string()));
// Switching to another known profile works and persists.
settings.set_active_profile(&settings_path, Some("worker".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("worker".to_string())).unwrap();
let reloaded = Settings::load(&settings_path); let reloaded = Settings::load(&settings_path);
assert_eq!(reloaded.active_profile, Some("worker".to_string())); assert_eq!(reloaded.active_profile, Some("worker".to_string()));
// Setting an unknown profile active is rejected, leaving the previous value.
let err = settings.set_active_profile(&settings_path, Some("ghost".to_string())); let err = settings.set_active_profile(&settings_path, Some("ghost".to_string()));
assert!(matches!(err, Err(ProfileError::NotFound))); assert!(matches!(err, Err(ProfileError::NotFound)));
assert_eq!(settings.active_profile, Some("worker".to_string())); assert_eq!(settings.active_profile, Some("worker".to_string()));
// Clearing the active profile works.
settings.set_active_profile(&settings_path, None).unwrap(); settings.set_active_profile(&settings_path, None).unwrap();
assert_eq!(settings.active_profile, None); assert_eq!(settings.active_profile, None);
} }
@@ -1064,14 +911,12 @@ mod tests {
assert_eq!(profile.metrics.len(), 1); assert_eq!(profile.metrics.len(), 1);
assert_eq!(profile.metrics[0].name, "requests"); assert_eq!(profile.metrics[0].name, "requests");
// Re-declaring the same name updates in place rather than duplicating.
let param_req_v2 = param_request("level", r"lvl=(?P<level>\w+)", "level"); let param_req_v2 = param_request("level", r"lvl=(?P<level>\w+)", "level");
settings.record_param_declaration(&settings_path, "web", &param_req_v2).unwrap(); settings.record_param_declaration(&settings_path, "web", &param_req_v2).unwrap();
let profile = settings.profile("web").unwrap(); let profile = settings.profile("web").unwrap();
assert_eq!(profile.params.len(), 1); assert_eq!(profile.params.len(), 1);
assert_eq!(profile.params[0].message_match_regex, r"lvl=(?P<level>\w+)"); assert_eq!(profile.params[0].message_match_regex, r"lvl=(?P<level>\w+)");
// Reload from disk: recorded declarations survive a restart.
let reloaded = Settings::load(&settings_path); let reloaded = Settings::load(&settings_path);
let profile = reloaded.profile("web").unwrap(); let profile = reloaded.profile("web").unwrap();
assert_eq!(profile.params.len(), 1); assert_eq!(profile.params.len(), 1);
@@ -1092,22 +937,18 @@ mod tests {
settings.remove_metric_declaration(&settings_path, "web", "requests").unwrap(); settings.remove_metric_declaration(&settings_path, "web", "requests").unwrap();
assert!(settings.profile("web").unwrap().metrics.is_empty()); assert!(settings.profile("web").unwrap().metrics.is_empty());
// Removing an unknown metric name is a harmless no-op, not an error.
settings.remove_metric_declaration(&settings_path, "web", "never-declared").unwrap(); settings.remove_metric_declaration(&settings_path, "web", "never-declared").unwrap();
// Reload from disk: the removal itself survives a restart (ticket 051's acceptance criterion).
let reloaded = Settings::load(&settings_path); let reloaded = Settings::load(&settings_path);
assert!(reloaded.profile("web").unwrap().metrics.is_empty()); assert!(reloaded.profile("web").unwrap().metrics.is_empty());
} }
/// Acceptance criterion: declaring params/metrics, "restarting" (reloading settings from disk), and replaying into a fresh engine reproduces the same declarations and still ingests lines, without re-declaring by hand.
#[tokio::test] #[tokio::test]
async fn profile_round_trip_reproduces_declarations_in_fresh_engine() { async fn profile_round_trip_reproduces_declarations_in_fresh_engine() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let settings_path = dir.path().join("settings.json"); let settings_path = dir.path().join("settings.json");
let mut settings = Settings::default(); let mut settings = Settings::default();
// Declare directly into a profile, as the HTTP API will do via the `record_*_declaration` hooks.
settings.create_profile(&settings_path, "web".to_string()).unwrap(); settings.create_profile(&settings_path, "web".to_string()).unwrap();
settings settings
.record_param_declaration( .record_param_declaration(
@@ -1116,8 +957,6 @@ mod tests {
&param_request("level", r"level=(?P<level>\w+)", "level"), &param_request("level", r"level=(?P<level>\w+)", "level"),
) )
.unwrap(); .unwrap();
// A metric now references params by name (ticket 037), so "duration" must be declared as a
// standalone param before the metric below can reference it as a label param.
settings settings
.record_param_declaration( .record_param_declaration(
&settings_path, &settings_path,
@@ -1135,18 +974,15 @@ mod tests {
.unwrap(); .unwrap();
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap();
// Simulate a restart: drop everything in memory, reload from disk.
drop(settings); drop(settings);
let settings = Settings::load(&settings_path); let settings = Settings::load(&settings_path);
let active_name = settings.active_profile.clone().unwrap(); let active_name = settings.active_profile.clone().unwrap();
let profile = settings.profile(&active_name).unwrap().clone(); let profile = settings.profile(&active_name).unwrap().clone();
// Spawn a fresh engine and replay the profile's declarations into it.
let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone())); let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone()));
let engine_tx = crate::engine::spawn(settings_handle, 16); let engine_tx = crate::engine::spawn(settings_handle, 16);
load_into_engine(&engine_tx, &profile).await; load_into_engine(&engine_tx, &profile).await;
// Ingest a line and confirm the engine works with the replayed declarations (no re-declaration needed).
engine_tx engine_tx
.tx .tx
.send(EngineMessage::Ingest(IngestedLine { .send(EngineMessage::Ingest(IngestedLine {
@@ -1198,7 +1034,6 @@ mod tests {
assert_eq!(logs[0].text, "level=info request duration=42"); assert_eq!(logs[0].text, "level=info request duration=42");
} }
/// Acceptance criterion: a dashboard (with a panel) survives reloading settings from disk.
#[test] #[test]
fn dashboard_create_save_delete_round_trip_and_persists_across_reload() { fn dashboard_create_save_delete_round_trip_and_persists_across_reload() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1224,7 +1059,6 @@ mod tests {
assert_eq!(saved.panels.len(), 1); assert_eq!(saved.panels.len(), 1);
assert_eq!(saved.id, dashboard.id); assert_eq!(saved.id, dashboard.id);
// Simulate a restart: drop and reload from disk.
drop(settings); drop(settings);
let mut reloaded = Settings::load(&settings_path); let mut reloaded = Settings::load(&settings_path);
let dashboards = reloaded.list_dashboards("web").unwrap(); let dashboards = reloaded.list_dashboards("web").unwrap();
@@ -1233,23 +1067,17 @@ mod tests {
assert_eq!(dashboards[0].panels.len(), 1); assert_eq!(dashboards[0].panels.len(), 1);
assert_eq!(dashboards[0].panels[0].promql, r#"requests{path="/a"}"#); assert_eq!(dashboards[0].panels[0].promql, r#"requests{path="/a"}"#);
// Deleting removes it, and that too persists.
reloaded.delete_dashboard(&settings_path, "web", &dashboard.id).unwrap(); reloaded.delete_dashboard(&settings_path, "web", &dashboard.id).unwrap();
assert!(reloaded.list_dashboards("web").unwrap().is_empty()); assert!(reloaded.list_dashboards("web").unwrap().is_empty());
let reloaded_again = Settings::load(&settings_path); let reloaded_again = Settings::load(&settings_path);
assert!(reloaded_again.list_dashboards("web").unwrap().is_empty()); assert!(reloaded_again.list_dashboards("web").unwrap().is_empty());
// Unknown profile/dashboard both fail with NotFound.
assert!(matches!( assert!(matches!(
reloaded_again.list_dashboards("ghost"), reloaded_again.list_dashboards("ghost"),
Err(ProfileError::NotFound) Err(ProfileError::NotFound)
)); ));
} }
/// Writes a pre-037 `settings.json` (with an inline `profiles` array whose metrics carry legacy
/// `"params"` matchers rather than `label_params`/`value_param`) directly to disk, bypassing
/// `Settings`/`Profile` entirely so this doesn't accidentally exercise the current
/// (post-migration) shape. `profiles_json` is the raw JSON array of profiles.
fn write_legacy_settings_json(settings_path: &Path, profiles_json: &str) { fn write_legacy_settings_json(settings_path: &Path, profiles_json: &str) {
fs::create_dir_all(settings_path.parent().unwrap()).unwrap(); fs::create_dir_all(settings_path.parent().unwrap()).unwrap();
let contents = format!( let contents = format!(
@@ -1258,9 +1086,6 @@ mod tests {
fs::write(settings_path, contents).unwrap(); fs::write(settings_path, contents).unwrap();
} }
/// Acceptance criterion: a profile saved before ticket 037 (inline `param_name`/`capture_group`/
/// `kind`/`role` matchers embedded in the metric) loads with those matchers turned into
/// standalone param declarations referenced by name, and the metric still ingests identically.
#[test] #[test]
fn migrates_legacy_inline_metric_params_into_standalone_declarations() { fn migrates_legacy_inline_metric_params_into_standalone_declarations() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1289,7 +1114,6 @@ mod tests {
let settings = Settings::load(&settings_path); let settings = Settings::load(&settings_path);
let profile = settings.profile("legacy").unwrap(); let profile = settings.profile("legacy").unwrap();
// No pre-existing bare "path"/"duration" param, so implicit ones were created, named after the metric.
assert_eq!(profile.params.len(), 2); assert_eq!(profile.params.len(), 2);
let path_param = profile.params.iter().find(|p| p.name == "requests.path").unwrap(); let path_param = profile.params.iter().find(|p| p.name == "requests.path").unwrap();
assert_eq!(path_param.message_match_regex, profile.metrics[0].message_match_regex); assert_eq!(path_param.message_match_regex, profile.metrics[0].message_match_regex);
@@ -1302,11 +1126,9 @@ mod tests {
assert_eq!(profile.metrics[0].label_params, vec!["requests.path".to_string()]); assert_eq!(profile.metrics[0].label_params, vec!["requests.path".to_string()]);
assert_eq!(profile.metrics[0].value_param, Some("requests.duration".to_string())); assert_eq!(profile.metrics[0].value_param, Some("requests.duration".to_string()));
// The migration is persisted, so the on-disk file no longer carries the legacy "params" shape.
let reloaded_raw = fs::read_to_string(&settings_path).unwrap(); let reloaded_raw = fs::read_to_string(&settings_path).unwrap();
assert!(!reloaded_raw.contains("param_name")); assert!(!reloaded_raw.contains("param_name"));
// Replaying into a fresh engine behaves identically to before migration: the metric still ingests and buckets.
let profile = profile.clone(); let profile = profile.clone();
let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(Settings::default())); let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(Settings::default()));
tokio::runtime::Runtime::new().unwrap().block_on(async { tokio::runtime::Runtime::new().unwrap().block_on(async {
@@ -1332,10 +1154,6 @@ mod tests {
}); });
} }
/// Acceptance criterion (ticket 038): a pre-037 legacy profile whose inline `Value`-role matcher
/// used `kind: "duration"` (the old "elapsed time since previous event" special case) migrates
/// into a standalone param with the new explicit `elapsed_since_last: true` extractor, not just a
/// bare `Duration`-kind param that would now (post-038) mean a literal parsed millisecond count.
#[test] #[test]
fn migrates_legacy_value_role_duration_matcher_into_elapsed_since_last_param() { fn migrates_legacy_value_role_duration_matcher_into_elapsed_since_last_param() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1370,11 +1188,6 @@ mod tests {
assert_eq!(profile.metrics[0].value_param, Some("login_gap.gap".to_string())); assert_eq!(profile.metrics[0].value_param, Some("login_gap.gap".to_string()));
} }
/// Acceptance criterion (ticket 038): a profile saved after ticket 037 landed but before ticket
/// 038 — so it already carries the unified `label_params`/`value_param` shape rather than legacy
/// inline matchers, with a bare `Duration`-kind param referenced as a metric's `value_param` — is
/// migrated the same way: the param gains `elapsed_since_last: true` so its behavior is unchanged
/// (elapsed time since the previous same-labeled event) under the new explicit extractor.
#[test] #[test]
fn migrates_post_037_bare_duration_value_param_into_elapsed_since_last() { fn migrates_post_037_bare_duration_value_param_into_elapsed_since_last() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1406,13 +1219,9 @@ mod tests {
let gap_param = profile.params.iter().find(|p| p.name == "gap").unwrap(); let gap_param = profile.params.iter().find(|p| p.name == "gap").unwrap();
assert!(gap_param.elapsed_since_last); assert!(gap_param.elapsed_since_last);
// A Duration-kind param not referenced as any value_param is left untouched (still a literal parse).
assert!(!profile.params.iter().find(|p| p.name == "user").unwrap().elapsed_since_last); assert!(!profile.params.iter().find(|p| p.name == "user").unwrap().elapsed_since_last);
} }
/// When a bare-named param already declared on the profile matches a legacy matcher's shape
/// exactly (same regex/capture-group/kind), migration reuses it by name instead of creating a
/// redundant `<metric>.<param>` declaration.
#[test] #[test]
fn migration_reuses_an_existing_matching_bare_named_param() { fn migration_reuses_an_existing_matching_bare_named_param() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1442,14 +1251,11 @@ mod tests {
let settings = Settings::load(&settings_path); let settings = Settings::load(&settings_path);
let profile = settings.profile("legacy").unwrap(); let profile = settings.profile("legacy").unwrap();
// Reused the existing "path" declaration rather than creating "requests.path".
assert_eq!(profile.params.len(), 1); assert_eq!(profile.params.len(), 1);
assert_eq!(profile.params[0].name, "path"); assert_eq!(profile.params[0].name, "path");
assert_eq!(profile.metrics[0].label_params, vec!["path".to_string()]); assert_eq!(profile.metrics[0].label_params, vec!["path".to_string()]);
} }
/// Migration is idempotent: loading an already-migrated (or never-legacy) profile a second time
/// doesn't change anything or re-trigger a save.
#[test] #[test]
fn migration_is_a_noop_on_an_already_migrated_profile() { fn migration_is_a_noop_on_an_already_migrated_profile() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1473,11 +1279,6 @@ mod tests {
assert_eq!(before, after); 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] #[tokio::test]
async fn metric_referencing_auto_derived_param_survives_restart_via_promotion() { async fn metric_referencing_auto_derived_param_survives_restart_via_promotion() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -1487,8 +1288,6 @@ mod tests {
settings.create_profile(&settings_path, "web".to_string()).unwrap(); settings.create_profile(&settings_path, "web".to_string()).unwrap();
settings.set_active_profile(&settings_path, Some("web".to_string())).unwrap(); settings.set_active_profile(&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 settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone())); let settings_handle = std::sync::Arc::new(std::sync::Mutex::new(settings.clone()));
let engine_tx = crate::engine::spawn(settings_handle, 16); let engine_tx = crate::engine::spawn(settings_handle, 16);
engine_tx engine_tx
@@ -1501,8 +1300,6 @@ mod tests {
.await .await
.unwrap(); .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 metric_req = metric_request("direction_x", r"dx=(?P<dx>\S+) moved", "dx");
let (respond_to, response) = oneshot::channel(); let (respond_to, response) = oneshot::channel();
engine_tx engine_tx
@@ -1512,7 +1309,6 @@ mod tests {
.unwrap(); .unwrap();
response.await.unwrap().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) = let EngineResponse::ParamDeclarations(live_params) =
ask(&engine_tx, EngineQuery::ParamDeclarations).await ask(&engine_tx, EngineQuery::ParamDeclarations).await
else { else {
@@ -1530,7 +1326,6 @@ mod tests {
}; };
settings.record_metric_declaration(&settings_path, "web", &metric_req, &[promoted]).unwrap(); settings.record_metric_declaration(&settings_path, "web", &metric_req, &[promoted]).unwrap();
// Simulate a restart: drop everything, reload from disk into a *fresh* engine with no prior ingestion.
drop(settings); drop(settings);
let reloaded_settings = Settings::load(&settings_path); let reloaded_settings = Settings::load(&settings_path);
let profile = reloaded_settings.profile("web").unwrap().clone(); let profile = reloaded_settings.profile("web").unwrap().clone();
src/promql.rs
-24
View File
@@ -1,21 +1,12 @@
//! Hand-rolled parser for a small PromQL subset: metric name + label matchers, an optional `[5m]` range, and a top-level `sum()`/`avg()`/`count()`/`rate()`/`histogram()`/`gauge()` wrapper.
//! Only exact-match (`=`) label matchers are supported; `rate()` requires a duration range selector, while `histogram(metric[width])` reuses that `[...]` slot for a bare bucket width instead.
//! `sum()`/`avg()` may additionally wrap a `rate(...)` call directly (e.g. `sum(rate(requests[5m]))`), and either may be followed by a `by (label, ...)` clause to preserve one series per distinct value of the named labels instead of collapsing to a single series.
//! `count()` is a plain vector selector wrapper only (no `rate()` nesting) — it counts occurrences per group regardless of the metric's value, which is the deliberate opt-in for occurrence counting that `sum()`/`avg()` no longer fall back to for a `String`-kind metric (see `crate::engine::evaluate_promql`).
use std::time::Duration; use std::time::Duration;
#[derive(Debug, Clone, Copy, PartialEq, Eq)] #[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AggOp { pub enum AggOp {
Sum, Sum,
Avg, Avg,
/// Occurrence count per group, ignoring the metric's stored value entirely — the explicit,
/// opt-in-by-name alternative to summing a `String`-kind metric's coerced-to-1.0 values.
Count, Count,
Rate, Rate,
/// Buckets counter values into fixed-width buckets; requires `bucket_width`.
Histogram, Histogram,
/// The most-recent counter value per label combination.
Gauge, Gauge,
} }
@@ -30,25 +21,17 @@ pub struct PromqlQuery {
pub agg: Option<AggOp>, pub agg: Option<AggOp>,
pub metric_name: String, pub metric_name: String,
pub matchers: Vec<LabelMatcher>, pub matchers: Vec<LabelMatcher>,
/// Duration range selector, e.g. `[5m]`; only meaningful for `rate()`.
pub range: Option<Duration>, pub range: Option<Duration>,
/// Bare-number range selector, e.g. `[10]`; only meaningful for `histogram()`, where it's the bucket width.
pub bucket_width: Option<f64>, pub bucket_width: Option<f64>,
/// Set when `agg` is `Sum`/`Avg` and it wraps an inner `rate(metric[range])` rather than a bare vector
/// selector, e.g. `sum(rate(requests[5m]))`. `range` above carries the inner `rate()`'s duration in that case.
pub inner_rate: bool, pub inner_rate: bool,
/// Labels named in a trailing `by (...)` clause; empty means "collapse to a single series" (the historical
/// behavior of `sum()`/`avg()`). When non-empty, one series is kept per distinct combination of these labels.
pub by_labels: Vec<String>, pub by_labels: Vec<String>,
} }
/// What a `[...]` range selector turned out to hold: a duration, or a bare width.
enum RangeSelector { enum RangeSelector {
Duration(Duration), Duration(Duration),
BareWidth(f64), BareWidth(f64),
} }
/// Parses a PromQL query string, returning a human-readable error message on malformed input.
pub fn parse_query(input: &str) -> Result<PromqlQuery, String> { pub fn parse_query(input: &str) -> Result<PromqlQuery, String> {
let mut parser = Parser::new(input); let mut parser = Parser::new(input);
let query = parser.parse_query()?; let query = parser.parse_query()?;
@@ -177,7 +160,6 @@ impl<'a> Parser<'a> {
Ok(matchers) Ok(matchers)
} }
/// Parses `[<digits><unit>]` (a duration, e.g. `[5m]`) or a bare `[<digits>]` (a bucket width).
fn parse_range_selector(&mut self) -> Result<Option<RangeSelector>, String> { fn parse_range_selector(&mut self) -> Result<Option<RangeSelector>, String> {
self.skip_whitespace(); self.skip_whitespace();
if self.peek() != Some('[') { if self.peek() != Some('[') {
@@ -239,7 +221,6 @@ impl<'a> Parser<'a> {
}) })
} }
/// Parses a trailing `by (label, ...)` clause, if present; returns an empty vec otherwise.
fn parse_by_clause(&mut self) -> Result<Vec<String>, String> { fn parse_by_clause(&mut self) -> Result<Vec<String>, String> {
self.skip_whitespace(); self.skip_whitespace();
let checkpoint = self.pos; let checkpoint = self.pos;
@@ -289,14 +270,11 @@ impl<'a> Parser<'a> {
_ => None, _ => None,
}; };
self.skip_whitespace(); self.skip_whitespace();
// `sum by (label) (metric)`: a `by (...)` clause may also precede the aggregation's
// parenthesized argument, not just follow it.
let prefix_by_labels = if agg.is_some() { self.parse_by_clause()? } else { Vec::new() }; let prefix_by_labels = if agg.is_some() { self.parse_by_clause()? } else { Vec::new() };
self.skip_whitespace(); self.skip_whitespace();
if agg.is_some() && self.peek() == Some('(') { if agg.is_some() && self.peek() == Some('(') {
self.expect_char('(')?; self.expect_char('(')?;
self.skip_whitespace(); self.skip_whitespace();
// `sum()`/`avg()` may wrap `rate(metric[range])` directly instead of a bare vector selector.
let mut inner = if matches!(agg, Some(AggOp::Sum) | Some(AggOp::Avg)) { let mut inner = if matches!(agg, Some(AggOp::Sum) | Some(AggOp::Avg)) {
let inner_checkpoint = self.pos; let inner_checkpoint = self.pos;
let took_inner_rate = self.parse_identifier().is_ok_and(|id| id == "rate"); let took_inner_rate = self.parse_identifier().is_ok_and(|id| id == "rate");
@@ -338,7 +316,6 @@ impl<'a> Parser<'a> {
if prefix_by_labels.is_empty() { trailing_by_labels } else { prefix_by_labels }; if prefix_by_labels.is_empty() { trailing_by_labels } else { prefix_by_labels };
Ok(inner) Ok(inner)
} else { } else {
// Rewind and parse as a plain vector selector, covering metric names that collide with an agg_op keyword.
self.pos = checkpoint; self.pos = checkpoint;
self.parse_vector_selector() self.parse_vector_selector()
} }
@@ -432,7 +409,6 @@ mod tests {
#[test] #[test]
fn rejects_unknown_aggregation() { fn rejects_unknown_aggregation() {
// `median` isn't a supported agg_op, so this parses as a bare metric name and trips on the trailing `(...)`.
let err = parse_query("median(requests)").unwrap_err(); let err = parse_query("median(requests)").unwrap_err();
assert!(err.contains("unexpected trailing input")); assert!(err.contains("unexpected trailing input"));
} }
src/rate.rs
-35
View File
@@ -1,24 +1,7 @@
//! Ticket 040: the single windowed-rate/count implementation in the codebase, shared by
//! PromQL's `rate()` (`crate::engine::evaluate_promql`, via the `windowed_rate_series` adapter
//! there) and alert `Threshold`/`Ratio` conditions (`crate::alerts`). Previously each computed
//! its own trailing window independently — PromQL scanning indexed bucket data, alerts scanning
//! raw log-line matches with a hand-rolled `VecDeque` — and could disagree for equivalent-looking
//! rules simply because the window boundary was implemented twice.
//!
//! `count_in_window` is the one place that decides what "falls in the trailing window" means;
//! everything else in this module is built on top of it, either by batch-rescanning a full point
//! series (`rate_at`/`windowed_rate_series`, fine for PromQL's historical/one-shot evaluation) or
//! incrementally (`RateWindow`, for alerts, which see one point at a time and need cost
//! proportional to the window rather than a full rescan per ingested line).
use std::collections::VecDeque; use std::collections::VecDeque;
use crate::timestamp::Timestamp; use crate::timestamp::Timestamp;
/// How many `points` fall in the trailing `(now - window_ms, now]` window. Points are occurrence
/// markers rather than a continuous reading, so only their timestamps matter — the boundary is
/// exclusive on the old end and inclusive of `now` itself, matching the original
/// `windowed_rate_series` behavior this replaces.
fn count_in_window(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64) -> usize { fn count_in_window(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64) -> usize {
let now_ms = now.as_millis(); let now_ms = now.as_millis();
let window_start = now_ms - window_ms; let window_start = now_ms - window_ms;
@@ -31,33 +14,21 @@ fn count_in_window(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64)
.count() .count()
} }
/// Trailing-window occurrence rate (points per second) ending at `now`: how many `points` fall in
/// `(now - window_ms, now]`, divided by the window width in seconds.
pub fn rate_at(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64) -> f64 { pub fn rate_at(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64) -> f64 {
let window_seconds = window_ms as f64 / 1000.0; let window_seconds = window_ms as f64 / 1000.0;
count_in_window(points, now, window_ms) as f64 / window_seconds count_in_window(points, now, window_ms) as f64 / window_seconds
} }
/// Evaluates `rate_at` at every one of `points`' own timestamps, turning an occurrence series
/// into a windowed rate series — used by PromQL's `rate()` so it plots as a real line rather than
/// one trailing point.
pub fn windowed_rate_series(points: &[(Timestamp, f64)], window_ms: i64) -> Vec<(Timestamp, f64)> { pub fn windowed_rate_series(points: &[(Timestamp, f64)], window_ms: i64) -> Vec<(Timestamp, f64)> {
points.iter().map(|(ts, _)| (*ts, rate_at(points, *ts, window_ms))).collect() points.iter().map(|(ts, _)| (*ts, rate_at(points, *ts, window_ms))).collect()
} }
/// Incrementally maintained trailing window: for a caller that sees points arrive one at a time
/// (alert `Threshold`/`Ratio` conditions, evaluated per ingested line) and needs cost proportional
/// to the window rather than `count_in_window`'s full rescan per call. Assumes points are pushed
/// in non-decreasing timestamp order, true for ingested log lines.
#[derive(Debug, Default, Clone)] #[derive(Debug, Default, Clone)]
pub struct RateWindow { pub struct RateWindow {
timestamps: VecDeque<Timestamp>, timestamps: VecDeque<Timestamp>,
} }
impl RateWindow { impl RateWindow {
/// Builds a window pre-populated from already-seen history (e.g. currently-ingested logs
/// matching an alert's query), so a freshly-created window starts with whatever of that
/// history already falls in the trailing window instead of empty.
pub fn backfill(timestamps: impl IntoIterator<Item = Timestamp>, now: Timestamp, window_ms: i64) -> Self { pub fn backfill(timestamps: impl IntoIterator<Item = Timestamp>, now: Timestamp, window_ms: i64) -> Self {
let mut window = Self::default(); let mut window = Self::default();
for ts in timestamps { for ts in timestamps {
@@ -67,14 +38,11 @@ impl RateWindow {
window window
} }
/// Records one occurrence at `timestamp` and drops anything that has aged out of the
/// resulting trailing window.
pub fn push(&mut self, timestamp: Timestamp, window_ms: i64) { pub fn push(&mut self, timestamp: Timestamp, window_ms: i64) {
self.timestamps.push_back(timestamp); self.timestamps.push_back(timestamp);
self.prune(timestamp, window_ms); self.prune(timestamp, window_ms);
} }
/// Same boundary as `count_in_window`: drops entries at or before `now - window_ms`.
fn prune(&mut self, now: Timestamp, window_ms: i64) { fn prune(&mut self, now: Timestamp, window_ms: i64) {
let window_start = now.as_millis() - window_ms; let window_start = now.as_millis() - window_ms;
while let Some(&front) = self.timestamps.front() { while let Some(&front) = self.timestamps.front() {
@@ -102,9 +70,7 @@ mod tests {
(Timestamp::from_millis(1000), 1.0), (Timestamp::from_millis(1000), 1.0),
(Timestamp::from_millis(2000), 1.0), (Timestamp::from_millis(2000), 1.0),
]; ];
// (1000, 2000]: only the point at 2000.
assert_eq!(rate_at(&points, Timestamp::from_millis(2000), 1000), 1.0); assert_eq!(rate_at(&points, Timestamp::from_millis(2000), 1000), 1.0);
// (0, 2000]: points at 1000 and 2000 -> 2 points / 2s.
assert_eq!(rate_at(&points, Timestamp::from_millis(2000), 2000), 1.0); assert_eq!(rate_at(&points, Timestamp::from_millis(2000), 2000), 1.0);
} }
@@ -136,7 +102,6 @@ mod tests {
#[test] #[test]
fn rate_window_backfill_seeds_from_history_within_the_window() { fn rate_window_backfill_seeds_from_history_within_the_window() {
let history = [0, 500, 2000, 2500].map(Timestamp::from_millis); let history = [0, 500, 2000, 2500].map(Timestamp::from_millis);
// now=2500ms, window=1000ms -> window (1500, 2500]: only 2000 and 2500 qualify.
let window = RateWindow::backfill(history, Timestamp::from_millis(2500), 1000); let window = RateWindow::backfill(history, Timestamp::from_millis(2500), 1000);
assert_eq!(window.count(), 2); assert_eq!(window.count(), 2);
} }
src/sessions.rs
-30
View File
@@ -1,8 +1,3 @@
//! Named session registry (ticket 068): each running `ottyl` process registers itself under a
//! name by writing a small file to `<settings dir>/sessions/<name>.json`, so a second CLI
//! invocation (e.g. `ottyl mcp --session <name>`, ticket 067) can resolve a name to a port without
//! the user copying one around by hand.
use std::fs; use std::fs;
use std::io; use std::io;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
@@ -10,7 +5,6 @@ use std::process;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
/// On-disk shape of a `sessions/<name>.json` file.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct SessionInfo { pub struct SessionInfo {
pub name: String, pub name: String,
@@ -18,8 +12,6 @@ pub struct SessionInfo {
pub pid: u32, pub pid: u32,
} }
/// The `sessions/` directory for a given settings file, following the same
/// sibling-directory convention as `profiles/` (see `settings::migrate_legacy_profiles_file`).
pub fn sessions_dir(settings_path: &Path) -> PathBuf { pub fn sessions_dir(settings_path: &Path) -> PathBuf {
settings_path settings_path
.parent() .parent()
@@ -31,10 +23,6 @@ fn session_file_path(dir: &Path, name: &str) -> PathBuf {
dir.join(format!("{name}.json")) dir.join(format!("{name}.json"))
} }
/// Best-effort liveness check for a PID. Shells out to `kill -0` on unix (no signal is delivered,
/// only permission/existence is checked); assumed alive on other platforms, which just means a
/// stale session file there waits for an explicit name collision to be overwritten instead of
/// being proactively cleaned up.
#[cfg(unix)] #[cfg(unix)]
fn is_pid_alive(pid: u32) -> bool { fn is_pid_alive(pid: u32) -> bool {
process::Command::new("kill") process::Command::new("kill")
@@ -51,9 +39,6 @@ fn is_pid_alive(_pid: u32) -> bool {
true true
} }
/// Removes session files whose owning process is no longer running. Best-effort: there's no
/// shutdown hook to unregister a session on graceful exit, so this runs on every startup instead.
/// Any I/O error reading a single entry is ignored — a future call gets another chance.
pub fn cleanup_stale(dir: &Path) { pub fn cleanup_stale(dir: &Path) {
let Ok(entries) = fs::read_dir(dir) else { let Ok(entries) = fs::read_dir(dir) else {
return; return;
@@ -75,16 +60,10 @@ pub fn cleanup_stale(dir: &Path) {
} }
} }
/// Generates a random human-readable session name (e.g. `comic-gazelle`) for when `--name` isn't
/// given explicitly.
pub fn generate_name() -> String { pub fn generate_name() -> String {
petname::petname(2, "-").unwrap_or_else(|| format!("session-{}", process::id())) petname::petname(2, "-").unwrap_or_else(|| format!("session-{}", process::id()))
} }
/// Registers this process as session `name` on `port`: cleans up stale entries, then writes
/// `sessions/<name>.json`, overwriting any existing file for that name (documented behavior — a
/// second instance started under the same explicit `--name` as a still-running one simply
/// replaces its registration; the older process keeps running, just without a discoverable entry).
pub fn register_session(settings_path: &Path, name: &str, port: u16) -> io::Result<PathBuf> { pub fn register_session(settings_path: &Path, name: &str, port: u16) -> io::Result<PathBuf> {
let dir = sessions_dir(settings_path); let dir = sessions_dir(settings_path);
fs::create_dir_all(&dir)?; fs::create_dir_all(&dir)?;
@@ -99,9 +78,6 @@ pub fn register_session(settings_path: &Path, name: &str, port: u16) -> io::Resu
Ok(path) Ok(path)
} }
/// Looks up a session by name. Returns `None` if no such file exists, it fails to parse, or its
/// process is no longer alive (a stale entry left by a process that didn't exit through the normal
/// next-`register_session` cleanup path).
pub fn resolve_session(settings_path: &Path, name: &str) -> Option<SessionInfo> { pub fn resolve_session(settings_path: &Path, name: &str) -> Option<SessionInfo> {
let dir = sessions_dir(settings_path); let dir = sessions_dir(settings_path);
let contents = fs::read_to_string(session_file_path(&dir, name)).ok()?; let contents = fs::read_to_string(session_file_path(&dir, name)).ok()?;
@@ -109,8 +85,6 @@ pub fn resolve_session(settings_path: &Path, name: &str) -> Option<SessionInfo>
is_pid_alive(info.pid).then_some(info) is_pid_alive(info.pid).then_some(info)
} }
/// Lists every session file under `dir`, live or stale (callers that only want live sessions
/// should filter on `is_pid_alive`-backed `resolve_session` instead). Used by `ottyl sessions`.
pub fn list_sessions(dir: &Path) -> Vec<SessionInfo> { pub fn list_sessions(dir: &Path) -> Vec<SessionInfo> {
let Ok(entries) = fs::read_dir(dir) else { let Ok(entries) = fs::read_dir(dir) else {
return Vec::new(); return Vec::new();
@@ -157,8 +131,6 @@ mod tests {
let sessions_dir_path = sessions_dir(&settings_path); let sessions_dir_path = sessions_dir(&settings_path);
fs::create_dir_all(&sessions_dir_path).unwrap(); fs::create_dir_all(&sessions_dir_path).unwrap();
// A PID essentially guaranteed not to be alive (PID 1 exists but a huge, unlikely-assigned
// PID stands in as "not alive" without depending on system PID-reuse specifics).
let stale = SessionInfo { name: "foo".to_string(), port: 1111, pid: 999_999 }; let stale = SessionInfo { name: "foo".to_string(), port: 1111, pid: 999_999 };
fs::write( fs::write(
session_file_path(&sessions_dir_path, "foo"), session_file_path(&sessions_dir_path, "foo"),
@@ -166,8 +138,6 @@ mod tests {
) )
.unwrap(); .unwrap();
// Registering a live session under the same name succeeds and the stale cleanup pass (run
// as part of registration) doesn't error out on the bogus pre-existing entry.
register_session(&settings_path, "foo", 5555).unwrap(); register_session(&settings_path, "foo", 5555).unwrap();
let info = resolve_session(&settings_path, "foo").unwrap(); let info = resolve_session(&settings_path, "foo").unwrap();
assert_eq!(info.port, 5555); assert_eq!(info.port, 5555);
src/settings.rs
-92
View File
@@ -1,6 +1,3 @@
//! Process-wide settings model: colour scheme, CSS entries, and profiles (ticket 049 folded
//! profile storage inline here instead of a separate `profiles.json`), persisted as JSON.
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
use std::fmt; use std::fmt;
use std::fs; use std::fs;
@@ -16,15 +13,8 @@ use serde::{Deserialize, Serialize};
use crate::alerts::AlertRule; use crate::alerts::AlertRule;
use crate::profile::Profile; use crate::profile::Profile;
/// Shared, process-wide settings handle. Cheap to clone.
pub type SettingsHandle = Arc<Mutex<Settings>>; pub type SettingsHandle = Arc<Mutex<Settings>>;
/// User's preferred colour scheme for the webapp.
///
/// `Dark` accepts the legacy `"grey"` serde value (ticket 004: "grey" was a
/// near-duplicate of "dark" and has been removed as a selectable option) so a
/// settings file saved before that change still loads cleanly instead of
/// falling back to `System`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
#[serde(rename_all = "lowercase")] #[serde(rename_all = "lowercase")]
pub enum ColorScheme { pub enum ColorScheme {
@@ -37,33 +27,22 @@ pub enum ColorScheme {
Yellow, Yellow,
Pink, Pink,
Orange, Orange,
/// Brighter, light-surfaced counterpart to `Green` (ticket 048).
#[serde(rename = "light_green")] #[serde(rename = "light_green")]
LightGreen, LightGreen,
/// Brighter, light-surfaced counterpart to `Red` (ticket 056).
#[serde(rename = "light_red")] #[serde(rename = "light_red")]
LightRed, LightRed,
/// Brighter, light-surfaced counterpart to `Blue` (ticket 056).
#[serde(rename = "light_blue")] #[serde(rename = "light_blue")]
LightBlue, LightBlue,
/// Brighter, light-surfaced counterpart to `Yellow` (ticket 056).
#[serde(rename = "light_yellow")] #[serde(rename = "light_yellow")]
LightYellow, LightYellow,
/// Brighter, light-surfaced counterpart to `Pink` (ticket 056).
#[serde(rename = "light_pink")] #[serde(rename = "light_pink")]
LightPink, LightPink,
/// Brighter, light-surfaced counterpart to `Orange` (ticket 056).
#[serde(rename = "light_orange")] #[serde(rename = "light_orange")]
LightOrange, LightOrange,
#[default] #[default]
System, System,
} }
/// A single CSS entry: content ottyl stores itself ("imported") or a reference to an external
/// path/URL ("linked", not deletable via the API). Every entry is always a selectable
/// colour-scheme theme (ticket 020 removed the "use as theme"/always-on-overlay distinction);
/// an entry persisted before that change with a stored `is_theme: false` still loads fine (the
/// field is simply ignored) and becomes selectable like any other entry.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "kind", rename_all = "lowercase")] #[serde(tag = "kind", rename_all = "lowercase")]
pub enum CssEntry { pub enum CssEntry {
@@ -71,25 +50,15 @@ pub enum CssEntry {
Linked { path_or_url: String }, Linked { path_or_url: String },
} }
/// Legacy on-disk shape of the (pre-049) `Profiles` pointer struct: only names/active-profile/dir
/// pointers, with the actual profile content living in a separate `profiles.json`. Kept only so an
/// old `settings.json` still parses; see `ProfilesOnDisk` and `Settings::migrate_legacy_profiles_file`.
#[derive(Debug, Clone, Deserialize, Default)] #[derive(Debug, Clone, Deserialize, Default)]
struct LegacyProfilesPointer { struct LegacyProfilesPointer {
// `names` is intentionally not captured: it was always redundant with the legacy
// `profiles.json`'s actual profile list, which is what migration reads instead.
#[serde(default)] #[serde(default)]
active_profile: Option<String>, active_profile: Option<String>,
/// Directory (relative to the settings file's directory, or absolute) the legacy `profiles.json`
/// lived under. Every version of this codebase ever wrote `"profiles"` here (no API ever let a
/// user change it), so migration doesn't bother re-deriving a custom value from this field.
#[serde(default)] #[serde(default)]
#[allow(dead_code)] #[allow(dead_code)]
profiles_dir: Option<String>, profiles_dir: Option<String>,
} }
/// On-disk shape of the `profiles` field: either the new inline `Vec<Profile>` (ticket 049) or the
/// old pointer-struct shape, so a `settings.json` written before this ticket still deserializes.
#[derive(Debug, Clone, Deserialize)] #[derive(Debug, Clone, Deserialize)]
#[serde(untagged)] #[serde(untagged)]
enum ProfilesOnDisk { enum ProfilesOnDisk {
@@ -103,9 +72,6 @@ impl Default for ProfilesOnDisk {
} }
} }
/// Raw on-disk shape of `Settings`, used only to support deserializing an old `settings.json` whose
/// `profiles` field was the pointer struct and had no top-level `active_profile` — see `Settings`'s
/// `#[serde(from = ...)]`.
#[derive(Debug, Clone, Deserialize)] #[derive(Debug, Clone, Deserialize)]
struct SettingsOnDisk { struct SettingsOnDisk {
color_scheme: ColorScheme, color_scheme: ColorScheme,
@@ -147,40 +113,25 @@ impl From<SettingsOnDisk> for Settings {
} }
} }
/// The single, process-wide settings instance. Only `PartialEq` (not `Eq`): `alert_rules` carries `f64` fields, which aren't `Eq`.
///
/// `profiles`/`active_profile` used to be a separate `Profiles` pointer struct referencing a
/// standalone `profiles.json` (see `crate::profile::ProfileStore`, removed by ticket 049); profile
/// data now lives inline here, as the single source of truth. `Deserialize` goes through
/// `SettingsOnDisk` so a `settings.json` written before this ticket still loads (see
/// `migrate_legacy_profiles_file`, which then imports the old `profiles.json`, if any, on `load`).
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
#[serde(from = "SettingsOnDisk")] #[serde(from = "SettingsOnDisk")]
pub struct Settings { pub struct Settings {
pub color_scheme: ColorScheme, pub color_scheme: ColorScheme,
pub css_entries: Vec<CssEntry>, pub css_entries: Vec<CssEntry>,
/// Id of the selected custom theme entry, if any; `None` means `color_scheme` alone determines the active palette.
#[serde(default)] #[serde(default)]
pub custom_theme_id: Option<String>, pub custom_theme_id: Option<String>,
/// Every known profile, inline (ticket 049).
pub profiles: Vec<Profile>, pub profiles: Vec<Profile>,
/// Name of the currently active profile, if any.
pub active_profile: Option<String>, pub active_profile: Option<String>,
/// Maximum number of ingested logs kept in memory; `None` means unlimited.
#[serde(default)] #[serde(default)]
pub max_logs: Option<usize>, pub max_logs: Option<usize>,
/// Alert rule definitions.
#[serde(default)] #[serde(default)]
pub alert_rules: Vec<AlertRule>, pub alert_rules: Vec<AlertRule>,
/// Ntfy topic every alert rule delivers to; `None` until configured in Settings.
#[serde(default)] #[serde(default)]
pub ntfy_topic: Option<String>, pub ntfy_topic: Option<String>,
/// Ntfy server override; `None` means `https://ntfy.sh`.
#[serde(default)] #[serde(default)]
pub ntfy_server: Option<String>, pub ntfy_server: Option<String>,
} }
/// Computes the default settings file path (`<config_dir>/settings.json`) from OS-conventional config directories; `None` if none could be determined.
pub fn default_settings_path() -> Option<PathBuf> { pub fn default_settings_path() -> Option<PathBuf> {
ProjectDirs::from( ProjectDirs::from(
crate::consts::APP_QUALIFIER, crate::consts::APP_QUALIFIER,
@@ -191,7 +142,6 @@ pub fn default_settings_path() -> Option<PathBuf> {
} }
impl Settings { impl Settings {
/// Loads settings from `path`, creating and persisting defaults if missing. Any I/O or parse error on an existing file is logged and treated as defaults, without overwriting the unreadable file. Also runs the one-time legacy `profiles.json` migration (see `migrate_legacy_profiles_file`).
pub fn load(path: &Path) -> Settings { pub fn load(path: &Path) -> Settings {
match fs::read_to_string(path) { match fs::read_to_string(path) {
Ok(contents) => match serde_json::from_str::<Settings>(&contents) { Ok(contents) => match serde_json::from_str::<Settings>(&contents) {
@@ -229,7 +179,6 @@ impl Settings {
} }
} }
/// Persists these settings to `path` as pretty-printed JSON, creating any missing parent directories first.
pub fn save(&self, path: &Path) -> io::Result<()> { pub fn save(&self, path: &Path) -> io::Result<()> {
if let Some(parent) = path.parent() { if let Some(parent) = path.parent() {
fs::create_dir_all(parent)?; fs::create_dir_all(parent)?;
@@ -239,14 +188,6 @@ impl Settings {
fs::write(path, contents) fs::write(path, contents)
} }
/// One-time migration (ticket 049): if `self.profiles` is empty (a fresh `Settings`, or one
/// parsed from a pre-049 `settings.json` whose profiles lived in a separate file) and a legacy
/// `<settings_path's parent>/profiles/profiles.json` exists, reads it in, merges it into
/// `self.profiles`, persists `self`, and — only once that save succeeds — renames the old file
/// (`.migrated` suffix) so it isn't re-imported on a later load. Every prior version of this
/// codebase always used the fixed `"profiles"` subdirectory name (no API ever let it be
/// reconfigured), so that's the only location checked. Best-effort: any failure is logged and
/// left for a future load to retry rather than panicking.
fn migrate_legacy_profiles_file(&mut self, settings_path: &Path) { fn migrate_legacy_profiles_file(&mut self, settings_path: &Path) {
if !self.profiles.is_empty() { if !self.profiles.is_empty() {
return; return;
@@ -276,12 +217,6 @@ impl Settings {
} }
} }
/// Migrates any pre-037/pre-038 metric or param shapes still present in `self.profiles` into
/// their current form (see `crate::profile::migrate_legacy_profile_shapes`), persisting `self`
/// if anything changed. Runs on every load — previously done inside the now-removed
/// `ProfileStore::load` (ticket 049 folded profile storage into `settings.json`) — so a profile
/// saved by an older version of ottyl, whether loaded inline here or just merged in by
/// `migrate_legacy_profiles_file`, still gets migrated forward.
fn migrate_legacy_profile_shapes(&mut self, settings_path: &Path) { fn migrate_legacy_profile_shapes(&mut self, settings_path: &Path) {
if crate::profile::migrate_legacy_profile_shapes(&mut self.profiles) if crate::profile::migrate_legacy_profile_shapes(&mut self.profiles)
&& let Err(err) = self.save(settings_path) && let Err(err) = self.save(settings_path)
@@ -294,14 +229,10 @@ impl Settings {
} }
} }
/// Error returned by CSS-entry mutations and lookups.
#[derive(Debug)] #[derive(Debug)]
pub enum CssError { pub enum CssError {
/// No entry with the given id exists.
NotFound, NotFound,
/// The entry is a `Linked` entry, not `Imported` — ottyl doesn't own a linked entry's content, so there's nothing to delete or serve back.
NotImported, NotImported,
/// Underlying I/O failure: writing/reading/removing the CSS file, or persisting `settings.json`.
Io(io::Error), Io(io::Error),
} }
@@ -323,7 +254,6 @@ impl From<io::Error> for CssError {
} }
} }
/// Computes the id used to reference a `CssEntry` over the API: the stored id for `Imported`, or a hash of `path_or_url` for `Linked` (which has no stored id), so the frontend can key both the same way.
pub fn css_entry_id(entry: &CssEntry) -> String { pub fn css_entry_id(entry: &CssEntry) -> String {
match entry { match entry {
CssEntry::Imported { id, .. } => id.clone(), CssEntry::Imported { id, .. } => id.clone(),
@@ -337,7 +267,6 @@ fn linked_entry_id(path_or_url: &str) -> String {
format!("linked-{:016x}", hasher.finish()) format!("linked-{:016x}", hasher.finish())
} }
/// Generates an id for a newly-imported CSS entry by hashing filename, content, and time, avoiding a `uuid` dependency for what's otherwise an opaque string.
fn generate_imported_css_id(filename: &str, content: &str) -> String { fn generate_imported_css_id(filename: &str, content: &str) -> String {
let mut hasher = DefaultHasher::new(); let mut hasher = DefaultHasher::new();
filename.hash(&mut hasher); filename.hash(&mut hasher);
@@ -355,7 +284,6 @@ fn imported_css_file_path(css_dir: &Path, id: &str) -> PathBuf {
} }
impl Settings { impl Settings {
/// Imports raw CSS `content`: writes it to `<css_dir>/<id>.css`, records a `CssEntry::Imported` in `css_entries`, and persists settings. Returns the generated id. `css_dir` is expected to be the settings file's parent directory.
pub fn import_css( pub fn import_css(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -374,14 +302,12 @@ impl Settings {
Ok(id) Ok(id)
} }
/// Registers a reference to an external CSS path/URL without copying its content, and persists settings.
pub fn link_css(&mut self, settings_path: &Path, path_or_url: String) -> Result<(), CssError> { pub fn link_css(&mut self, settings_path: &Path, path_or_url: String) -> Result<(), CssError> {
self.css_entries.push(CssEntry::Linked { path_or_url }); self.css_entries.push(CssEntry::Linked { path_or_url });
self.save(settings_path)?; self.save(settings_path)?;
Ok(()) Ok(())
} }
/// Removes an imported CSS entry identified by `id`: deletes its file under `css_dir`, drops it from `css_entries`, and persists settings. Fails with `NotFound` or `NotImported` (linked entries aren't deletable this way).
pub fn remove_css( pub fn remove_css(
&mut self, &mut self,
settings_path: &Path, settings_path: &Path,
@@ -398,7 +324,6 @@ impl Settings {
Some(CssEntry::Imported { .. }) => { Some(CssEntry::Imported { .. }) => {
let idx = index.unwrap(); let idx = index.unwrap();
self.css_entries.remove(idx); self.css_entries.remove(idx);
// Best-effort: the settings entry is already removed even if the file is gone.
let _ = fs::remove_file(imported_css_file_path(css_dir, id)); let _ = fs::remove_file(imported_css_file_path(css_dir, id));
self.save(settings_path)?; self.save(settings_path)?;
Ok(()) Ok(())
@@ -406,7 +331,6 @@ impl Settings {
} }
} }
/// Reads back the raw content of an imported CSS entry identified by `id`. Fails with `NotFound` or `NotImported` (linked entries have no local content — the frontend fetches `path_or_url` directly).
pub fn css_content(&self, css_dir: &Path, id: &str) -> Result<String, CssError> { pub fn css_content(&self, css_dir: &Path, id: &str) -> Result<String, CssError> {
match self.css_entries.iter().find(|entry| css_entry_id(entry) == id) { match self.css_entries.iter().find(|entry| css_entry_id(entry) == id) {
None => Err(CssError::NotFound), None => Err(CssError::NotFound),
@@ -424,7 +348,6 @@ mod tests {
#[test] #[test]
fn default_settings_path_ends_with_settings_json() { fn default_settings_path_ends_with_settings_json() {
// Can't assert the exact OS path without touching real config dirs, so just check the shape.
if let Some(path) = default_settings_path() { if let Some(path) = default_settings_path() {
assert_eq!(path.file_name().unwrap(), "settings.json"); assert_eq!(path.file_name().unwrap(), "settings.json");
assert!(path.parent().is_some()); assert!(path.parent().is_some());
@@ -471,10 +394,6 @@ mod tests {
assert_eq!(settings.color_scheme, ColorScheme::Dark); assert_eq!(settings.color_scheme, ColorScheme::Dark);
} }
/// A `settings.json` written before ticket 049 (nested `Profiles` pointer struct, no top-level
/// `active_profile`) still parses: the pointer's `active_profile` carries over, and `profiles`
/// starts empty (the real content lived in a separate `profiles.json`, migrated separately by
/// `Settings::load`, not by plain deserialization).
#[test] #[test]
fn legacy_profiles_pointer_shape_deserializes_with_empty_inline_profiles() { fn legacy_profiles_pointer_shape_deserializes_with_empty_inline_profiles() {
let settings: Settings = serde_json::from_str( let settings: Settings = serde_json::from_str(
@@ -548,7 +467,6 @@ mod tests {
assert!(css_file.exists()); assert!(css_file.exists());
assert_eq!(fs::read_to_string(&css_file).unwrap(), "body { color: red; }"); assert_eq!(fs::read_to_string(&css_file).unwrap(), "body { color: red; }");
// Drop and reload settings from disk (simulates a restart).
drop(settings); drop(settings);
let reloaded = Settings::load(&settings_path); let reloaded = Settings::load(&settings_path);
assert!(reloaded.css_entries.iter().any(|entry| matches!( assert!(reloaded.css_entries.iter().any(|entry| matches!(
@@ -598,7 +516,6 @@ mod tests {
let result = settings.remove_css(&settings_path, &css_dir, &linked_id); let result = settings.remove_css(&settings_path, &css_dir, &linked_id);
assert!(matches!(result, Err(CssError::NotImported))); assert!(matches!(result, Err(CssError::NotImported)));
// Entry must still be there — rejection shouldn't silently drop it.
assert_eq!(settings.css_entries.len(), 1); assert_eq!(settings.css_entries.len(), 1);
} }
@@ -629,9 +546,6 @@ mod tests {
assert!(matches!(result, Err(CssError::NotImported))); assert!(matches!(result, Err(CssError::NotImported)));
} }
/// Acceptance criterion (ticket 049): a pre-existing legacy `profiles_dir/profiles.json` is
/// merged into `settings.json` on the next load, and the old file is renamed (not re-imported on
/// a subsequent load).
#[test] #[test]
fn load_migrates_legacy_profiles_file_into_inline_settings() { fn load_migrates_legacy_profiles_file_into_inline_settings() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
@@ -644,8 +558,6 @@ mod tests {
r#"[{"name":"web","params":[],"metrics":[],"dashboards":[]}]"#, r#"[{"name":"web","params":[],"metrics":[],"dashboards":[]}]"#,
) )
.unwrap(); .unwrap();
// No settings.json yet: simulates upgrading a pre-049 install that never wrote one either,
// or one that did but whose nested pointer already carried an active_profile name.
fs::write( fs::write(
&settings_path, &settings_path,
r#"{"color_scheme":"system","css_entries":[],"custom_theme_id":null,"profiles":{"names":["web"],"active_profile":"web","profiles_dir":"profiles"},"max_logs":null,"alert_rules":[],"ntfy_topic":null,"ntfy_server":null}"#, r#"{"color_scheme":"system","css_entries":[],"custom_theme_id":null,"profiles":{"names":["web"],"active_profile":"web","profiles_dir":"profiles"},"max_logs":null,"alert_rules":[],"ntfy_topic":null,"ntfy_server":null}"#,
@@ -657,19 +569,15 @@ mod tests {
assert_eq!(settings.profiles[0].name, "web"); assert_eq!(settings.profiles[0].name, "web");
assert_eq!(settings.active_profile, Some("web".to_string())); assert_eq!(settings.active_profile, Some("web".to_string()));
// The legacy file is gone (renamed), so it isn't re-imported.
assert!(!legacy_path.exists()); assert!(!legacy_path.exists());
let migrated_path = legacy_path.with_extension("json.migrated"); let migrated_path = legacy_path.with_extension("json.migrated");
assert!(migrated_path.exists()); assert!(migrated_path.exists());
// Reload from disk: the migrated settings.json now carries profiles inline directly.
let reloaded = Settings::load(&settings_path); let reloaded = Settings::load(&settings_path);
assert_eq!(reloaded.profiles.len(), 1); assert_eq!(reloaded.profiles.len(), 1);
assert_eq!(reloaded.profiles[0].name, "web"); assert_eq!(reloaded.profiles[0].name, "web");
} }
/// A fresh install (no legacy file at all) never creates `profiles.json` — it's not even looked
/// for beyond the one migration check, and `profiles` simply stays empty.
#[test] #[test]
fn load_without_legacy_file_leaves_profiles_empty() { fn load_without_legacy_file_leaves_profiles_empty() {
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
src/timestamp.rs
-1
View File
@@ -1,7 +1,6 @@
use serde::Serialize; use serde::Serialize;
use std::time::{SystemTime, UNIX_EPOCH}; use std::time::{SystemTime, UNIX_EPOCH};
/// Wall-clock instant stored as nanoseconds since the Unix epoch (not `std::time::Instant`, which has no epoch and can't be serialized).
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize)] #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize)]
pub struct Timestamp(i64); pub struct Timestamp(i64);