From 2d7491dece46a364951e82610b71ea25fc8d3390 Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Fri, 20 Feb 2026 09:31:00 +0100 Subject: [PATCH 1/6] test: add scaling benchmarks for Inhibitor.Mutes Add BenchmarkMutesScaling with three cases to measure how Mutes() performance scales with rule count: - different_targets: Each rule has unique target matcher, only one matches the alert (best case for selective lookup) - same_target: All rules have same target matcher, all must be checked - no_match: Alert matches no rule's target, all must be checked These benchmarks establish baseline performance for potential optimizations to the inhibition rule matching logic. Signed-off-by: Titouan Chary --- inhibit/inhibit_bench_test.go | 169 ++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/inhibit/inhibit_bench_test.go b/inhibit/inhibit_bench_test.go index f5fa536f4a..56ff56d06e 100644 --- a/inhibit/inhibit_bench_test.go +++ b/inhibit/inhibit_bench_test.go @@ -228,3 +228,172 @@ func mustNewMatcher(b *testing.B, op labels.MatchType, name, value string) *labe require.NoError(b, err) return m } + +func BenchmarkMutesScaling(b *testing.B) { + b.Run("different_targets", func(b *testing.B) { + for _, numRules := range []int{10, 100, 1000} { + b.Run("rules="+strconv.Itoa(numRules), func(b *testing.B) { + benchmarkDifferentTargets(b, numRules) + }) + } + }) + + b.Run("same_target", func(b *testing.B) { + for _, numRules := range []int{10, 100, 1000} { + b.Run("rules="+strconv.Itoa(numRules), func(b *testing.B) { + benchmarkSameTarget(b, numRules) + }) + } + }) + + b.Run("no_match", func(b *testing.B) { + for _, numRules := range []int{10, 100, 1000} { + b.Run("rules="+strconv.Itoa(numRules), func(b *testing.B) { + benchmarkNoMatch(b, numRules) + }) + } + }) +} + +func benchmarkDifferentTargets(b *testing.B, numRules int) { + r := prometheus.NewRegistry() + m := types.NewMarker(r) + s, err := mem.NewAlerts(context.TODO(), m, time.Minute, 0, nil, promslog.NewNopLogger(), r, nil) + require.NoError(b, err) + defer s.Close() + + rules := make([]config.InhibitRule, numRules) + for i := 0; i < numRules; i++ { + rules[i] = config.InhibitRule{ + SourceMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "alertname", "SourceAlert"), + mustNewMatcher(b, labels.MatchEqual, "cluster", strconv.Itoa(i)), + }, + TargetMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "severity", "warning"), + mustNewMatcher(b, labels.MatchEqual, "cluster", strconv.Itoa(i)), + }, + } + } + + // Source alert for the LAST rule (worst case for linear scan) + lastCluster := strconv.Itoa(numRules - 1) + alert := types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{ + "alertname": "SourceAlert", + "cluster": model.LabelValue(lastCluster), + }, + }, + } + require.NoError(b, s.Put(context.Background(), &alert)) + + ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) + defer ih.Stop() + go ih.Run() + <-time.After(time.Second) + + targetLset := model.LabelSet{ + "alertname": "TargetAlert", + "severity": "warning", + "cluster": model.LabelValue(lastCluster), + } + ctx := context.Background() + + b.ResetTimer() + b.ReportAllocs() + + for b.Loop() { + if !ih.Mutes(ctx, targetLset) { + b.Fatal("expected alert to be muted") + } + } +} + +func benchmarkSameTarget(b *testing.B, numRules int) { + r := prometheus.NewRegistry() + m := types.NewMarker(r) + s, err := mem.NewAlerts(context.TODO(), m, time.Minute, 0, nil, promslog.NewNopLogger(), r, nil) + require.NoError(b, err) + defer s.Close() + + rules := make([]config.InhibitRule, numRules) + for i := 0; i < numRules; i++ { + rules[i] = config.InhibitRule{ + SourceMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "src", strconv.Itoa(i)), + }, + TargetMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "dst", "0"), + }, + } + } + + // Source alert for the LAST rule only + alert := types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{ + "src": model.LabelValue(strconv.Itoa(numRules - 1)), + }, + }, + } + require.NoError(b, s.Put(context.Background(), &alert)) + + ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) + defer ih.Stop() + go ih.Run() + <-time.After(time.Second) + + targetLset := model.LabelSet{"dst": "0"} + ctx := context.Background() + + b.ResetTimer() + b.ReportAllocs() + + for b.Loop() { + if !ih.Mutes(ctx, targetLset) { + b.Fatal("expected alert to be muted") + } + } +} + +func benchmarkNoMatch(b *testing.B, numRules int) { + r := prometheus.NewRegistry() + m := types.NewMarker(r) + s, err := mem.NewAlerts(context.TODO(), m, time.Minute, 0, nil, promslog.NewNopLogger(), r, nil) + require.NoError(b, err) + defer s.Close() + + rules := make([]config.InhibitRule, numRules) + for i := 0; i < numRules; i++ { + rules[i] = config.InhibitRule{ + SourceMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "alertname", "SourceAlert"), + }, + TargetMatchers: config.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "cluster", strconv.Itoa(i)), + }, + } + } + + ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) + defer ih.Stop() + go ih.Run() + <-time.After(time.Second) + + // Alert with cluster that doesn't match any rule + targetLset := model.LabelSet{ + "alertname": "TargetAlert", + "cluster": "nonexistent", + } + ctx := context.Background() + + b.ResetTimer() + b.ReportAllocs() + + for b.Loop() { + if ih.Mutes(ctx, targetLset) { + b.Fatal("expected alert to NOT be muted") + } + } +} From fad05f6753dc3a3924ba41230b76d70623c372c8 Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Fri, 20 Feb 2026 09:31:30 +0100 Subject: [PATCH 2/6] refactor: extract checkInhibit from Inhibitor.Mutes Extract the core inhibition checking logic into a separate checkInhibit method. This separates concerns: - Mutes(): handles tracing span lifecycle and marker updates - checkInhibit(): contains the rule iteration logic This refactoring prepares for future optimizations to the rule matching logic without changing the public API or tracing behavior. No functional changes. Signed-off-by: Titouan Chary --- inhibit/inhibit.go | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/inhibit/inhibit.go b/inhibit/inhibit.go index 1c681de385..75b40d6d2a 100644 --- a/inhibit/inhibit.go +++ b/inhibit/inhibit.go @@ -189,33 +189,45 @@ func (ih *Inhibitor) Mutes(ctx context.Context, lset model.LabelSet) bool { ) defer span.End() - now := time.Now() + inhibitedByFP, inhibited := ih.checkInhibit(lset, time.Now(), span) + if inhibited { + ih.marker.SetInhibited(fp, inhibitedByFP.String()) + span.AddEvent("alert inhibited", + trace.WithAttributes( + attribute.String("alerting.inhibit_rule.source.fingerprint", inhibitedByFP.String()), + ), + ) + return true + } + + ih.marker.SetInhibited(fp) + span.AddEvent("alert not inhibited") + return false +} + +// checkInhibit checks whether the given label set is inhibited by any rule. +// Returns the fingerprint of the inhibiting alert and true if inhibited. +// The span parameter is optional and used for per-rule tracing events. +func (ih *Inhibitor) checkInhibit(lset model.LabelSet, now time.Time, span trace.Span) (model.Fingerprint, bool) { for _, r := range ih.rules { if !r.TargetMatchers.Matches(lset) { // If target side of rule doesn't match, we don't need to look any further. continue } - span.AddEvent("alert matched rule target", - trace.WithAttributes( - attribute.String("alerting.inhibit_rule.name", r.Name), - ), - ) - // If we are here, the target side matches. If the source side matches, too, we - // need to exclude inhibiting alerts for which the same is true. - if inhibitedByFP, eq := r.hasEqual(lset, r.SourceMatchers.Matches(lset), now); eq { - ih.marker.SetInhibited(fp, inhibitedByFP.String()) - span.AddEvent("alert inhibited", + if span != nil { + span.AddEvent("alert matched rule target", trace.WithAttributes( - attribute.String("alerting.inhibit_rule.source.fingerprint", inhibitedByFP.String()), + attribute.String("alerting.inhibit_rule.name", r.Name), ), ) - return true + } + // If we are here, the target side matches. If the source side matches, too, we + // need to exclude inhibiting alerts for which the same is true. + if inhibitedByFP, eq := r.hasEqual(lset, r.SourceMatchers.Matches(lset), now); eq { + return inhibitedByFP, true } } - ih.marker.SetInhibited(fp) - span.AddEvent("alert not inhibited") - - return false + return 0, false } // An InhibitRule specifies that a class of (source) alerts should inhibit From be88a9f556cc8cb21aefb1bbec3d51c0d6ad204d Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Fri, 20 Feb 2026 09:33:34 +0100 Subject: [PATCH 3/6] perf: add inverted index for inhibit rule target matcher lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ruleIndex to Inhibitor for O(k) rule lookup instead of O(N) linear scan, where k = number of labels and N = number of inhibit rules. When the index IS effective (O(1) lookup): - Rules have different equality target matchers (e.g., cluster=X) - Alert labels allow direct lookup into the index - Example: 1000 rules each targeting different clusters, checking an alert for cluster=999 → only examines 1 rule instead of 1000 When the index is NOT effective (falls back to O(N) scan): - All rules share the same target matchers (e.g., all target dst=0) - Rules use regex or not-equal matchers (cannot be indexed) - High-overlap matchers excluded from indexing (>50% of rules) Implementation details: - Index rules by exact match target matchers at construction time - Use callback pattern (forEachCandidate) to avoid slice allocation - Pool visited map to reduce GC pressure - Skip deduplication for single-matcher rules Benchmark results (BenchmarkMutesScaling, 1000 rules): different_targets: 32µs → 2.1µs (15x faster, index effective) no_match: 15µs → 1.0µs (15x faster, index effective) same_target: 218µs → 209µs (no change, index not effective) Signed-off-by: Titouan Chary --- inhibit/inhibit.go | 20 ++- inhibit/rule_index.go | 206 ++++++++++++++++++++++ inhibit/rule_index_test.go | 345 +++++++++++++++++++++++++++++++++++++ 3 files changed, 564 insertions(+), 7 deletions(-) create mode 100644 inhibit/rule_index.go create mode 100644 inhibit/rule_index_test.go diff --git a/inhibit/inhibit.go b/inhibit/inhibit.go index 75b40d6d2a..957476b8a8 100644 --- a/inhibit/inhibit.go +++ b/inhibit/inhibit.go @@ -42,6 +42,7 @@ var tracer = otel.Tracer("github.com/prometheus/alertmanager/inhibit") type Inhibitor struct { alerts provider.Alerts rules []*InhibitRule + ruleIdx *ruleIndex marker types.AlertMarker logger *slog.Logger propagator propagation.TextMapPropagator @@ -74,6 +75,7 @@ func NewInhibitor(ap provider.Alerts, rs []config.InhibitRule, mk types.AlertMar ruleNames[cr.Name] = struct{}{} } } + ih.ruleIdx = newRuleIndex(ih.rules) return ih } @@ -209,10 +211,11 @@ func (ih *Inhibitor) Mutes(ctx context.Context, lset model.LabelSet) bool { // Returns the fingerprint of the inhibiting alert and true if inhibited. // The span parameter is optional and used for per-rule tracing events. func (ih *Inhibitor) checkInhibit(lset model.LabelSet, now time.Time, span trace.Span) (model.Fingerprint, bool) { - for _, r := range ih.rules { + var inhibitedByFP model.Fingerprint + + inhibited := ih.ruleIdx.forEachCandidate(lset, func(r *InhibitRule) bool { if !r.TargetMatchers.Matches(lset) { - // If target side of rule doesn't match, we don't need to look any further. - continue + return false } if span != nil { span.AddEvent("alert matched rule target", @@ -223,11 +226,14 @@ func (ih *Inhibitor) checkInhibit(lset model.LabelSet, now time.Time, span trace } // If we are here, the target side matches. If the source side matches, too, we // need to exclude inhibiting alerts for which the same is true. - if inhibitedByFP, eq := r.hasEqual(lset, r.SourceMatchers.Matches(lset), now); eq { - return inhibitedByFP, true + if foundFP, eq := r.hasEqual(lset, r.SourceMatchers.Matches(lset), now); eq { + inhibitedByFP = foundFP + return true } - } - return 0, false + return false + }) + + return inhibitedByFP, inhibited } // An InhibitRule specifies that a class of (source) alerts should inhibit diff --git a/inhibit/rule_index.go b/inhibit/rule_index.go new file mode 100644 index 0000000000..50f5264fb2 --- /dev/null +++ b/inhibit/rule_index.go @@ -0,0 +1,206 @@ +// Copyright 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 inhibit + +import ( + "sync" + + "github.com/prometheus/common/model" + + "github.com/prometheus/alertmanager/pkg/labels" +) + +const ( + // minRulesForIndex is the minimum number of rules needed before indexing + // provides benefit over linear scan. Below this threshold, the overhead + // of map lookups and pool operations exceeds linear scan cost. + // Set very low (2) to enable indexing for most configurations, relying + // on high-overlap detection to avoid the pathological cases. + minRulesForIndex = 2 + + // maxMatcherOverlapRatio is the maximum fraction of rules a matcher can + // appear in before it's excluded from indexing. Matchers appearing in + // too many rules don't provide good filtering. + maxMatcherOverlapRatio = 0.5 +) + +// matcherKey is used as a map key to avoid string concatenation allocations. +type matcherKey struct { + name string + value string +} + +var visitedRulePool = sync.Pool{ + New: func() interface{} { + return make(map[*InhibitRule]struct{}, 16) + }, +} + +func getVisitedRules() map[*InhibitRule]struct{} { + return visitedRulePool.Get().(map[*InhibitRule]struct{}) +} + +func putVisitedRules(m map[*InhibitRule]struct{}) { + clear(m) + visitedRulePool.Put(m) +} + +// ruleIndex provides O(k) rule candidate lookup instead of O(N) linear scan, +// where k = number of labels and N = number of inhibit rules. +type ruleIndex struct { + // labelName -> labelValue -> rules with that equality target matcher + exactIndex map[string]map[string][]*InhibitRule + + // rules with exactly one indexed equality target matcher (no dedup needed) + singleMatcherRules map[*InhibitRule]struct{} + + // rules without indexed equality target matchers (must check linearly) + linearRules []*InhibitRule + + // useLinearScan is true when rule count is below threshold + useLinearScan bool + + allRules []*InhibitRule +} + +func newRuleIndex(rules []*InhibitRule) *ruleIndex { + idx := &ruleIndex{ + exactIndex: make(map[string]map[string][]*InhibitRule), + singleMatcherRules: make(map[*InhibitRule]struct{}), + linearRules: nil, + useLinearScan: false, + allRules: rules, + } + + // For small rule sets, linear scan is faster than index overhead + if len(rules) < minRulesForIndex { + idx.useLinearScan = true + return idx + } + + // First pass: count how many rules each matcher appears in to detect high-overlap + matcherCount := make(map[matcherKey]int) + for _, rule := range rules { + for _, m := range rule.TargetMatchers { + if m.Type == labels.MatchEqual { + matcherCount[matcherKey{m.Name, m.Value}]++ + } + } + } + + // Determine which matchers are high-overlap and should be excluded + maxOverlap := int(float64(len(rules)) * maxMatcherOverlapRatio) + highOverlapMatchers := make(map[matcherKey]struct{}, len(matcherCount)) + for key, count := range matcherCount { + if count > maxOverlap { + highOverlapMatchers[key] = struct{}{} + } + } + + // Second pass: build index excluding high-overlap matchers + for _, rule := range rules { + // Count indexable matchers to determine if rule needs deduplication + var indexableCount int + for _, m := range rule.TargetMatchers { + if m.Type != labels.MatchEqual { + continue + } + if _, isHighOverlap := highOverlapMatchers[matcherKey{m.Name, m.Value}]; !isHighOverlap { + indexableCount++ + } + } + + if indexableCount == 0 { + // No good indexable matchers, use linear scan for this rule + idx.linearRules = append(idx.linearRules, rule) + continue + } + + if indexableCount == 1 { + idx.singleMatcherRules[rule] = struct{}{} + } + + // Add rule to index for each indexable matcher + for _, m := range rule.TargetMatchers { + if m.Type != labels.MatchEqual { + continue + } + if _, isHighOverlap := highOverlapMatchers[matcherKey{m.Name, m.Value}]; isHighOverlap { + continue + } + if idx.exactIndex[m.Name] == nil { + idx.exactIndex[m.Name] = make(map[string][]*InhibitRule) + } + idx.exactIndex[m.Name][m.Value] = append(idx.exactIndex[m.Name][m.Value], rule) + } + } + + return idx +} + +// forEachCandidate calls fn for each rule that might match the given label set. +// Returns true if any rule's callback returned true (indicating a match was found). +func (idx *ruleIndex) forEachCandidate(lset model.LabelSet, fn func(*InhibitRule) bool) bool { + if len(idx.allRules) == 0 { + return false + } + + // Fast path: if rule count is small or no index was built, iterate all rules + if idx.useLinearScan || len(idx.exactIndex) == 0 { + for _, rule := range idx.allRules { + if fn(rule) { + return true + } + } + return false + } + + visited := getVisitedRules() + defer putVisitedRules(visited) + + for labelName, labelValue := range lset { + valueMap, ok := idx.exactIndex[string(labelName)] + if !ok { + continue + } + + rules, ok := valueMap[string(labelValue)] + if !ok { + continue + } + + for _, rule := range rules { + // Rules with multiple indexed matchers need deduplication since they + // appear in multiple index entries. Single-matcher rules can skip this. + if _, isSingleMatcher := idx.singleMatcherRules[rule]; !isSingleMatcher { + if _, seen := visited[rule]; seen { + continue + } + visited[rule] = struct{}{} + } + + if fn(rule) { + return true + } + } + } + + for _, rule := range idx.linearRules { + if fn(rule) { + return true + } + } + + return false +} diff --git a/inhibit/rule_index_test.go b/inhibit/rule_index_test.go new file mode 100644 index 0000000000..fa33f42001 --- /dev/null +++ b/inhibit/rule_index_test.go @@ -0,0 +1,345 @@ +// Copyright 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 inhibit + +import ( + "testing" + + "github.com/prometheus/common/model" + "github.com/stretchr/testify/require" + + "github.com/prometheus/alertmanager/pkg/labels" +) + +func TestNewRuleIndex_EmptyRules(t *testing.T) { + idx := newRuleIndex(nil) + + require.NotNil(t, idx) + require.True(t, idx.useLinearScan) + require.Empty(t, idx.allRules) +} + +func TestNewRuleIndex_BelowThreshold(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + } + + idx := newRuleIndex(rules) + + require.True(t, idx.useLinearScan) + require.Empty(t, idx.exactIndex) +} + +func TestNewRuleIndex_IndexedRules(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "dev"), + }, + }, + } + + idx := newRuleIndex(rules) + + require.False(t, idx.useLinearScan) + require.Contains(t, idx.exactIndex, "cluster") + require.Len(t, idx.exactIndex["cluster"], 3) + require.Len(t, idx.exactIndex["cluster"]["prod"], 1) + require.Len(t, idx.exactIndex["cluster"]["staging"], 1) + require.Len(t, idx.exactIndex["cluster"]["dev"], 1) +} + +func TestNewRuleIndex_HighOverlapMatchers(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "severity", "warning"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "severity", "warning"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "severity", "warning"), + }, + }, + } + + idx := newRuleIndex(rules) + + require.False(t, idx.useLinearScan) + require.Empty(t, idx.exactIndex) + require.Len(t, idx.linearRules, 3) +} + +func TestNewRuleIndex_RegexMatchers(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchRegexp, "cluster", "prod-.*"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchRegexp, "cluster", "staging-.*"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "env", "test"), + }, + }, + } + + idx := newRuleIndex(rules) + + require.False(t, idx.useLinearScan) + require.Contains(t, idx.exactIndex, "env") + require.Len(t, idx.linearRules, 2) +} + +func TestNewRuleIndex_MultipleMatchers(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + newTestMatcher(t, labels.MatchEqual, "region", "us-east"), + }, + }, + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }, + } + + idx := newRuleIndex(rules) + + require.False(t, idx.useLinearScan) + require.Contains(t, idx.exactIndex, "cluster") + require.Contains(t, idx.exactIndex, "region") + require.Len(t, idx.exactIndex["cluster"]["prod"], 1) + require.Len(t, idx.exactIndex["region"]["us-east"], 1) + require.NotContains(t, idx.singleMatcherRules, rules[0]) + require.Contains(t, idx.singleMatcherRules, rules[1]) +} + +func TestForEachCandidate_EmptyIndex(t *testing.T) { + idx := newRuleIndex(nil) + + called := false + result := idx.forEachCandidate(model.LabelSet{"foo": "bar"}, func(r *InhibitRule) bool { + called = true + return false + }) + + require.False(t, result) + require.False(t, called) +} + +func TestForEachCandidate_LinearScan(t *testing.T) { + rules := []*InhibitRule{ + { + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + } + idx := newRuleIndex(rules) + + var visited []*InhibitRule + result := idx.forEachCandidate(model.LabelSet{"cluster": "prod"}, func(r *InhibitRule) bool { + visited = append(visited, r) + return false + }) + + require.False(t, result) + require.Len(t, visited, 1) + require.Equal(t, rules[0], visited[0]) +} + +func TestForEachCandidate_IndexedLookup(t *testing.T) { + rules := []*InhibitRule{ + { + Name: "rule-prod", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + Name: "rule-staging", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }, + { + Name: "rule-dev", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "dev"), + }, + }, + } + idx := newRuleIndex(rules) + + var visited []*InhibitRule + result := idx.forEachCandidate(model.LabelSet{"cluster": "staging"}, func(r *InhibitRule) bool { + visited = append(visited, r) + return false + }) + + require.False(t, result) + require.Len(t, visited, 1) + require.Equal(t, "rule-staging", visited[0].Name) +} + +func TestForEachCandidate_EarlyTermination(t *testing.T) { + rules := []*InhibitRule{ + { + Name: "rule-1", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + Name: "rule-2", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + } + idx := newRuleIndex(rules) + + var visited []*InhibitRule + result := idx.forEachCandidate(model.LabelSet{"cluster": "prod"}, func(r *InhibitRule) bool { + visited = append(visited, r) + return true + }) + + require.True(t, result) + require.Len(t, visited, 1) +} + +func TestForEachCandidate_Deduplication(t *testing.T) { + rule := &InhibitRule{ + Name: "multi-matcher", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + newTestMatcher(t, labels.MatchEqual, "region", "us-east"), + }, + } + rules := []*InhibitRule{rule, { + Name: "other", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }} + idx := newRuleIndex(rules) + + var visited []*InhibitRule + result := idx.forEachCandidate(model.LabelSet{ + "cluster": "prod", + "region": "us-east", + }, func(r *InhibitRule) bool { + visited = append(visited, r) + return false + }) + + require.False(t, result) + count := 0 + for _, v := range visited { + if v.Name == "multi-matcher" { + count++ + } + } + require.Equal(t, 1, count) +} + +func TestForEachCandidate_LinearRulesIncluded(t *testing.T) { + rules := []*InhibitRule{ + { + Name: "indexed", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + Name: "linear-regex", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchRegexp, "env", ".*"), + }, + }, + } + idx := newRuleIndex(rules) + + var visited []string + result := idx.forEachCandidate(model.LabelSet{"cluster": "prod", "env": "test"}, func(r *InhibitRule) bool { + visited = append(visited, r.Name) + return false + }) + + require.False(t, result) + require.Contains(t, visited, "indexed") + require.Contains(t, visited, "linear-regex") +} + +func TestForEachCandidate_NoMatchingLabels(t *testing.T) { + rules := []*InhibitRule{ + { + Name: "rule-prod", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + Name: "rule-staging", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }, + } + idx := newRuleIndex(rules) + + var visited []*InhibitRule + result := idx.forEachCandidate(model.LabelSet{"cluster": "dev"}, func(r *InhibitRule) bool { + visited = append(visited, r) + return false + }) + + require.False(t, result) + require.Empty(t, visited) +} + +func newTestMatcher(t *testing.T, op labels.MatchType, name, value string) *labels.Matcher { + t.Helper() + m, err := labels.NewMatcher(op, name, value) + require.NoError(t, err) + return m +} From 905b02b8a09820deaf7a695962d6f77de5b6ce73 Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Fri, 20 Feb 2026 12:48:55 +0100 Subject: [PATCH 4/6] refactor: make rule index thresholds configurable Replace hardcoded constants with RuleIndexOptions struct to allow testing different threshold values. Benchmark results for MinRulesForIndex (ns/op): rules | linear | indexed 1 | 17 | 17 2 | 29 | 85 5 | 68 | 84 10 | 135 | 94 Crossover at ~7 rules. Default of 2 enables indexing early since high-overlap detection handles pathological cases. Benchmark results for MaxMatcherOverlapRatio (ns/op): ratio | time 0.10 | 183 0.50 | 186 0.60 | 552 1.00 | 571 Clear cliff between 0.5 and 0.6 with 3x degradation. Default of 0.5 is optimal - highest value before performance degrades. Signed-off-by: Titouan Chary --- inhibit/inhibit_bench_test.go | 130 +++++++++++++++++++++++++++++++++- inhibit/rule_index.go | 58 ++++++++------- inhibit/rule_index_test.go | 52 ++++++++++++++ 3 files changed, 207 insertions(+), 33 deletions(-) diff --git a/inhibit/inhibit_bench_test.go b/inhibit/inhibit_bench_test.go index 56ff56d06e..76b074c695 100644 --- a/inhibit/inhibit_bench_test.go +++ b/inhibit/inhibit_bench_test.go @@ -263,7 +263,7 @@ func benchmarkDifferentTargets(b *testing.B, numRules int) { defer s.Close() rules := make([]config.InhibitRule, numRules) - for i := 0; i < numRules; i++ { + for i := range numRules { rules[i] = config.InhibitRule{ SourceMatchers: config.Matchers{ mustNewMatcher(b, labels.MatchEqual, "alertname", "SourceAlert"), @@ -318,7 +318,7 @@ func benchmarkSameTarget(b *testing.B, numRules int) { defer s.Close() rules := make([]config.InhibitRule, numRules) - for i := 0; i < numRules; i++ { + for i := range numRules { rules[i] = config.InhibitRule{ SourceMatchers: config.Matchers{ mustNewMatcher(b, labels.MatchEqual, "src", strconv.Itoa(i)), @@ -365,7 +365,7 @@ func benchmarkNoMatch(b *testing.B, numRules int) { defer s.Close() rules := make([]config.InhibitRule, numRules) - for i := 0; i < numRules; i++ { + for i := range numRules { rules[i] = config.InhibitRule{ SourceMatchers: config.Matchers{ mustNewMatcher(b, labels.MatchEqual, "alertname", "SourceAlert"), @@ -397,3 +397,127 @@ func benchmarkNoMatch(b *testing.B, numRules int) { } } } + +// BenchmarkMinRulesForIndexThreshold compares linear vs indexed lookup at various rule counts. +// +// Results (ns/op): +// +// rules | linear | indexed +// 1 | 17 | 17 +// 2 | 29 | 85 +// 5 | 68 | 84 +// 10 | 135 | 94 +// +// Crossover at ~7 rules. Default MinRulesForIndex=2 enables indexing early since +// high-overlap detection handles pathological cases. +func BenchmarkMinRulesForIndexThreshold(b *testing.B) { + for _, numRules := range []int{1, 2, 3, 5, 10} { + b.Run("rules="+strconv.Itoa(numRules), func(b *testing.B) { + benchmarkRuleIndexThreshold(b, numRules) + }) + } +} + +func benchmarkRuleIndexThreshold(b *testing.B, numRules int) { + rules := make([]*InhibitRule, numRules) + for i := range numRules { + rules[i] = &InhibitRule{ + TargetMatchers: labels.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "cluster", strconv.Itoa(i)), + }, + } + } + + lset := model.LabelSet{"cluster": "0"} + + b.Run("linear", func(b *testing.B) { + opts := RuleIndexOptions{MinRulesForIndex: numRules + 1, MaxMatcherOverlapRatio: 0.5} + idx := newRuleIndexWithOptions(rules, opts) + + b.ResetTimer() + for b.Loop() { + idx.forEachCandidate(lset, func(r *InhibitRule) bool { + r.TargetMatchers.Matches(lset) + return false + }) + } + }) + + b.Run("indexed", func(b *testing.B) { + opts := RuleIndexOptions{MinRulesForIndex: 1, MaxMatcherOverlapRatio: 0.5} + idx := newRuleIndexWithOptions(rules, opts) + + b.ResetTimer() + for b.Loop() { + idx.forEachCandidate(lset, func(r *InhibitRule) bool { + r.TargetMatchers.Matches(lset) + return false + }) + } + }) +} + +// BenchmarkMaxMatcherOverlapRatio compares performance at various overlap thresholds. +// +// Results (ns/op): +// +// ratio | time +// 0.10 | 183 +// 0.20 | 185 +// 0.30 | 182 +// 0.40 | 185 +// 0.50 | 186 +// 0.60 | 552 +// 0.70 | 533 +// 0.80 | 546 +// 0.90 | 524 +// 1.00 | 571 +// +// Clear cliff between 0.5 and 0.6 with 3x degradation. Default MaxMatcherOverlapRatio=0.5 +// is optimal - highest value before performance degrades. +func BenchmarkMaxMatcherOverlapRatio(b *testing.B) { + for _, ratio := range []float64{0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0} { + b.Run("ratio="+strconv.FormatFloat(ratio, 'f', 2, 64), func(b *testing.B) { + benchmarkOverlapRatio(b, ratio) + }) + } +} + +func benchmarkOverlapRatio(b *testing.B, ratio float64) { + numRules := 100 + highOverlapCount := int(float64(numRules) * 0.6) + + rules := make([]*InhibitRule, numRules) + for i := range highOverlapCount { + rules[i] = &InhibitRule{ + TargetMatchers: labels.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "severity", "warning"), + }, + } + } + for i := highOverlapCount; i < numRules; i++ { + rules[i] = &InhibitRule{ + TargetMatchers: labels.Matchers{ + mustNewMatcher(b, labels.MatchEqual, "cluster", strconv.Itoa(i)), + }, + } + } + + opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: ratio} + idx := newRuleIndexWithOptions(rules, opts) + + lset := model.LabelSet{"severity": "warning", "cluster": model.LabelValue(strconv.Itoa(highOverlapCount))} + + b.ResetTimer() + b.ReportAllocs() + + var visited int + for b.Loop() { + visited = 0 + idx.forEachCandidate(lset, func(r *InhibitRule) bool { + visited++ + return false + }) + } + _ = visited +} diff --git a/inhibit/rule_index.go b/inhibit/rule_index.go index 50f5264fb2..d0b97069b6 100644 --- a/inhibit/rule_index.go +++ b/inhibit/rule_index.go @@ -14,6 +14,7 @@ package inhibit import ( + "slices" "sync" "github.com/prometheus/common/model" @@ -21,28 +22,32 @@ import ( "github.com/prometheus/alertmanager/pkg/labels" ) -const ( - // minRulesForIndex is the minimum number of rules needed before indexing - // provides benefit over linear scan. Below this threshold, the overhead - // of map lookups and pool operations exceeds linear scan cost. - // Set very low (2) to enable indexing for most configurations, relying - // on high-overlap detection to avoid the pathological cases. - minRulesForIndex = 2 - - // maxMatcherOverlapRatio is the maximum fraction of rules a matcher can - // appear in before it's excluded from indexing. Matchers appearing in - // too many rules don't provide good filtering. - maxMatcherOverlapRatio = 0.5 -) - // matcherKey is used as a map key to avoid string concatenation allocations. type matcherKey struct { name string value string } +// RuleIndexOptions configures the rule index behavior. +type RuleIndexOptions struct { + // MinRulesForIndex is the minimum number of rules before indexing is used. + MinRulesForIndex int + + // MaxMatcherOverlapRatio is the maximum fraction of rules a matcher can + // appear in before being excluded from the index. + MaxMatcherOverlapRatio float64 +} + +// DefaultRuleIndexOptions returns the default options for rule indexing. +func DefaultRuleIndexOptions() RuleIndexOptions { + return RuleIndexOptions{ + MinRulesForIndex: 2, + MaxMatcherOverlapRatio: 0.5, + } +} + var visitedRulePool = sync.Pool{ - New: func() interface{} { + New: func() any { return make(map[*InhibitRule]struct{}, 16) }, } @@ -75,6 +80,10 @@ type ruleIndex struct { } func newRuleIndex(rules []*InhibitRule) *ruleIndex { + return newRuleIndexWithOptions(rules, DefaultRuleIndexOptions()) +} + +func newRuleIndexWithOptions(rules []*InhibitRule, opts RuleIndexOptions) *ruleIndex { idx := &ruleIndex{ exactIndex: make(map[string]map[string][]*InhibitRule), singleMatcherRules: make(map[*InhibitRule]struct{}), @@ -84,7 +93,7 @@ func newRuleIndex(rules []*InhibitRule) *ruleIndex { } // For small rule sets, linear scan is faster than index overhead - if len(rules) < minRulesForIndex { + if len(rules) < opts.MinRulesForIndex { idx.useLinearScan = true return idx } @@ -100,7 +109,7 @@ func newRuleIndex(rules []*InhibitRule) *ruleIndex { } // Determine which matchers are high-overlap and should be excluded - maxOverlap := int(float64(len(rules)) * maxMatcherOverlapRatio) + maxOverlap := int(float64(len(rules)) * opts.MaxMatcherOverlapRatio) highOverlapMatchers := make(map[matcherKey]struct{}, len(matcherCount)) for key, count := range matcherCount { if count > maxOverlap { @@ -158,12 +167,7 @@ func (idx *ruleIndex) forEachCandidate(lset model.LabelSet, fn func(*InhibitRule // Fast path: if rule count is small or no index was built, iterate all rules if idx.useLinearScan || len(idx.exactIndex) == 0 { - for _, rule := range idx.allRules { - if fn(rule) { - return true - } - } - return false + return slices.ContainsFunc(idx.allRules, fn) } visited := getVisitedRules() @@ -196,11 +200,5 @@ func (idx *ruleIndex) forEachCandidate(lset model.LabelSet, fn func(*InhibitRule } } - for _, rule := range idx.linearRules { - if fn(rule) { - return true - } - } - - return false + return slices.ContainsFunc(idx.linearRules, fn) } diff --git a/inhibit/rule_index_test.go b/inhibit/rule_index_test.go index fa33f42001..878b8b0f8a 100644 --- a/inhibit/rule_index_test.go +++ b/inhibit/rule_index_test.go @@ -343,3 +343,55 @@ func newTestMatcher(t *testing.T, op labels.MatchType, name, value string) *labe require.NoError(t, err) return m } + +func TestRuleIndexOptions_MinRulesForIndex(t *testing.T) { + rules := []*InhibitRule{ + {TargetMatchers: labels.Matchers{newTestMatcher(t, labels.MatchEqual, "cluster", "prod")}}, + } + + t.Run("threshold=1_uses_index", func(t *testing.T) { + opts := RuleIndexOptions{MinRulesForIndex: 1, MaxMatcherOverlapRatio: 0.5} + idx := newRuleIndexWithOptions(rules, opts) + require.False(t, idx.useLinearScan) + }) + + t.Run("threshold=2_uses_linear", func(t *testing.T) { + opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 0.5} + idx := newRuleIndexWithOptions(rules, opts) + require.True(t, idx.useLinearScan) + }) +} + +func TestRuleIndexOptions_MaxMatcherOverlapRatio(t *testing.T) { + rules := []*InhibitRule{ + {TargetMatchers: labels.Matchers{newTestMatcher(t, labels.MatchEqual, "severity", "warning")}}, + {TargetMatchers: labels.Matchers{newTestMatcher(t, labels.MatchEqual, "severity", "warning")}}, + {TargetMatchers: labels.Matchers{newTestMatcher(t, labels.MatchEqual, "severity", "warning")}}, + {TargetMatchers: labels.Matchers{newTestMatcher(t, labels.MatchEqual, "cluster", "prod")}}, + } + + t.Run("ratio=0.5_excludes_high_overlap", func(t *testing.T) { + opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 0.5} + idx := newRuleIndexWithOptions(rules, opts) + + require.NotContains(t, idx.exactIndex, "severity") + require.Contains(t, idx.exactIndex, "cluster") + require.Len(t, idx.linearRules, 3) + }) + + t.Run("ratio=1.0_includes_all", func(t *testing.T) { + opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 1.0} + idx := newRuleIndexWithOptions(rules, opts) + + require.Contains(t, idx.exactIndex, "severity") + require.Contains(t, idx.exactIndex, "cluster") + require.Empty(t, idx.linearRules) + }) +} + +func TestDefaultRuleIndexOptions(t *testing.T) { + opts := DefaultRuleIndexOptions() + + require.Equal(t, 2, opts.MinRulesForIndex) + require.Equal(t, 0.5, opts.MaxMatcherOverlapRatio) +} From 113e756ed309ce51a743e599bd964e5f5f05364b Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Mon, 2 Mar 2026 09:10:58 +0100 Subject: [PATCH 5/6] refactor: address PR review comments - Replace time.After sleeps with WaitForLoading() in benchmarks - Make ruleIndexOptions and defaultRuleIndexOptions unexported - Expand early termination test to use more rules Co-Authored-By: Claude Opus 4.5 Signed-off-by: Titouan Chary --- inhibit/inhibit_bench_test.go | 16 +++++++--------- inhibit/rule_index.go | 30 +++++++++++++++--------------- inhibit/rule_index_test.go | 28 +++++++++++++++++++++------- 3 files changed, 43 insertions(+), 31 deletions(-) diff --git a/inhibit/inhibit_bench_test.go b/inhibit/inhibit_bench_test.go index 76b074c695..3577e13b50 100644 --- a/inhibit/inhibit_bench_test.go +++ b/inhibit/inhibit_bench_test.go @@ -201,9 +201,7 @@ func benchmarkMutes(b *testing.B, opts benchmarkOptions) { ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) defer ih.Stop() go ih.Run() - - // Wait some time for the inhibitor to seed its cache. - <-time.After(time.Second) + ih.WaitForLoading() for b.Loop() { require.NoError(b, opts.benchFunc(ih.Mutes)) @@ -291,7 +289,7 @@ func benchmarkDifferentTargets(b *testing.B, numRules int) { ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) defer ih.Stop() go ih.Run() - <-time.After(time.Second) + ih.WaitForLoading() targetLset := model.LabelSet{ "alertname": "TargetAlert", @@ -342,7 +340,7 @@ func benchmarkSameTarget(b *testing.B, numRules int) { ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) defer ih.Stop() go ih.Run() - <-time.After(time.Second) + ih.WaitForLoading() targetLset := model.LabelSet{"dst": "0"} ctx := context.Background() @@ -379,7 +377,7 @@ func benchmarkNoMatch(b *testing.B, numRules int) { ih := NewInhibitor(s, rules, m, promslog.NewNopLogger()) defer ih.Stop() go ih.Run() - <-time.After(time.Second) + ih.WaitForLoading() // Alert with cluster that doesn't match any rule targetLset := model.LabelSet{ @@ -431,7 +429,7 @@ func benchmarkRuleIndexThreshold(b *testing.B, numRules int) { lset := model.LabelSet{"cluster": "0"} b.Run("linear", func(b *testing.B) { - opts := RuleIndexOptions{MinRulesForIndex: numRules + 1, MaxMatcherOverlapRatio: 0.5} + opts := ruleIndexOptions{minRulesForIndex: numRules + 1, maxMatcherOverlapRatio: 0.5} idx := newRuleIndexWithOptions(rules, opts) b.ResetTimer() @@ -444,7 +442,7 @@ func benchmarkRuleIndexThreshold(b *testing.B, numRules int) { }) b.Run("indexed", func(b *testing.B) { - opts := RuleIndexOptions{MinRulesForIndex: 1, MaxMatcherOverlapRatio: 0.5} + opts := ruleIndexOptions{minRulesForIndex: 1, maxMatcherOverlapRatio: 0.5} idx := newRuleIndexWithOptions(rules, opts) b.ResetTimer() @@ -503,7 +501,7 @@ func benchmarkOverlapRatio(b *testing.B, ratio float64) { } } - opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: ratio} + opts := ruleIndexOptions{minRulesForIndex: 2, maxMatcherOverlapRatio: ratio} idx := newRuleIndexWithOptions(rules, opts) lset := model.LabelSet{"severity": "warning", "cluster": model.LabelValue(strconv.Itoa(highOverlapCount))} diff --git a/inhibit/rule_index.go b/inhibit/rule_index.go index d0b97069b6..e3775c63c0 100644 --- a/inhibit/rule_index.go +++ b/inhibit/rule_index.go @@ -28,21 +28,21 @@ type matcherKey struct { value string } -// RuleIndexOptions configures the rule index behavior. -type RuleIndexOptions struct { - // MinRulesForIndex is the minimum number of rules before indexing is used. - MinRulesForIndex int +// ruleIndexOptions configures the rule index behavior. +type ruleIndexOptions struct { + // minRulesForIndex is the minimum number of rules before indexing is used. + minRulesForIndex int - // MaxMatcherOverlapRatio is the maximum fraction of rules a matcher can + // maxMatcherOverlapRatio is the maximum fraction of rules a matcher can // appear in before being excluded from the index. - MaxMatcherOverlapRatio float64 + maxMatcherOverlapRatio float64 } -// DefaultRuleIndexOptions returns the default options for rule indexing. -func DefaultRuleIndexOptions() RuleIndexOptions { - return RuleIndexOptions{ - MinRulesForIndex: 2, - MaxMatcherOverlapRatio: 0.5, +// defaultRuleIndexOptions returns the default options for rule indexing. +func defaultRuleIndexOptions() ruleIndexOptions { + return ruleIndexOptions{ + minRulesForIndex: 2, + maxMatcherOverlapRatio: 0.5, } } @@ -80,10 +80,10 @@ type ruleIndex struct { } func newRuleIndex(rules []*InhibitRule) *ruleIndex { - return newRuleIndexWithOptions(rules, DefaultRuleIndexOptions()) + return newRuleIndexWithOptions(rules, defaultRuleIndexOptions()) } -func newRuleIndexWithOptions(rules []*InhibitRule, opts RuleIndexOptions) *ruleIndex { +func newRuleIndexWithOptions(rules []*InhibitRule, opts ruleIndexOptions) *ruleIndex { idx := &ruleIndex{ exactIndex: make(map[string]map[string][]*InhibitRule), singleMatcherRules: make(map[*InhibitRule]struct{}), @@ -93,7 +93,7 @@ func newRuleIndexWithOptions(rules []*InhibitRule, opts RuleIndexOptions) *ruleI } // For small rule sets, linear scan is faster than index overhead - if len(rules) < opts.MinRulesForIndex { + if len(rules) < opts.minRulesForIndex { idx.useLinearScan = true return idx } @@ -109,7 +109,7 @@ func newRuleIndexWithOptions(rules []*InhibitRule, opts RuleIndexOptions) *ruleI } // Determine which matchers are high-overlap and should be excluded - maxOverlap := int(float64(len(rules)) * opts.MaxMatcherOverlapRatio) + maxOverlap := int(float64(len(rules)) * opts.maxMatcherOverlapRatio) highOverlapMatchers := make(map[matcherKey]struct{}, len(matcherCount)) for key, count := range matcherCount { if count > maxOverlap { diff --git a/inhibit/rule_index_test.go b/inhibit/rule_index_test.go index 878b8b0f8a..aee8c2ddb4 100644 --- a/inhibit/rule_index_test.go +++ b/inhibit/rule_index_test.go @@ -221,6 +221,8 @@ func TestForEachCandidate_IndexedLookup(t *testing.T) { } func TestForEachCandidate_EarlyTermination(t *testing.T) { + // Use more than 2 rules to ensure we're testing index-based iteration, + // not linear scan (default minRulesForIndex=2) rules := []*InhibitRule{ { Name: "rule-1", @@ -234,6 +236,18 @@ func TestForEachCandidate_EarlyTermination(t *testing.T) { newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), }, }, + { + Name: "rule-3", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + { + Name: "rule-4", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "staging"), + }, + }, } idx := newRuleIndex(rules) @@ -350,13 +364,13 @@ func TestRuleIndexOptions_MinRulesForIndex(t *testing.T) { } t.Run("threshold=1_uses_index", func(t *testing.T) { - opts := RuleIndexOptions{MinRulesForIndex: 1, MaxMatcherOverlapRatio: 0.5} + opts := ruleIndexOptions{minRulesForIndex: 1, maxMatcherOverlapRatio: 0.5} idx := newRuleIndexWithOptions(rules, opts) require.False(t, idx.useLinearScan) }) t.Run("threshold=2_uses_linear", func(t *testing.T) { - opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 0.5} + opts := ruleIndexOptions{minRulesForIndex: 2, maxMatcherOverlapRatio: 0.5} idx := newRuleIndexWithOptions(rules, opts) require.True(t, idx.useLinearScan) }) @@ -371,7 +385,7 @@ func TestRuleIndexOptions_MaxMatcherOverlapRatio(t *testing.T) { } t.Run("ratio=0.5_excludes_high_overlap", func(t *testing.T) { - opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 0.5} + opts := ruleIndexOptions{minRulesForIndex: 2, maxMatcherOverlapRatio: 0.5} idx := newRuleIndexWithOptions(rules, opts) require.NotContains(t, idx.exactIndex, "severity") @@ -380,7 +394,7 @@ func TestRuleIndexOptions_MaxMatcherOverlapRatio(t *testing.T) { }) t.Run("ratio=1.0_includes_all", func(t *testing.T) { - opts := RuleIndexOptions{MinRulesForIndex: 2, MaxMatcherOverlapRatio: 1.0} + opts := ruleIndexOptions{minRulesForIndex: 2, maxMatcherOverlapRatio: 1.0} idx := newRuleIndexWithOptions(rules, opts) require.Contains(t, idx.exactIndex, "severity") @@ -390,8 +404,8 @@ func TestRuleIndexOptions_MaxMatcherOverlapRatio(t *testing.T) { } func TestDefaultRuleIndexOptions(t *testing.T) { - opts := DefaultRuleIndexOptions() + opts := defaultRuleIndexOptions() - require.Equal(t, 2, opts.MinRulesForIndex) - require.Equal(t, 0.5, opts.MaxMatcherOverlapRatio) + require.Equal(t, 2, opts.minRulesForIndex) + require.Equal(t, 0.5, opts.maxMatcherOverlapRatio) } From ebda3dffa8d5026a15ee02f396b3d18c4c31ab01 Mon Sep 17 00:00:00 2001 From: Titouan Chary Date: Tue, 17 Mar 2026 15:46:42 +0100 Subject: [PATCH 6/6] fix: exclude empty-value matchers from rule index Rules with MatchEqual(label, "") should match alerts that don't have the label at all, since Go's map lookup returns "" for absent keys. However, forEachCandidate only iterates over labels present in the alert's label set, causing such rules to be missed when indexed. Fix by treating empty-value matchers as non-indexable, so rules with only such matchers fall back to linearRules where they're always checked. Co-Authored-By: Claude Opus 4.5 Signed-off-by: Titouan Chary --- inhibit/rule_index.go | 11 +++++++---- inhibit/rule_index_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/inhibit/rule_index.go b/inhibit/rule_index.go index e3775c63c0..7dd500d731 100644 --- a/inhibit/rule_index.go +++ b/inhibit/rule_index.go @@ -99,10 +99,13 @@ func newRuleIndexWithOptions(rules []*InhibitRule, opts ruleIndexOptions) *ruleI } // First pass: count how many rules each matcher appears in to detect high-overlap + // Note: MatchEqual with empty value is excluded from indexing because it matches + // alerts that don't have the label at all, but forEachCandidate only iterates + // over labels present in the alert's label set. matcherCount := make(map[matcherKey]int) for _, rule := range rules { for _, m := range rule.TargetMatchers { - if m.Type == labels.MatchEqual { + if m.Type == labels.MatchEqual && m.Value != "" { matcherCount[matcherKey{m.Name, m.Value}]++ } } @@ -117,12 +120,12 @@ func newRuleIndexWithOptions(rules []*InhibitRule, opts ruleIndexOptions) *ruleI } } - // Second pass: build index excluding high-overlap matchers + // Second pass: build index excluding high-overlap matchers and empty-value matchers for _, rule := range rules { // Count indexable matchers to determine if rule needs deduplication var indexableCount int for _, m := range rule.TargetMatchers { - if m.Type != labels.MatchEqual { + if m.Type != labels.MatchEqual || m.Value == "" { continue } if _, isHighOverlap := highOverlapMatchers[matcherKey{m.Name, m.Value}]; !isHighOverlap { @@ -142,7 +145,7 @@ func newRuleIndexWithOptions(rules []*InhibitRule, opts ruleIndexOptions) *ruleI // Add rule to index for each indexable matcher for _, m := range rule.TargetMatchers { - if m.Type != labels.MatchEqual { + if m.Type != labels.MatchEqual || m.Value == "" { continue } if _, isHighOverlap := highOverlapMatchers[matcherKey{m.Name, m.Value}]; isHighOverlap { diff --git a/inhibit/rule_index_test.go b/inhibit/rule_index_test.go index aee8c2ddb4..64ec39246b 100644 --- a/inhibit/rule_index_test.go +++ b/inhibit/rule_index_test.go @@ -409,3 +409,40 @@ func TestDefaultRuleIndexOptions(t *testing.T) { require.Equal(t, 2, opts.minRulesForIndex) require.Equal(t, 0.5, opts.maxMatcherOverlapRatio) } + +// TestForEachCandidate_EmptyValueMatcherWithAbsentLabel tests that a rule with +// MatchEqual("label", "") correctly matches alerts that don't have that label. +// This is a regression test: the index must not miss such rules when the label +// is absent from the alert's label set. +func TestForEachCandidate_EmptyValueMatcherWithAbsentLabel(t *testing.T) { + rules := []*InhibitRule{ + { + Name: "empty-value-rule", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "optional_label", ""), + }, + }, + { + Name: "other-rule", + TargetMatchers: labels.Matchers{ + newTestMatcher(t, labels.MatchEqual, "cluster", "prod"), + }, + }, + } + idx := newRuleIndex(rules) + + // Alert does NOT have "optional_label" at all. + // MatchEqual("optional_label", "") should match because Go's map lookup + // returns "" for absent keys, so lset["optional_label"] == "" is true. + lset := model.LabelSet{"cluster": "staging", "severity": "warning"} + + var visited []string + result := idx.forEachCandidate(lset, func(r *InhibitRule) bool { + visited = append(visited, r.Name) + return false + }) + + require.False(t, result) + require.Contains(t, visited, "empty-value-rule", + "Rule with MatchEqual(label, \"\") should be a candidate when label is absent from alert") +}