Merge pull request #1207 from cloudflare/rate

Use label source for more checks
cloudflare · Nov 28, 2024 · 2a74f5b · 2a74f5b
2 parents aeb0788 + 43111f1
commit 2a74f5b
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 271 deletions.
diff --git a/internal/checks/promql_fragile.go b/internal/checks/promql_fragile.go
@@ -53,7 +53,7 @@ func (c FragileCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rul
 		return nil
 	}
 
-	for _, problem := range c.checkAggregation(expr.Query) {
+	for _, problem := range c.checkAggregation(expr.Value.Value, expr.Query) {
 		problems = append(problems, Problem{
 			Lines:    expr.Value.Lines,
 			Reporter: c.Reporter(),
@@ -78,7 +78,7 @@ func (c FragileCheck) Check(_ context.Context, _ discovery.Path, rule parser.Rul
 	return problems
 }
 
-func (c FragileCheck) checkAggregation(node *parser.PromQLNode) (problems []exprProblem) {
+func (c FragileCheck) checkAggregation(query string, node *parser.PromQLNode) (problems []exprProblem) {
 	if n := utils.HasOuterBinaryExpr(node); n != nil && n.Op != promParser.LOR && n.Op != promParser.LUNLESS {
 		if n.VectorMatching != nil && n.VectorMatching.On {
 			goto NEXT
@@ -91,8 +91,8 @@ func (c FragileCheck) checkAggregation(node *parser.PromQLNode) (problems []expr
 		}
 		var isFragile bool
 		for _, child := range node.Children {
-			for _, agg := range utils.HasOuterAggregation(child) {
-				if agg.Without {
+			for _, src := range utils.LabelsSource(query, child.Expr) {
+				if src.Type == utils.AggregateSource && !src.FixedLabels {
 					isFragile = true
 				}
 			}
@@ -120,7 +120,7 @@ func (c FragileCheck) checkAggregation(node *parser.PromQLNode) (problems []expr
 
 NEXT:
 	for _, child := range node.Children {
-		problems = append(problems, c.checkAggregation(child)...)
+		problems = append(problems, c.checkAggregation(query, child)...)
 	}
 
 	return problems

diff --git a/internal/parser/utils/aggregation.go b/internal/parser/utils/aggregation.go
diff --git a/internal/parser/utils/aggregation_test.go b/internal/parser/utils/aggregation_test.go