Alerting: Revert Revert "Alerting: Calculate diff for two AlertRules" (#46034)

* Revert "Revert "Alerting: Calculate diff for two AlertRules (#45877)" (#46023)"

This reverts commit 82aa5acba6.

* remove flakiness

pkg/services/ngalert/models/alert_rule.go

@@ -1,9 +1,15 @@
 package models
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"time"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+
+	"github.com/grafana/grafana/pkg/util/cmputil"
 )
 
 var (
@@ -104,6 +110,24 @@ type AlertRule struct {
 	Labels      map[string]string
 }
 
+// Diff calculates diff between two alert rules. Returns nil if two rules are equal. Otherwise, returns cmputil.DiffReport
+func (alertRule *AlertRule) Diff(rule *AlertRule, ignore ...string) cmputil.DiffReport {
+	var reporter cmputil.DiffReporter
+	ops := make([]cmp.Option, 0, 4)
+
+	// json.RawMessage is a slice of bytes and therefore cmp's default behavior is to compare it by byte, which is not really useful
+	var jsonCmp = cmp.Transformer("", func(in json.RawMessage) string {
+		return string(in)
+	})
+	ops = append(ops, cmp.Reporter(&reporter), cmpopts.IgnoreFields(AlertQuery{}, "modelProps"), jsonCmp)
+
+	if len(ignore) > 0 {
+		ops = append(ops, cmpopts.IgnoreFields(AlertRule{}, ignore...))
+	}
+	cmp.Equal(alertRule, rule, ops...)
+	return reporter.Diffs
+}
+
 // AlertRuleKey is the alert definition identifier
 type AlertRuleKey struct {
 	OrgID int64

pkg/services/ngalert/models/alert_rule_test.go

@@ -1,12 +1,14 @@
 package models
 
 import (
+	"encoding/json"
 	"math/rand"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/grafana/grafana/pkg/util"
@@ -225,3 +227,294 @@ func TestPatchPartialAlertRule(t *testing.T) {
 		}
 	})
 }
+
+func TestDiff(t *testing.T) {
+	t.Run("should return nil if there is no diff", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := CopyRule(rule1)
+		result := rule1.Diff(rule2)
+		require.Emptyf(t, result, "expected diff to be empty. rule1: %#v, rule2: %#v\ndiff: %s", rule1, rule2, result)
+	})
+
+	t.Run("should respect fields to ignore", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := CopyRule(rule1)
+		rule2.ID = rule1.ID/2 + 1
+		rule2.Version = rule1.Version/2 + 1
+		rule2.Updated = rule1.Updated.Add(1 * time.Second)
+		result := rule1.Diff(rule2, "ID", "Version", "Updated")
+		require.Emptyf(t, result, "expected diff to be empty. rule1: %#v, rule2: %#v\ndiff: %s", rule1, rule2, result)
+	})
+
+	t.Run("should find diff in simple fields", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := AlertRuleGen()()
+
+		diffs := rule1.Diff(rule2, "Data", "Annotations", "Labels") // these fields will be tested separately
+
+		difCnt := 0
+		if rule1.ID != rule2.ID {
+			diff := diffs.GetDiffsForField("ID")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.ID, diff[0].Left.Int())
+			assert.Equal(t, rule2.ID, diff[0].Right.Int())
+			difCnt++
+		}
+		if rule1.OrgID != rule2.OrgID {
+			diff := diffs.GetDiffsForField("OrgID")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.OrgID, diff[0].Left.Int())
+			assert.Equal(t, rule2.OrgID, diff[0].Right.Int())
+			difCnt++
+		}
+		if rule1.Title != rule2.Title {
+			diff := diffs.GetDiffsForField("Title")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.Title, diff[0].Left.String())
+			assert.Equal(t, rule2.Title, diff[0].Right.String())
+			difCnt++
+		}
+		if rule1.Condition != rule2.Condition {
+			diff := diffs.GetDiffsForField("Condition")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.Condition, diff[0].Left.String())
+			assert.Equal(t, rule2.Condition, diff[0].Right.String())
+			difCnt++
+		}
+		if rule1.Updated != rule2.Updated {
+			diff := diffs.GetDiffsForField("Updated")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.Updated, diff[0].Left.Interface())
+			assert.Equal(t, rule2.Updated, diff[0].Right.Interface())
+			difCnt++
+		}
+		if rule1.IntervalSeconds != rule2.IntervalSeconds {
+			diff := diffs.GetDiffsForField("IntervalSeconds")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.IntervalSeconds, diff[0].Left.Int())
+			assert.Equal(t, rule2.IntervalSeconds, diff[0].Right.Int())
+			difCnt++
+		}
+		if rule1.Version != rule2.Version {
+			diff := diffs.GetDiffsForField("Version")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.Version, diff[0].Left.Int())
+			assert.Equal(t, rule2.Version, diff[0].Right.Int())
+			difCnt++
+		}
+		if rule1.UID != rule2.UID {
+			diff := diffs.GetDiffsForField("UID")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.UID, diff[0].Left.String())
+			assert.Equal(t, rule2.UID, diff[0].Right.String())
+			difCnt++
+		}
+		if rule1.NamespaceUID != rule2.NamespaceUID {
+			diff := diffs.GetDiffsForField("NamespaceUID")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.NamespaceUID, diff[0].Left.String())
+			assert.Equal(t, rule2.NamespaceUID, diff[0].Right.String())
+			difCnt++
+		}
+		if rule1.DashboardUID != rule2.DashboardUID {
+			diff := diffs.GetDiffsForField("DashboardUID")
+			assert.Len(t, diff, 1)
+			difCnt++
+		}
+		if rule1.PanelID != rule2.PanelID {
+			diff := diffs.GetDiffsForField("PanelID")
+			assert.Len(t, diff, 1)
+			difCnt++
+		}
+		if rule1.RuleGroup != rule2.RuleGroup {
+			diff := diffs.GetDiffsForField("RuleGroup")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.RuleGroup, diff[0].Left.String())
+			assert.Equal(t, rule2.RuleGroup, diff[0].Right.String())
+			difCnt++
+		}
+		if rule1.NoDataState != rule2.NoDataState {
+			diff := diffs.GetDiffsForField("NoDataState")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.NoDataState, diff[0].Left.Interface())
+			assert.Equal(t, rule2.NoDataState, diff[0].Right.Interface())
+			difCnt++
+		}
+		if rule1.ExecErrState != rule2.ExecErrState {
+			diff := diffs.GetDiffsForField("ExecErrState")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.ExecErrState, diff[0].Left.Interface())
+			assert.Equal(t, rule2.ExecErrState, diff[0].Right.Interface())
+			difCnt++
+		}
+		if rule1.For != rule2.For {
+			diff := diffs.GetDiffsForField("For")
+			assert.Len(t, diff, 1)
+			assert.Equal(t, rule1.For, diff[0].Left.Interface())
+			assert.Equal(t, rule2.For, diff[0].Right.Interface())
+			difCnt++
+		}
+
+		require.Lenf(t, diffs, difCnt, "Got some unexpected diffs. Either add to ignore or add assert to it")
+
+		if t.Failed() {
+			t.Logf("rule1: %#v, rule2: %#v\ndiff: %s", rule1, rule2, diffs)
+		}
+	})
+
+	t.Run("should detect changes in Annotations", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := CopyRule(rule1)
+
+		rule1.Annotations = map[string]string{
+			"key1": "value1",
+			"key2": "value2",
+		}
+
+		rule2.Annotations = map[string]string{
+			"key2": "value22",
+			"key3": "value3",
+		}
+		diff := rule1.Diff(rule2)
+
+		assert.Len(t, diff, 3)
+
+		d := diff.GetDiffsForField("Annotations[key1]")
+		assert.Len(t, d, 1)
+		assert.Equal(t, "value1", d[0].Left.String())
+		assert.False(t, d[0].Right.IsValid())
+
+		d = diff.GetDiffsForField("Annotations[key2]")
+		assert.Len(t, d, 1)
+		assert.Equal(t, "value2", d[0].Left.String())
+		assert.Equal(t, "value22", d[0].Right.String())
+
+		d = diff.GetDiffsForField("Annotations[key3]")
+		assert.Len(t, d, 1)
+		assert.False(t, d[0].Left.IsValid())
+		assert.Equal(t, "value3", d[0].Right.String())
+
+		if t.Failed() {
+			t.Logf("rule1: %#v, rule2: %#v\ndiff: %v", rule1, rule2, diff)
+		}
+	})
+
+	t.Run("should detect changes in Labels", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := CopyRule(rule1)
+
+		rule1.Labels = map[string]string{
+			"key1": "value1",
+			"key2": "value2",
+		}
+
+		rule2.Labels = map[string]string{
+			"key2": "value22",
+			"key3": "value3",
+		}
+		diff := rule1.Diff(rule2)
+
+		assert.Len(t, diff, 3)
+
+		d := diff.GetDiffsForField("Labels[key1]")
+		assert.Len(t, d, 1)
+		assert.Equal(t, "value1", d[0].Left.String())
+		assert.False(t, d[0].Right.IsValid())
+
+		d = diff.GetDiffsForField("Labels[key2]")
+		assert.Len(t, d, 1)
+		assert.Equal(t, "value2", d[0].Left.String())
+		assert.Equal(t, "value22", d[0].Right.String())
+
+		d = diff.GetDiffsForField("Labels[key3]")
+		assert.Len(t, d, 1)
+		assert.False(t, d[0].Left.IsValid())
+		assert.Equal(t, "value3", d[0].Right.String())
+
+		if t.Failed() {
+			t.Logf("rule1: %#v, rule2: %#v\ndiff: %s", rule1, rule2, d)
+		}
+	})
+
+	t.Run("should detect changes in Data", func(t *testing.T) {
+		rule1 := AlertRuleGen()()
+		rule2 := CopyRule(rule1)
+
+		query1 := AlertQuery{
+			RefID:     "A",
+			QueryType: util.GenerateShortUID(),
+			RelativeTimeRange: RelativeTimeRange{
+				From: Duration(5 * time.Hour),
+				To:   0,
+			},
+			DatasourceUID: util.GenerateShortUID(),
+			Model:         json.RawMessage(`{ "test": "data"}`),
+			modelProps: map[string]interface{}{
+				"test": 1,
+			},
+		}
+
+		rule1.Data = []AlertQuery{query1}
+
+		t.Run("should ignore modelProps", func(t *testing.T) {
+			query2 := query1
+			query2.modelProps = map[string]interface{}{
+				"some": "other value",
+			}
+			rule2.Data = []AlertQuery{query2}
+
+			diff := rule1.Diff(rule2)
+
+			assert.Nil(t, diff)
+
+			if t.Failed() {
+				t.Logf("rule1: %#v, rule2: %#v\ndiff: %v", rule1, rule2, diff)
+			}
+		})
+
+		t.Run("should detect changes inside the query", func(t *testing.T) {
+			query2 := query1
+			query2.QueryType = "test"
+			query2.RefID = "test"
+			rule2.Data = []AlertQuery{query2}
+
+			diff := rule1.Diff(rule2)
+
+			assert.Len(t, diff, 2)
+
+			d := diff.GetDiffsForField("Data[0].QueryType")
+			assert.Len(t, d, 1)
+			d = diff.GetDiffsForField("Data[0].RefID")
+			assert.Len(t, d, 1)
+			if t.Failed() {
+				t.Logf("rule1: %#v, rule2: %#v\ndiff: %v", rule1, rule2, diff)
+			}
+		})
+
+		t.Run("should detect new changes in array if too many fields changed", func(t *testing.T) {
+			query2 := query1
+			query2.QueryType = "test"
+			query2.RefID = "test"
+			query2.DatasourceUID = "test"
+			query2.Model = json.RawMessage(`{ "test": "da2ta"}`)
+
+			rule2.Data = []AlertQuery{query2}
+
+			diff := rule1.Diff(rule2)
+
+			assert.Len(t, diff, 2)
+
+			for _, d := range diff {
+				assert.Equal(t, "Data", d.Path)
+				if d.Left.IsValid() {
+					assert.Equal(t, query1, d.Left.Interface())
+				} else {
+					assert.Equal(t, query2, d.Right.Interface())
+				}
+			}
+			if t.Failed() {
+				t.Logf("rule1: %#v, rule2: %#v\ndiff: %v", rule1, rule2, diff)
+			}
+		})
+	})
+}

