From ac1efd9e53e186cf54a2ec0e4908ce6183b7bfc7 Mon Sep 17 00:00:00 2001 From: Hugh Kaznowski Date: Sun, 6 Sep 2026 01:31:32 +0100 Subject: [PATCH] Update wishlist and remove done ticets --- Wishlist.md | 10 ++++ tickets/037-unify-param-declarations.md | 54 ------------------- tickets/038-duration-as-derived-param-kind.md | 44 --------------- ...ic-param-query-resolves-declared-params.md | 32 ----------- .../040-unify-rate-window-implementation.md | 49 ----------------- tickets/041-fix-string-metric-coercion.md | 32 ----------- tickets/042-fix-sum-avg-timestamp-merge.md | 38 ------------- tickets/043-histogram-by-labels-support.md | 32 ----------- 8 files changed, 10 insertions(+), 281 deletions(-) create mode 100644 Wishlist.md delete mode 100644 tickets/037-unify-param-declarations.md delete mode 100644 tickets/038-duration-as-derived-param-kind.md delete mode 100644 tickets/039-metric-param-query-resolves-declared-params.md delete mode 100644 tickets/040-unify-rate-window-implementation.md delete mode 100644 tickets/041-fix-string-metric-coercion.md delete mode 100644 tickets/042-fix-sum-avg-timestamp-merge.md delete mode 100644 tickets/043-histogram-by-labels-support.md diff --git a/Wishlist.md b/Wishlist.md new file mode 100644 index 0000000..39bc720 --- /dev/null +++ b/Wishlist.md @@ -0,0 +1,10 @@ +There is another reserved param, `ottyl_fmt`. It is a text enum of the format that the log line was derived with. The other params - sequence and timestamp should be renamed `ottyl_seq` and `ottyl_ts`. + +Timeline by default is ordered by `ottyl_ts`. It should be possible to order it by other params. Logs that don't have the param either are ordered by `ottyl_seq` or dropped from view. A toggle should control that. + +Bar charts that have too many bars, get their bars squashed. Legends for all visualisations that are too big also get simplified. + +There is no difference between "colour scheme" and "css". The presets are literally entire stlye makeovers. It just so happens coincidentally that it's basically the same style. There should be nothing stopping people from adding their own style that not only changes colours, but also completely changes layout etc. The UI in settings needs to reflect that, and also the impl. + +The existing colour schemes have the issue that they are basically monotone with a single accent. Give 2 or 3 accents. Also create brighter variations. For example "Green" right now should stay but with added accents, but a new "Light Green" should also be added that is brighter. The default "Light" and "Dark" themes should also have additional accents that are suitable. + diff --git a/tickets/037-unify-param-declarations.md b/tickets/037-unify-param-declarations.md deleted file mode 100644 index 2d4b997..0000000 --- a/tickets/037-unify-param-declarations.md +++ /dev/null @@ -1,54 +0,0 @@ -# 037 — Unify param declarations: one `Param` type, referenced by name from metrics - -## Summary - -"Parameter" currently names two structurally near-identical but separate -types: `ParamDeclaration` (`src/data_value.rs`, driven by the Params panel, -used only to add a display column to the logs table) and -`RegexToMetricPrimitiveMatcher` (`src/dto.rs`), embedded inline inside a -metric declaration. A metric cannot reference an already-declared param by -name — `src/engine.rs` says so explicitly ("referencing an already-declared -`ParamDeclaration` by name isn't supported yet") — so the same -regex/capture-group/kind gets redefined from scratch per metric, and nothing -keeps a param declared in the Params panel consistent with a same-named -matcher inside a metric. - -## Scope - -- Collapse `ParamDeclaration` and `RegexToMetricPrimitiveMatcher` into one - `ParamDeclaration` type: `{ name, extractor (regex+capture_group | - json_field_path), kind }`. -- Change `MetricDeclaration` to reference params by name instead of embedding - its own extractor: `label_params: Vec`, `value_param: - Option`. Role (label vs. value) becomes a property of how a metric - uses a param, not a property of the param itself. -- Update `apply_metric_to_log` (`src/engine.rs`) to resolve `label_params`/ - `value_param` against the shared param declaration store instead of reading - an inline matcher. -- Update declare/validate paths (`validate_metric_request`, `engine.rs`) to - reject a metric referencing an undeclared param name. -- Update frontend types (`webapp/src/lib/api.ts`) and `Metrics.tsx`'s - declaration form to pick existing declared params (e.g. a select populated - from the Params panel) rather than re-entering a regex/capture-group/kind - per metric. -- Migration: existing stored `MetricDeclaration` records with inline matchers - need their matchers turned into standalone `ParamDeclaration` entries - (created implicitly, named e.g. `.` if no matching - declared param exists) so existing profiles keep working after upgrade. - -## Acceptance criteria - -- A param declared once in the Params panel can be selected as a label or - value param by multiple metric declarations without re-entering its - extractor. -- Declaring a metric that references a nonexistent param name is rejected at - declare-time with a clear error. -- Existing profiles with metrics declared under the old inline-matcher shape - load and behave identically after migration. -- `Metrics.tsx` no longer has a separate "regex + capture group + kind" entry - form duplicated from the Params panel's form. - -## Depends on - -None. [038](038-duration-as-derived-param-kind.md) and -[039](039-metric-param-query-resolves-declared-params.md) build on this. diff --git a/tickets/038-duration-as-derived-param-kind.md b/tickets/038-duration-as-derived-param-kind.md deleted file mode 100644 index 5fdcdc8..0000000 --- a/tickets/038-duration-as-derived-param-kind.md +++ /dev/null @@ -1,44 +0,0 @@ -# 038 — Make Duration an explicit derived-param kind, not a hidden case of Value - -## Summary - -Today a `Value`-role param with `kind: Duration` silently means something -different from every other kind: instead of parsing extracted text, it means -"elapsed time since the previous same-labeled event" -(`apply_metric_to_log`, `src/engine.rs`, backed by a `LastEventTimestamp` -index entry). Same enum variant (`MetricPrimitiveKind::Duration`), two -unrelated meanings depending on role — one is "parse this text as a -duration," which doesn't apply here at all; the other is "compute elapsed -time from event sequencing." This is a silent special case a reader has to -already know about to understand the code. - -## Scope - -- Add an explicit extractor variant, e.g. `Extractor::ElapsedSinceLast { - grouped_by: Vec }`, distinct from `Regex`/`JsonPath`. -- A param declared with this extractor is computed (elapsed time since the - previous event sharing the given label set), never text-extracted. - `MetricPrimitiveKind` no longer needs a role-dependent special case for - `Duration`. -- Update `apply_metric_to_log` to branch on extractor kind instead of on - `(role, kind)` combination. -- Update the Params panel / metric declaration UI (`Params.tsx`, - `Metrics.tsx`) to offer "elapsed since previous event" as its own - extractor option rather than a regex+kind combination that happens to mean - something else. -- Migration: existing declared/embedded params using `Value` role + - `Duration` kind convert to the new extractor variant. - -## Acceptance criteria - -- A "duration" param's behavior (elapsed time since last matching event) is - selectable directly in the UI without going through a regex field that is - never actually used. -- `apply_metric_to_log` has no branch that changes meaning based on role for - a given kind. -- Existing profiles using the old Value+Duration combination behave - identically after migration. - -## Depends on - -[037](037-unify-param-declarations.md). diff --git a/tickets/039-metric-param-query-resolves-declared-params.md b/tickets/039-metric-param-query-resolves-declared-params.md deleted file mode 100644 index 4c69dbe..0000000 --- a/tickets/039-metric-param-query-resolves-declared-params.md +++ /dev/null @@ -1,32 +0,0 @@ -# 039 — `metric[param]` query syntax should resolve against declared params - -## Summary - -`compile_metric_comparison` (`src/engine.rs`) resolves the `param` in -`metric[param] value` by reading whatever key happens to be present in a -bucket's `labels` map at query time. There's no link back to a metric's -actual declared label params, so a typo or a param that was never a label on -that metric fails silently (empty match) instead of being rejected up front. - -## Scope - -- Once [037](037-unify-param-declarations.md) lands, validate `metric[param]` - filter expressions against the target metric's declared `label_params` at - parse/compile time. -- Return a clear filter-validation error when `param` is not one of the - metric's label params, instead of silently matching nothing. -- Confirm `src/filter_parser.rs` and `compile_metric_comparison` share the - same notion of "param" as the Params panel and metric declarations after - this change (no separate resolution path). - -## Acceptance criteria - -- `metric[nonexistent_param] == "x"` is rejected with a validation error - referencing the metric's actual label params, rather than silently - returning no matches. -- Existing valid filter queries using real label params continue to work - unchanged. - -## Depends on - -[037](037-unify-param-declarations.md). diff --git a/tickets/040-unify-rate-window-implementation.md b/tickets/040-unify-rate-window-implementation.md deleted file mode 100644 index 35cdab2..0000000 --- a/tickets/040-unify-rate-window-implementation.md +++ /dev/null @@ -1,49 +0,0 @@ -# 040 — One rate/window implementation, shared by PromQL and alerts - -## Summary - -"Rate over a window" is implemented twice with different semantics and no -shared code: - -- PromQL's `rate()` (`windowed_rate_series`, `src/engine.rs`) scans the - trailing window at every historical point over indexed bucket data - (occurrences-per-second, backfilled). -- Alerts' `Threshold`/`Ratio` conditions (`src/alerts.rs`) maintain an - independent live sliding `VecDeque` window over raw log-line matches - (current count only, resets on process restart, no backfill), deliberately - bypassing the bucket/PromQL pipeline for cost reasons (module doc: a - `metric[param]` comparison would require a full bucket rescan per line). - -These can and will disagree for equivalent-looking rate/ratio rules, since -they read different data and compute the window differently. - -## Scope - -- Extract a single windowed-rate function, e.g. in a new `src/rate.rs`: - `fn rate_at(points: &[(Timestamp, f64)], now: Timestamp, window_ms: i64) -> f64`. -- Have PromQL's `rate()` evaluation call this shared function (behavior - should be unchanged for existing dashboard queries). -- Have alerts call the same function against the bucket store instead of - maintaining a separate `VecDeque` over raw matches. If alerts' proportional - per-line cost guarantee can't be met by reading buckets directly, solve - that inside the shared module (e.g. an incrementally maintained window - index) rather than keeping two implementations. -- As a consequence, alert rate/ratio conditions gain backfilled history - (they no longer reset to empty on process restart) — call this out in - release notes since it changes alert-firing behavior on restart. - -## Acceptance criteria - -- Only one windowed-rate implementation exists in the codebase. -- A dashboard `rate()` chart and an alert `Threshold`/`Ratio` condition over - the same metric/window agree on the computed rate for the same historical - data, verified with a seeded fixture. -- Alert conditions no longer silently reset their window on process restart - (or this is an explicitly flagged, deliberate behavior change). -- Existing dashboard `rate()` queries produce unchanged output for existing - test fixtures. - -## Depends on - -None — independent of the param unification work -([037](037-unify-param-declarations.md)–[039](039-metric-param-query-resolves-declared-params.md)). diff --git a/tickets/041-fix-string-metric-coercion.md b/tickets/041-fix-string-metric-coercion.md deleted file mode 100644 index c13d07f..0000000 --- a/tickets/041-fix-string-metric-coercion.md +++ /dev/null @@ -1,32 +0,0 @@ -# 041 — Stop silently coercing string metric values to 1.0 in sum()/avg() - -## Summary - -`coerce_to_f64` (`src/engine.rs`) maps every `MetricPrimitive::String` value -to a flat `1.0`. This means `sum()`/`avg()` on a string-valued metric -silently becomes an occurrence count instead of erroring or behaving -predictably, with no signal to the user that their aggregate doesn't mean -what it looks like it means. - -## Scope - -- At PromQL query-evaluation time, detect when the target metric's declared - value kind is `String` and the requested aggregation is `sum`/`avg`. -- Either reject the query with a clear error ("cannot sum/avg a string-typed - metric; use count() or a numeric-typed metric"), or introduce an explicit - `count()` aggregation so occurrence-counting is something a user opts into - by name rather than something `sum()` does silently for the wrong metric - kind. -- Audit existing dashboards/panels for `sum()`/`avg()` over string-kind - metrics that would now be rejected or need updating to `count()`. - -## Acceptance criteria - -- Querying `sum()`/`avg()` over a string-kind metric either fails with a - clear, actionable error or is redirected to an explicit `count()` semantic - — it no longer silently returns a count while looking like a sum. -- Numeric-kind metrics are unaffected. - -## Depends on - -None. diff --git a/tickets/042-fix-sum-avg-timestamp-merge.md b/tickets/042-fix-sum-avg-timestamp-merge.md deleted file mode 100644 index 5dae257..0000000 --- a/tickets/042-fix-sum-avg-timestamp-merge.md +++ /dev/null @@ -1,38 +0,0 @@ -# 042 — Verify/fix exact-timestamp merging for non-rate sum()/avg() - -## Summary - -Non-rate `sum()`/`avg()` queries merge per-label series via -`merge_series_exact` (`src/engine.rs`), which only combines values whose -timestamps match *exactly*. In practice, raw occurrence timestamps across -different label groups essentially never land on the same millisecond, so -`sum(requests) by (path)` over raw occurrence counts likely produces sparse, -near-empty merged series rather than a meaningful "total requests over -time," even though that's the obvious expectation for such a query. The -existing code comment justifying exact-match merging ("nothing to carry -forward") only holds if the query isn't meant to sum sparse counts over -time — which is exactly the common case for occurrence-count dashboards. - -## Scope - -- Build a seeded fixture with multiple label groups whose events arrive at - different (non-coincident) timestamps and confirm whether - `sum(...) by (...)` currently produces the expected combined total or the - suspected sparse/near-empty result. -- If confirmed broken: bucket points into fixed time windows (the same way - `rate()` already windows) before summing/averaging across label groups, - instead of requiring exact timestamp coincidence. -- Add a regression test covering multi-label-group `sum()`/`avg()` over - time. - -## Acceptance criteria - -- `sum(metric) by (label)` over multiple label groups with staggered event - timestamps produces a sensible combined time series (not sparse/empty), - verified by a new test fixture. -- `avg()` behaves analogously. -- Existing single-label-group queries are unaffected. - -## Depends on - -None. diff --git a/tickets/043-histogram-by-labels-support.md b/tickets/043-histogram-by-labels-support.md deleted file mode 100644 index de25270..0000000 --- a/tickets/043-histogram-by-labels-support.md +++ /dev/null @@ -1,32 +0,0 @@ -# 043 — Histogram queries ignore by() grouping, inconsistent with sum/avg/rate - -## Summary - -PromQL evaluation (`evaluate_promql`, `src/engine.rs`) has inconsistent -grouping semantics across aggregation ops: `sum`/`avg`/`rate` respect the -query's `by_labels` and group series accordingly, but `Histogram` collapses -all label combinations into a single histogram regardless of `by()`. The -same query language should not silently vary in what `by()` means depending -on the aggregation used. - -## Scope - -- Decide the intended behavior: either (a) support `by()` grouping for - histograms, producing one histogram per label group, or (b) explicitly - reject/ignore `by()` on histogram queries with a clear validation error - rather than silently accepting and discarding it. -- Implement the chosen behavior in `evaluate_promql`'s histogram branch. -- Update any dashboard panel UI (`Dashboards.tsx`) that lets a user combine - histogram with `by()` to reflect the supported behavior (either offering - grouped histograms or disabling `by()` for histogram panels). - -## Acceptance criteria - -- `histogram(metric) by (label)` either produces one histogram per label - group (verified by a seeded fixture) or is rejected/flagged clearly at - query time — it no longer silently accepts and ignores `by()`. -- `sum`/`avg`/`rate` behavior with `by()` is unchanged. - -## Depends on - -None.