diff --git a/types/types.go b/types/types.go index 94545f8271..1299f45acb 100644 --- a/types/types.go +++ b/types/types.go @@ -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 diff --git a/types/types_test.go b/types/types_test.go index 4e3fc2d7e8..bb1970a7f2 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -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) + } + }) } }