Simplify paginated implementation

* Remove maxAlerts parameter.
* Reuse existing API responses by using omitempty in some fields

Signed-off-by: Raphael Silva <rapphil@gmail.com>

web/api/v1/api.go

@@ -170,7 +170,6 @@ type apiFuncResult struct {
 type apiFunc func(r *http.Request) apiFuncResult
 
 type listRulesPaginationRequest struct {
-	MaxAlerts     int64
 	MaxRuleGroups int64
 	NextToken     string
 }
@@ -1339,12 +1338,7 @@ 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"`
-
-// PaginatedRuleDiscovery has info for all rules with pagination.
-type PaginatedRuleDiscovery struct {
-	RuleGroups []*RuleGroup `json:"groups"`
-	NextToken  string       `json:"nextToken"`
 }
 
 // RuleGroup has info for rules which are part of a group.
@@ -1381,29 +1375,6 @@ type AlertingRule struct {
 	Type string `json:"type"`
 }
 
-type AlertingRulePaginated struct {
-	// State can be "pending", "firing", "inactive".
-	State          string           `json:"state"`
-	Name           string           `json:"name"`
-	Query          string           `json:"query"`
-	Duration       float64          `json:"duration"`
-	KeepFiringFor  float64          `json:"keepFiringFor"`
-	Labels         labels.Labels    `json:"labels"`
-	Annotations    labels.Labels    `json:"annotations"`
-	PartialAlerts  *PartialAlerts   `json:"alertInfo"`
-	Health         rules.RuleHealth `json:"health"`
-	LastError      string           `json:"lastError,omitempty"`
-	EvaluationTime float64          `json:"evaluationTime"`
-	LastEvaluation time.Time        `json:"lastEvaluation"`
-	// Type of an alertingRule is always "alerting".
-	Type string `json:"type"`
-}
-
-type PartialAlerts struct {
-	Alerts  []*Alert `json:"alerts"`
-	HasMore bool     `json:"hasMore"`
-}
-
 type RecordingRule struct {
 	Name           string           `json:"name"`
 	Query          string           `json:"query"`
@@ -1459,6 +1430,7 @@ func (api *API) rules(r *http.Request) apiFuncResult {
 	}
 
 	rgs := make([]*RuleGroup, 0, len(ruleGroups))
+
 	for _, grp := range ruleGroups {
 		if len(rgSet) > 0 {
 			if _, ok := rgSet[grp.Name()]; !ok {
@@ -1509,48 +1481,23 @@ func (api *API) rules(r *http.Request) apiFuncResult {
 				if !excludeAlerts {
 					activeAlerts = rulesAlertsToAPIAlerts(rule.ActiveAlerts())
 				}
-				hasMore := false
+
-				if paginationRequest != nil && paginationRequest.MaxAlerts >= 0 && len(activeAlerts) > int(paginationRequest.MaxAlerts) {
+				enrichedRule = AlertingRule{
-					activeAlerts = activeAlerts[:int(paginationRequest.MaxAlerts)]
+					State:          rule.State().String(),
-					hasMore = true
+					Name:           rule.Name(),
+					Query:          rule.Query().String(),
+					Duration:       rule.HoldDuration().Seconds(),
+					KeepFiringFor:  rule.KeepFiringFor().Seconds(),
+					Labels:         rule.Labels(),
+					Annotations:    rule.Annotations(),
+					Alerts:         activeAlerts,
+					Health:         rule.Health(),
+					LastError:      lastError,
+					EvaluationTime: rule.GetEvaluationDuration().Seconds(),
+					LastEvaluation: rule.GetEvaluationTimestamp(),
+					Type:           "alerting",
 				}
 
-				if paginationRequest != nil {
-					enrichedRule = AlertingRulePaginated{
-						State:         rule.State().String(),
-						Name:          rule.Name(),
-						Query:         rule.Query().String(),
-						Duration:      rule.HoldDuration().Seconds(),
-						KeepFiringFor: rule.KeepFiringFor().Seconds(),
-						Labels:        rule.Labels(),
-						Annotations:   rule.Annotations(),
-						PartialAlerts: &PartialAlerts{
-							Alerts:  activeAlerts,
-							HasMore: hasMore,
-						},
-						Health:         rule.Health(),
-						LastError:      lastError,
-						EvaluationTime: rule.GetEvaluationDuration().Seconds(),
-						LastEvaluation: rule.GetEvaluationTimestamp(),
-						Type:           "alerting",
-					}
-				} else {
-					enrichedRule = AlertingRule{
-						State:          rule.State().String(),
-						Name:           rule.Name(),
-						Query:          rule.Query().String(),
-						Duration:       rule.HoldDuration().Seconds(),
-						KeepFiringFor:  rule.KeepFiringFor().Seconds(),
-						Labels:         rule.Labels(),
-						Annotations:    rule.Annotations(),
-						Alerts:         activeAlerts,
-						Health:         rule.Health(),
-						LastError:      lastError,
-						EvaluationTime: rule.GetEvaluationDuration().Seconds(),
-						LastEvaluation: rule.GetEvaluationTimestamp(),
-						Type:           "alerting",
-					}
-				}
 			case *rules.RecordingRule:
 				if !returnRecording {
 					break
@@ -1582,11 +1529,10 @@ func (api *API) rules(r *http.Request) apiFuncResult {
 	}
 
 	if paginationRequest != nil {
-		paginatedRes := &PaginatedRuleDiscovery{RuleGroups: make([]*RuleGroup, 0, len(ruleGroups))}
 		returnGroups, nextToken := TruncateGroupInfos(rgs, int(paginationRequest.MaxRuleGroups))
-		paginatedRes.RuleGroups = returnGroups
+		res.NextToken = nextToken
-		paginatedRes.NextToken = nextToken
+		res.RuleGroups = returnGroups
-		return apiFuncResult{paginatedRes, nil, nil, nil}
+		return apiFuncResult{res, nil, nil, nil}
 	}
 
 	res.RuleGroups = rgs
@@ -1609,28 +1555,17 @@ func parseExcludeAlerts(r *http.Request) (bool, error) {
 
 func parseListRulesPaginationRequest(r *http.Request) (*listRulesPaginationRequest, *parsePaginationError) {
 	var (
-		maxAlerts     int64 = -1
 		maxRuleGroups int64 = -1
 		nextToken           = ""
 		err           error
 	)
 
-	if r.URL.Query().Get("maxAlerts") != "" {
-		maxAlerts, err = strconv.ParseInt(r.URL.Query().Get("maxAlerts"), 10, 32)
-		if err != nil || maxAlerts < 0 {
-			return nil, &parsePaginationError{
-				err:       fmt.Errorf("maxAlerts need to be a valid number greater than or equal to 0: %w", err),
-				parameter: "maxAlerts",
-			}
-		}
-	}
-
 	if r.URL.Query().Get("maxRuleGroups") != "" {
 		maxRuleGroups, err = strconv.ParseInt(r.URL.Query().Get("maxRuleGroups"), 10, 32)
 		if err != nil || maxRuleGroups < 0 {
 			return nil, &parsePaginationError{
 				err:       fmt.Errorf("maxRuleGroups need to be a valid number greater than or equal to 0: %w", err),
-				parameter: "maxAlerts",
+				parameter: "maxRuleGroups",
 			}
 		}
 	}
@@ -1639,10 +1574,9 @@ func parseListRulesPaginationRequest(r *http.Request) (*listRulesPaginationReque
 		nextToken = r.URL.Query().Get("nextToken")
 	}
 
-	if maxAlerts >= 0 || maxRuleGroups >= 0 || nextToken != "" {
+	if maxRuleGroups >= 0 || nextToken != "" {
 		return &listRulesPaginationRequest{
 			MaxRuleGroups: maxRuleGroups,
-			MaxAlerts:     maxAlerts,
 			NextToken:     nextToken,
 		}, nil
 	}

web/api/v1/api_test.go

@@ -1073,54 +1073,28 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 	}
 
 	rulesZeroFunc := func(i interface{}) {
-		if i != nil {
+		v := i.(*RuleDiscovery)
-			switch v := i.(type) {
+		if v != nil {
-			case *RuleDiscovery:
+			for _, ruleGroup := range v.RuleGroups {
-				for _, ruleGroup := range v.RuleGroups {
+				ruleGroup.EvaluationTime = float64(0)
-					ruleGroup.EvaluationTime = float64(0)
+				ruleGroup.LastEvaluation = time.Time{}
-					ruleGroup.LastEvaluation = time.Time{}
+				for k, rule := range ruleGroup.Rules {
-					for k, rule := range ruleGroup.Rules {
+					switch r := rule.(type) {
-						switch r := rule.(type) {
+					case AlertingRule:
-						case AlertingRule:
+						r.LastEvaluation = time.Time{}
-							r.LastEvaluation = time.Time{}
+						r.EvaluationTime = float64(0)
-							r.EvaluationTime = float64(0)
+						r.LastError = ""
-							r.LastError = ""
+						r.Health = "ok"
-							r.Health = "ok"
+						for _, alert := range r.Alerts {
-							for _, alert := range r.Alerts {
+							alert.ActiveAt = nil
-								alert.ActiveAt = nil
-							}
-							ruleGroup.Rules[k] = r
-						case RecordingRule:
-							r.LastEvaluation = time.Time{}
-							r.EvaluationTime = float64(0)
-							r.LastError = ""
-							r.Health = "ok"
-							ruleGroup.Rules[k] = r
-						}
-					}
-				}
-			case *PaginatedRuleDiscovery:
-				for _, ruleGroup := range v.RuleGroups {
-					ruleGroup.EvaluationTime = float64(0)
-					ruleGroup.LastEvaluation = time.Time{}
-					for k, rule := range ruleGroup.Rules {
-						switch r := rule.(type) {
-						case AlertingRulePaginated:
-							r.LastEvaluation = time.Time{}
-							r.EvaluationTime = float64(0)
-							r.LastError = ""
-							r.Health = "ok"
-							for _, alert := range r.PartialAlerts.Alerts {
-								alert.ActiveAt = nil
-							}
-							ruleGroup.Rules[k] = r
-						case RecordingRule:
-							r.LastEvaluation = time.Time{}
-							r.EvaluationTime = float64(0)
-							r.LastError = ""
-							r.Health = "ok"
-							ruleGroup.Rules[k] = r
 						}
+						ruleGroup.Rules[k] = r
+					case RecordingRule:
+						r.LastEvaluation = time.Time{}
+						r.EvaluationTime = float64(0)
+						r.LastError = ""
+						r.Health = "ok"
+						ruleGroup.Rules[k] = r
 					}
 				}
 			}
@@ -2432,7 +2406,7 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 			query: url.Values{
 				"maxRuleGroups": []string{"1"},
 			},
-			response: &PaginatedRuleDiscovery{
+			response: &RuleDiscovery{
 				NextToken: getRuleGroupNextToken("/path/to/file", "grp"),
 				RuleGroups: []*RuleGroup{
 					{
@@ -2441,43 +2415,41 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 						Interval: 1,
 						Limit:    0,
 						Rules: []Rule{
-							AlertingRulePaginated{
+							AlertingRule{
-								State:         "inactive",
+								State:       "inactive",
-								Name:          "test_metric3",
+								Name:        "test_metric3",
-								Query:         "absent(test_metric3) != 1",
+								Query:       "absent(test_metric3) != 1",
-								Duration:      1,
+								Duration:    1,
-								Labels:        labels.Labels{},
+								Labels:      labels.Labels{},
-								Annotations:   labels.Labels{},
+								Annotations: labels.Labels{},
-								PartialAlerts: &PartialAlerts{Alerts: []*Alert{}},
+								Alerts:      []*Alert{},
-								Health:        "ok",
+								Health:      "ok",
-								Type:          "alerting",
+								Type:        "alerting",
 							},
-							AlertingRulePaginated{
+							AlertingRule{
-								State:         "inactive",
+								State:       "inactive",
-								Name:          "test_metric4",
+								Name:        "test_metric4",
-								Query:         "up == 1",
+								Query:       "up == 1",
-								Duration:      1,
+								Duration:    1,
-								Labels:        labels.Labels{},
+								Labels:      labels.Labels{},
-								Annotations:   labels.Labels{},
+								Annotations: labels.Labels{},
-								PartialAlerts: &PartialAlerts{Alerts: []*Alert{}},
+								Alerts:      []*Alert{},
-								Health:        "ok",
+								Health:      "ok",
-								Type:          "alerting",
+								Type:        "alerting",
 							},
-							AlertingRulePaginated{
+							AlertingRule{
 								State:       "pending",
 								Name:        "test_metric5",
 								Query:       "vector(1)",
 								Duration:    1,
 								Labels:      labels.FromStrings("name", "tm5"),
 								Annotations: labels.Labels{},
-								PartialAlerts: &PartialAlerts{
+								Alerts: []*Alert{
-									Alerts: []*Alert{
+									{
-										{
+										Labels:      labels.FromStrings("alertname", "test_metric5", "name", "tm5"),
-											Labels:      labels.FromStrings("alertname", "test_metric5", "name", "tm5"),
+										Annotations: labels.Labels{},
-											Annotations: labels.Labels{},
+										State:       "pending",
-											State:       "pending",
+										Value:       "1e+00",
-											Value:       "1e+00",
-										},
 									},
 								},
 								Health: "ok",
@@ -2496,87 +2468,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E
 			},
 			zeroFunc: rulesZeroFunc,
 		},
-		{
-			endpoint: api.rules,
-			query: url.Values{
-				"maxAlerts": []string{"0"},
-			},
-			response: &PaginatedRuleDiscovery{
-				RuleGroups: []*RuleGroup{
-					{
-						Name:     "grp",
-						File:     "/path/to/file",
-						Interval: 1,
-						Limit:    0,
-						Rules: []Rule{
-							AlertingRulePaginated{
-								State:         "inactive",
-								Name:          "test_metric3",
-								Query:         "absent(test_metric3) != 1",
-								Duration:      1,
-								Labels:        labels.Labels{},
-								Annotations:   labels.Labels{},
-								PartialAlerts: &PartialAlerts{Alerts: []*Alert{}},
-								Health:        "ok",
-								Type:          "alerting",
-							},
-							AlertingRulePaginated{
-								State:         "inactive",
-								Name:          "test_metric4",
-								Query:         "up == 1",
-								Duration:      1,
-								Labels:        labels.Labels{},
-								Annotations:   labels.Labels{},
-								PartialAlerts: &PartialAlerts{Alerts: []*Alert{}},
-								Health:        "ok",
-								Type:          "alerting",
-							},
-							AlertingRulePaginated{
-								State:       "pending",
-								Name:        "test_metric5",
-								Query:       "vector(1)",
-								Duration:    1,
-								Labels:      labels.FromStrings("name", "tm5"),
-								Annotations: labels.Labels{},
-								PartialAlerts: &PartialAlerts{
-									Alerts:  []*Alert{},
-									HasMore: true,
-								},
-								Health: "ok",
-								Type:   "alerting",
-							},
-							RecordingRule{
-								Name:   "recording-rule-1",
-								Query:  "vector(1)",
-								Labels: labels.Labels{},
-								Health: "ok",
-								Type:   "recording",
-							},
-						},
-					},
-					{
-						Name:     "grp2",
-						File:     "/path/to/file",
-						Interval: 1,
-						Limit:    0,
-						Rules: []Rule{
-							AlertingRulePaginated{
-								State:         "inactive",
-								Name:          "test_metric3",
-								Query:         "absent(test_metric3) != 1",
-								Duration:      1,
-								Labels:        labels.Labels{},
-								Annotations:   labels.Labels{},
-								PartialAlerts: &PartialAlerts{Alerts: []*Alert{}},
-								Health:        "ok",
-								Type:          "alerting",
-							},
-						},
-					},
-				},
-			},
-			zeroFunc: rulesZeroFunc,
-		},
 		{
 			endpoint: api.queryExemplars,
 			query: url.Values{