From 57592168b090f0aab1d50f115047947278baabcf Mon Sep 17 00:00:00 2001 From: Raphael Silva Date: Thu, 4 Jul 2024 05:26:20 +0000 Subject: [PATCH] Simplify paginated implementation * Remove maxAlerts parameter. * Reuse existing API responses by using omitempty in some fields Signed-off-by: Raphael Silva --- web/api/v1/api.go | 110 +++++----------------- web/api/v1/api_test.go | 205 ++++++++++------------------------------- 2 files changed, 70 insertions(+), 245 deletions(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index e70991df330..e42620c3445 100644 --- a/web/api/v1/api.go +++ b/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"` -} - -// PaginatedRuleDiscovery has info for all rules with pagination. -type PaginatedRuleDiscovery struct { - RuleGroups []*RuleGroup `json:"groups"` - NextToken string `json:"nextToken"` + NextToken string `json:"nextToken:omitempty"` } // 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) { - activeAlerts = activeAlerts[:int(paginationRequest.MaxAlerts)] - hasMore = true - } - 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", - } + 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 - paginatedRes.NextToken = nextToken - return apiFuncResult{paginatedRes, nil, nil, nil} + res.NextToken = nextToken + res.RuleGroups = returnGroups + 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 } diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index d90d2cfeb52..ca6cc30515c 100644 --- a/web/api/v1/api_test.go +++ b/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 { - switch v := i.(type) { - case *RuleDiscovery: - for _, ruleGroup := range v.RuleGroups { - ruleGroup.EvaluationTime = float64(0) - ruleGroup.LastEvaluation = time.Time{} - for k, rule := range ruleGroup.Rules { - switch r := rule.(type) { - case AlertingRule: - r.LastEvaluation = time.Time{} - r.EvaluationTime = float64(0) - r.LastError = "" - r.Health = "ok" - for _, alert := range r.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 - } - } - } - 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 + v := i.(*RuleDiscovery) + if v != nil { + for _, ruleGroup := range v.RuleGroups { + ruleGroup.EvaluationTime = float64(0) + ruleGroup.LastEvaluation = time.Time{} + for k, rule := range ruleGroup.Rules { + switch r := rule.(type) { + case AlertingRule: + r.LastEvaluation = time.Time{} + r.EvaluationTime = float64(0) + r.LastError = "" + r.Health = "ok" + for _, alert := range r.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 } } } @@ -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,106 +2415,42 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E 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)", + AlertingRule{ + State: "inactive", + Name: "test_metric3", + Query: "absent(test_metric3) != 1", Duration: 1, - Labels: labels.FromStrings("name", "tm5"), + Labels: labels.Labels{}, Annotations: labels.Labels{}, - PartialAlerts: &PartialAlerts{ - Alerts: []*Alert{ - { - Labels: labels.FromStrings("alertname", "test_metric5", "name", "tm5"), - Annotations: labels.Labels{}, - State: "pending", - Value: "1e+00", - }, - }, - }, - Health: "ok", - Type: "alerting", - }, - RecordingRule{ - Name: "recording-rule-1", - Query: "vector(1)", - Labels: labels.Labels{}, - Health: "ok", - Type: "recording", - }, - }, - }, - }, - }, - 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", + 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", + AlertingRule{ + State: "inactive", + Name: "test_metric4", + Query: "up == 1", + Duration: 1, + Labels: labels.Labels{}, + Annotations: labels.Labels{}, + Alerts: []*Alert{}, + Health: "ok", + 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{}, - HasMore: true, + Alerts: []*Alert{ + { + Labels: labels.FromStrings("alertname", "test_metric5", "name", "tm5"), + Annotations: labels.Labels{}, + State: "pending", + Value: "1e+00", + }, }, Health: "ok", Type: "alerting", @@ -2554,25 +2464,6 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E }, }, }, - { - 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,