From b7541552662e49e9607577551a3840b5391308e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= <l.mierzwa@gmail.com>
Date: Tue, 28 Mar 2017 18:30:58 -0700
Subject: [PATCH] Store alert sha1 internally and use it as a secondary key
 when sorting

We already compute alert sha1, it's a unique string so it's a good match for secondary sort key, used when two alerts have the exact same
timestamp. This helps to ensure that we always have a stable sort order and don't flash the UI if we have a group with alerts created at the exact same time
---
 models/models.go      | 22 ++++++++----
 models/models_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++
 timer.go              |  8 ++---
 3 files changed, 97 insertions(+), 11 deletions(-)
 create mode 100644 models/models_test.go

diff --git a/models/models.go b/models/models.go
index a60adbb3a..ea8d09ab8 100644
--- a/models/models.go
+++ b/models/models.go
@@ -47,13 +47,15 @@ type UnseeSilence struct {
 }
 
 // UnseeAlert is vanilla alert + some additional attributes
-// Unsee extends an alert object with Links map, it's generated from annotations
-// if annotation value is an url it's pulled out of annotation map
-// and returned under links field, Unsee UI used this to show links differently
-// than other annotations
+// unsee extends an alert object with:
+// * Links map, it's generated from annotations if annotation value is an url
+//   it's pulled out of annotation map and returned under links field,
+//   unsee UI used this to show links differently than other annotations
+// * Fingerprint, which is a sha1 of the entire alert
 type UnseeAlert struct {
 	AlertmanagerAlert
-	Links map[string]string `json:"links"`
+	Links       map[string]string `json:"links"`
+	Fingerprint string            `json:"-"`
 }
 
 // UnseeAlertList is flat list of UnseeAlert objects
@@ -67,8 +69,14 @@ func (a UnseeAlertList) Swap(i, j int) {
 	a[i], a[j] = a[j], a[i]
 }
 func (a UnseeAlertList) Less(i, j int) bool {
-	// compare timestamps, if equal compare labels
-	return a[i].StartsAt.After(a[j].StartsAt)
+	// compare timestamps, if equal compare fingerprints to stable sort order
+	if a[i].StartsAt.After(a[j].StartsAt) {
+		return true
+	}
+	if a[i].StartsAt.Before(a[j].StartsAt) {
+		return false
+	}
+	return a[i].Fingerprint < a[j].Fingerprint
 }
 
 // UnseeAlertGroup is vanilla Alertmanager group, but alerts are flattened
diff --git a/models/models_test.go b/models/models_test.go
new file mode 100644
index 000000000..b67e8e09d
--- /dev/null
+++ b/models/models_test.go
@@ -0,0 +1,78 @@
+package models_test
+
+import (
+	"sort"
+	"testing"
+	"time"
+
+	"github.com/cloudflare/unsee/models"
+)
+
+type alertListSortTest struct {
+	startsAt    time.Time
+	fingerprint string
+	position    int
+}
+
+var alertListSortTests = []alertListSortTest{
+	alertListSortTest{
+		startsAt:    time.Date(2017, time.January, 10, 0, 0, 0, 5, time.UTC),
+		fingerprint: "abcdefg",
+		position:    0,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2017, time.January, 10, 0, 0, 0, 1, time.UTC),
+		fingerprint: "bbbbbb",
+		position:    1,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2017, time.January, 10, 0, 0, 0, 0, time.UTC),
+		fingerprint: "cdfddfg",
+		position:    2,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2015, time.March, 10, 0, 0, 0, 0, time.UTC),
+		fingerprint: "xlfjdf",
+		position:    6,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2016, time.December, 10, 0, 0, 0, 0, time.UTC),
+		fingerprint: "011m",
+		position:    4,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2017, time.January, 10, 0, 0, 0, 0, time.UTC),
+		fingerprint: "cxzfg",
+		position:    3,
+	},
+	alertListSortTest{
+		startsAt:    time.Date(2015, time.March, 10, 0, 0, 0, 0, time.UTC),
+		fingerprint: "abv",
+		position:    5,
+	},
+}
+
+func TestUnseeAlertListSort(t *testing.T) {
+	al := models.UnseeAlertList{}
+	for _, testCase := range alertListSortTests {
+		a := models.UnseeAlert{}
+		a.StartsAt = testCase.startsAt
+		a.Fingerprint = testCase.fingerprint
+		al = append(al, a)
+	}
+
+	// repeat sort 100 times to ensure we're always sorting same way
+	iterations := 100
+	failures := 0
+	for i := 1; i <= iterations; i++ {
+		sort.Sort(al)
+		for _, testCase := range alertListSortTests {
+			if al[testCase.position].Fingerprint != testCase.fingerprint {
+				failures++
+			}
+		}
+	}
+	if failures > 0 {
+		t.Errorf("%d sort failures for %d checks", failures, iterations*len(al))
+	}
+}
diff --git a/timer.go b/timer.go
index 5d275c414..c41ffe2d0 100644
--- a/timer.go
+++ b/timer.go
@@ -100,12 +100,12 @@ func PullFromAlertmanager() {
 
 				apiAlert.Labels = transform.StripLables(ignoredLabels, apiAlert.Labels)
 
-				hash := fmt.Sprintf("%x", structhash.Sha1(apiAlert, 1))
+				apiAlert.Fingerprint = fmt.Sprintf("%x", structhash.Sha1(apiAlert, 1))
 
 				// add alert to map if not yet present
-				if _, found := alerts[hash]; !found {
-					alerts[hash] = apiAlert
-					io.WriteString(agHasher, hash) // alert group hasher
+				if _, found := alerts[apiAlert.Fingerprint]; !found {
+					alerts[apiAlert.Fingerprint] = apiAlert
+					io.WriteString(agHasher, apiAlert.Fingerprint) // alert group hasher
 				}
 
 				for k, v := range alert.Labels {