Skip to content

Commit

Permalink
Merge pull request #51 from s-urbaniak/bz-1926598
Browse files Browse the repository at this point in the history 
Bug 1926598: pkg/rules: fix deduplication of equal alerts with different labels
  • Loading branch information
@openshift-merge-robot
openshift-merge-robot committed Mar 30, 2021
2 parents 3615fc5 + 06e14f9 commit 5153f7d
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 13 deletions.
9 changes: 4 additions & 5 deletions pkg/rules/rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,23 +68,22 @@ func dedupRules(rules []*rulespb.Rule, replicaLabels map[string]struct{}) []*rul
return rules
}

// Sort each rule's label names such that they are comparable.
// Remove replica labels and sort each rule's label names such that they are comparable.
for _, r := range rules {
removeReplicaLabels(r, replicaLabels)
sort.Slice(r.GetLabels(), func(i, j int) bool {
return r.GetLabels()[i].Name < r.GetLabels()[j].Name
})
}

// Sort rules globally based on synthesized deduplication labels, also considering replica labels and their values.
// Sort rules globally.
sort.Slice(rules, func(i, j int) bool {
return rules[i].Compare(rules[j]) < 0
})

// Remove rules based on synthesized deduplication labels, this time ignoring replica labels and last evaluation.
// Remove rules based on synthesized deduplication labels.
i := 0
removeReplicaLabels(rules[i], replicaLabels)
for j := 1; j < len(rules); j++ {
removeReplicaLabels(rules[j], replicaLabels)
if rules[i].Compare(rules[j]) != 0 {
// Effectively retain rules[j] in the resulting slice.
i++
Expand Down
108 changes: 100 additions & 8 deletions pkg/rules/rules_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,14 +448,14 @@ func TestDedupRules(t *testing.T) {
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Query: "up",
DurationSeconds: 2.0,
DurationSeconds: 1.0,
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "a", Value: "1"},
}}}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Query: "up",
DurationSeconds: 1.0,
DurationSeconds: 2.0,
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "a", Value: "1"},
}}}),
Expand Down Expand Up @@ -505,13 +505,13 @@ func TestDedupRules(t *testing.T) {
}}}),
},
want: []*rulespb.Rule{
rulespb.NewRecordingRule(&rulespb.RecordingRule{Name: "a1"}),
rulespb.NewRecordingRule(&rulespb.RecordingRule{Name: "a1", Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "a", Value: "1"},
}}}),
rulespb.NewRecordingRule(&rulespb.RecordingRule{Name: "a1", Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "a", Value: "2"},
}}}),
rulespb.NewRecordingRule(&rulespb.RecordingRule{Name: "a1"}),
},
replicaLabels: []string{"replica"},
},
Expand Down Expand Up @@ -613,6 +613,11 @@ func TestDedupRules(t *testing.T) {
}),
},
want: []*rulespb.Rule{
rulespb.NewAlertingRule(&rulespb.Alert{
State: rulespb.AlertState_FIRING,
Name: "a1",
LastEvaluation: time.Unix(2, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
State: rulespb.AlertState_FIRING,
Name: "a1",
Expand All @@ -621,11 +626,6 @@ func TestDedupRules(t *testing.T) {
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
State: rulespb.AlertState_FIRING,
Name: "a1",
LastEvaluation: time.Unix(2, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
State: rulespb.AlertState_PENDING,
Name: "a2",
Expand All @@ -634,6 +634,98 @@ func TestDedupRules(t *testing.T) {
},
replicaLabels: []string{"replica"},
},
{
name: "alerts with different severity",
rules: []*rulespb.Rule{
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "1"},
{Name: "severity", Value: "warning"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "1"},
{Name: "severity", Value: "critical"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "2"},
{Name: "severity", Value: "warning"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "2"},
{Name: "severity", Value: "critical"},
}},
LastEvaluation: time.Unix(1, 0),
}),
},
want: []*rulespb.Rule{
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "severity", Value: "critical"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "severity", Value: "warning"},
}},
LastEvaluation: time.Unix(1, 0),
}),
},
replicaLabels: []string{"replica"},
},
{
name: "alerts with missing replica labels",
rules: []*rulespb.Rule{
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "1"},
{Name: "label", Value: "foo"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "replica", Value: "2"},
{Name: "label", Value: "foo"},
}},
LastEvaluation: time.Unix(1, 0),
}),
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "label", Value: "foo"},
}},
LastEvaluation: time.Unix(1, 0),
}),
},
want: []*rulespb.Rule{
rulespb.NewAlertingRule(&rulespb.Alert{
Name: "a1",
Labels: labelpb.ZLabelSet{Labels: []labelpb.ZLabel{
{Name: "label", Value: "foo"},
}},
LastEvaluation: time.Unix(1, 0),
}),
},
replicaLabels: []string{"replica"},
},
} {
t.Run(tc.name, func(t *testing.T) {
replicaLabels := make(map[string]struct{})
Expand Down

0 comments on commit 5153f7d

Please sign in to comment.