Merge pull request #882 from iksaif/dedup-matchers

Make sure Matchers are always ordered

dispatch/dispatch_test.go

@@ -286,7 +286,7 @@ func TestAggrGroup(t *testing.T) {
 		}
 
 		if !ag.empty() {
-			t.Fatalf("Expected aggregation group to be empty after resolving alerts")
+			t.Fatalf("Expected aggregation group to be empty after resolving alerts: %v", ag)
 		}
 	}
 

dispatch/route.go

@@ -16,6 +16,7 @@ package dispatch
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 	"time"
 
 	"github.com/prometheus/common/model"
@@ -89,6 +90,7 @@ func NewRoute(cr *config.Route, parent *Route) *Route {
 	for ln, lv := range cr.MatchRE {
 		matchers = append(matchers, types.NewRegexMatcher(model.LabelName(ln), lv.Regexp))
 	}
+	sort.Sort(matchers)
 
 	route := &Route{
 		parent:    parent,

dispatch/route_test.go

@@ -51,6 +51,7 @@ routes:
 
   - match_re:
       env: "produ.*"
+      job: ".*"
 
     receiver: 'notify-productionB'
     group_wait: 30s
@@ -99,6 +100,7 @@ routes:
 	tests := []struct {
 		input  model.LabelSet
 		result []*RouteOpts
+		keys   []string
 	}{
 		{
 			input: model.LabelSet{
@@ -113,6 +115,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{owner=\"team-A\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -128,6 +131,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{owner=\"team-A\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -142,6 +146,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{owner=~\"^(?:team-(B|C))$\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -157,6 +162,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{owner=\"team-A\"}/{env=\"testing\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -179,6 +185,10 @@ routes:
 					RepeatInterval: 1 * time.Hour,
 				},
 			},
+			keys: []string{
+				"{}/{owner=\"team-A\"}/{env=\"production\"}",
+				"{}/{owner=\"team-A\"}/{env=~\"^(?:produ.*)$\",job=~\"^(?:.*)$\"}",
+			},
 		},
 		{
 			input: model.LabelSet{
@@ -193,6 +203,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{group_by=\"role\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -208,6 +219,7 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{group_by=\"role\"}/{env=\"testing\"}"},
 		},
 		{
 			input: model.LabelSet{
@@ -224,17 +236,25 @@ routes:
 					RepeatInterval: def.RepeatInterval,
 				},
 			},
+			keys: []string{"{}/{group_by=\"role\"}/{env=\"testing\"}/{wait=\"long\"}"},
 		},
 	}
 
 	for _, test := range tests {
 		var matches []*RouteOpts
+		var keys []string
+
 		for _, r := range tree.Match(test.input) {
 			matches = append(matches, &r.RouteOpts)
+			keys = append(keys, r.Key())
 		}
 
 		if !reflect.DeepEqual(matches, test.result) {
 			t.Errorf("\nexpected:\n%v\ngot:\n%v", test.result, matches)
 		}
+
+		if !reflect.DeepEqual(keys, test.keys) {
+			t.Errorf("\nexpected:\n%v\ngot:\n%v", test.keys, keys)
+		}
 	}
 }

types/types_test.go

@@ -15,12 +15,49 @@ package types
 
 import (
 	"reflect"
+	"regexp"
 	"testing"
 	"time"
 
 	"github.com/prometheus/common/model"
 )
 
+func TestMatcher(t *testing.T) {
+	m := NewMatcher("foo", "bar")
+
+	if m.String() != "foo=\"bar\"" {
+		t.Errorf("unexpected matcher string %#v", m.String())
+	}
+
+	re, err := regexp.Compile(".*")
+	if err != nil {
+		t.Errorf("unexpected error: %s", err)
+	}
+
+	m = NewRegexMatcher("foo", re)
+
+	if m.String() != "foo=~\".*\"" {
+		t.Errorf("unexpected matcher string %#v", m.String())
+	}
+}
+
+func TestMatchers(t *testing.T) {
+	m1 := NewMatcher("foo", "bar")
+
+	re, err := regexp.Compile(".*")
+	if err != nil {
+		t.Errorf("unexpected error: %s", err)
+	}
+
+	m2 := NewRegexMatcher("bar", re)
+
+	matchers := NewMatchers(m1, m2)
+
+	if matchers.String() != "{bar=~\".*\",foo=\"bar\"}" {
+		t.Errorf("unexpected matcher string %#v", matchers.String())
+	}
+}
+
 func TestAlertMerge(t *testing.T) {
 	now := time.Now()