diff --git a/CHANGELOG.md b/CHANGELOG.md index a5182671a2..10a63e60e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## main / (unreleased) +## 0.34.1 / 2026-09-11 + +* [BUGFIX] inhibit: Fix several issues related to inhibitions that caused alerts to be improperly un-muted in some cases. #5542, #5449, #5559 + ## 0.34.0 / 2026-08-16 * [CHANGE] notify: The `reason` label on `alertmanager_notifications_failed_total` now distinguishes `authError` (HTTP 401/403) and `rateLimited` (HTTP 429) from the generic `clientError`. Dashboards/alerts matching `reason="clientError"` for these codes must be updated. #5332 diff --git a/VERSION b/VERSION index 85e60ed180..cd46610fe4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.34.0 +0.34.1 diff --git a/go.mod b/go.mod index 830cfac35a..5f1d957846 100644 --- a/go.mod +++ b/go.mod @@ -54,10 +54,10 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.44.0 go.opentelemetry.io/otel/sdk v1.44.0 go.opentelemetry.io/otel/trace v1.44.0 - golang.org/x/mod v0.38.0 - golang.org/x/net v0.57.0 - golang.org/x/text v0.40.0 - google.golang.org/grpc v1.82.1 + golang.org/x/mod v0.40.0 + golang.org/x/net v0.58.0 + golang.org/x/text v0.41.0 + google.golang.org/grpc v1.83.1 google.golang.org/protobuf v1.36.11 gopkg.in/telebot.v3 v3.3.8 gopkg.in/yaml.v2 v2.4.0 @@ -124,12 +124,12 @@ require ( go.opentelemetry.io/proto/otlp v1.10.0 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.54.0 // indirect + golang.org/x/crypto v0.55.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/time v0.15.0 // indirect - golang.org/x/tools v0.47.0 // indirect + golang.org/x/tools v0.49.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260526163538-3dc84a4a5aaa // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index fafd12a0bd..e0e494f924 100644 --- a/go.sum +++ b/go.sum @@ -625,8 +625,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4= golang.org/x/crypto v0.0.0-20211108221036-ceb1ce70b4fa/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20220411220226-7b82a4e95df4/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= -golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8= @@ -662,8 +662,8 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= -golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= +golang.org/x/mod v0.40.0 h1:hUv+3cXcdRHz08UmSiOob7sadHig73uo5bkXxQ/tvUs= +golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE= golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= @@ -711,8 +711,8 @@ golang.org/x/net v0.0.0-20220325170049-de3da57026de/go.mod h1:CfG3xpIq0wQ8r1q4Su golang.org/x/net v0.0.0-20220412020605-290c469a71a5/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220425223048-2871e0cb64e4/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= -golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= -golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -840,8 +840,8 @@ golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= -golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= -golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -902,8 +902,8 @@ golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= -golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -1070,8 +1070,8 @@ google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ5 google.golang.org/grpc v1.45.0/go.mod h1:lN7owxKUQEqMfSyQikvvk5tf/6zMPsrK+ONuO11+0rQ= google.golang.org/grpc v1.46.0/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk= google.golang.org/grpc v1.46.2/go.mod h1:vN9eftEi1UMyUsIF80+uQXhHjbXYbm0uXoFCACuMGWk= -google.golang.org/grpc v1.82.1 h1:NnAxzGRA0677vCa4BUkOAnO5+FfQqVl9iUXeD0IqcGE= -google.golang.org/grpc v1.82.1/go.mod h1:yzTZ1TB1Z3SG+LIYaI+WiE8D5+PZ3ArnrSp8zF3+/ZA= +google.golang.org/grpc v1.83.1 h1:HIO0+BEtBP6soyqvqC8sNUjZ7bTs+0hFQuFF+RAy++Y= +google.golang.org/grpc v1.83.1/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.1.0/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= diff --git a/inhibit/cache.go b/inhibit/cache.go new file mode 100644 index 0000000000..31180a2688 --- /dev/null +++ b/inhibit/cache.go @@ -0,0 +1,124 @@ +// 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 ( + "context" + "sync" + "time" + + "github.com/prometheus/common/model" + + "github.com/prometheus/alertmanager/alert" +) + +// cache contains the runtime state of the inhibit rule. +type cache struct { + equal map[model.LabelName]struct{} + + mtx sync.RWMutex + // alerts is the map of alerts that match the source matchers of the inhibit rule. + alerts map[model.Fingerprint]*alert.Alert + // index is a map of equal label fingerprint to the set of source alert fingerprints stored + // in the cache. + index map[model.Fingerprint]model.FingerprintSet +} + +func newCache(equal map[model.LabelName]struct{}) *cache { + return &cache{ + equal: equal, + alerts: make(map[model.Fingerprint]*alert.Alert), + index: make(map[model.Fingerprint]model.FingerprintSet), + } +} + +// fingerprintEquals returns the fingerprint of the equal labels of the given label set. +func (c *cache) fingerprintEquals(lset model.LabelSet) model.Fingerprint { + equalSet := make(model.LabelSet, len(c.equal)) + for n := range c.equal { + equalSet[n] = lset[n] + } + return equalSet.Fingerprint() +} + +// set adds or replaces the given source alert. +func (c *cache) set(a *alert.Alert) { + fp := a.Fingerprint() + eq := c.fingerprintEquals(a.Labels) + + c.mtx.Lock() + defer c.mtx.Unlock() + + c.alerts[fp] = a + set, ok := c.index[eq] + if !ok { + set = model.FingerprintSet{} + c.index[eq] = set + } + set[fp] = struct{}{} +} + +// find returns the fingerprint of a cached source alert that shares the equal +// labels of lset, is active at now, and satisfies match. +func (c *cache) find(lset model.LabelSet, now time.Time, match func(*alert.Alert) bool) (model.Fingerprint, bool) { + eq := c.fingerprintEquals(lset) + + c.mtx.RLock() + defer c.mtx.RUnlock() + + for fp := range c.index[eq] { + a := c.alerts[fp] + if a.ResolvedAt(now) { + continue + } + if !match(a) { + continue + } + return fp, true + } + + return model.Fingerprint(0), false +} + +func (c *cache) run(ctx context.Context, interval time.Duration) { + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + c.gc() + } + } +} + +func (c *cache) gc() { + c.mtx.Lock() + defer c.mtx.Unlock() + + for fp, a := range c.alerts { + if !a.Resolved() { + continue + } + delete(c.alerts, fp) + + eq := c.fingerprintEquals(a.Labels) + set := c.index[eq] + delete(set, fp) + if len(set) == 0 { + delete(c.index, eq) + } + } +} diff --git a/inhibit/index.go b/inhibit/index.go deleted file mode 100644 index fd60e48701..0000000000 --- a/inhibit/index.go +++ /dev/null @@ -1,64 +0,0 @@ -// 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" -) - -// index contains map of fingerprints to fingerprints. -// The keys are fingerprints of the equal labels of source alerts. -// The values are fingerprints of the source alerts. -// For more info see comments on inhibitor and InhibitRule. -type index struct { - mtx sync.RWMutex - items map[model.Fingerprint]model.Fingerprint -} - -func newIndex() *index { - return &index{ - items: make(map[model.Fingerprint]model.Fingerprint), - } -} - -func (c *index) Get(key model.Fingerprint) (model.Fingerprint, bool) { - c.mtx.RLock() - defer c.mtx.RUnlock() - - fp, ok := c.items[key] - return fp, ok -} - -func (c *index) Set(key, value model.Fingerprint) { - c.mtx.Lock() - defer c.mtx.Unlock() - - c.items[key] = value -} - -func (c *index) Delete(key model.Fingerprint) { - c.mtx.Lock() - defer c.mtx.Unlock() - - delete(c.items, key) -} - -func (c *index) Len() int { - c.mtx.RLock() - defer c.mtx.RUnlock() - - return len(c.items) -} diff --git a/inhibit/inhibit.go b/inhibit/inhibit.go index c441054be6..5df5ee2491 100644 --- a/inhibit/inhibit.go +++ b/inhibit/inhibit.go @@ -23,19 +23,17 @@ import ( "github.com/prometheus/common/model" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/propagation" "go.opentelemetry.io/otel/trace" + "github.com/prometheus/alertmanager/alert" amcommoncfg "github.com/prometheus/alertmanager/config/common" "github.com/prometheus/alertmanager/eventrecorder" "github.com/prometheus/alertmanager/eventrecorder/eventrecorderpb" "github.com/prometheus/alertmanager/marker" "github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/alertmanager/provider" - "github.com/prometheus/alertmanager/store" "github.com/prometheus/alertmanager/tracing" - "github.com/prometheus/alertmanager/types" ) var tracer = tracing.NewTracer("github.com/prometheus/alertmanager/inhibit") @@ -109,7 +107,7 @@ func (ih *Inhibitor) run(ctx context.Context) { } } -func (ih *Inhibitor) processAlert(ctx context.Context, a *types.Alert) { +func (ih *Inhibitor) processAlert(ctx context.Context, a *alert.Alert) { _, span := tracer.Start(ctx, "inhibit.Inhibitor.processAlert", trace.WithAttributes( attribute.String("alerting.alert.name", a.Name()), @@ -124,15 +122,8 @@ func (ih *Inhibitor) processAlert(ctx context.Context, a *types.Alert) { if r.SourceMatchers.Matches(a.Labels) { attr := attribute.String("alerting.inhibit_rule.name", r.Name) span.AddEvent("alert matched rule source", trace.WithAttributes(attr)) - if err := r.scache.Set(a); err != nil { - message := "error on set alert" - ih.logger.Error(message, "err", err) - span.SetStatus(codes.Error, message) - span.RecordError(err) - continue - } span.SetAttributes(attr) - r.updateIndex(a) + r.cache.set(a) } } } @@ -154,7 +145,7 @@ func (ih *Inhibitor) Run() { runCtx, runCancel := context.WithCancel(ctx) for _, rule := range ih.rules { - go rule.scache.Run(runCtx, 15*time.Minute) + go rule.cache.run(runCtx, 15*time.Minute) } g.Add(func() error { @@ -257,13 +248,7 @@ type InhibitRule struct { Equal map[model.LabelName]struct{} // Cache of alerts matching source labels. - scache *store.Alerts - - // Index of fingerprints of source alert equal labels to fingerprint of source alert. - // The index helps speed up source alert lookups from scache significantely in scenarios with 100s of source alerts cached. - // The index items might overwrite eachother if multiple source alerts have exact equal labels. - // Overwrites only happen if the new source alert has bigger EndsAt value. - sindex *index + cache *cache } // NewInhibitRule returns a new InhibitRule based on a configuration definition. @@ -320,87 +305,12 @@ func NewInhibitRule(cr amcommoncfg.InhibitRule) *InhibitRule { equal[model.LabelName(ln)] = struct{}{} } - rule := &InhibitRule{ + return &InhibitRule{ Name: cr.Name, SourceMatchers: sourcem, TargetMatchers: targetm, Equal: equal, - scache: store.NewAlerts(), - sindex: newIndex(), - } - - rule.scache.SetGCCallback(rule.gcCallback) - - return rule -} - -// fingerprintEquals returns the fingerprint of the equal labels of the given label set. -func (r *InhibitRule) fingerprintEquals(lset model.LabelSet) model.Fingerprint { - equalSet := make(model.LabelSet, len(r.Equal)) - for n := range r.Equal { - equalSet[n] = lset[n] - } - return equalSet.Fingerprint() -} - -// updateIndex updates the source alert index if necessary. -func (r *InhibitRule) updateIndex(alert *types.Alert) { - fp := alert.Fingerprint() - // Calculate source labelset subset which is in equals. - eq := r.fingerprintEquals(alert.Labels) - - // Check if the equal labelset is already in the index. - indexed, ok := r.sindex.Get(eq) - if !ok { - // If not, add it. - r.sindex.Set(eq, fp) - return - } - // If the indexed fingerprint is the same as the new fingerprint, do nothing. - if indexed == fp { - return - } - - // New alert and existing index are not the same, compare them. - existing, err := r.scache.Get(indexed) - if err != nil { - // failed to get the existing alert, overwrite the index. - r.sindex.Set(eq, fp) - return - } - - // If the new alert resolves after the existing alert, replace the index. - if existing.ResolvedAt(alert.EndsAt) { - r.sindex.Set(eq, fp) - return - } - // If the existing alert resolves after the new alert, do nothing. -} - -// findEqualSourceAlert returns the source alert that matches the equal labels of the given label set. -func (r *InhibitRule) findEqualSourceAlert(lset model.LabelSet, now time.Time) (*types.Alert, bool) { - equalsFP := r.fingerprintEquals(lset) - sourceFP, ok := r.sindex.Get(equalsFP) - if ok { - alert, err := r.scache.Get(sourceFP) - if err != nil { - return nil, false - } - - if alert.ResolvedAt(now) { - return nil, false - } - - return alert, true - } - - return nil, false -} - -func (r *InhibitRule) gcCallback(alerts []*types.Alert) { - for _, a := range alerts { - fp := r.fingerprintEquals(a.Labels) - r.sindex.Delete(fp) + cache: newCache(equal), } } @@ -409,13 +319,7 @@ func (r *InhibitRule) gcCallback(alerts []*types.Alert) { // is returned. If excludeTwoSidedMatch is true, alerts that match both the // source and the target side of the rule are disregarded. func (r *InhibitRule) hasEqual(lset model.LabelSet, excludeTwoSidedMatch bool, now time.Time) (model.Fingerprint, bool) { - equal, found := r.findEqualSourceAlert(lset, now) - if found { - if excludeTwoSidedMatch && r.TargetMatchers.Matches(equal.Labels) { - return model.Fingerprint(0), false - } - return equal.Fingerprint(), found - } - - return model.Fingerprint(0), false + return r.cache.find(lset, now, func(a *alert.Alert) bool { + return !excludeTwoSidedMatch || !r.TargetMatchers.Matches(a.Labels) + }) } 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 2ae6ef38fc..91e57a92f6 100644 --- a/inhibit/inhibit_test.go +++ b/inhibit/inhibit_test.go @@ -29,7 +29,6 @@ import ( "github.com/prometheus/alertmanager/marker" "github.com/prometheus/alertmanager/pkg/labels" "github.com/prometheus/alertmanager/provider" - "github.com/prometheus/alertmanager/store" ) var nopLogger = promslog.NewNopLogger() @@ -52,16 +51,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,30 +158,121 @@ 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{}{} for _, ln := range c.equal { - r.Equal[ln] = struct{}{} + equal[ln] = struct{}{} + } + r := &InhibitRule{ + TargetMatchers: c.targetMatchers, + Equal: equal, + cache: newCache(equal), } for _, v := range c.initial { - r.scache.Set(v) - r.updateIndex(v) + r.cache.set(v) } - 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.cache.gc() + + _, found = r.hasEqual(target, true, now) + require.True(t, found) +} + +func TestInhibitRuleGCCallbackDoesNotRemoveRefreshedSameFingerprintSourceAlert(t *testing.T) { + t.Parallel() + + 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.cache.gc() + + _, found := r.hasEqual(model.LabelSet{"t": "1", "e": "1"}, false, now) + require.True(t, found) +} + func TestInhibitRuleMatches(t *testing.T) { t.Parallel() @@ -198,15 +306,8 @@ func TestInhibitRuleMatches(t *testing.T) { }, } - ih.rules[0].scache = store.NewAlerts() - ih.rules[0].scache.Set(sourceAlert1) - ih.rules[0].sindex = newIndex() - ih.rules[0].updateIndex(sourceAlert1) - - ih.rules[1].scache = store.NewAlerts() - ih.rules[1].scache.Set(sourceAlert2) - ih.rules[1].sindex = newIndex() - ih.rules[1].updateIndex(sourceAlert2) + ih.rules[0].cache.set(sourceAlert1) + ih.rules[1].cache.set(sourceAlert2) cases := []struct { target model.LabelSet @@ -296,15 +397,8 @@ func TestInhibitRuleMatchers(t *testing.T) { }, } - ih.rules[0].scache = store.NewAlerts() - ih.rules[0].scache.Set(sourceAlert1) - ih.rules[0].sindex = newIndex() - ih.rules[0].updateIndex(sourceAlert1) - - ih.rules[1].scache = store.NewAlerts() - ih.rules[1].scache.Set(sourceAlert2) - ih.rules[1].sindex = newIndex() - ih.rules[1].updateIndex(sourceAlert2) + ih.rules[0].cache.set(sourceAlert1) + ih.rules[1].cache.set(sourceAlert2) cases := []struct { target model.LabelSet @@ -566,12 +660,10 @@ func TestInhibit(t *testing.T) { } func TestInhibitRule_fingerprintEquals(t *testing.T) { - rule := &InhibitRule{ - Equal: map[model.LabelName]struct{}{ - "cluster": {}, - "service": {}, - }, - } + c := newCache(map[model.LabelName]struct{}{ + "cluster": {}, + "service": {}, + }) lset := model.LabelSet{ "cluster": "prod", @@ -579,7 +671,7 @@ func TestInhibitRule_fingerprintEquals(t *testing.T) { "instance": "host1", } - fp := rule.fingerprintEquals(lset) + fp := c.fingerprintEquals(lset) // Same equal labels should produce same fingerprint lset2 := model.LabelSet{ @@ -587,14 +679,80 @@ func TestInhibitRule_fingerprintEquals(t *testing.T) { "service": "api", "instance": "host2", // different non-equal label } - require.Equal(t, fp, rule.fingerprintEquals(lset2)) + require.Equal(t, fp, c.fingerprintEquals(lset2)) // Different equal label value should produce different fingerprint lset3 := model.LabelSet{ "cluster": "staging", "service": "api", } - require.NotEqual(t, fp, rule.fingerprintEquals(lset3)) + require.NotEqual(t, fp, c.fingerprintEquals(lset3)) +} + +func TestInhibitRuleIndexSurvivesGC(t *testing.T) { + now := time.Now() + r := NewInhibitRule(amcommoncfg.InhibitRule{Equal: []string{"cluster"}}) + + active := &alert.Alert{Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "S1", "cluster": "c1"}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(2 * time.Hour), + }} + resolved := &alert.Alert{Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "S2", "cluster": "c1"}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(-time.Minute), + }} + r.cache.set(active) + r.cache.set(resolved) + + target := model.LabelSet{"alertname": "T", "cluster": "c1"} + fp, ok := r.hasEqual(target, false, now) + require.True(t, ok) + require.Equal(t, active.Fingerprint(), fp) + + r.cache.gc() + require.Len(t, r.cache.alerts, 1) + require.Contains(t, r.cache.alerts, active.Fingerprint()) + + fp, ok = r.hasEqual(target, false, now) + require.True(t, ok, "active source alert must still inhibit after GC of a sibling") + require.Equal(t, active.Fingerprint(), fp) + require.Len(t, r.cache.index, 1) + + active.EndsAt = now.Add(-time.Second) + r.cache.set(active) + r.cache.gc() + _, ok = r.hasEqual(target, false, now) + require.False(t, ok) + require.Empty(t, r.cache.alerts) + require.Empty(t, r.cache.index, "empty index keys must be removed") +} + +func TestInhibitRuleTwoSidedDoesNotShadow(t *testing.T) { + now := time.Now() + r := NewInhibitRule(amcommoncfg.InhibitRule{ + TargetMatchers: amcommoncfg.Matchers{&labels.Matcher{Type: labels.MatchEqual, Name: "severity", Value: "warning"}}, + Equal: []string{"cluster"}, + }) + + sourceOnly := &alert.Alert{Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "S1", "cluster": "c1", "severity": "critical"}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(time.Hour), + }} + twoSided := &alert.Alert{Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "S2", "cluster": "c1", "severity": "warning"}, + StartsAt: now.Add(-time.Hour), + EndsAt: now.Add(2 * time.Hour), + }} + r.cache.set(sourceOnly) + r.cache.set(twoSided) + + target := model.LabelSet{"alertname": "T", "cluster": "c1", "severity": "warning"} + fp, ok := r.hasEqual(target, true, now) + require.True(t, ok) + require.Equal(t, sourceOnly.Fingerprint(), fp) } func BenchmarkFingerprintEquals(b *testing.B) { @@ -606,7 +764,7 @@ func BenchmarkFingerprintEquals(b *testing.B) { equalLabels[model.LabelName(fmt.Sprintf("label_%d", i))] = struct{}{} } - rule := &InhibitRule{Equal: equalLabels} + c := newCache(equalLabels) // Create a label set with matching values lset := make(model.LabelSet, numLabels+2) @@ -620,7 +778,7 @@ func BenchmarkFingerprintEquals(b *testing.B) { b.ReportAllocs() for b.Loop() { - _ = rule.fingerprintEquals(lset) + _ = c.fingerprintEquals(lset) } }) }