Skip to content

Commit

Permalink
Simplify and rename notification info struct.
Browse files Browse the repository at this point in the history 
The delivered field was previously unused and is removed by this commit.
Only successful notifications are stored. The type was renamed to NotifyInfo.
  • Loading branch information
@fabxc
fabxc committed Nov 6, 2015
1 parent 14001eb commit 8d2bbc3
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 136 deletions.
51 changes: 21 additions & 30 deletions notify/notify.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,35 +219,27 @@ func (n *DedupingNotifier) Notify(ctx context.Context, alerts ...*types.Alert) e

if last != nil {
if a.Resolved() {
if !sendResolved {
continue
}
// If the initial alert was not delivered successfully,
// there is no point in sending a resolved notification.
if !last.Resolved && !last.Delivered {
continue
}
if last.Resolved && last.Delivered {
if !sendResolved || last.Resolved {
continue
}
} else if !last.Resolved {
// Do not send again if last was delivered unless
// the repeat interval has already passed.
if last.Delivered {
if !now.After(last.Timestamp.Add(repeatInterval)) {
// To not repeat initial batch fragmentation after the repeat interval
// has passed, store them and send them anyway if on of the other
// alerts has already passed the repeat interval.
// This way we unify batches again.
resendQueue = append(resendQueue, a)

continue
} else {
doResend = true
}
if !now.After(last.Timestamp.Add(repeatInterval)) {
// To not repeat initial batch fragmentation after the repeat interval
// has passed, store them and send them anyway if on of the other
// alerts has already passed the repeat interval.
// This way we unify batches again.
resendQueue = append(resendQueue, a)

continue
} else {
doResend = true
}
}
} else if a.Resolved() {
// If the alert is resolved but we never notified about it firing,
// there is nothing to do.
continue
}

Expand All @@ -260,24 +252,23 @@ func (n *DedupingNotifier) Notify(ctx context.Context, alerts ...*types.Alert) e
filtered = append(filtered, resendQueue...)
}

var newNotifies []*types.Notify
// The deduping notifier is the last one before actually sending notifications.
// Thus, this is the place where we abort if after all filtering, nothing is left.
if len(filtered) == 0 {
return nil
}

var newNotifies []*types.NotifyInfo

for _, a := range filtered {
newNotifies = append(newNotifies, &types.Notify{
newNotifies = append(newNotifies, &types.NotifyInfo{
Alert: a.Fingerprint(),
SendTo: name,
Delivered: true,
Resolved: a.Resolved(),
Timestamp: now,
})
}

// The deduping notifier is the last one before actually sending notifications.
// Thus, this is the place where we abort if after all filtering, nothing is left.
if len(filtered) == 0 {
return nil
}

if err := n.notifier.Notify(ctx, filtered...); err != nil {
return err
}
Expand Down
121 changes: 47 additions & 74 deletions notify/notify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,31 +59,31 @@ func TestDedupingNotifier(t *testing.T) {

alerts := []*types.Alert{
{
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "0"},
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "1"},
EndsAt: now.Add(-5 * time.Minute),
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "2"},
EndsAt: now.Add(-9 * time.Minute),
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "3"},
EndsAt: now.Add(-20 * time.Minute),
EndsAt: now.Add(-10 * time.Minute),
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "4"},
EndsAt: now.Add(-10 * time.Minute),
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "5"},
EndsAt: now.Add(-10 * time.Minute),
},
}, {
Alert: model.Alert{
Labels: model.LabelSet{"alertname": "6"},
},
},
}
Expand All @@ -93,55 +93,43 @@ func TestDedupingNotifier(t *testing.T) {
fps = append(fps, a.Fingerprint())
}

