From a2a6e24f9fb641e1fbfe82be035df8ebb83d82e8 Mon Sep 17 00:00:00 2001
From: Tariq Ibrahim <tariq181290@gmail.com>
Date: Sat, 9 Feb 2019 01:17:52 -0800
Subject: [PATCH] show list of offending labels in the error message in
 many-to-many scenarios (#5189)

Signed-off-by: tariqibrahim <tariq181290@gmail.com>
---
 pkg/labels/labels.go      |  19 +++++++
 pkg/labels/labels_test.go | 104 ++++++++++++++++++++++++++++++++++++++
 promql/engine.go          |  11 +++-
 3 files changed, 132 insertions(+), 2 deletions(-)
 create mode 100644 pkg/labels/labels_test.go

diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go
index a10f636d7..bfc0ce586 100644
--- a/pkg/labels/labels.go
+++ b/pkg/labels/labels.go
@@ -81,6 +81,25 @@ func (ls *Labels) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+// MatchLabels returns a subset of Labels that matches/does not match with the provided label names based on the 'on' boolean.
+// If on is set to true, it returns the subset of labels that match with the provided label names and its inverse when 'on' is set to false.
+func (ls Labels) MatchLabels(on bool, names ...string) Labels {
+	matchedLabels := Labels{}
+
+	nameSet := map[string]struct{}{}
+	for _, n := range names {
+		nameSet[n] = struct{}{}
+	}
+
+	for _, v := range ls {
+		if _, ok := nameSet[v.Name]; on == ok {
+			matchedLabels = append(matchedLabels, v)
+		}
+	}
+
+	return matchedLabels
+}
+
 // Hash returns a hash value for the label set.
 func (ls Labels) Hash() uint64 {
 	b := make([]byte, 0, 1024)
diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go
new file mode 100644
index 000000000..3651ae5f1
--- /dev/null
+++ b/pkg/labels/labels_test.go
@@ -0,0 +1,104 @@
+// Copyright 2019 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package labels
+
+import (
+	"testing"
+)
+
+func TestLabels_MatchLabels(t *testing.T) {
+	labels := Labels{
+		{
+			Name:  "__name__",
+			Value: "ALERTS",
+		},
+		{
+			Name:  "alertname",
+			Value: "HTTPRequestRateLow",
+		},
+		{
+			Name:  "alertstate",
+			Value: "pending",
+		},
+		{
+			Name:  "instance",
+			Value: "0",
+		},
+		{
+			Name:  "job",
+			Value: "app-server",
+		},
+		{
+			Name:  "severity",
+			Value: "critical",
+		},
+	}
+
+	providedNames := []string{
+		"__name__",
+		"alertname",
+		"alertstate",
+		"instance",
+	}
+
+	got := labels.MatchLabels(true, providedNames...)
+	expected := Labels{
+		{
+			Name:  "__name__",
+			Value: "ALERTS",
+		},
+		{
+			Name:  "alertname",
+			Value: "HTTPRequestRateLow",
+		},
+		{
+			Name:  "alertstate",
+			Value: "pending",
+		},
+		{
+			Name:  "instance",
+			Value: "0",
+		},
+	}
+
+	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)
+		}
+	}
+}
diff --git a/promql/engine.go b/promql/engine.go
index f405724d3..7450dcd38 100644
--- a/promql/engine.go
+++ b/promql/engine.go
@@ -1434,9 +1434,16 @@ func (ev *evaluator) VectorBinop(op ItemType, lhs, rhs Vector, matching *VectorM
 		sig := sigf(rs.Metric)
 		// The rhs is guaranteed to be the 'one' side. Having multiple samples
 		// with the same signature means that the matching is many-to-many.
-		if _, found := rightSigs[sig]; found {
+		if duplSample, found := rightSigs[sig]; found {
+			// oneSide represents which side of the vector represents the 'one' in the many-to-one relationship.
+			oneSide := "right"
+			if matching.Card == CardOneToMany {
+				oneSide = "left"
+			}
+			matchedLabels := rs.Metric.MatchLabels(matching.On, matching.MatchingLabels...)
 			// Many-to-many matching not allowed.
-			ev.errorf("many-to-many matching not allowed: matching labels must be unique on one side")
+			ev.errorf("found duplicate series for the match group %s on the %s hand-side of the operation: [%s, %s]"+
+				";many-to-many matching not allowed: matching labels must be unique on one side", matchedLabels.String(), oneSide, rs.Metric.String(), duplSample.Metric.String())
 		}
 		rightSigs[sig] = rs
 	}