Skip to content

Commit 20eb118

Browse files
zeckegotjosh
zecke
authored and
gotjosh
committed
Fix zeroed EndsAt timestamp
This commit fixes a bug where firing alerts had zeroed EndsAt timestamps in notifications. This bug was introduced in another PR (#1686) to fix issues where alerts are resolved during a flush. (Commit message from George Robinson) Update the acceptance tests as the webhook now has a non-zero EndsAt timestamp for firing alerts. The models.GettableAlert doesn't havea `status` field which means the current test is unable to differentiate between firing and resolved alerts. Signed-off-by: Holger Hans Peter Freyther <[email protected]>
1 parent 9144ad4 commit 20eb118

File tree

15 files changed

+121
-103
lines changed

15 files changed

+121
-103
lines changed

‎dispatch/dispatch_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -425,22 +425,22 @@ route:
425425

426426
require.Equal(t, AlertGroups{
427427
&AlertGroup{
428-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[0], now)},
428+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[0]}, now),
429429
Labels: model.LabelSet{
430430
"alertname": "OtherAlert",
431431
},
432432
Receiver: "prod",
433433
},
434434
&AlertGroup{
435-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[1], now)},
435+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[1]}, now),
436436
Labels: model.LabelSet{
437437
"alertname": "TestingAlert",
438438
"service": "api",
439439
},
440440
Receiver: "testing",
441441
},
442442
&AlertGroup{
443-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[2], now), types.NewAlertSnapshot(inputAlerts[3], now)},
443+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[2], inputAlerts[3]}, now),
444444
Labels: model.LabelSet{
445445
"alertname": "HighErrorRate",
446446
"service": "api",
@@ -449,7 +449,7 @@ route:
449449
Receiver: "prod",
450450
},
451451
&AlertGroup{
452-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[4], now)},
452+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[4]}, now),
453453
Labels: model.LabelSet{
454454
"alertname": "HighErrorRate",
455455
"service": "api",
@@ -458,7 +458,7 @@ route:
458458
Receiver: "prod",
459459
},
460460
&AlertGroup{
461-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[5], now)},
461+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[5]}, now),
462462
Labels: model.LabelSet{
463463
"alertname": "HighLatency",
464464
"service": "db",
@@ -467,7 +467,7 @@ route:
467467
Receiver: "kafka",
468468
},
469469
&AlertGroup{
470-
Alerts: []*types.AlertSnapshot{types.NewAlertSnapshot(inputAlerts[5], now)},
470+
Alerts: types.SnapshotAlerts([]*types.Alert{inputAlerts[5]}, now),
471471
Labels: model.LabelSet{
472472
"alertname": "HighLatency",
473473
"service": "db",

‎notify/discord/discord.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.AlertSnapshot) (bool
9393

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

96-
alerts := types.Snapshot(as...)
96+
alerts := types.AlertsSnapshot(as)
9797
data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
9898
tmpl := notify.TmplText(n.tmpl, data, &err)
9999
if err != nil {

‎notify/msteams/msteams.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.AlertSnapshot) (bool
107107
return false, err
108108
}
109109

110-
alerts := types.Snapshot(as...)
110+
alerts := types.AlertsSnapshot(as)
111111
color := colorGrey
112112
switch alerts.Status() {
113113
case model.AlertFiring:

‎notify/opsgenie/opsgenie.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func (n *Notifier) createRequests(ctx context.Context, as ...*types.AlertSnapsho
153153

154154
var (
155155
alias = key.Hash()
156-
alerts = types.Snapshot(as...)
156+
alerts = types.AlertsSnapshot(as)
157157
)
158158
switch alerts.Status() {
159159
case model.AlertResolved:

‎notify/pagerduty/pagerduty.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.AlertSnapshot) (bool
311311
}
312312

313313
var (
314-
alerts = types.Snapshot(as...)
314+
alerts = types.AlertsSnapshot(as)
315315
data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
316316
eventType = pagerDutyEventTrigger
317317
)

‎notify/victorops/victorops.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (n *Notifier) createVictorOpsPayload(ctx context.Context, as ...*types.Aler
124124
}
125125

126126
var (
127-
alerts = types.Snapshot(as...)
127+
alerts = types.AlertsSnapshot(as)
128128
data = notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
129129
tmpl = notify.TmplText(n.tmpl, data, &err)
130130

‎template/template.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ func (as Alerts) Resolved() []Alert {
360360
func (t *Template) Data(recv string, groupLabels model.LabelSet, alerts ...*types.AlertSnapshot) *Data {
361361
data := &Data{
362362
Receiver: regexp.QuoteMeta(recv),
363-
Status: string(types.Snapshot(alerts...).Status()),
363+
Status: string(types.AlertsSnapshot(alerts).Status()),
364364
Alerts: make(Alerts, 0, len(alerts)),
365365
GroupLabels: KV{},
366366
CommonLabels: KV{},

‎test/cli/acceptance/cli_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ receivers:
7373

7474
alert1 := Alert("alertname", "test1").Active(1, 2)
7575
am.AddAlertsAt(false, 0, alert1)
76-
co.Want(Between(1, 2), Alert("alertname", "test1").Active(1))
76+
co.Want(Between(1, 2), Alert("alertname", "test1").Active(1, 2))
7777

7878
at.Run()
7979

‎test/with_api_v2/acceptance/cluster_test.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ func TestClusterDeduplication(t *testing.T) {
2828
t.Parallel()
2929

3030
conf := `
31+
global:
32+
resolve_timeout: %v
3133
route:
3234
receiver: "default"
3335
group_by: []
@@ -47,11 +49,11 @@ receivers:
4749
co := at.Collector("webhook")
4850
wh := a.NewWebhook(t, co)
4951

50-
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 3)
52+
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, resolveTimeout, wh.Address()), 3)
5153

5254
amc.Push(a.At(1), a.Alert("alertname", "test1"))
5355

54-
co.Want(a.Between(2, 3), a.Alert("alertname", "test1").Active(1))
56+
co.Want(a.Between(2, 3), a.Alert("alertname", "test1").Active(1, 1+resolveTimeout.Seconds()))
5557

5658
at.Run()
5759

@@ -64,6 +66,8 @@ func TestClusterVSInstance(t *testing.T) {
6466
t.Parallel()
6567

6668
conf := `
69+
global:
70+
resolve_timeout: %v
6771
route:
6872
receiver: "default"
6973
group_by: [ "alertname" ]
@@ -98,16 +102,16 @@ receivers:
98102
collectors = append(collectors, tc.Collector("webhook"))
99103
webhook := a.NewWebhook(t, collectors[i])
100104

101-
amClusters = append(amClusters, tc.AlertmanagerCluster(fmt.Sprintf(conf, webhook.Address()), clusterSizes[i]))
105+
amClusters = append(amClusters, tc.AlertmanagerCluster(fmt.Sprintf(conf, resolveTimeout, webhook.Address()), clusterSizes[i]))
102106

103107
wg.Add(1)
104108
}
105109

106-
for _, alertTime := range []float64{0, 2, 4, 6, 8} {
110+
for _, alertTime := range []float64{1, 2, 4, 6, 8} {
107111
for i, amc := range amClusters {
108112
alert := a.Alert("alertname", fmt.Sprintf("test1-%v", alertTime))
109113
amc.Push(a.At(alertTime), alert)
110-
collectors[i].Want(a.Between(alertTime, alertTime+5), alert.Active(alertTime))
114+
collectors[i].Want(a.Between(alertTime, alertTime+5), alert.Active(alertTime, alertTime+resolveTimeout.Seconds()))
111115
}
112116
}
113117

@@ -124,4 +128,8 @@ receivers:
124128
if err != nil {
125129
t.Fatal(err)
126130
}
131+
132+
for _, co := range collectors {
133+
t.Log(co.Check())
134+
}
127135
}

‎test/with_api_v2/acceptance/inhibit_test.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ func TestInhibiting(t *testing.T) {
2929
// gets resolved.
3030

3131
conf := `
32+
global:
33+
resolve_timeout: %v
3234
route:
3335
receiver: "default"
3436
group_by: []
@@ -58,7 +60,7 @@ inhibit_rules:
5860
co := at.Collector("webhook")
5961
wh := NewWebhook(t, co)
6062

61-
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1)
63+
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, resolveTimeout, wh.Address()), 1)
6264

6365
amc.Push(At(1), Alert("alertname", "test1", "job", "testjob", "zone", "aa"))
6466
amc.Push(At(1), Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa"))
@@ -73,21 +75,21 @@ inhibit_rules:
7375
amc.Push(At(3.6), Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(2.2, 3.6))
7476

7577
co.Want(Between(2, 2.5),
76-
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1),
77-
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa").Active(1),
78-
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1),
78+
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
79+
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
80+
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1, 1+resolveTimeout.Seconds()),
7981
)
8082

8183
co.Want(Between(3, 3.5),
82-
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1),
83-
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1),
84-
Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(2.2),
84+
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
85+
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1, 1+resolveTimeout.Seconds()),
86+
Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(2.2, 2.2+resolveTimeout.Seconds()),
8587
)
8688

8789
co.Want(Between(4, 4.5),
88-
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1),
89-
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa").Active(1),
90-
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1),
90+
Alert("alertname", "test1", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
91+
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
92+
Alert("alertname", "InstanceDown", "job", "testjob", "zone", "ab").Active(1, 1+resolveTimeout.Seconds()),
9193
Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(2.2, 3.6),
9294
)
9395

@@ -104,6 +106,8 @@ func TestAlwaysInhibiting(t *testing.T) {
104106
// alerts.
105107

106108
conf := `
109+
global:
110+
resolve_timeout: %v
107111
route:
108112
receiver: "default"
109113
group_by: []
@@ -133,7 +137,7 @@ inhibit_rules:
133137
co := at.Collector("webhook")
134138
wh := NewWebhook(t, co)
135139

136-
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1)
140+
amc := at.AlertmanagerCluster(fmt.Sprintf(conf, resolveTimeout, wh.Address()), 1)
137141

138142
amc.Push(At(1), Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa"))
139143
amc.Push(At(1), Alert("alertname", "JobDown", "job", "testjob", "zone", "aa"))
@@ -142,7 +146,7 @@ inhibit_rules:
142146
amc.Push(At(2.6), Alert("alertname", "InstanceDown", "job", "testjob", "zone", "aa").Active(1, 2.6))
143147

144148
co.Want(Between(2, 2.5),
145-
Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(1),
149+
Alert("alertname", "JobDown", "job", "testjob", "zone", "aa").Active(1, 1+resolveTimeout.Seconds()),
146150
)
147151

148152
co.Want(Between(3, 3.5),

0 commit comments

Comments
 (0)