Add tickets to improve metric abstractions

tickets/037-unify-param-declarations.md

@@ -0,0 +1,54 @@
+# 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<String>`, `value_param:
+  Option<String>`. 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. `<metric_name>.<param_name>` 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.

tickets/038-duration-as-derived-param-kind.md

@@ -0,0 +1,44 @@
+# 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<String> }`, 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).

tickets/039-metric-param-query-resolves-declared-params.md

@@ -0,0 +1,32 @@
+# 039 — `metric[param]` query syntax should resolve against declared params
+
+## Summary
+
+`compile_metric_comparison` (`src/engine.rs`) resolves the `param` in
+`metric[param] <op> 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).

tickets/040-unify-rate-window-implementation.md

@@ -0,0 +1,49 @@
+# 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)).

tickets/041-fix-string-metric-coercion.md

@@ -0,0 +1,32 @@
+# 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.

tickets/042-fix-sum-avg-timestamp-merge.md

@@ -0,0 +1,38 @@
+# 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.

tickets/043-histogram-by-labels-support.md

@@ -0,0 +1,32 @@
+# 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.