Refactor utils.LabelsSource

cloudflare · Oct 29, 2024 · 4b70d4a · 4b70d4a
1 parent 7d2a022
commit 4b70d4a
Show file tree

Hide file tree

Showing 6 changed files with 1,294 additions and 1,133 deletions.
diff --git a/internal/checks/alerts_absent.go b/internal/checks/alerts_absent.go
@@ -59,8 +59,7 @@ func (c AlertsAbsentCheck) Check(ctx context.Context, _ discovery.Path, rule par
 	}
 
 	var hasAbsent bool
-	src := utils.LabelsSource(rule.AlertingRule.Expr.Value.Value, rule.AlertingRule.Expr.Query.Expr)
-	for _, s := range append(src.Alternatives, src) {
+	for _, s := range utils.LabelsSource(rule.AlertingRule.Expr.Value.Value, rule.AlertingRule.Expr.Query.Expr) {
 		if s.Operation == "absent" {
 			hasAbsent = true
 		}

diff --git a/internal/checks/alerts_template.go b/internal/checks/alerts_template.go
@@ -430,7 +430,7 @@ func findTemplateVariables(name, text string) (vars [][]string, aliases aliasMap
 	return vars, aliases, true
 }
 
-func checkQueryLabels(query, labelName, labelValue string, src utils.Source) (problems []exprProblem) {
+func checkQueryLabels(query, labelName, labelValue string, src []utils.Source) (problems []exprProblem) {
 	vars, aliases, ok := findTemplateVariables(labelName, labelValue)
 	if !ok {
 		return nil
@@ -444,7 +444,7 @@ func checkQueryLabels(query, labelName, labelValue string, src utils.Source) (pr
 				if _, ok := done[v[1]]; ok {
 					continue
 				}
-				for _, s := range append(src.Alternatives, src) {
+				for _, s := range src {
 					if s.FixedLabels && !slices.Contains(s.IncludedLabels, v[1]) {
 						problems = append(problems, textForProblem(query, v[1], "", s, Bug))
 						goto NEXT

diff --git a/internal/checks/alerts_template_test.go b/internal/checks/alerts_template_test.go
@@ -646,7 +646,7 @@ func TestTemplateCheck(t *testing.T) {
 							Last:  5,
 						},
 						Reporter: checks.TemplateCheckName,
-						Text:     "Template is using `job` label but `absent(bar)` is not passing it.",
+						Text:     "Template is using `job` label but `absent(foo)` is not passing it.",
 						Details:  checks.TemplateCheckAbsentDetails,
 						Severity: checks.Bug,
 					},

diff --git a/internal/checks/promql_fragile.go b/internal/checks/promql_fragile.go
@@ -127,8 +127,7 @@ NEXT:
 }
 
 func (c FragileCheck) checkSampling(expr string, node promParser.Node) (problems []exprProblem) {
-	s := utils.LabelsSource(expr, node)
-	for _, src := range append(s.Alternatives, s) {
+	for _, src := range utils.LabelsSource(expr, node) {
 		if src.Type != utils.AggregateSource {
 			continue
 		}

diff --git a/internal/parser/utils/source.go b/internal/parser/utils/source.go
@@ -35,13 +35,17 @@ type Source struct {
 	IncludedLabels   []string // Labels that are included by filters, they will be present if exist on source series (by).
 	ExcludedLabels   []string // Labels guaranteed to be excluded from the results (without).
 	GuaranteedLabels []string // Labels guaranteed to be present on the results (matchers).
-	Alternatives     []Source // Alternative lable sources
+	alternatives     []Source // Alternative lable sources
 	Type             SourceType
 	FixedLabels      bool // Labels are fixed and only allowed labels can be present.
 }
 
-func LabelsSource(expr string, node promParser.Node) Source {
-	return walkNode(expr, node)
+func LabelsSource(expr string, node promParser.Node) (src []Source) {
+	s := walkNode(expr, node)
+	src = make([]Source, 0, len(s.alternatives)+1)
+	src = append(src, s)
+	src = append(src, s.alternatives...)
+	return src
 }
 
 func walkNode(expr string, node promParser.Node) (s Source) {
@@ -480,7 +484,7 @@ func parseBinOps(expr string, n *promParser.BinaryExpr) (s Source) {
 			}
 		}
 		if n.Op == promParser.LOR {
-			s.Alternatives = append(s.Alternatives, walkNode(expr, n.RHS))
+			s.alternatives = append(s.alternatives, walkNode(expr, n.RHS))
 		}
 	}