grafana / grafana/alerting

Notification history: range counts do not respect query `limit` when not grouped by rule

Open Beginner friendly
#618 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.