-
Notifications
You must be signed in to change notification settings - Fork 523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ruler: Add pagination for api/v1/rules #9563
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -7,12 +7,14 @@ package ruler | |||||||
|
||||||||
import ( | ||||||||
"context" | ||||||||
"encoding/base64" | ||||||||
"flag" | ||||||||
"fmt" | ||||||||
"hash/fnv" | ||||||||
"net/http" | ||||||||
"net/url" | ||||||||
"path/filepath" | ||||||||
"slices" | ||||||||
"strings" | ||||||||
"sync" | ||||||||
"time" | ||||||||
|
@@ -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) { | ||||||||
userID, err := tenant.TenantID(ctx) | ||||||||
if err != nil { | ||||||||
return nil, fmt.Errorf("no user id found in context") | ||||||||
return nil, "", fmt.Errorf("no user id found in context") | ||||||||
} | ||||||||
|
||||||||
rr := ring.ReadRing(r.ring) | ||||||||
|
@@ -965,7 +967,7 @@ func (r *Ruler) GetRules(ctx context.Context, req RulesRequest) ([]*GroupStateDe | |||||||
|
||||||||
ctx, err = user.InjectIntoGRPCRequest(ctx) | ||||||||
if err != nil { | ||||||||
return nil, fmt.Errorf("unable to inject user ID into grpc request, %v", err) | ||||||||
return nil, "", fmt.Errorf("unable to inject user ID into grpc request, %v", err) | ||||||||
} | ||||||||
|
||||||||
var ( | ||||||||
|
@@ -993,7 +995,31 @@ func (r *Ruler) GetRules(ctx context.Context, req RulesRequest) ([]*GroupStateDe | |||||||
return nil | ||||||||
}) | ||||||||
|
||||||||
return merged, err | ||||||||
// If the request asks for pagination, we fetch req.MaxGroups number | ||||||||
// of rule groups from each replica. We then merge and sort these and | ||||||||
// take the top k (k = MaxGroups) | ||||||||
if req.MaxGroups > 0 { | ||||||||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the sorting in |
||||||||
|
||||||||
if len(merged) > int(req.MaxGroups) { | ||||||||
groupForToken := merged[req.MaxGroups] | ||||||||
return merged[:req.MaxGroups], getRuleGroupNextToken(groupForToken.Group.Namespace, groupForToken.Group.Name), err | ||||||||
} | ||||||||
|
||||||||
// If len(merged) <= req.MaxGroups we are | ||||||||
// on the last page so there is no token to return | ||||||||
return merged, "", err | ||||||||
} | ||||||||
|
||||||||
return merged, "", err | ||||||||
} | ||||||||
|
||||||||
// SyncRules implements the gRPC Ruler service. | ||||||||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rules are sorted at this point, right? From There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, it's done here:
|
||||||||
if groupSet.IsFiltered(group.Name()) { | ||||||||
continue | ||||||||
} | ||||||||
|
||||||||
interval := group.Interval() | ||||||||
|
||||||||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
if !tokenGreaterThanOrEqual(getRuleGroupNextToken(decodedNamespace, group.Name()), req.NextToken) { | ||||||||
continue | ||||||||
} | ||||||||
foundToken = true | ||||||||
} | ||||||||
|
||||||||
interval := group.Interval() | ||||||||
|
||||||||
if fileSet.IsFiltered(decodedNamespace) { | ||||||||
continue | ||||||||
} | ||||||||
|
@@ -1171,12 +1206,27 @@ func (r *Ruler) getLocalRules(ctx context.Context, userID string, req RulesReque | |||||||
|
||||||||
// Prometheus does not return a rule group if it has no rules after filtering. | ||||||||
if len(groupDesc.ActiveRules) > 0 { | ||||||||
if req.MaxGroups > 0 && len(groupDescs) == int(req.MaxGroups)+1 { | ||||||||
break | ||||||||
} | ||||||||
groupDescs = append(groupDescs, groupDesc) | ||||||||
} | ||||||||
} | ||||||||
return groupDescs, nil | ||||||||
} | ||||||||
|
||||||||
func getRuleGroupNextToken(file, group string) string { | ||||||||
return base64.URLEncoding.EncodeToString([]byte(file + "/" + group)) | ||||||||
} | ||||||||
|
||||||||
// Returns true if tokenA >= tokenB | ||||||||
func tokenGreaterThanOrEqual(tokenA string, tokenB string) bool { | ||||||||
decodedTokenA, _ := base64.URLEncoding.DecodeString(tokenA) | ||||||||
decodedTokenB, _ := base64.URLEncoding.DecodeString(tokenB) | ||||||||
|
||||||||
return string(decodedTokenA) >= string(decodedTokenB) | ||||||||
} | ||||||||
|
||||||||
// IsMaxRuleGroupsLimited returns true if there is a limit set for the max | ||||||||
// number of rule groups for the tenant and namespace. | ||||||||
func (r *Ruler) IsMaxRuleGroupsLimited(userID, namespace string) bool { | ||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed offline, but decided against this for now to prevent an API level struct being introduced to this function.