Remove duplicated reports from promql/vector_matching

docs/changelog.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## v0.80.0
+
+### Fixed
+
+- [promql/vector_matching](checks/promql/vector_matching.md) check no longer reports problems that are also reported by [promql/impossible](checks/promql/impossible.md).
+
 ## v0.79.0
 
 ### Changed

internal/checks/promql_impossible_test.snap

@@ -2513,11 +2513,6 @@
 
 ---
 
-[TestImpossibleCheck/match_on_group_right_label - 1]
-[]
-
----
-
 [TestImpossibleCheck/max_by()_unless - 1]
 []
 

internal/checks/promql_vector_matching.go

@@ -139,6 +139,12 @@ func (c VectorMatchingCheck) checkNode(ctx context.Context, rule parser.Rule, ex
 			}
 		}
 
+		lhsSources := source.LabelsSource(expr.Value.Value, node.Expr.(*promParser.BinaryExpr).LHS)
+		rhsSources := source.LabelsSource(expr.Value.Value, node.Expr.(*promParser.BinaryExpr).RHS)
+		if !canJoinStatically(lhsSources, rhsSources, n.VectorMatching) {
+			goto NEXT
+		}
+
 		leftLabels, leftURI, err := c.seriesLabels(ctx, n.LHS.String(), ignored...)
 		if err != nil {
 			problems = append(problems, problemFromError(err, rule, c.Reporter(), c.prom.Name(), Bug))
@@ -284,6 +290,39 @@ NEXT:
 	return problems
 }
 
+func canJoinStatically(lhsSources, rhsSources []source.Source, vm *promParser.VectorMatching) bool {
+	for _, ls := range lhsSources {
+		for _, rs := range rhsSources {
+			switch {
+			case vm.On && len(vm.MatchingLabels) == 0:
+				return true
+			case vm.On:
+				for _, name := range vm.MatchingLabels {
+					if ls.CanHaveLabel(name) && !rs.CanHaveLabel(name) {
+						return false
+					}
+					if rs.CanHaveLabel(name) && !ls.CanHaveLabel(name) {
+						return false
+					}
+				}
+			default:
+				for name, l := range ls.Labels {
+					if l.Kind != source.GuaranteedLabel {
+						continue
+					}
+					if slices.Contains(vm.MatchingLabels, name) {
+						continue
+					}
+					if !rs.CanHaveLabel(name) {
+						return false
+					}
+				}
+			}
+		}
+	}
+	return true
+}
+
 func (c VectorMatchingCheck) seriesLabels(ctx context.Context, query string, ignored ...model.LabelName) (labelSets, string, error) {
 	var expr strings.Builder
 	expr.WriteString(wrapExpr(query, "count"))

internal/checks/promql_vector_matching_test.go

@@ -1178,6 +1178,106 @@ func TestVectorMatchingCheck(t *testing.T) {
 			},
 			problems: true,
 		},
+		// Verifies that promql_vector_matching doesn't report when the join is statically
+		// impossible - the promql_impossible check already handles this case.
+		// sum by(instance) removes job from LHS, but on(instance, job) requires it.
+		{
+			description: "skip when join is statically impossible via on()",
+			content:     "- record: foo\n  expr: sum by(instance) (foo) * on(instance, job) bar\n",
+			checker:     newVectorMatchingCheck,
+			prometheus:  newSimpleProm,
+			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nsum by (instance) (foo) * on (instance, job) bar\n)"},
+					},
+					resp: respondWithEmptyVector(),
+				},
+			},
+		},
+		// Verifies that vector_matching doesn't report when the join is statically
+		// impossible due to label mismatch without on/ignoring modifiers.
+		// LHS has instance as guaranteed label via the selector, but sum(bar)
+		// removes all labels making instance structurally impossible on RHS.
+		{
+			description: "skip when join is statically impossible via guaranteed labels",
+			content:     "- record: foo\n  expr: foo{instance=\"a\"} / sum(bar)\n",
+			checker:     newVectorMatchingCheck,
+			prometheus:  newSimpleProm,
+			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nfoo{instance=\"a\"} / sum(bar)\n)"},
+					},
+					resp: respondWithEmptyVector(),
+				},
+			},
+		},
+		// Verifies that vector_matching skips when on() label is structurally
+		// impossible on the RHS. LHS (foo) can have job, but RHS sum by(instance)
+		// removes job, hitting the ls.CanHaveLabel && !rs.CanHaveLabel branch.
+		{
+			description: "skip when on() label is statically impossible on RHS",
+			content:     "- record: foo\n  expr: foo * on(instance, job) sum by(instance) (bar)\n",
+			checker:     newVectorMatchingCheck,
+			prometheus:  newSimpleProm,
+			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nfoo * on (instance, job) sum by (instance) (bar)\n)"},
+					},
+					resp: respondWithEmptyVector(),
+				},
+			},
+		},
+		// Verifies that ignoring() with a guaranteed label in the ignore list
+		// doesn't cause a false skip. LHS guarantees instance via the selector,
+		// and instance is in the ignoring() list so it should be excluded from
+		// the label comparison, allowing the check to proceed normally.
+		{
+			description: "ignoring() skips guaranteed label in ignore list",
+			content:     "- record: foo\n  expr: foo{instance=\"a\"} / ignoring(instance) bar\n",
+			checker:     newVectorMatchingCheck,
+			prometheus:  newSimpleProm,
+			mocks: []*prometheusMock{
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nfoo{instance=\"a\"} / ignoring (instance) bar\n)"},
+					},
+					resp: respondWithEmptyVector(),
+				},
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nfoo{instance=\"a\"}\n) without(__name__,instance)"},
+					},
+					resp: vectorResponse{
+						samples: []*model.Sample{
+							generateSample(map[string]string{
+								"job": "aaa",
+							}),
+						},
+					},
+				},
+				{
+					conds: []requestCondition{
+						requireQueryPath,
+						formCond{key: "query", value: "count(\nbar\n) without(__name__,instance)"},
+					},
+					resp: vectorResponse{
+						samples: []*model.Sample{
+							generateSample(map[string]string{
+								"job": "aaa",
+							}),
+						},
+					},
+				},
+			},
+		},
 	}
 	runTests(t, testCases)
 }

internal/checks/promql_vector_matching_test.snap

@@ -190,6 +190,11 @@
 
 ---
 
+[TestVectorMatchingCheck/ignoring()_skips_guaranteed_label_in_ignore_list - 1]
+[]
+
+---
+
 [TestVectorMatchingCheck/max()_by(a)_and_on(a)_foo - 1]
 - description: max() by(a) and on(a) foo
   content: "\n- record: foo\n  expr: |\n    max by (cluster, env) (\n      increase(error_total{}[10m])\n    ) > 0\n    and on (cluster)\n    cluster_metadata{cluster=\"foo\", environment=\"prod\"} > 0\n  "
@@ -400,6 +405,21 @@
 
 ---
 
+[TestVectorMatchingCheck/skip_when_join_is_statically_impossible_via_guaranteed_labels - 1]
+[]
+
+---
+
+[TestVectorMatchingCheck/skip_when_join_is_statically_impossible_via_on() - 1]
+[]
+
+---
+
+[TestVectorMatchingCheck/skip_when_on()_label_is_statically_impossible_on_RHS - 1]
+[]
+
+---
+
 [TestVectorMatchingCheck/skips_number_comparison_on_LHS - 1]
 []