nsBefore := []*types.Notify{
// The first alert is another attempt to send a previously
// failing and firing notification.
{
Alert: fps[0],
SendTo: "name",
Resolved: false,
Delivered: false,
Timestamp: now.Add(-20 * time.Minute),
},
// The second alert comes through for the first time and
// is omitted here.
nsBefore := []*types.NotifyInfo{
// The first a new alert starting now.
nil,
// The second alert was not previously notified about and
// is already resolved.
nil,
// The third alert is another attempt to send a previously
// failing and resolved notification.
// The third alert is an attempt to resolve a previously
// firing alert.
{
Alert: fps[2],
SendTo: "name",
Resolved: true,
Delivered: false,
Timestamp: now.Add(-10 * time.Minute),
},
// The fourth alert is an attempt to resolve a previously
// firing and delivered alert.
{
Alert: fps[3],
SendTo: "name",
Resolved: false,
Delivered: true,
Timestamp: now.Add(-10 * time.Minute),
},
// The fifth alert is an attempt to resolve an alert again
// The fourth alert is an attempt to resolve an alert again
// even though the previous notification succeeded.
{
Alert: fps[4],
Alert: fps[3],
SendTo: "name",
Resolved: true,
Delivered: true,
Timestamp: now.Add(-10 * time.Minute),
},
// The sixth alert resends a previously successful notification
// The fifth alert resends a previously successful notification
// that was longer than ago than the repeat interval.
{
Alert: fps[5],
Alert: fps[4],
SendTo: "name",
Resolved: false,
Delivered: true,
Timestamp: now.Add(-110 * time.Minute),
},
// The sixth alert is a firing again after being resolved before.
{
Alert: fps[5],
SendTo: "name",
Resolved: true,
Timestamp: now.Add(3 * time.Minute),
},
}

if err := notifies.Set(nsBefore...); err != nil {
Expand All @@ -155,10 +143,10 @@ func TestDedupingNotifier(t *testing.T) {
// After a failing notify the notifies data must be unchanged.
nsCur, err := notifies.Get("name", fps...)
if err != nil {
t.Fatalf("Error getting notifies: %s", err)
t.Fatalf("Error getting notify info: %s", err)
}
if !reflect.DeepEqual(nsBefore, nsCur) {
t.Fatalf("Notifies data has changed unexpectedly")
t.Fatalf("Notify info data has changed unexpectedly")
}

deduper.notifier = record
Expand All @@ -168,50 +156,33 @@ func TestDedupingNotifier(t *testing.T) {

alertsExp := []*types.Alert{
alerts[0],
alerts[1],
alerts[2],
alerts[3],
alerts[4],
alerts[5],
}

nsAfter := []*types.Notify{
{
Alert: fps[0],
SendTo: "name",
Resolved: false,
Delivered: true,
},
nsAfter := []*types.NotifyInfo{
{
Alert: fps[1],
SendTo: "name",
Resolved: false,
Delivered: true,
},
{
Alert: fps[2],
SendTo: "name",
Resolved: true,
Delivered: true,
Alert: fps[0],
SendTo: "name",
Resolved: false,
},
nil,
{
Alert: fps[3],
SendTo: "name",
Resolved: true,
Delivered: true,
Alert: fps[2],
SendTo: "name",
Resolved: true,
},
// Unmodified.
nsBefore[3],
{
Alert: fps[4],
SendTo: "name",
Resolved: true,
Delivered: true,
Timestamp: now.Add(-10 * time.Minute),
Alert: fps[4],
SendTo: "name",
Resolved: false,
},
{
Alert: fps[5],
SendTo: "name",
Resolved: false,
Delivered: true,
Alert: fps[5],
SendTo: "name",
Resolved: false,
},
}

Expand All @@ -223,15 +194,17 @@ func TestDedupingNotifier(t *testing.T) {
t.Fatalf("Error getting notifies: %s", err)
}

// Hack correct timestamps back in if they are sane.
for i, after := range nsAfter {
cur := nsCur[i]
if after.Timestamp.IsZero() {

// Hack correct timestamps back in if they are sane.
if cur != nil && after.Timestamp.IsZero() {
if cur.Timestamp.Before(now) {
t.Fatalf("Wrong timestamp for notify %v", cur)
}
after.Timestamp = cur.Timestamp
}

if !reflect.DeepEqual(after, cur) {
t.Errorf("Unexpected notifies, expected: %v, got: %v", after, cur)
}
Expand Down
14 changes: 7 additions & 7 deletions provider/mem.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ var (
type MemData struct {
mtx sync.RWMutex
alerts map[model.Fingerprint]*types.Alert
notifies map[string]map[model.Fingerprint]*types.Notify
notifies map[string]map[model.Fingerprint]*types.NotifyInfo
}

func NewMemData() *MemData {
return &MemData{
alerts: map[model.Fingerprint]*types.Alert{},
notifies: map[string]map[model.Fingerprint]*types.Notify{},
notifies: map[string]map[model.Fingerprint]*types.NotifyInfo{},
}
}

Expand Down Expand Up @@ -145,7 +145,7 @@ func (a *MemAlerts) getPending() []*types.Alert {
over := map[model.Fingerprint]struct{}{}
for _, ns := range a.data.notifies {
for fp, notify := range ns {
if notify.Resolved && notify.Delivered {
if notify.Resolved {
over[fp] = struct{}{}
}
}
Expand Down Expand Up @@ -205,7 +205,7 @@ func NewMemNotifies(data *MemData) *MemNotifies {
return &MemNotifies{data: data}
}

func (n *MemNotifies) Set(ns ...*types.Notify) error {
func (n *MemNotifies) Set(ns ...*types.NotifyInfo) error {
n.data.mtx.Lock()
defer n.data.mtx.Unlock()

Expand All @@ -215,19 +215,19 @@ func (n *MemNotifies) Set(ns ...*types.Notify) error {
}
am, ok := n.data.notifies[notify.SendTo]
if !ok {
am = map[model.Fingerprint]*types.Notify{}
am = map[model.Fingerprint]*types.NotifyInfo{}
n.data.notifies[notify.SendTo] = am
}
am[notify.Alert] = notify
}
return nil
}

func (n *MemNotifies) Get(dest string, fps ...model.Fingerprint) ([]*types.Notify, error) {
func (n *MemNotifies) Get(dest string, fps ...model.Fingerprint) ([]*types.NotifyInfo, error) {
n.data.mtx.RLock()
defer n.data.mtx.RUnlock()

res := make([]*types.Notify, len(fps))
res := make([]*types.NotifyInfo, len(fps))

ns, ok := n.data.notifies[dest]
if !ok {
Expand Down
4 changes: 2 additions & 2 deletions provider/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type Silences interface {
// Notifies provides information about pending and successful
// notifications.
type Notifies interface {
Get(dest string, fps ...model.Fingerprint) ([]*types.Notify, error)
Get(dest string, fps ...model.Fingerprint) ([]*types.NotifyInfo, error)
// Set several notifies at once. All or none must succeed.
Set(ns ...*types.Notify) error
Set(ns ...*types.NotifyInfo) error
}
Loading

0 comments on commit 8d2bbc3

Please sign in to comment.