diff --git a/CHANGELOG.md b/CHANGELOG.md index 95fd37e173..1c089000b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ * [ENHANCEMENT] Upgrade Thanos and promql-engine to latest. #7740 * [ENHANCEMENT] Ruler: Adjust ruler frontend decoder to not wrap query error messages with execution prefix, this makes error responses consistent between internal and external ruler paths. #7741 * [ENHANCEMENT] Distributor: Deduplicate metric metadata when converting PRW 2.0 requests. PRW 2.0 attaches metadata to every series, so a metric family was previously expanded into one `MetricMetadata` per series. #7760 +* [ENHANCEMENT] Ruler: Add new limit `-ruler.list-rules-max-rules` on the total number of rules returned by the Prometheus ListRules API. Responses exceeding the limit are truncated on a rule group boundary and return a `groupNextToken` for retrieving the remaining groups. A rule group is never split, so a single group larger than the limit is still returned whole. Defaults to 0, which is unlimited. #7785 * [BUGFIX] Querier: Fix queryWithRetry and labelsWithRetry returning (nil, nil) on cancelled context by propagating ctx.Err(). #7370 * [BUGFIX] Metrics Helper: Fix non-deterministic bucket order in merged histograms by sorting buckets after map iteration, matching Prometheus client library behavior. #7380 * [BUGFIX] Distributor: Return HTTP 401 Unauthorized when tenant ID resolution fails in the Prometheus Remote Write 2.0 path. #7389 diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index 2b11f47f82..2b3a1fbc84 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -6242,6 +6242,14 @@ ring: # CLI flag: -ruler.disabled-tenants [disabled_tenants: | default = ""] +# Maximum number of rules returned by the Prometheus ListRules API. If there are +# more rulegroups, the response will include a pagination token which can be +# used to fetch the next set. The API will always return at least one rulegroup, +# even if it contains more rules than the limit. Defaults to 0, which is +# unlimited +# CLI flag: -ruler.list-rules-max-rules +[list_rules_max_rules: | default = 0] + # Report query statistics for ruler queries to complete as a per user metric and # as an info level log message. # CLI flag: -ruler.query-stats-enabled diff --git a/pkg/ruler/api.go b/pkg/ruler/api.go index 177782e607..1f8b450261 100644 --- a/pkg/ruler/api.go +++ b/pkg/ruler/api.go @@ -186,7 +186,7 @@ func (a *API) PrometheusRules(w http.ResponseWriter, req *http.Request) { } w.Header().Set("Content-Type", "application/json") - response, err := a.ruler.GetRules(req.Context(), rulesRequest) + response, err := a.ruler.GetRules(req.Context(), rulesRequest, a.ruler.cfg.ListRulesMaxRules) if err != nil { util_api.RespondError(logger, w, v1.ErrServer, err.Error(), http.StatusInternalServerError) @@ -346,7 +346,7 @@ func (a *API) PrometheusAlerts(w http.ResponseWriter, req *http.Request) { Type: alertingRuleFilter, MaxRuleGroups: -1, } - rulesResponse, err := a.ruler.GetRules(req.Context(), rulesRequest) + rulesResponse, err := a.ruler.GetRules(req.Context(), rulesRequest, 0) if err != nil { util_api.RespondError(logger, w, v1.ErrServer, err.Error(), http.StatusInternalServerError) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index f55b1e0e31..c8b9635fbf 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -14,6 +14,7 @@ import ( "github.com/go-kit/log" "github.com/gorilla/mux" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/stretchr/testify/require" "github.com/weaveworks/common/user" @@ -426,6 +427,135 @@ func TestRuler_rules_limit(t *testing.T) { require.JSONEq(t, string(expectedResponse), string(actual)) } +func TestRuler_rules_max_rules(t *testing.T) { + // mockRulesNamespaces gives user1 two groups of two rules each: namespace2/fail + // sorts before namespace1/group1 in rule group token order, so it is the group + // that survives truncation. Filters and pagination vary how many rules a single + // request returns. + for _, tc := range []struct { + name string + rules map[string]rulespb.RuleGroupList + userID string + maxRules uint + queryParams string + expectedGroups int + expectedRules int + expectedNextToken string + }{ + { + name: "limit disabled returns every rule", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 0, + expectedGroups: 2, + expectedRules: 4, + }, + { + name: "count below the limit", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 10, + expectedGroups: 2, + expectedRules: 4, + }, + { + name: "count exactly at the limit", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 4, + expectedGroups: 2, + expectedRules: 4, + }, + { + name: "count over the limit is truncated, summed across groups", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 3, + expectedGroups: 1, + expectedRules: 2, + expectedNextToken: GetRuleGroupNextToken("namespace2", "fail"), + }, + { + name: "only the rules matching the filters are counted", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 3, + queryParams: "?type=alert", + expectedGroups: 2, + expectedRules: 2, + }, + { + name: "paginating below the limit succeeds", + rules: mockRulesNamespaces, + userID: "user1", + maxRules: 3, + queryParams: "?group_limit=1", + expectedGroups: 1, + expectedRules: 2, + expectedNextToken: GetRuleGroupNextToken("namespace2", "fail"), + }, + { + // A rule group is indivisible, so the response is allowed to exceed the + // limit rather than return an unusable empty page. It is the only group, so + // there is nothing left to page to and no token is returned. + name: "a lone rule group over the limit is returned whole", + rules: mockRules, + userID: "user1", + maxRules: 1, + expectedGroups: 1, + expectedRules: 2, + }, + { + name: "limit applies per request: tenant under the limit", + rules: mockRules, + userID: "user2", + maxRules: 1, + expectedGroups: 1, + expectedRules: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := defaultRulerConfig(t) + cfg.ListRulesMaxRules = tc.maxRules + + r := newTestRuler(t, cfg, newMockRuleStore(tc.rules, nil), nil) + defer services.StopAndAwaitTerminated(context.Background(), r) //nolint:errcheck + + a := NewAPI(r, r.store, log.NewNopLogger()) + + req := requestFor(t, http.MethodGet, "https://localhost:8080/api/prom/api/v1/rules"+tc.queryParams, nil, tc.userID) + w := httptest.NewRecorder() + a.PrometheusRules(w, req) + + resp := w.Result() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + + // util_api.Response holds Data as an any, so decode into a shape that + // exposes the rule groups directly. + parsed := struct { + Status string `json:"status"` + Data RuleDiscovery `json:"data"` + ErrorType v1.ErrorType `json:"errorType"` + Error string `json:"error"` + }{} + require.NoError(t, json.Unmarshal(body, &parsed)) + + require.Equal(t, http.StatusOK, resp.StatusCode) + require.Equal(t, "success", parsed.Status) + require.Empty(t, parsed.Error) + require.Len(t, parsed.Data.RuleGroups, tc.expectedGroups) + + rulesCount := 0 + for _, g := range parsed.Data.RuleGroups { + rulesCount += len(g.Rules) + } + require.Equal(t, tc.expectedRules, rulesCount) + require.Equal(t, tc.expectedNextToken, parsed.Data.GroupNextToken) + }) + } +} + func TestRuler_alerts(t *testing.T) { store := newMockRuleStore(mockRules, nil) cfg := defaultRulerConfig(t) diff --git a/pkg/ruler/merger.go b/pkg/ruler/merger.go index 1a14e67c4c..59c9984903 100644 --- a/pkg/ruler/merger.go +++ b/pkg/ruler/merger.go @@ -10,8 +10,8 @@ import ( // mergeGroupStateDesc removes duplicates from the provided []*GroupStateDesc by keeping the GroupStateDesc with the // latest information. It uses the EvaluationTimestamp of the GroupStateDesc and the EvaluationTimestamp of the // ActiveRules in a GroupStateDesc to determine the which GroupStateDesc has the latest information. -// It also truncates rule groups if maxRuleGroups > 0 -func mergeGroupStateDesc(ruleResponses []*RulesResponse, maxRuleGroups int32, dedup bool) *RulesResponse { +// It also truncates rule groups if maxRuleGroups > 0 or maxRules > 0 +func mergeGroupStateDesc(ruleResponses []*RulesResponse, maxRuleGroups int32, maxRules uint, dedup bool) *RulesResponse { var groupsStateDescs []*GroupStateDesc @@ -44,10 +44,10 @@ func mergeGroupStateDesc(ruleResponses []*RulesResponse, maxRuleGroups int32, de groups = groupsStateDescs } - if maxRuleGroups > 0 { + if maxRuleGroups > 0 || maxRules > 0 { //Need to sort here before we truncate sort.Sort(PaginatedGroupStates(groups)) - result, nextToken := generatePage(groups, int(maxRuleGroups)) + result, nextToken := generatePage(groups, int(maxRuleGroups), maxRules) return &RulesResponse{ Groups: result, NextToken: nextToken, diff --git a/pkg/ruler/merger_test.go b/pkg/ruler/merger_test.go index bc002b112b..96c0dfee32 100644 --- a/pkg/ruler/merger_test.go +++ b/pkg/ruler/merger_test.go @@ -72,6 +72,7 @@ func TestMergeGroupStateDesc(t *testing.T) { input []*RulesResponse expectedOutput *RulesResponse maxRuleGroups int32 + maxRules uint } testCases := map[string]testCase{ @@ -189,12 +190,52 @@ func TestMergeGroupStateDesc(t *testing.T) { }, maxRuleGroups: 1, }, + // gs1 (ns1/g1) sorts before gs2 (ns1/g2) in token order, and both hold two + // active rules, so a maxRules of 2 keeps gs1 and pages at gs1. + "maxRules truncates the merged page without maxRuleGroups": { + input: []*RulesResponse{ + { + Groups: []*GroupStateDesc{&gs1, &gs2}, + NextToken: "", + }, + }, + expectedOutput: &RulesResponse{ + Groups: []*GroupStateDesc{&gs1}, + NextToken: GetRuleGroupNextToken(gs1.Group.Namespace, gs1.Group.Name), + }, + maxRules: 2, + }, + "maxRules leaves the merged page intact when the total fits": { + input: []*RulesResponse{ + { + Groups: []*GroupStateDesc{&gs1, &gs2}, + NextToken: "", + }, + }, + expectedOutput: &RulesResponse{ + Groups: []*GroupStateDesc{&gs1, &gs2}, + NextToken: "", + }, + maxRules: 4, + }, + "both limits disabled returns everything untruncated": { + input: []*RulesResponse{ + { + Groups: []*GroupStateDesc{&gs1, &gs2}, + NextToken: "", + }, + }, + expectedOutput: &RulesResponse{ + Groups: []*GroupStateDesc{&gs1, &gs2}, + NextToken: "", + }, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { - out := mergeGroupStateDesc(tc.input, tc.maxRuleGroups, true) + out := mergeGroupStateDesc(tc.input, tc.maxRuleGroups, tc.maxRules, true) slices.SortFunc(out.Groups, func(a, b *GroupStateDesc) int { fileCompare := strings.Compare(a.Group.Namespace, b.Group.Namespace) if fileCompare != 0 { @@ -202,7 +243,7 @@ func TestMergeGroupStateDesc(t *testing.T) { } return strings.Compare(a.Group.Name, b.Group.Name) }) - require.Equal(t, int(tc.maxRuleGroups), len(out.Groups)) + require.Len(t, out.Groups, len(tc.expectedOutput.Groups)) t.Log(tc.expectedOutput) t.Log(out) require.True(t, reflect.DeepEqual(tc.expectedOutput, out)) diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index 82f7c57fb0..b874027787 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -168,6 +168,8 @@ type Config struct { EnabledTenants flagext.StringSliceCSV `yaml:"enabled_tenants"` DisabledTenants flagext.StringSliceCSV `yaml:"disabled_tenants"` + ListRulesMaxRules uint `yaml:"list_rules_max_rules"` + RingCheckPeriod time.Duration `yaml:"-"` // Field will be populated during runtime. @@ -268,6 +270,8 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.Var(&cfg.EnabledTenants, "ruler.enabled-tenants", "Comma separated list of tenants whose rules this ruler can evaluate. If specified, only these tenants will be handled by ruler, otherwise this ruler can process rules from all tenants. Subject to sharding.") f.Var(&cfg.DisabledTenants, "ruler.disabled-tenants", "Comma separated list of tenants whose rules this ruler cannot evaluate. If specified, a ruler that would normally pick the specified tenant(s) for processing will ignore them instead. Subject to sharding.") + f.UintVar(&cfg.ListRulesMaxRules, "ruler.list-rules-max-rules", 0, "Maximum number of rules returned by the Prometheus ListRules API. If there are more rulegroups, the response will include a pagination token which can be used to fetch the next set. The API will always return at least one rulegroup, even if it contains more rules than the limit. Defaults to 0, which is unlimited") + f.BoolVar(&cfg.EnableQueryStats, "ruler.query-stats-enabled", false, "Report query statistics for ruler queries to complete as a per user metric and as an info level log message.") f.BoolVar(&cfg.DisableRuleGroupLabel, "ruler.disable-rule-group-label", false, "Disable the rule_group label on exported metrics") @@ -1192,14 +1196,14 @@ func (r *Ruler) filterBackupRuleGroups(userID string, ruleGroups []*rulespb.Rule // GetRules retrieves the running rules from this ruler and all running rulers in the ring if // sharding is enabled -func (r *Ruler) GetRules(ctx context.Context, rulesRequest RulesRequest) (*RulesResponse, error) { +func (r *Ruler) GetRules(ctx context.Context, rulesRequest RulesRequest, maxRules uint) (*RulesResponse, error) { userID, err := users.TenantID(ctx) if err != nil { return nil, fmt.Errorf("no user id found in context") } if r.cfg.EnableSharding { - resp, err := r.getShardedRules(ctx, userID, rulesRequest) + resp, err := r.getShardedRules(ctx, userID, rulesRequest, maxRules) if resp == nil { return &RulesResponse{ Groups: make([]*GroupStateDesc, 0), @@ -1209,11 +1213,11 @@ func (r *Ruler) GetRules(ctx context.Context, rulesRequest RulesRequest) (*Rules return resp, err } - response, err := r.getLocalRules(userID, rulesRequest, false) + response, err := r.getLocalRules(userID, rulesRequest, maxRules, false) return &response, err } -func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, includeBackups bool) (RulesResponse, error) { +func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, maxRules uint, includeBackups bool) (RulesResponse, error) { groups := r.manager.GetRules(userID) groupDescs := make([]*GroupStateDesc, 0, len(groups)) @@ -1375,7 +1379,7 @@ func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, includeB combinedRuleStateDescs = append(combinedRuleStateDescs, backupGroupDescs...) } - if rulesRequest.MaxRuleGroups <= 0 { + if rulesRequest.MaxRuleGroups <= 0 && maxRules == 0 { return RulesResponse{ Groups: combinedRuleStateDescs, NextToken: "", @@ -1398,7 +1402,7 @@ func (r *Ruler) getLocalRules(userID string, rulesRequest RulesRequest, includeB } } - resultingGroupDescs, nextToken := generatePage(resultingGroupDescs, int(rulesRequest.MaxRuleGroups)) + resultingGroupDescs, nextToken := generatePage(resultingGroupDescs, int(rulesRequest.MaxRuleGroups), maxRules) return RulesResponse{ Groups: resultingGroupDescs, NextToken: nextToken, @@ -1519,7 +1523,7 @@ func (r *Ruler) getShardSizeForUser(userID string) int { return max(newShardSize, r.cfg.Ring.ReplicationFactor) } -func (r *Ruler) getShardedRules(ctx context.Context, userID string, rulesRequest RulesRequest) (*RulesResponse, error) { +func (r *Ruler) getShardedRules(ctx context.Context, userID string, rulesRequest RulesRequest, maxRules uint) (*RulesResponse, error) { ring := ring.ReadRing(r.ring) if shardSize := r.limits.RulerTenantShardSize(userID); shardSize > 0 && r.cfg.ShardingStrategy == util.ShardingStrategyShuffle { @@ -1598,17 +1602,15 @@ func (r *Ruler) getShardedRules(ctx context.Context, userID string, rulesRequest return nil }) - if err == nil { - if r.cfg.RulesBackupEnabled() || r.cfg.APIDeduplicateRules { - return mergeGroupStateDesc(merged, rulesRequest.MaxRuleGroups, true), nil - } - return mergeGroupStateDesc(merged, rulesRequest.MaxRuleGroups, false), nil + if err != nil { + return &RulesResponse{ + Groups: make([]*GroupStateDesc, 0), + NextToken: "", + }, err } - return &RulesResponse{ - Groups: make([]*GroupStateDesc, 0), - NextToken: "", - }, err + backup := r.cfg.RulesBackupEnabled() || r.cfg.APIDeduplicateRules + return mergeGroupStateDesc(merged, rulesRequest.MaxRuleGroups, maxRules, backup), nil } // Rules implements the rules service @@ -1619,7 +1621,7 @@ func (r *Ruler) Rules(ctx context.Context, in *RulesRequest) (*RulesResponse, er return nil, fmt.Errorf("no user id found in context") } - response, err := r.getLocalRules(userID, *in, r.cfg.RulesBackupEnabled()) + response, err := r.getLocalRules(userID, *in, 0, r.cfg.RulesBackupEnabled()) if err != nil { return nil, err } diff --git a/pkg/ruler/ruler_pagination.go b/pkg/ruler/ruler_pagination.go index b1a1eb7169..b8d7764232 100644 --- a/pkg/ruler/ruler_pagination.go +++ b/pkg/ruler/ruler_pagination.go @@ -20,26 +20,34 @@ func GetRuleGroupNextToken(namespace string, group string) string { } // generatePage function takes in a sorted list of groups and returns a page of groups and the next token which can be -// used to in subsequent requests. The # of groups per page is at most equal to maxRuleGroups. If the total passed in -// rule group count is greater than maxRuleGroups, then a next token is returned. Otherwise, next token is empty -func generatePage(groups []*GroupStateDesc, maxRuleGroups int) ([]*GroupStateDesc, string) { - resultNumber := 0 +// used to in subsequent requests. The # of groups per page is at most equal to maxRuleGroups and the number of rules is +// at most maxRules, unless one rulegroup contains more rules, then that entire rulegroup is returned. +// If the rule or rule group count is greater than their limit, a next token is returned. Otherwise, next token is empty +func generatePage(groups []*GroupStateDesc, maxRuleGroups int, maxRules uint) ([]*GroupStateDesc, string) { var returnPaginationToken string returnGroupDescs := make([]*GroupStateDesc, 0, len(groups)) + resultNumber := 0 + ruleCount := 0 + truncated := false + for _, groupInfo := range groups { + ruleLimit := maxRules > 0 && uint(ruleCount+len(groupInfo.ActiveRules)) > maxRules + groupLimit := maxRuleGroups > 0 && resultNumber >= maxRuleGroups - // Add the rule group to the return slice if the maxRuleGroups is not hit - if maxRuleGroups < 0 || resultNumber < maxRuleGroups { + // Add the rule group to the return slice if the maxRules and maxRuleGroups is not hit, or if the first rulegroup exceeds maxRules + if (!groupLimit && !ruleLimit) || (ruleLimit && resultNumber == 0) { returnGroupDescs = append(returnGroupDescs, groupInfo) + ruleCount += len(groupInfo.ActiveRules) resultNumber++ - continue - } - - // Return the next token if there are more groups - if maxRuleGroups > 0 && resultNumber == maxRuleGroups { - returnPaginationToken = GetRuleGroupNextToken(returnGroupDescs[maxRuleGroups-1].Group.Namespace, returnGroupDescs[maxRuleGroups-1].Group.Name) + } else { + truncated = true break } } + + // Return the next token if there are more groups. The guard above ensures resultNumber can never be 0 if truncated==true + if truncated { + returnPaginationToken = GetRuleGroupNextToken(returnGroupDescs[resultNumber-1].Group.Namespace, returnGroupDescs[resultNumber-1].Group.Name) + } return returnGroupDescs, returnPaginationToken } diff --git a/pkg/ruler/ruler_pagination_test.go b/pkg/ruler/ruler_pagination_test.go index fe55312673..8123b64182 100644 --- a/pkg/ruler/ruler_pagination_test.go +++ b/pkg/ruler/ruler_pagination_test.go @@ -61,19 +61,19 @@ func TestGeneratePage(t *testing.T) { } t.Run("returns all groups when maxRuleGroups exceeds total", func(t *testing.T) { - result, token := generatePage(groups, 10) + result, token := generatePage(groups, 10, 0) assert.Len(t, result, 5) assert.Empty(t, token) }) t.Run("returns all groups when maxRuleGroups equals total", func(t *testing.T) { - result, token := generatePage(groups, 5) + result, token := generatePage(groups, 5, 0) assert.Len(t, result, 5) assert.Empty(t, token) }) t.Run("returns page with next token when more groups exist", func(t *testing.T) { - result, token := generatePage(groups, 3) + result, token := generatePage(groups, 3, 0) require.Len(t, result, 3) assert.NotEmpty(t, token) expectedToken := GetRuleGroupNextToken(result[2].Group.Namespace, result[2].Group.Name) @@ -81,20 +81,153 @@ func TestGeneratePage(t *testing.T) { }) t.Run("returns all groups when maxRuleGroups is negative", func(t *testing.T) { - result, token := generatePage(groups, -1) + result, token := generatePage(groups, -1, 0) assert.Len(t, result, 5) assert.Empty(t, token) }) t.Run("empty input", func(t *testing.T) { - result, token := generatePage(nil, 10) + result, token := generatePage(nil, 10, 0) assert.Empty(t, result) assert.Empty(t, token) }) t.Run("page of one", func(t *testing.T) { - result, token := generatePage(groups, 1) + result, token := generatePage(groups, 1, 0) require.Len(t, result, 1) assert.NotEmpty(t, token) }) } + +// groupWithRules builds a group holding ruleCount active rules. generatePage only +// looks at how many active rules a group has, not at their contents. +func groupWithRules(name string, ruleCount int) *GroupStateDesc { + activeRules := make([]*RuleStateDesc, ruleCount) + for i := range activeRules { + activeRules[i] = &RuleStateDesc{Rule: &rulespb.RuleDesc{Expr: "up"}} + } + return &GroupStateDesc{ + Group: &rulespb.RuleGroupDesc{Namespace: "namespace", Name: name}, + ActiveRules: activeRules, + } +} + +func TestGeneratePage_MaxRules(t *testing.T) { + // ruleCounts describes the input groups, which generatePage expects to already + // be sorted. Group names ascend with the slice index so that the input order is + // also token order, letting the cases below assert on the surviving prefix. + for _, tc := range []struct { + name string + ruleCounts []int + maxRuleGroups int + maxRules uint + expectedGroups int + expectedToken int // index into ruleCounts of the group the token points at, or -1 for no token + }{ + { + name: "maxRules disabled returns every group", + ruleCounts: []int{2, 2, 2}, + maxRuleGroups: 0, + maxRules: 0, + expectedGroups: 3, + expectedToken: -1, + }, + { + name: "cumulative rule count below maxRules", + ruleCounts: []int{2, 2, 2}, + maxRuleGroups: 0, + maxRules: 10, + expectedGroups: 3, + expectedToken: -1, + }, + { + name: "cumulative rule count exactly at maxRules is not truncated", + ruleCounts: []int{2, 2, 2}, + maxRuleGroups: 0, + maxRules: 6, + expectedGroups: 3, + expectedToken: -1, + }, + { + name: "maxRules truncates part way through the list", + ruleCounts: []int{2, 2, 2}, + maxRuleGroups: 0, + maxRules: 5, + expectedGroups: 2, + expectedToken: 1, + }, + { + // A rule group is indivisible, so a group larger than maxRules is returned + // whole rather than dropped. It is the only group, so there is nothing left + // to page to and the token must be empty. + name: "lone group larger than maxRules is returned without a token", + ruleCounts: []int{5}, + maxRuleGroups: 0, + maxRules: 2, + expectedGroups: 1, + expectedToken: -1, + }, + { + name: "oversized first group still yields a token when more groups follow", + ruleCounts: []int{5, 1}, + maxRuleGroups: 0, + maxRules: 2, + expectedGroups: 1, + expectedToken: 0, + }, + { + name: "maxRuleGroups binds before maxRules", + ruleCounts: []int{1, 1, 1}, + maxRuleGroups: 2, + maxRules: 100, + expectedGroups: 2, + expectedToken: 1, + }, + { + name: "maxRules binds before maxRuleGroups", + ruleCounts: []int{2, 2, 2}, + maxRuleGroups: 3, + maxRules: 3, + expectedGroups: 1, + expectedToken: 0, + }, + { + name: "groups without active rules are never limited by maxRules", + ruleCounts: []int{0, 0, 0}, + maxRuleGroups: 0, + maxRules: 1, + expectedGroups: 3, + expectedToken: -1, + }, + { + name: "empty input with maxRules set", + ruleCounts: nil, + maxRuleGroups: 0, + maxRules: 5, + expectedGroups: 0, + expectedToken: -1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + groups := make([]*GroupStateDesc, len(tc.ruleCounts)) + for i, count := range tc.ruleCounts { + groups[i] = groupWithRules(string(rune('a'+i)), count) + } + + result, token := generatePage(groups, tc.maxRuleGroups, tc.maxRules) + + require.Len(t, result, tc.expectedGroups) + // The page must be the leading prefix of the input. + for i := range result { + assert.Same(t, groups[i], result[i]) + } + + if tc.expectedToken < 0 { + assert.Empty(t, token) + return + } + expected := groups[tc.expectedToken] + assert.Equal(t, GetRuleGroupNextToken(expected.Group.Namespace, expected.Group.Name), token) + }) + } +} diff --git a/pkg/ruler/ruler_test.go b/pkg/ruler/ruler_test.go index cf42a97067..cebcb2502d 100644 --- a/pkg/ruler/ruler_test.go +++ b/pkg/ruler/ruler_test.go @@ -669,6 +669,7 @@ func TestGetRules(t *testing.T) { shardingStrategy string shuffleShardSize float64 rulesRequest RulesRequest + maxRules uint expectedCount map[string]int expectedClientCallCount int rulerStateMap map[string]ring.InstanceState @@ -1006,6 +1007,52 @@ func TestGetRules(t *testing.T) { replicationFactor: 3, expectedClientCallCount: len(expectedRules), }, + // maxRules caps the page by rule count rather than by group count, so it + // applies even though the request asks for no group limit. Groups are paged in + // token order, and a group is only included if it fits whole: user1 gets + // namespace/third (2 rules) then namespace/second (1), stopping before + // namespace/first (2) would take it to 5. + "No Sharding with maxRules truncating the page": { + sharding: false, + rulesRequest: RulesRequest{MaxRuleGroups: -1}, + maxRules: 3, + rulerStateMap: rulerStateMapAllActive, + expectedCount: map[string]int{ + "user1": 3, + "user2": 3, + "user3": 3, + }, + }, + "Default Sharding with maxRules truncating the page": { + sharding: true, + shardingStrategy: util.ShardingStrategyDefault, + rulerStateMap: rulerStateMapAllActive, + rulesRequest: RulesRequest{MaxRuleGroups: -1}, + maxRules: 3, + expectedCount: map[string]int{ + "user1": 3, + "user2": 3, + "user3": 3, + }, + expectedClientCallCount: len(expectedRules), + }, + // A rule group is indivisible, so when the first group in token order is bigger + // than maxRules it is returned whole and the response exceeds the limit. user1 + // and user3 both lead with a 2 rule group; user2 leads with a 1 rule group and + // so stays at the limit. + "Default Sharding with maxRules smaller than the leading rule group": { + sharding: true, + shardingStrategy: util.ShardingStrategyDefault, + rulerStateMap: rulerStateMapAllActive, + rulesRequest: RulesRequest{MaxRuleGroups: -1}, + maxRules: 1, + expectedCount: map[string]int{ + "user1": 2, + "user2": 1, + "user3": 2, + }, + expectedClientCallCount: len(expectedRules), + }, "Shuffle Sharding and ShardSize = 2 with Rule Type Filter": { sharding: true, shuffleShardSize: 2, @@ -1383,7 +1430,7 @@ func TestGetRules(t *testing.T) { for u := range allRulesByUser { ctx := user.InjectOrgID(context.Background(), u) forEachRuler(func(_ string, r *Ruler) { - ruleStateDescriptions, err := r.GetRules(ctx, tc.rulesRequest) + ruleStateDescriptions, err := r.GetRules(ctx, tc.rulesRequest, tc.maxRules) if tc.expectedError != nil { require.Error(t, tc.expectedError) return @@ -1639,7 +1686,7 @@ func TestGetRulesFromBackup(t *testing.T) { } } ctx := user.InjectOrgID(context.Background(), tenantId) - ruleStateDescriptions, err := rulerAddrMap["ruler1"].GetRules(ctx, RulesRequest{MaxRuleGroups: -1}) + ruleStateDescriptions, err := rulerAddrMap["ruler1"].GetRules(ctx, RulesRequest{MaxRuleGroups: -1}, 0) require.NoError(t, err) require.Equal(t, 5, len(ruleStateDescriptions.Groups)) stateByKey := map[string]*GroupStateDesc{} @@ -1660,7 +1707,7 @@ func TestGetRulesFromBackup(t *testing.T) { RuleGroupNames: []string{"b1"}, Type: recordingRuleFilter, MaxRuleGroups: -1, - }) + }, 0) require.NoError(t, err) require.Equal(t, 1, len(ruleStateDescriptions.Groups)) require.Equal(t, "b1", ruleStateDescriptions.Groups[0].Group.Name) @@ -1869,7 +1916,7 @@ func getRulesHATest(replicationFactor int) func(t *testing.T) { getRules := func(ruler string) { ctx := user.InjectOrgID(context.Background(), tenantId) - ruleStateDescriptions, err := rulerAddrMap[ruler].GetRules(ctx, RulesRequest{MaxRuleGroups: -1}) + ruleStateDescriptions, err := rulerAddrMap[ruler].GetRules(ctx, RulesRequest{MaxRuleGroups: -1}, 0) require.NoError(t, err) require.Equal(t, 5, len(ruleStateDescriptions.Groups)) stateByKey := map[string]*GroupStateDesc{} diff --git a/schemas/cortex-config-schema.json b/schemas/cortex-config-schema.json index 939e9134db..42cfed620e 100644 --- a/schemas/cortex-config-schema.json +++ b/schemas/cortex-config-schema.json @@ -7552,6 +7552,12 @@ }, "type": "object" }, + "list_rules_max_rules": { + "default": 0, + "description": "Maximum number of rules returned by the Prometheus ListRules API. If there are more rulegroups, the response will include a pagination token which can be used to fetch the next set. The API will always return at least one rulegroup, even if it contains more rules than the limit. Defaults to 0, which is unlimited", + "type": "number", + "x-cli-flag": "ruler.list-rules-max-rules" + }, "liveness_check_timeout": { "default": "1s", "description": "Timeout duration for non-primary rulers during liveness checks. If the check times out, the non-primary ruler will evaluate the rule group. Applicable when ruler.enable-ha-evaluation is true.",