pkg/services/ngalert/models/testing.go

@@ -2,6 +2,7 @@ package models
 
 import (
 	"encoding/json"
+	"fmt"
 	"math/rand"
 	"time"
 
@@ -60,24 +61,11 @@ func AlertRuleGen(mutators ...func(*AlertRule)) func() *AlertRule {
 		}
 
 		rule := &AlertRule{
-			ID:        rand.Int63(),
-			OrgID:     rand.Int63(),
-			Title:     "TEST-ALERT-" + util.GenerateShortUID(),
-			Condition: "A",
-			Data: []AlertQuery{
-				{
-					DatasourceUID: "-100",
-					Model: json.RawMessage(`{
-			"datasourceUid": "-100",
-			"type":"math",
-			"expression":"2 + 1 < 1"
-		}`),
-					RelativeTimeRange: RelativeTimeRange{
-						From: Duration(5 * time.Hour),
-						To:   Duration(3 * time.Hour),
-					},
-					RefID: "A",
-				}},
+			ID:              rand.Int63(),
+			OrgID:           rand.Int63(),
+			Title:           "TEST-ALERT-" + util.GenerateShortUID(),
+			Condition:       "A",
+			Data:            []AlertQuery{GenerateAlertQuery()},
 			Updated:         time.Now().Add(-time.Duration(rand.Intn(100) + 1)),
 			IntervalSeconds: rand.Int63n(60) + 1,
 			Version:         rand.Int63(),
@@ -100,6 +88,25 @@ func AlertRuleGen(mutators ...func(*AlertRule)) func() *AlertRule {
 	}
 }
 
+func GenerateAlertQuery() AlertQuery {
+	f := rand.Intn(10) + 5
+	t := rand.Intn(f)
+
+	return AlertQuery{
+		DatasourceUID: util.GenerateShortUID(),
+		Model: json.RawMessage(fmt.Sprintf(`{
+			"%s": "%s",
+			"%s":"%d"
+		}`, util.GenerateShortUID(), util.GenerateShortUID(), util.GenerateShortUID(), rand.Int())),
+		RelativeTimeRange: RelativeTimeRange{
+			From: Duration(time.Duration(f) * time.Minute),
+			To:   Duration(time.Duration(t) * time.Minute),
+		},
+		RefID:     util.GenerateShortUID(),
+		QueryType: util.GenerateShortUID(),
+	}
+}
+
 // GenerateUniqueAlertRules generates many random alert rules and makes sure that they have unique UID.
 // It returns a tuple where first element is a map where keys are UID of alert rule and the second element is a slice of the same rules
 func GenerateUniqueAlertRules(count int, f func() *AlertRule) (map[string]*AlertRule, []*AlertRule) {
@@ -125,3 +132,59 @@ func GenerateAlertRules(count int, f func() *AlertRule) []*AlertRule {
 	}
 	return result
 }
+
+// CopyRule creates a deep copy of AlertRule
+func CopyRule(r *AlertRule) *AlertRule {
+	result := AlertRule{
+		ID:              r.ID,
+		OrgID:           r.OrgID,
+		Title:           r.Title,
+		Condition:       r.Condition,
+		Updated:         r.Updated,
+		IntervalSeconds: r.IntervalSeconds,
+		Version:         r.Version,
+		UID:             r.UID,
+		NamespaceUID:    r.NamespaceUID,
+		RuleGroup:       r.RuleGroup,
+		NoDataState:     r.NoDataState,
+		ExecErrState:    r.ExecErrState,
+		For:             r.For,
+	}
+
+	if r.DashboardUID != nil {
+		dash := *r.DashboardUID
+		result.DashboardUID = &dash
+	}
+	if r.PanelID != nil {
+		p := *r.PanelID
+		result.PanelID = &p
+	}
+
+	for _, d := range r.Data {
+		q := AlertQuery{
+			RefID:             d.RefID,
+			QueryType:         d.QueryType,
+			RelativeTimeRange: d.RelativeTimeRange,
+			DatasourceUID:     d.DatasourceUID,
+		}
+		q.Model = make([]byte, 0, cap(d.Model))
+		q.Model = append(q.Model, d.Model...)
+		result.Data = append(result.Data, q)
+	}
+
+	if r.Annotations != nil {
+		result.Annotations = make(map[string]string, len(r.Annotations))
+		for s, s2 := range r.Annotations {
+			result.Annotations[s] = s2
+		}
+	}
+
+	if r.Labels != nil {
+		result.Labels = make(map[string]string, len(r.Labels))
+		for s, s2 := range r.Labels {
+			result.Labels[s] = s2
+		}
+	}
+
+	return &result
+}

pkg/util/cmputil/reporter.go

@@ -0,0 +1,101 @@
+package cmputil
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+type DiffReport []Diff
+
+// GetDiffsForField returns subset of the diffs which path starts with the provided path
+func (r DiffReport) GetDiffsForField(path string) DiffReport {
+	var result []Diff
+	for _, diff := range r {
+		if strings.HasPrefix(path, diff.Path) {
+			result = append(result, diff)
+		}
+	}
+	return result
+}
+
+// DiffReporter is a simple custom reporter that only records differences
+// detected during comparison. Implements an interface required by cmp.Reporter option
+type DiffReporter struct {
+	path  cmp.Path
+	Diffs DiffReport
+}
+
+func (r *DiffReporter) PushStep(ps cmp.PathStep) {
+	r.path = append(r.path, ps)
+}
+
+func (r *DiffReporter) PopStep() {
+	r.path = r.path[:len(r.path)-1]
+}
+
+func (r *DiffReporter) Report(rs cmp.Result) {
+	if !rs.Equal() {
+		vx, vy := r.path.Last().Values()
+		r.Diffs = append(r.Diffs, Diff{
+			Path:  printPath(r.path),
+			Left:  vx,
+			Right: vy,
+		})
+	}
+}
+
+func printPath(p cmp.Path) string {
+	ss := strings.Builder{}
+	for _, s := range p {
+		toAdd := ""
+		switch v := s.(type) {
+		case cmp.StructField:
+			toAdd = v.String()
+		case cmp.MapIndex:
+			toAdd = fmt.Sprintf("[%s]", v.Key())
+		case cmp.SliceIndex:
+			if v.Key() >= 0 {
+				toAdd = fmt.Sprintf("[%d]", v.Key())
+			}
+		}
+		if toAdd == "" {
+			continue
+		}
+		ss.WriteString(toAdd)
+	}
+	return strings.TrimPrefix(ss.String(), ".")
+}
+
+func (r DiffReport) String() string {
+	b := strings.Builder{}
+	for _, diff := range r {
+		b.WriteString(diff.String())
+		b.WriteByte('\n')
+	}
+	return b.String()
+}
+
+type Diff struct {
+	// Path to the field that has difference separated by period. Array index and key are designated by square brackets.
+	// For example, Annotations[12345].Data.Fields[0].ID
+	Path  string
+	Left  reflect.Value
+	Right reflect.Value
+}
+
+func (d *Diff) String() string {
+	left := d.Left.String()
+	// invalid reflect.Value is produced when two collections (slices\maps) are compared and one misses value.
+	// This way go-cmp indicates that an element was added\removed from a list.
+	if !d.Left.IsValid() {
+		left = "<none>"
+	}
+	right := d.Right.String()
+	if !d.Right.IsValid() {
+		right = "<none>"
+	}
+	return fmt.Sprintf("%v:\n\t-: %+v\n\t+: %+v", d.Path, left, right)
+}