Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zeroed EndsAt timestamp #3805

Closed
wants to merge 5 commits into from
Closed

Fix zeroed EndsAt timestamp #3805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
deb9b02
Bump prometheus/common to v0.52.3
grobinson-grafana Apr 12, 2024
65b00ac
Fix zeroed EndsAt timestamp
grobinson-grafana Apr 12, 2024
43a4212
Fix lint
grobinson-grafana Apr 12, 2024
fb2794c
Fix tests
grobinson-grafana Apr 12, 2024
7d19a60
Update go.mod
grobinson-grafana Apr 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/alertmanager/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/collectors"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promlog"
Expand Down Expand Up @@ -114,7 +115,7 @@ func init() {
prometheus.MustRegister(configuredReceivers)
prometheus.MustRegister(configuredIntegrations)
prometheus.MustRegister(configuredInhibitionRules)
prometheus.MustRegister(version.NewCollector("alertmanager"))
prometheus.MustRegister(collectors.NewBuildInfoCollector())
}

func instrumentHandler(handlerName string, handler http.HandlerFunc) http.HandlerFunc {
Expand Down
21 changes: 10 additions & 11 deletions dispatch/dispatch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,7 @@ func (ag *aggrGroup) run(nf notifyFunc) {
ag.hasFlushed = true
ag.mtx.Unlock()

ag.flush(func(alerts ...*types.Alert) bool {
return nf(ctx, alerts...)
})
ag.flush(ctx, nf)

cancel()

Expand Down Expand Up @@ -493,29 +491,30 @@ func (ag *aggrGroup) empty() bool {
}

// flush sends notifications for all new alerts.
func (ag *aggrGroup) flush(notify func(...*types.Alert) bool) {
func (ag *aggrGroup) flush(ctx context.Context, nf notifyFunc) {
if ag.empty() {
return
}

now, ok := notify.Now(ctx)
if !ok {
level.Error(ag.logger).Log("msg", "now missing")
return
}

var (
alerts = ag.alerts.List()
alertsSlice = make(types.AlertSlice, 0, len(alerts))
now = time.Now()
)
for _, alert := range alerts {
a := *alert
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect we can remove this, but will do so in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the intent correctly this is passing the reference time through the context and every notifier should use the .*At(now time.Time) variant. As we don't (plan) to modify the alert we don't need to take a deep copy (some notifiers call types.Alerts).

Dropping this copy might be hard. On line 528 (529) the UpdatedAt time is compared between the copy and the storage of the aggregation group. Without a copy these numbers will always be the same.

I am wondering if there are ways to make sure that integrations never call the Resolved(), Status()? Given that alertmanager is likely to see an increase in number of integrations that appears desirable to me to ease writing and reviewing these.

Given that the model.Alert is copied and one needs the reference time. What is your thought on creating something like a model.AlertsSnapshot that holds an immutable snapshot of the alert and the status/reference time? This would ease writing and reviewing the notifiers, protect the core from accidental modificiations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping this copy might be hard. On line 528 (529) the UpdatedAt time is compared between the copy and the storage of the aggregation group. Without a copy these numbers will always be the same.

Oh well spotted!

What is your thought on creating something like a model.AlertsSnapshot that holds an immutable snapshot of the alert and the status/reference time?

This is what I tried at first, but I found it to be quite an intrusive change (#3351 (comment)). I haven't ruled it out though - do you think that would be a better way to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think longterm we are better off with a genuine snapshot. It eases writing integrations, it eases code review of them. I think some of the type changes we can probably automate with gofmt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this?

type AlertSnapshot struct {
	Alert
	// status of the alert at the time it was snapshot.
	status model.AlertStatus
}

func (a *AlertSnapshot) Resolved() bool {
	return a.status == model.AlertResolved
}

func (a *AlertSnapshot) Status() model.AlertStatus {
	return a.status
}

// Need to override String() as otherwise it will call Resolved()
// of model.Alert not AlertSnapshot.
func (a *AlertSnapshot) String() string {
	s := fmt.Sprintf("%s[%s]", a.Name(), a.Fingerprint().String()[:7])
	if a.Resolved() {
		return s + "[resolved]"
	}
	return s + "[active]"
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but from my point of view the trade-off is whether we want all of these fields to be mutable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh – I think I misunderstood. By mutable/immutable are you talking about the fields being package-public (capitalized) or package-private (starting with lowercase)?

Copy link
Contributor

@zecke zecke Apr 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Starting with lower case. I wanted to see how an implementation of this looks like and got the below to compile (and pass most tests but it requires some clean ups).

For me the biggest benefit is that it creates an immutable snapshot and simplifies API. The notification handlers are unable to modify the underlying Alert and the API removes the *At(time.Time) functions and we don't have to remember using notify.Now.

Please see:
main...zecke:alertmanager:zecke-alternative-ends-at

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think you have time to open a PR for this? @gotjosh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Let me fix the formatting and remove some of the repeated code in the tests.

// Ensure that alerts don't resolve as time move forwards.
if !a.ResolvedAt(now) {
a.EndsAt = time.Time{}
}
alertsSlice = append(alertsSlice, &a)
}
sort.Stable(alertsSlice)

level.Debug(ag.logger).Log("msg", "flushing", "alerts", fmt.Sprintf("%v", alertsSlice))

if notify(alertsSlice...) {
if nf(ctx, alertsSlice...) {
for _, a := range alertsSlice {
// Only delete if the fingerprint has not been inserted
// again since we notified about it.
Expand All @@ -526,7 +525,7 @@ func (ag *aggrGroup) flush(notify func(...*types.Alert) bool) {
level.Error(ag.logger).Log("msg", "failed to get alert", "err", err, "alert", a.String())
continue
}
if a.Resolved() && got.UpdatedAt == a.UpdatedAt {
if a.ResolvedAt(now) && got.UpdatedAt == a.UpdatedAt {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems fine. If got has any modification reflected in UpdatedAt then we do not remove the alert.

if err := ag.alerts.Delete(fp); err != nil {
level.Error(ag.logger).Log("msg", "error on delete alert", "err", err, "alert", a.String())
}
Expand Down
14 changes: 7 additions & 7 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
github.com/oklog/run v1.1.0
github.com/oklog/ulid v1.3.1
github.com/prometheus/client_golang v1.19.0
github.com/prometheus/common v0.48.0
github.com/prometheus/common v0.52.4-0.20240415124502-c1b9b7252566
github.com/prometheus/common/assets v0.2.0
github.com/prometheus/common/sigv4 v0.1.0
github.com/prometheus/exporter-toolkit v0.11.0
Expand All @@ -40,7 +40,7 @@ require (
github.com/xlab/treeprint v1.2.0
go.uber.org/atomic v1.11.0
golang.org/x/mod v0.14.0
golang.org/x/net v0.20.0
golang.org/x/net v0.22.0
golang.org/x/text v0.14.0
golang.org/x/tools v0.17.0
gopkg.in/telebot.v3 v3.2.1
Expand Down Expand Up @@ -77,19 +77,19 @@ require (
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.5.0 // indirect
github.com/prometheus/client_model v0.6.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 // indirect
github.com/xhit/go-str2duration/v2 v2.1.0 // indirect
go.mongodb.org/mongo-driver v1.13.1 // indirect
go.opentelemetry.io/otel v1.17.0 // indirect
go.opentelemetry.io/otel/metric v1.17.0 // indirect
go.opentelemetry.io/otel/trace v1.17.0 // indirect
golang.org/x/crypto v0.18.0 // indirect
golang.org/x/oauth2 v0.16.0 // indirect
golang.org/x/crypto v0.21.0 // indirect
golang.org/x/oauth2 v0.18.0 // indirect
golang.org/x/sync v0.6.0 // indirect
golang.org/x/sys v0.16.0 // indirect
golang.org/x/sys v0.18.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/protobuf v1.32.0 // indirect
google.golang.org/protobuf v1.33.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
28 changes: 14 additions & 14 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,15 +425,15 @@ github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:
github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
github.com/prometheus/client_model v0.5.0 h1:VQw1hfvPvk3Uv6Qf29VrPF32JB6rtbgI6cYPYQjL0Qw=
github.com/prometheus/client_model v0.5.0/go.mod h1:dTiFglRmd66nLR9Pv9f0mZi7B7fk5Pm3gvsjB5tr+kI=
github.com/prometheus/client_model v0.6.0 h1:k1v3CzpSRUTrKMppY35TLwPvxHqBu0bYgxZzqGIgaos=
github.com/prometheus/client_model v0.6.0/go.mod h1:NTQHnmxFpouOD0DpvP4XujX3CdOAGQPoaGhyTchlyt8=
github.com/prometheus/common v0.4.1/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4=
github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4=
github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo=
github.com/prometheus/common v0.26.0/go.mod h1:M7rCNAaPfAosfx8veZJCuw84e35h3Cfd9VFqTh1DIvc=
github.com/prometheus/common v0.29.0/go.mod h1:vu+V0TpY+O6vW9J44gczi3Ap/oXXR10b+M/gUGO4Hls=
github.com/prometheus/common v0.48.0 h1:QO8U2CdOzSn1BBsmXJXduaaW+dY/5QLjfB8svtSzKKE=
github.com/prometheus/common v0.48.0/go.mod h1:0/KsvlIEfPQCQ5I2iNSAWKPZziNCvRs5EC6ILDTlAPc=
github.com/prometheus/common v0.52.4-0.20240415124502-c1b9b7252566 h1:1KtXd1thPcL3UnJ0F2X08aEQDeFPZQ56/W1AUeqiazY=
github.com/prometheus/common v0.52.4-0.20240415124502-c1b9b7252566/go.mod h1:BrxBKv3FWBIGXw89Mg1AeBq7FSyRzXWI3l3e7W3RN5U=
github.com/prometheus/common/assets v0.2.0 h1:0P5OrzoHrYBOSM1OigWL3mY8ZvV2N4zIE/5AahrSrfM=
github.com/prometheus/common/assets v0.2.0/go.mod h1:D17UVUE12bHbim7HzwUvtqm6gwBEaDQ0F+hIGbFbccI=
github.com/prometheus/common/sigv4 v0.1.0 h1:qoVebwtwwEhS85Czm2dSROY5fTo2PAPEVdDeppTwGX4=
Expand Down Expand Up @@ -540,8 +540,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y
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.0.0-20220622213112-05595931fe9d/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc=
golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg=
golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA=
golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs=
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=
Expand Down Expand Up @@ -629,8 +629,8 @@ golang.org/x/net v0.0.0-20220412020605-290c469a71a5/go.mod h1:CfG3xpIq0wQ8r1q4Su
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.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo=
golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY=
golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
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=
Expand All @@ -651,8 +651,8 @@ golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8/go.mod h1:KelEdhl1UZF7XfJ
golang.org/x/oauth2 v0.0.0-20220223155221-ee480838109b/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5/go.mod h1:DAh4E804XQdzx2j+YRIaUnCqCV2RuMz24cGBJ5QYIrc=
golang.org/x/oauth2 v0.16.0 h1:aDkGMBSYxElaoP81NpoUoz2oo2R2wHdZpGToUxfyQrQ=
golang.org/x/oauth2 v0.16.0/go.mod h1:hqZ+0LWXsiVoZpeld6jVt06P3adbS2Uu911W1SsJv2o=
golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI=
golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8=
golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
Expand Down Expand Up @@ -748,8 +748,8 @@ golang.org/x/sys v0.0.0-20220502124256-b6088ccd6cba/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU=
golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down Expand Up @@ -1002,8 +1002,8 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I=
google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI=
google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
9 changes: 7 additions & 2 deletions notify/discord/discord.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,11 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
return false, err
}

now, err := notify.ExtractNow(ctx)
if err != nil {
return false, err
}

level.Debug(n.logger).Log("incident", key)

alerts := types.Alerts(as...)
Expand All @@ -116,10 +121,10 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
}

color := colorGrey
if alerts.Status() == model.AlertFiring {
if alerts.StatusAt(now) == model.AlertFiring {
color = colorRed
}
if alerts.Status() == model.AlertResolved {
if alerts.StatusAt(now) == model.AlertResolved {
color = colorGreen
}

Expand Down
1 change: 1 addition & 0 deletions notify/discord/discord_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func TestDiscordTemplating(t *testing.T) {

ctx := context.Background()
ctx = notify.WithGroupKey(ctx, "1")
ctx = notify.WithNow(ctx, time.Now())

ok, err := pd.Notify(ctx, []*types.Alert{
{
Expand Down
7 changes: 6 additions & 1 deletion notify/msteams/msteams.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
return false, err
}

now, err := notify.ExtractNow(ctx)
if err != nil {
return false, err
}

level.Debug(n.logger).Log("incident", key)

data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
Expand All @@ -109,7 +114,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)

alerts := types.Alerts(as...)
color := colorGrey
switch alerts.Status() {
switch alerts.StatusAt(now) {
case model.AlertFiring:
color = colorRed
case model.AlertResolved:
Expand Down
2 changes: 2 additions & 0 deletions notify/msteams/msteams_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ func TestMSTeamsTemplating(t *testing.T) {

ctx := context.Background()
ctx = notify.WithGroupKey(ctx, "1")
ctx = notify.WithNow(ctx, time.Now())

ok, err := pd.Notify(ctx, []*types.Alert{
{
Expand Down Expand Up @@ -175,6 +176,7 @@ func TestNotifier_Notify_WithReason(t *testing.T) {
}
ctx := context.Background()
ctx = notify.WithGroupKey(ctx, "1")
ctx = notify.WithNow(ctx, time.Now())

alert1 := &types.Alert{
Alert: model.Alert{
Expand Down
23 changes: 16 additions & 7 deletions notify/notify.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,9 +683,14 @@ func (n *DedupStage) needsUpdate(entry *nflogpb.Entry, firing, resolved map[uint

// Exec implements the Stage interface.
func (n *DedupStage) Exec(ctx context.Context, _ log.Logger, alerts ...*types.Alert) (context.Context, []*types.Alert, error) {
gkey, ok := GroupKey(ctx)
if !ok {
return ctx, nil, errors.New("group key missing")
gkey, err := ExtractGroupKey(ctx)
if err != nil {
return ctx, nil, err
}

now, err := ExtractNow(ctx)
if err != nil {
return ctx, nil, err
}

repeatInterval, ok := RepeatInterval(ctx)
Expand All @@ -701,7 +706,7 @@ func (n *DedupStage) Exec(ctx context.Context, _ log.Logger, alerts ...*types.Al
var hash uint64
for _, a := range alerts {
hash = n.hash(a)
if a.Resolved() {
if a.ResolvedAt(now) {
resolved = append(resolved, hash)
resolvedSet[hash] = struct{}{}
} else {
Expand All @@ -713,7 +718,7 @@ func (n *DedupStage) Exec(ctx context.Context, _ log.Logger, alerts ...*types.Al
ctx = WithFiringAlerts(ctx, firing)
ctx = WithResolvedAlerts(ctx, resolved)

entries, err := n.nflog.Query(nflog.QGroupKey(gkey), nflog.QReceiver(n.recv))
entries, err := n.nflog.Query(nflog.QGroupKey(gkey.String()), nflog.QReceiver(n.recv))
if err != nil && !errors.Is(err, nflog.ErrNotFound) {
return ctx, nil, err
}
Expand Down Expand Up @@ -774,8 +779,12 @@ func (r RetryStage) Exec(ctx context.Context, l log.Logger, alerts ...*types.Ale
}

func (r RetryStage) exec(ctx context.Context, l log.Logger, alerts ...*types.Alert) (context.Context, []*types.Alert, error) {
var sent []*types.Alert
now, err := ExtractNow(ctx)
if err != nil {
return ctx, nil, err
}

var sent []*types.Alert
// If we shouldn't send notifications for resolved alerts, but there are only
// resolved alerts, report them all as successfully notified (we still want the
// notification log to log them for the next run of DedupStage).
Expand All @@ -788,7 +797,7 @@ func (r RetryStage) exec(ctx context.Context, l log.Logger, alerts ...*types.Ale
return ctx, alerts, nil
}
for _, a := range alerts {
if a.Status() != model.AlertResolved {
if a.StatusAt(now) != model.AlertResolved {
sent = append(sent, a)
}
}
Expand Down
15 changes: 15 additions & 0 deletions notify/notify_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ func TestDedupStage(t *testing.T) {

ctx = WithGroupKey(ctx, "1")

_, _, err = s.Exec(ctx, log.NewNopLogger())
require.EqualError(t, err, "now missing")
ctx = WithNow(ctx, time.Now())

_, _, err = s.Exec(ctx, log.NewNopLogger())
require.EqualError(t, err, "repeat interval missing")

Expand Down Expand Up @@ -404,6 +408,7 @@ func TestRetryStageWithError(t *testing.T) {

ctx := context.Background()
ctx = WithFiringAlerts(ctx, []uint64{0})
ctx = WithNow(ctx, time.Now())

// Notify with a recoverable error should retry and succeed.
resctx, res, err := r.Exec(ctx, log.NewNopLogger(), alerts...)
Expand Down Expand Up @@ -457,6 +462,7 @@ func TestRetryStageWithErrorCode(t *testing.T) {

ctx := context.Background()
ctx = WithFiringAlerts(ctx, []uint64{0})
ctx = WithNow(ctx, time.Now())

// Notify with a non-recoverable error.
resctx, _, err := r.Exec(ctx, log.NewNopLogger(), alerts...)
Expand Down Expand Up @@ -491,6 +497,7 @@ func TestRetryStageWithContextCanceled(t *testing.T) {
}

ctx = WithFiringAlerts(ctx, []uint64{0})
ctx = WithNow(ctx, time.Now())

// Notify with a non-recoverable error.
resctx, _, err := r.Exec(ctx, log.NewNopLogger(), alerts...)
Expand Down Expand Up @@ -529,6 +536,13 @@ func TestRetryStageNoResolved(t *testing.T) {
ctx := context.Background()

resctx, res, err := r.Exec(ctx, log.NewNopLogger(), alerts...)
require.EqualError(t, err, "now missing")
require.Nil(t, res)
require.NotNil(t, resctx)

ctx = WithNow(ctx, time.Now())

resctx, res, err = r.Exec(ctx, log.NewNopLogger(), alerts...)
require.EqualError(t, err, "firing alerts missing")
require.Nil(t, res)
require.NotNil(t, resctx)
Expand Down Expand Up @@ -579,6 +593,7 @@ func TestRetryStageSendResolved(t *testing.T) {

ctx := context.Background()
ctx = WithFiringAlerts(ctx, []uint64{0})
ctx = WithNow(ctx, time.Now())

resctx, res, err := r.Exec(ctx, log.NewNopLogger(), alerts...)
require.NoError(t, err)
Expand Down
Loading
Loading