From e4b3aca90a0a1008c210dc95642f4de81359ca2b Mon Sep 17 00:00:00 2001 From: Raphael Silva Date: Thu, 17 Oct 2024 19:18:21 +0000 Subject: [PATCH] Simplify pagination implementation * Eliminate need for extra structs to store pagination parameters Signed-off-by: Raphael Silva --- docs/querying/api.md | 4 +-- web/api/v1/api.go | 66 +++++++++++++++++------------------------- web/api/v1/api_test.go | 20 ++++++------- 3 files changed, 38 insertions(+), 52 deletions(-) diff --git a/docs/querying/api.md b/docs/querying/api.md index 6a5feaf0ce0..ff2e4f9ee9b 100644 --- a/docs/querying/api.md +++ b/docs/querying/api.md @@ -764,8 +764,8 @@ URL query parameters: - `file[]=`: only return rules with the given filepath. If the parameter is repeated, rules with any of the provided filepaths are returned. When the parameter is absent or empty, no filtering is done. - `exclude_alerts=`: only return rules, do not return active alerts. - `match[]=`: only return rules that have configured labels that satisfy the label selectors. If the parameter is repeated, rules that match any of the sets of label selectors are returned. Note that matching is on the labels in the definition of each rule, not on the values after template expansion (for alerting rules). Optional. -- `max_groups=`: The `max_groups` parameter allows you to specify the maximum number of rule groups to return in a single response. If the total number of rule groups exceeds the specified `max_groups` value, the response will include a `nextToken` property. You can use the value of this `nextToken` property in subsequent requests in the `next_token` parameter to paginate over the remaining rule groups. The `nextToken` property will not be present in the final response, indicating that you have retrieved all the available rule groups. Please note that there are no guarantees regarding the consistency of the response if the rule groups are being modified during the pagination process. If a rule group is removed while you are paginating over the rule groups, an error might be raised if the removed rule group coincides with the next token. -- `next_token`: the pagination token that was returned in previous request when the `max_groups` property is set. The pagination token is used to iteratively paginate over a large number of rule groups. To use the `next_token` parameter, the `max_groups` parameter also need to be present. +- `group_limit=`: The `group_limit` parameter allows you to specify a limit for the number of rule groups that is returned in a single response. If the total number of rule groups exceeds the specified `group_limit` value, the response will include a `nextGroupToken` property. You can use the value of this `nextGroupToken` property in subsequent requests in the `group_next_token` parameter to paginate over the remaining rule groups. The `nextGroupToken` property will not be present in the final response, indicating that you have retrieved all the available rule groups. Please note that there are no guarantees regarding the consistency of the response if the rule groups are being modified during the pagination process. +- `group_next_token`: the pagination token that was returned in previous request when the `group_limit` property is set. The pagination token is used to iteratively paginate over a large number of rule groups. To use the `group_next_token` parameter, the `group_limit` parameter also need to be present. If a rule group that coincides with the next token is removed while you are paginating over the rule groups, a response with status code 400 will be returned. ```json $ curl http://localhost:9090/api/v1/rules diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 8236fb0b651..9f99c30466d 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -1,5 +1,5 @@ // Copyright 2016 The Prometheus Authors - +// Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // @@ -175,16 +175,6 @@ type apiFuncResult struct { type apiFunc func(r *http.Request) apiFuncResult -type listRulesPaginationRequest struct { - MaxRuleGroups int64 - NextToken string -} - -type parsePaginationError struct { - err error - parameter string -} - // TSDBAdminStats defines the tsdb interfaces used by the v1 API for admin operations as well as statistics. type TSDBAdminStats interface { CleanTombstones() error @@ -1384,8 +1374,8 @@ func (api *API) metricMetadata(r *http.Request) apiFuncResult { // RuleDiscovery has info for all rules. type RuleDiscovery struct { - RuleGroups []*RuleGroup `json:"groups"` - NextToken string `json:"nextToken:omitempty"` + RuleGroups []*RuleGroup `json:"groups"` + GroupNextToken string `json:"groupNextToken:omitempty"` } // RuleGroup has info for rules which are part of a group. @@ -1472,9 +1462,9 @@ func (api *API) rules(r *http.Request) apiFuncResult { return invalidParamError(err, "exclude_alerts") } - paginationRequest, parseErr := parseListRulesPaginationRequest(r) + maxGroups, nextToken, parseErr := parseListRulesPaginationRequest(r) if parseErr != nil { - return invalidParamError(parseErr.err, parseErr.parameter) + return *parseErr } rgs := make([]*RuleGroup, 0, len(ruleGroups)) @@ -1482,8 +1472,8 @@ func (api *API) rules(r *http.Request) apiFuncResult { foundToken := false for _, grp := range ruleGroups { - if paginationRequest != nil && paginationRequest.NextToken != "" && !foundToken { - if paginationRequest.NextToken != getRuleGroupNextToken(grp.File(), grp.Name()) { + if maxGroups > 0 && nextToken != "" && !foundToken { + if nextToken != getRuleGroupNextToken(grp.File(), grp.Name()) { continue } foundToken = true @@ -1576,20 +1566,19 @@ func (api *API) rules(r *http.Request) apiFuncResult { // If the rule group response has no rules, skip it - this means we filtered all the rules of this group. if len(apiRuleGroup.Rules) > 0 { - if paginationRequest != nil && len(rgs) == int(paginationRequest.MaxRuleGroups) { + if maxGroups > 0 && len(rgs) == int(maxGroups) { // We've reached the capacity of our page. // We are looking ahead up to this point because this is ultimately where the presence of at least one rule // group in a subsequent response is determined, hence requiring a nextToken. - res.NextToken = getRuleGroupNextToken(grp.File(), grp.Name()) + res.GroupNextToken = getRuleGroupNextToken(grp.File(), grp.Name()) break } rgs = append(rgs, apiRuleGroup) } } - if paginationRequest != nil && paginationRequest.NextToken != "" && !foundToken { - err := fmt.Errorf("invalid nextToken '%v'. were rule groups changed?", paginationRequest.NextToken) - return invalidParamError(err, "next_token") + if maxGroups > 0 && nextToken != "" && !foundToken { + return invalidParamError(fmt.Errorf("invalid group_next_token '%v'. were rule groups changed?", nextToken), "group_next_token") } res.RuleGroups = rgs @@ -1610,39 +1599,36 @@ func parseExcludeAlerts(r *http.Request) (bool, error) { return excludeAlerts, nil } -func parseListRulesPaginationRequest(r *http.Request) (*listRulesPaginationRequest, *parsePaginationError) { +func parseListRulesPaginationRequest(r *http.Request) (int64, string, *apiFuncResult) { var ( parsedMaxGroups int64 = -1 err error ) - maxGroups := r.URL.Query().Get("max_groups") - nextToken := r.URL.Query().Get("next_token") + maxGroups := r.URL.Query().Get("group_limit") + nextToken := r.URL.Query().Get("group_next_token") if nextToken != "" && maxGroups == "" { - return nil, &parsePaginationError{ - err: fmt.Errorf("max_groups needs to be present in order to paginate"), - parameter: "max_groups", - } + errResult := invalidParamError(fmt.Errorf("group_limit needs to be present in order to paginate over the groups"), "group_next_token") + return -1, "", &errResult } if maxGroups != "" { parsedMaxGroups, err = strconv.ParseInt(maxGroups, 10, 32) - if err != nil || parsedMaxGroups <= 0 { - return nil, &parsePaginationError{ - err: fmt.Errorf("max_groups need to be a valid number greater than 0: %w", err), - parameter: "max_groups", - } + if err != nil { + errResult := invalidParamError(fmt.Errorf("group_limit needs to be a valid number: %w", err), "group_limit") + return -1, "", &errResult + } + if parsedMaxGroups <= 0 { + errResult := invalidParamError(fmt.Errorf("group_limit needs to be greater than 0"), "group_limit") + return -1, "", &errResult } } - if parsedMaxGroups >= 0 || nextToken != "" { - return &listRulesPaginationRequest{ - MaxRuleGroups: parsedMaxGroups, - NextToken: nextToken, - }, nil + if parsedMaxGroups > 0 { + return parsedMaxGroups, nextToken, nil } - return nil, nil + return -1, "", nil } func getRuleGroupNextToken(file, group string) string { diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 167eb536e66..2419517e07f 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -2750,10 +2750,10 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E { endpoint: api.rules, query: url.Values{ - "max_groups": []string{"1"}, + "group_limit": []string{"1"}, }, response: &RuleDiscovery{ - NextToken: getRuleGroupNextToken("/path/to/file", "grp2"), + GroupNextToken: getRuleGroupNextToken("/path/to/file", "grp2"), RuleGroups: []*RuleGroup{ { Name: "grp", @@ -2846,8 +2846,8 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E { endpoint: api.rules, query: url.Values{ - "max_groups": []string{"1"}, - "next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, + "group_limit": []string{"1"}, + "group_next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, }, response: &RuleDiscovery{ RuleGroups: []*RuleGroup{ @@ -2877,16 +2877,16 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E { // invalid pagination request endpoint: api.rules, query: url.Values{ - "next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, + "group_next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, }, errType: errorBadData, zeroFunc: rulesZeroFunc, }, - { // invalid max_groups + { // invalid group_limit endpoint: api.rules, query: url.Values{ - "max_groups": []string{"0"}, - "next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, + "group_limit": []string{"0"}, + "group_next_token": []string{getRuleGroupNextToken("/path/to/file", "grp2")}, }, errType: errorBadData, zeroFunc: rulesZeroFunc, @@ -2894,8 +2894,8 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E { // Pagination token is invalid due to changes in the rule groups endpoint: api.rules, query: url.Values{ - "max_groups": []string{"1"}, - "next_token": []string{getRuleGroupNextToken("/removed/file", "notfound")}, + "group_limit": []string{"1"}, + "group_next_token": []string{getRuleGroupNextToken("/removed/file", "notfound")}, }, errType: errorBadData, zeroFunc: rulesZeroFunc,