Fix: exclude metric name in labels.MatchLabels() (#6523)

This function is only used in one place to format an error message when
encountering multiple matches on the "one" side of a
one-to-many/many-to-one vector match, which is probably why nobodoy has
noticed this before. The hashing is already done correctly and excludes
the metric name label when using the "ignoring" matching modifier.

Signed-off-by: Julius Volz <julius.volz@gmail.com>

pkg/labels/labels.go

@@ -109,7 +109,7 @@ func (ls Labels) MatchLabels(on bool, names ...string) Labels {
 	}
 
 	for _, v := range ls {
-		if _, ok := nameSet[v.Name]; on == ok {
+		if _, ok := nameSet[v.Name]; on == ok && (on || v.Name != MetricName) {
 			matchedLabels = append(matchedLabels, v)
 		}
 	}

pkg/labels/labels_test.go

@@ -15,6 +15,8 @@ package labels
 
 import (
 	"testing"
+
+	"github.com/prometheus/prometheus/util/testutil"
 )
 
 func TestLabels_MatchLabels(t *testing.T) {
@@ -45,60 +47,105 @@ func TestLabels_MatchLabels(t *testing.T) {
 		},
 	}
 
-	providedNames := []string{
-		"__name__",
-		"alertname",
-		"alertstate",
-		"instance",
-	}
-
-	got := labels.MatchLabels(true, providedNames...)
-	expected := Labels{
+	tests := []struct {
+		providedNames []string
+		on            bool
+		expected      Labels
+	}{
+		// on = true, explicitly including metric name in matching.
 		{
-			Name:  "__name__",
-			Value: "ALERTS",
+			providedNames: []string{
+				"__name__",
+				"alertname",
+				"alertstate",
+				"instance",
+			},
+			on: true,
+			expected: Labels{
+				{
+					Name:  "__name__",
+					Value: "ALERTS",
+				},
+				{
+					Name:  "alertname",
+					Value: "HTTPRequestRateLow",
+				},
+				{
+					Name:  "alertstate",
+					Value: "pending",
+				},
+				{
+					Name:  "instance",
+					Value: "0",
+				},
+			},
 		},
+		// on = false, explicitly excluding metric name from matching.
 		{
-			Name:  "alertname",
-			Value: "HTTPRequestRateLow",
+			providedNames: []string{
+				"__name__",
+				"alertname",
+				"alertstate",
+				"instance",
+			},
+			on: false,
+			expected: Labels{
+				{
+					Name:  "job",
+					Value: "app-server",
+				},
+				{
+					Name:  "severity",
+					Value: "critical",
+				},
+			},
 		},
+		// on = true, explicitly excluding metric name from matching.
 		{
-			Name:  "alertstate",
-			Value: "pending",
+			providedNames: []string{
+				"alertname",
+				"alertstate",
+				"instance",
+			},
+			on: true,
+			expected: Labels{
+				{
+					Name:  "alertname",
+					Value: "HTTPRequestRateLow",
+				},
+				{
+					Name:  "alertstate",
+					Value: "pending",
+				},
+				{
+					Name:  "instance",
+					Value: "0",
+				},
+			},
 		},
+		// on = false, implicitly excluding metric name from matching.
 		{
-			Name:  "instance",
-			Value: "0",
+			providedNames: []string{
+				"alertname",
+				"alertstate",
+				"instance",
+			},
+			on: false,
+			expected: Labels{
+				{
+					Name:  "job",
+					Value: "app-server",
+				},
+				{
+					Name:  "severity",
+					Value: "critical",
+				},
+			},
 		},
 	}
 
-	assertSlice(t, got, expected)
-
-	// Now try with 'on' set to false.
-	got = labels.MatchLabels(false, providedNames...)
-
-	expected = Labels{
-		{
-			Name:  "job",
-			Value: "app-server",
-		},
-		{
-			Name:  "severity",
-			Value: "critical",
-		},
-	}
-
-	assertSlice(t, got, expected)
-}
-
-func assertSlice(t *testing.T, got, expected Labels) {
-	if len(expected) != len(got) {
-		t.Errorf("expected the length of matched label names to be %d, but got %d", len(expected), len(got))
-	}
-
-	for i, expectedLabel := range expected {
-		if expectedLabel.Name != got[i].Name {
-			t.Errorf("expected to get Label with name %s, but got %s instead", expectedLabel.Name, got[i].Name)
-		}
+	for i, test := range tests {
+		got := labels.MatchLabels(test.on, test.providedNames...)
+		testutil.Equals(t, test.expected, got, "unexpected labelset for test case %d", i)
 	}
 }