diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index 3216961469c..4e59f55c6d6 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -1,15 +1,9 @@ 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 ( @@ -110,24 +104,6 @@ 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 diff --git a/pkg/services/ngalert/models/alert_rule_test.go b/pkg/services/ngalert/models/alert_rule_test.go index aa9934abc1a..a5106fb7063 100644 --- a/pkg/services/ngalert/models/alert_rule_test.go +++ b/pkg/services/ngalert/models/alert_rule_test.go @@ -1,14 +1,12 @@ 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" @@ -227,316 +225,3 @@ 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) - - if rule1.DashboardUID == nil { - assert.True(t, diff[0].Left.IsNil()) - } else { - assert.Equal(t, *rule1.DashboardUID, diff[0].Left.Elem().String()) - } - if rule2.DashboardUID == nil { - assert.True(t, diff[0].Right.IsNil()) - } else { - assert.Equal(t, *rule2.DashboardUID, diff[0].Right.Elem().String()) - } - difCnt++ - } - if rule1.PanelID != rule2.PanelID { - diff := diffs.GetDiffsForField("PanelID") - assert.Len(t, diff, 1) - - if rule1.PanelID == nil { - assert.True(t, diff[0].Left.IsNil()) - } else { - assert.Equal(t, *rule1.PanelID, diff[0].Left.Elem().Int()) - } - if rule2.PanelID == nil { - assert.True(t, diff[0].Right.IsNil()) - } else { - assert.Equal(t, *rule2.PanelID, diff[0].Right.Elem().Int()) - } - 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) - } - }) - }) -} diff --git a/pkg/services/ngalert/models/testing.go b/pkg/services/ngalert/models/testing.go index 8c05872ba1d..3428086fc6a 100644 --- a/pkg/services/ngalert/models/testing.go +++ b/pkg/services/ngalert/models/testing.go @@ -2,7 +2,6 @@ package models import ( "encoding/json" - "fmt" "math/rand" "time" @@ -61,11 +60,24 @@ func AlertRuleGen(mutators ...func(*AlertRule)) func() *AlertRule { } rule := &AlertRule{ - ID: rand.Int63(), - OrgID: rand.Int63(), - Title: "TEST-ALERT-" + util.GenerateShortUID(), - Condition: "A", - Data: []AlertQuery{GenerateAlertQuery()}, + 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", + }}, Updated: time.Now().Add(-time.Duration(rand.Intn(100) + 1)), IntervalSeconds: rand.Int63n(60) + 1, Version: rand.Int63(), @@ -88,25 +100,6 @@ 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) { @@ -132,59 +125,3 @@ 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 -} diff --git a/pkg/util/cmputil/reporter.go b/pkg/util/cmputil/reporter.go deleted file mode 100644 index c9532d3cc0f..00000000000 --- a/pkg/util/cmputil/reporter.go +++ /dev/null @@ -1,101 +0,0 @@ -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 = "" - } - right := d.Right.String() - if !d.Right.IsValid() { - right = "" - } - return fmt.Sprintf("%v:\n\t-: %+v\n\t+: %+v", d.Path, left, right) -}