diff --git a/inhibit/inhibit_bench_test.go b/inhibit/inhibit_bench_test.go index fc93383964..98784248aa 100644 --- a/inhibit/inhibit_bench_test.go +++ b/inhibit/inhibit_bench_test.go @@ -62,6 +62,9 @@ func BenchmarkMutes(b *testing.B) { b.Run("1 inhibition rule, 10000 inhibiting alerts", func(b *testing.B) { benchmarkMutes(b, allRulesMatchBenchmark(b, 1, 10000)) }) + b.Run("1 inhibition rule, 10000 same-equal alerts, source-only candidate", func(b *testing.B) { + benchmarkMutes(b, sameEqualSourceOnlyBenchmark(b, 10000)) + }) b.Run("100 inhibition rules, 1000 inhibiting alerts", func(b *testing.B) { benchmarkMutes(b, allRulesMatchBenchmark(b, 100, 1000)) }) @@ -140,6 +143,58 @@ func allRulesMatchBenchmark(b *testing.B, numInhibitionRules, numInhibitingAlert } } +func sameEqualSourceOnlyBenchmark(b *testing.B, numInhibitingAlerts int) benchmarkOptions { + now := time.Now() + + return benchmarkOptions{ + n: 1, + newRuleFunc: func(_ int) amcommoncfg.InhibitRule { + return amcommoncfg.InhibitRule{ + SourceMatchers: amcommoncfg.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "src", "1"), + }, + TargetMatchers: amcommoncfg.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "dst", "1"), + }, + Equal: []string{"eq"}, + } + }, + newAlertsFunc: func(_ int, _ amcommoncfg.InhibitRule) []types.Alert { + alerts := make([]types.Alert, 0, numInhibitingAlerts+1) + for i := range numInhibitingAlerts { + alerts = append(alerts, types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{ + "src": model.LabelValue("1"), + "eq": model.LabelValue("1"), + "idx": model.LabelValue(strconv.Itoa(i)), + }, + EndsAt: now.Add(time.Hour), + }, + }) + } + alerts = append(alerts, types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{ + "src": model.LabelValue("1"), + "dst": model.LabelValue("1"), + "eq": model.LabelValue("1"), + "idx": model.LabelValue("two-sided"), + }, + EndsAt: now.Add(2 * time.Hour), + }, + }) + return alerts + }, + benchFunc: func(mutesFunc func(context.Context, model.LabelSet) bool) error { + if ok := mutesFunc(context.Background(), model.LabelSet{"src": "1", "dst": "1", "eq": "1"}); !ok { + return errors.New("expected source-and-target alert to be muted by a source-only alert") + } + return nil + }, + } +} + // lastRuleMatchesBenchmark returns a new benchmark where the last inhibition // rule inhibits the label dst=0. All other inhibition rules are no-ops. // diff --git a/inhibit/inhibit_test.go b/inhibit/inhibit_test.go index 3ceafffe7d..092a4c3bf4 100644 --- a/inhibit/inhibit_test.go +++ b/inhibit/inhibit_test.go @@ -52,16 +52,34 @@ func checkMutes(t *testing.T, ih *Inhibitor, target model.LabelSet, wantMuted bo } } +// runInhibitor returns an inhibitor that has processed alerts and stopped, so +// each rule's source cache and index hold what processAlert put there. +func runInhibitor(t *testing.T, rules []amcommoncfg.InhibitRule, alerts ...*alert.Alert) *Inhibitor { + t.Helper() + + ap := newFakeAlerts(alerts) + ih := NewInhibitor(ap, rules, nopLogger, eventrecorder.NopRecorder()) + go func() { + <-ap.finished + ih.Stop() + }() + ih.Run() + + return ih +} + func TestInhibitRuleHasEqual(t *testing.T) { t.Parallel() now := time.Now() cases := []struct { - name string - initial map[model.Fingerprint]*alert.Alert - equal model.LabelNames - input model.LabelSet - result bool + name string + initial map[model.Fingerprint]*alert.Alert + equal model.LabelNames + targetMatchers labels.Matchers + input model.LabelSet + excludeTwoSidedMatch bool + result bool }{ { name: "no source alerts", @@ -141,14 +159,41 @@ func TestInhibitRuleHasEqual(t *testing.T) { input: model.LabelSet{"a": "b"}, result: false, }, + { + name: "matching source-only alert still inhibits when newest equal source is two-sided", + initial: map[model.Fingerprint]*alert.Alert{ + 1: { + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "e": "1"}, + StartsAt: now.Add(-time.Minute), + EndsAt: now.Add(time.Hour), + }, + }, + 2: { + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "t": "1", "e": "1"}, + StartsAt: now.Add(-time.Minute), + EndsAt: now.Add(2 * time.Hour), + }, + }, + }, + equal: model.LabelNames{"e"}, + targetMatchers: labels.Matchers{{Type: labels.MatchEqual, Name: "t", Value: "1"}}, + input: model.LabelSet{"s": "1", "t": "1", "e": "1"}, + // The indexed two-sided source must be ignored, but the source-only + // alert with the same equal labels should still inhibit the target. + excludeTwoSidedMatch: true, + result: true, + }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { r := &InhibitRule{ - Equal: map[model.LabelName]struct{}{}, - scache: store.NewAlerts(), - sindex: newIndex(), + Equal: map[model.LabelName]struct{}{}, + TargetMatchers: c.targetMatchers, + scache: store.NewAlerts(), + sindex: newIndex(), } for _, ln := range c.equal { r.Equal[ln] = struct{}{} @@ -158,13 +203,79 @@ func TestInhibitRuleHasEqual(t *testing.T) { r.sindex.Add(r.fingerprintEquals(v.Labels), v.Fingerprint()) } - if _, have := r.hasEqual(c.input, false, time.Now()); have != c.result { + if _, have := r.hasEqual(c.input, c.excludeTwoSidedMatch, time.Now()); have != c.result { t.Errorf("Unexpected result %t, expected %t", have, c.result) } }) } } +func TestInhibitRuleHasEqualKeepsSourceOnlyAlertAfterGCSameEqual(t *testing.T) { + t.Parallel() + + now := time.Now() + sourceOnly := &alert.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "e": "1", "id": "source-only"}, + StartsAt: now.Add(-time.Minute), + EndsAt: now.Add(time.Hour), + }, + } + expiredSameEqual := &alert.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "e": "1", "id": "expired"}, + StartsAt: now.Add(-2 * time.Hour), + EndsAt: now.Add(-time.Hour), + }, + } + + ih := runInhibitor(t, []amcommoncfg.InhibitRule{{ + TargetMatch: map[string]string{"t": "1"}, + Equal: []string{"e"}, + }}, sourceOnly, expiredSameEqual) + r := ih.rules[0] + + target := model.LabelSet{"s": "1", "t": "1", "e": "1"} + _, found := r.hasEqual(target, true, now) + require.True(t, found) + + r.gcCallback([]*alert.Alert{expiredSameEqual}) + + _, found = r.hasEqual(target, true, now) + require.True(t, found) +} + +func TestInhibitRuleGCCallbackDoesNotRemoveRefreshedSameFingerprintSourceAlert(t *testing.T) { + t.Parallel() + t.Skip("gcCallback races the inhibitor and drops an index entry the cache still holds, skipped until the fix lands") + + now := time.Now() + oldSource := &alert.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "e": "1"}, + StartsAt: now.Add(-2 * time.Hour), + EndsAt: now.Add(-time.Hour), + }, + UpdatedAt: now.Add(-time.Hour), + } + refreshedSource := &alert.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"s": "1", "e": "1"}, + StartsAt: now.Add(-2 * time.Hour), + EndsAt: now.Add(time.Hour), + }, + UpdatedAt: now, + } + + ih := runInhibitor(t, []amcommoncfg.InhibitRule{{Equal: []string{"e"}}}, oldSource, refreshedSource) + r := ih.rules[0] + + r.gcCallback([]*alert.Alert{oldSource}) + + _, found := r.hasEqual(model.LabelSet{"t": "1", "e": "1"}, false, now) + require.True(t, found) +} + func TestInhibitRuleMatches(t *testing.T) { t.Parallel()