types: fix alert merging

Alert merging assumed that EndsAt would always be empty for firing
alerts. This is no longer true starting with Prometheus v2.4.0: EndsAt
is set to a multiple of the evaluation interval or resend interval
(whichever is the largest). This change updates the merging logic to
support both cases.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
This commit is contained in:
Simon Pasquier 2018-11-06 16:58:08 +01:00
parent d19fae3bae
commit b33509603e
2 changed files with 213 additions and 12 deletions

View File

@ -295,9 +295,8 @@ func Alerts(alerts ...*Alert) model.Alerts {
res := make(model.Alerts, 0, len(alerts))
for _, a := range alerts {
v := a.Alert
// If the end timestamp was set as the expected value in case
// of a timeout but is not reached yet, do not expose it.
if a.Timeout && !a.Resolved() {
// If the end timestamp is not reached yet, do not expose it.
if !a.Resolved() {
v.EndsAt = time.Time{}
}
res = append(res, &v)
@ -321,10 +320,16 @@ func (a *Alert) Merge(o *Alert) *Alert {
res.StartsAt = a.StartsAt
}
// A non-timeout resolved timestamp always rules.
// The latest explicit resolved timestamp wins.
if a.EndsAt.After(o.EndsAt) && !a.Timeout {
res.EndsAt = a.EndsAt
if o.Resolved() {
// The latest explicit resolved timestamp wins if both alerts are effectively resolved.
if a.Resolved() && a.EndsAt.After(o.EndsAt) {
res.EndsAt = a.EndsAt
}
} else {
// A non-timeout timestamp always rules if it is the latest.
if a.EndsAt.After(o.EndsAt) && !a.Timeout {
res.EndsAt = a.EndsAt
}
}
return &res

View File

@ -16,6 +16,7 @@ package types
import (
"reflect"
"sort"
"strconv"
"testing"
"time"
@ -26,13 +27,17 @@ import (
func TestAlertMerge(t *testing.T) {
now := time.Now()
// By convention, alert A is always older than alert B.
pairs := []struct {
A, B, Res *Alert
}{
{
// Both alerts have the Timeout flag set.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert B.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now.Add(2 * time.Minute),
},
UpdatedAt: now,
@ -46,6 +51,61 @@ func TestAlertMerge(t *testing.T) {
UpdatedAt: now.Add(time.Minute),
Timeout: true,
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
Timeout: true,
},
},
{
// Alert A has the Timeout flag set while Alert B has it unset.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert B.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now,
Timeout: true,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now,
EndsAt: now.Add(2 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(2 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
},
{
// Alert A has the Timeout flag unset while Alert B has it set.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert A.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now,
EndsAt: now.Add(2 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
Timeout: true,
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
@ -55,12 +115,148 @@ func TestAlertMerge(t *testing.T) {
Timeout: true,
},
},
{
// Both alerts have the Timeout flag unset and are not resolved.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert A.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now,
EndsAt: now.Add(2 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
},
{
// Both alerts have the Timeout flag unset and are not resolved.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert B.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(4 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-time.Minute),
EndsAt: now.Add(4 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
},
{
// Both alerts have the Timeout flag unset, A is resolved while B isn't.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert B.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-3 * time.Minute),
EndsAt: now.Add(-time.Minute),
},
UpdatedAt: now,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now.Add(time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-3 * time.Minute),
EndsAt: now.Add(time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
},
{
// Both alerts have the Timeout flag unset, B is resolved while A isn't.
// StartsAt is defined by Alert A.
// EndsAt is defined by Alert B.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now.Add(3 * time.Minute),
},
UpdatedAt: now,
},
B: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now,
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-2 * time.Minute),
EndsAt: now,
},
UpdatedAt: now.Add(time.Minute),
},
},
{
// Both alerts are resolved (EndsAt < now).
// StartsAt is defined by Alert B.
// EndsAt is defined by Alert A.
A: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-3 * time.Minute),
EndsAt: now.Add(-time.Minute),
},
UpdatedAt: now.Add(-time.Minute),
},
B: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-4 * time.Minute),
EndsAt: now.Add(-2 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
Res: &Alert{
Alert: model.Alert{
StartsAt: now.Add(-4 * time.Minute),
EndsAt: now.Add(-1 * time.Minute),
},
UpdatedAt: now.Add(time.Minute),
},
},
}
for _, p := range pairs {
if res := p.A.Merge(p.B); !reflect.DeepEqual(p.Res, res) {
t.Errorf("unexpected merged alert %#v", res)
}
for i, p := range pairs {
p := p
t.Run(strconv.Itoa(i), func(t *testing.T) {
if res := p.A.Merge(p.B); !reflect.DeepEqual(p.Res, res) {
t.Errorf("unexpected merged alert %#v", res)
}
if res := p.B.Merge(p.A); !reflect.DeepEqual(p.Res, res) {
t.Errorf("unexpected merged alert %#v", res)
}
})
}
}