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
Max Leonard Inden
parent
108388a72f
commit
008b4a93da
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue