diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 65fcf1523..fe533069e 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -72,7 +72,6 @@ import ( "github.com/prometheus/prometheus/tsdb/agent" "github.com/prometheus/prometheus/util/logging" prom_runtime "github.com/prometheus/prometheus/util/runtime" - "github.com/prometheus/prometheus/util/strutil" "github.com/prometheus/prometheus/web" ) @@ -619,7 +618,7 @@ func main() { Appendable: fanoutStorage, Queryable: localStorage, QueryFunc: rules.EngineQueryFunc(queryEngine, fanoutStorage), - NotifyFunc: sendAlerts(notifierManager, cfg.web.ExternalURL.String()), + NotifyFunc: rules.SendAlerts(notifierManager, cfg.web.ExternalURL.String()), Context: ctxRule, ExternalURL: cfg.web.ExternalURL, Registerer: prometheus.DefaultRegisterer, @@ -1270,36 +1269,6 @@ func computeExternalURL(u, listenAddr string) (*url.URL, error) { return eu, nil } -type sender interface { - Send(alerts ...*notifier.Alert) -} - -// sendAlerts implements the rules.NotifyFunc for a Notifier. -func sendAlerts(s sender, externalURL string) rules.NotifyFunc { - return func(ctx context.Context, expr string, alerts ...*rules.Alert) { - var res []*notifier.Alert - - for _, alert := range alerts { - a := ¬ifier.Alert{ - StartsAt: alert.FiredAt, - Labels: alert.Labels, - Annotations: alert.Annotations, - GeneratorURL: externalURL + strutil.TableLinkForExpression(expr), - } - if !alert.ResolvedAt.IsZero() { - a.EndsAt = alert.ResolvedAt - } else { - a.EndsAt = alert.ValidUntil - } - res = append(res, a) - } - - if len(alerts) > 0 { - s.Send(res...) - } - } -} - // readyStorage implements the Storage interface while allowing to set the actual // storage at a later point in time. type readyStorage struct { diff --git a/cmd/prometheus/main_test.go b/cmd/prometheus/main_test.go index 7dec5b9a5..9fbca5c33 100644 --- a/cmd/prometheus/main_test.go +++ b/cmd/prometheus/main_test.go @@ -198,7 +198,7 @@ func TestSendAlerts(t *testing.T) { } require.Equal(t, tc.exp, alerts) }) - sendAlerts(senderFunc, "http://localhost:9090")(context.TODO(), "up", tc.in...) + rules.SendAlerts(senderFunc, "http://localhost:9090")(context.TODO(), "up", tc.in...) }) } } diff --git a/rules/alerting.go b/rules/alerting.go index ccbb1b592..4cfb1fa85 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -507,6 +507,8 @@ func (r *AlertingRule) sendAlerts(ctx context.Context, ts time.Time, resendDelay } alert.ValidUntil = ts.Add(4 * delta) anew := *alert + // The notifier re-uses the labels slice, hence make a copy. + anew.Labels = alert.Labels.Copy() alerts = append(alerts, &anew) } }) diff --git a/rules/alerting_test.go b/rules/alerting_test.go index 5ef69a2ca..ccf3e52e7 100644 --- a/rules/alerting_test.go +++ b/rules/alerting_test.go @@ -20,10 +20,13 @@ import ( "time" "github.com/go-kit/log" + "github.com/prometheus/common/model" "github.com/stretchr/testify/require" "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/model/relabel" "github.com/prometheus/prometheus/model/timestamp" + "github.com/prometheus/prometheus/notifier" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/storage" @@ -659,3 +662,53 @@ func TestQueryForStateSeries(t *testing.T) { testFunc(tst) } } + +// TestSendAlertsDontAffectActiveAlerts tests a fix for https://github.com/prometheus/prometheus/issues/11424. +func TestSendAlertsDontAffectActiveAlerts(t *testing.T) { + rule := NewAlertingRule( + "TestRule", + nil, + time.Minute, + labels.FromStrings("severity", "critical"), + labels.EmptyLabels(), labels.EmptyLabels(), "", true, nil, + ) + + // Set an active alert. + lbls := labels.FromStrings("a1", "1") + h := lbls.Hash() + al := &Alert{State: StateFiring, Labels: lbls, ActiveAt: time.Now()} + rule.active[h] = al + + expr, err := parser.ParseExpr("foo") + require.NoError(t, err) + rule.vector = expr + + // The relabel rule reproduced the bug here. + opts := notifier.Options{ + QueueCapacity: 1, + RelabelConfigs: []*relabel.Config{ + { + SourceLabels: model.LabelNames{"a1"}, + Regex: relabel.MustNewRegexp("(.+)"), + TargetLabel: "a1", + Replacement: "bug", + Action: "replace", + }, + }, + } + nm := notifier.NewManager(&opts, log.NewNopLogger()) + + f := SendAlerts(nm, "") + notifyFunc := func(ctx context.Context, expr string, alerts ...*Alert) { + require.Len(t, alerts, 1) + require.Equal(t, al, alerts[0]) + f(ctx, expr, alerts...) + } + + rule.sendAlerts(context.Background(), time.Now(), 0, 0, notifyFunc) + nm.Stop() + + // The relabel rule changes a1=1 to a1=bug. + // But the labels with the AlertingRule should not be changed. + require.Equal(t, labels.FromStrings("a1", "1"), rule.active[h].Labels) +} diff --git a/rules/manager.go b/rules/manager.go index 5eed4bfb0..1ed469596 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -35,9 +35,11 @@ import ( "github.com/prometheus/prometheus/model/rulefmt" "github.com/prometheus/prometheus/model/timestamp" "github.com/prometheus/prometheus/model/value" + "github.com/prometheus/prometheus/notifier" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/strutil" ) // RuleHealth describes the health state of a rule. @@ -1168,3 +1170,33 @@ func (m *Manager) AlertingRules() []*AlertingRule { return alerts } + +type Sender interface { + Send(alerts ...*notifier.Alert) +} + +// SendAlerts implements the rules.NotifyFunc for a Notifier. +func SendAlerts(s Sender, externalURL string) NotifyFunc { + return func(ctx context.Context, expr string, alerts ...*Alert) { + var res []*notifier.Alert + + for _, alert := range alerts { + a := ¬ifier.Alert{ + StartsAt: alert.FiredAt, + Labels: alert.Labels, + Annotations: alert.Annotations, + GeneratorURL: externalURL + strutil.TableLinkForExpression(expr), + } + if !alert.ResolvedAt.IsZero() { + a.EndsAt = alert.ResolvedAt + } else { + a.EndsAt = alert.ValidUntil + } + res = append(res, a) + } + + if len(alerts) > 0 { + s.Send(res...) + } + } +}