Notification history: range counts do not respect query `limit` when not grouped by rule
- Dominant language
- Go
- Stars
- 89
- Forks
- 74
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 8
Description
# Notification history: range counts do not respect query `limit` when not grouped by rule
## Summary
In the notification history Loki reader, the **range counts** path does not truncate results to the requested `limit` for any grouping other than `RuleUID` (including the no-grouping case). The **instant counts** path does apply a client-side `topk`, so the two paths behave inconsistently and the range path can return an unbounded number of series.
Found during review of #616 (RBAC for notification history). Not RBAC-related: accessible-rule filtering still applies via the LogQL matcher, so this is a correctness/contract issue, not a data-leak.
## Details
`apps/historian/pkg/app/notification/lokireader.go`
- Instant path: `buildMetricsQuery` wraps the inner expression in `topk(limit, ...)` whenever grouping is not by `RuleUID`:
```go
// buildMetricsQuery (~L234)
if groupBy.RuleUID {
return inner
}
return fmt.Sprintf(`topk(%d, %s)`, limit, inner)
```
- Range path: `runMetricsRangeQuery` (~L389) calls `buildMetricsRangeQuery` directly, which never emits `topk`. Client-side truncation only happens inside `explodeRuleUIDRangeCounts`, which runs **only** when `groupBy.RuleUID` is true:
```go
// runMetricsRangeQuery (~L412)
if groupBy.RuleUID {
rangeCounts = explodeRuleUIDRangeCounts(rangeCounts, limit, filter)
}
return rangeCounts, nil // no limit applied for any other grouping
```
For groupings such as receiver, integration, status, outcome, error, or no grouping at all, the range path returns every series Loki produced, with no cap. The `limit` argument passed to `MetricsRangeQuery` is Loki's log-line page size, not a series-count cap on a metrics matrix, so it does not bound the result here.
## Impact
- Range counts can return far more series than the caller's `limit`.
- Inconsistent behavior between instant and range counts breaks the implicit contract that both honor `limit`.
- Payload-size / performance concern for wide groupings. No authorization impact.
## Suggested fix
Apply a client-side top-N truncation to the merged range counts for the non-`RuleUID` grouping case, mirroring what the instant path gets from `topk` and what `explodeRuleUIDRangeCounts` already does for the rule-UID case.
## Acceptance criteria
- Range counts respect `limit` for all groupings, not just `RuleUID`.
- Regression test asserting a range-count query with a small `limit` and multiple series returns at most `limit` series.
## Severity
Low-to-Medium. Correctness/contract inconsistency, no RBAC/authorization or data-leak impact.
Contributor guide
Research direction
Start in apps/historian/pkg/app/notification/lokireader.go, comparing buildMetricsQuery with runMetricsRangeQuery and buildMetricsRangeQuery. Add a regression test for a small limit with multiple non-RuleUID series, and confirm range counts return no more than the requested limit for every grouping.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, observability-sre
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100