Ruler: Add pagination for api/v1/rules #9563
Conversation
fayzal-g
force-pushed
the
rule-pagination-replica
branch
2 times, most recently
from
6e07e93
to
f832fe0
Compare
fayzal-g
force-pushed
the
rule-pagination-replica
branch
from
f832fe0
to
baeb8d9
Compare
fayzal-g
changed the title
| @@ -951,10 +953,10 @@ func filterRuleGroupsByNotMissing(configs map[string]rulespb.RuleGroupList, miss | |||
| } | |||
|
|
|||
| // GetRules retrieves the running rules from this ruler and all running rulers in the ring. | |||
| func (r *Ruler) GetRules(ctx context.Context, req RulesRequest) ([]*GroupStateDesc, error) { | |||
| func (r *Ruler) GetRules(ctx context.Context, req RulesRequest) ([]*GroupStateDesc, string, error) { | |||
There was a problem hiding this comment.
It would be nice if function could return RulesResponse, just so we add the token to that, just so it's named.
Not sure how invasive that is, so it could be done in a separate or follow up PR.
There was a problem hiding this comment.
Discussed offline, but decided against this for now to prevent an API level struct being introduced to this function.
pkg/ruler/ruler.go
Outdated
| slices.SortFunc(merged, func(a, b *GroupStateDesc) int { | ||
| nsCmp := strings.Compare(a.Group.Namespace, b.Group.Namespace) | ||
| if nsCmp != 0 { | ||
| return nsCmp | ||
| } | ||
|
|
||
| // If Namespaces are equal, check the group names | ||
| return strings.Compare(a.Group.Name, b.Group.Name) | ||
| }) |
There was a problem hiding this comment.
Non-blocking nit: this might be over-engineering..... but I see a potential issue where someone updates the sort strategy in one place, and not the other (in api.go). Could we share them somehow? Define a "SortableGroup" interface or something?
There was a problem hiding this comment.
I've removed the sorting in api.go - the groups will now get sorted after they're merged in GetRules()
| // The mapped filename is url path escaped encoded to make handling `/` characters easier | ||
| decodedNamespace, err := url.PathUnescape(strings.TrimPrefix(group.File(), prefix)) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "unable to decode rule filename") | ||
| } | ||
|
|
||
| if req.NextToken != "" && !foundToken { |
There was a problem hiding this comment.
| if req.NextToken != "" && !foundToken { | |
| // If a pagination token is provided, skip past groups until we reach the point of that token. | |
| if req.NextToken != "" && !foundToken { |
| @@ -1068,18 +1094,27 @@ func (r *Ruler) getLocalRules(ctx context.Context, userID string, req RulesReque | |||
| groupSet := makeStringFilterSet(req.RuleGroup) | |||
| ruleSet := makeStringFilterSet(req.RuleName) | |||
|
|
|||
| foundToken := false | |||
| for _, group := range groups { | |||
There was a problem hiding this comment.
The rules are sorted at this point, right? From GetRules?
There was a problem hiding this comment.
Yup, it's done here:
No description provided.