From 1624dc9dfd6d6a272c1758eab32631830c119204 Mon Sep 17 00:00:00 2001 From: bergquist Date: Tue, 3 May 2016 17:31:04 +0200 Subject: [PATCH] feat(alerting): separate operator and level --- pkg/api/dtos/alerting.go | 28 ++++----- pkg/models/alerts.go | 56 +++++++++--------- pkg/models/alerts_test.go | 25 +++++--- pkg/services/sqlstore/alert_rule.go | 2 + .../sqlstore/alert_rule_changes_test.go | 26 +++++---- pkg/services/sqlstore/alert_rule_test.go | 58 ++++++++++--------- pkg/services/sqlstore/alert_state_test.go | 26 +++++---- pkg/services/sqlstore/migrations/alert_mig.go | 6 +- .../app/plugins/panel/graph/alert_tab_ctrl.ts | 25 +++----- .../panel/graph/partials/tab_alerting.html | 10 +++- public/sass/components/_alerts.scss | 3 - 11 files changed, 143 insertions(+), 122 deletions(-) diff --git a/pkg/api/dtos/alerting.go b/pkg/api/dtos/alerting.go index f18f5b0ba18..f1081217f79 100644 --- a/pkg/api/dtos/alerting.go +++ b/pkg/api/dtos/alerting.go @@ -1,19 +1,21 @@ package dtos type AlertRuleDTO struct { - Id int64 `json:"id"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - Query string `json:"query"` - QueryRefId string `json:"queryRefId"` - WarnLevel string `json:"warnLevel"` - CritLevel string `json:"critLevel"` - Interval string `json:"interval"` - Title string `json:"title"` - Description string `json:"description"` - QueryRange string `json:"queryRange"` - Aggregator string `json:"aggregator"` - State string `json:"state"` + Id int64 `json:"id"` + DashboardId int64 `json:"dashboardId"` + PanelId int64 `json:"panelId"` + Query string `json:"query"` + QueryRefId string `json:"queryRefId"` + WarnLevel int64 `json:"warnLevel"` + CritLevel int64 `json:"critLevel"` + WarnOperator string `json:"warnOperator"` + CritOperator string `json:"critOperator"` + Interval string `json:"interval"` + Title string `json:"title"` + Description string `json:"description"` + QueryRange string `json:"queryRange"` + Aggregator string `json:"aggregator"` + State string `json:"state"` DashbboardUri string `json:"dashboardUri"` } diff --git a/pkg/models/alerts.go b/pkg/models/alerts.go index 159ea02f328..186b057af25 100644 --- a/pkg/models/alerts.go +++ b/pkg/models/alerts.go @@ -6,20 +6,22 @@ import ( ) type AlertRule struct { - Id int64 `json:"id"` - OrgId int64 `json:"-"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - Query string `json:"query"` - QueryRefId string `json:"queryRefId"` - WarnLevel string `json:"warnLevel"` - CritLevel string `json:"critLevel"` - Interval string `json:"interval"` - Title string `json:"title"` - Description string `json:"description"` - QueryRange string `json:"queryRange"` - Aggregator string `json:"aggregator"` - State string `json:"state"` + Id int64 `json:"id"` + OrgId int64 `json:"-"` + DashboardId int64 `json:"dashboardId"` + PanelId int64 `json:"panelId"` + Query string `json:"query"` + QueryRefId string `json:"queryRefId"` + WarnLevel int64 `json:"warnLevel"` + CritLevel int64 `json:"critLevel"` + WarnOperator string `json:"warnOperator"` + CritOperator string `json:"critOperator"` + Interval string `json:"interval"` + Title string `json:"title"` + Description string `json:"description"` + QueryRange string `json:"queryRange"` + Aggregator string `json:"aggregator"` + State string `json:"state"` } type AlertRuleChange struct { @@ -40,18 +42,20 @@ func (cmd *SaveDashboardCommand) GetAlertModels() *[]AlertRule { alerting := panel.Get("alerting") alert := AlertRule{ - DashboardId: cmd.Result.Id, - OrgId: cmd.Result.OrgId, - PanelId: panel.Get("id").MustInt64(), - Id: alerting.Get("id").MustInt64(), - QueryRefId: alerting.Get("queryRef").MustString(), - WarnLevel: alerting.Get("warnLevel").MustString(), - CritLevel: alerting.Get("critLevel").MustString(), - Interval: alerting.Get("interval").MustString(), - Title: alerting.Get("title").MustString(), - Description: alerting.Get("description").MustString(), - QueryRange: alerting.Get("queryRange").MustString(), - Aggregator: alerting.Get("aggregator").MustString(), + DashboardId: cmd.Result.Id, + OrgId: cmd.Result.OrgId, + PanelId: panel.Get("id").MustInt64(), + Id: alerting.Get("id").MustInt64(), + QueryRefId: alerting.Get("queryRef").MustString(), + WarnLevel: alerting.Get("warnLevel").MustInt64(), + CritLevel: alerting.Get("critLevel").MustInt64(), + WarnOperator: alerting.Get("warnOperator").MustString(), + CritOperator: alerting.Get("critOperator").MustString(), + Interval: alerting.Get("interval").MustString(), + Title: alerting.Get("title").MustString(), + Description: alerting.Get("description").MustString(), + QueryRange: alerting.Get("queryRange").MustString(), + Aggregator: alerting.Get("aggregator").MustString(), } for _, targetsObj := range panel.Get("targets").MustArray() { diff --git a/pkg/models/alerts_test.go b/pkg/models/alerts_test.go index e661d65f017..e70f97d6163 100644 --- a/pkg/models/alerts_test.go +++ b/pkg/models/alerts_test.go @@ -101,8 +101,10 @@ func TestAlertModel(t *testing.T) { "seriesOverrides": [], "alerting": { "queryRef": "A", - "warnLevel": "> 30", - "critLevel": "> 50", + "warnLevel": 30, + "critLevel": 50, + "warnOperator": ">", + "critOperator": ">", "aggregator": "sum", "queryRange": "10m", "interval": "10s", @@ -186,8 +188,10 @@ func TestAlertModel(t *testing.T) { "seriesOverrides": [], "alerting": { "queryRef": "A", - "warnLevel": "> 300", - "critLevel": "> 500", + "warnOperator": ">", + "critOperator": ">", + "warnLevel": 300, + "critLevel": 500, "aggregator": "avg", "queryRange": "10m", "interval": "10s", @@ -363,11 +367,16 @@ func TestAlertModel(t *testing.T) { So(v.Description, ShouldNotBeEmpty) } - So(alerts[0].WarnLevel, ShouldEqual, "> 30") - So(alerts[1].WarnLevel, ShouldEqual, "> 300") + So(alerts[0].WarnLevel, ShouldEqual, 30) + So(alerts[1].WarnLevel, ShouldEqual, 300) - So(alerts[0].CritLevel, ShouldEqual, "> 50") - So(alerts[1].CritLevel, ShouldEqual, "> 500") + So(alerts[0].CritLevel, ShouldEqual, 50) + So(alerts[1].CritLevel, ShouldEqual, 500) + + So(alerts[0].CritOperator, ShouldEqual, ">") + So(alerts[1].CritOperator, ShouldEqual, ">") + So(alerts[0].WarnOperator, ShouldEqual, ">") + So(alerts[1].WarnOperator, ShouldEqual, ">") So(alerts[0].Query, ShouldEqual, `{"refId":"A","target":"aliasByNode(statsd.fakesite.counters.session_start.desktop.count, 4)"}`) So(alerts[1].Query, ShouldEqual, `{"refId":"A","target":"aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}`) diff --git a/pkg/services/sqlstore/alert_rule.go b/pkg/services/sqlstore/alert_rule.go index 86743f5894b..f34319f35bc 100644 --- a/pkg/services/sqlstore/alert_rule.go +++ b/pkg/services/sqlstore/alert_rule.go @@ -75,6 +75,8 @@ func alertIsDifferent(rule1, rule2 m.AlertRule) bool { result = result || rule1.Aggregator != rule2.Aggregator result = result || rule1.CritLevel != rule2.CritLevel result = result || rule1.WarnLevel != rule2.WarnLevel + result = result || rule1.WarnOperator != rule2.WarnOperator + result = result || rule1.CritOperator != rule2.CritOperator result = result || rule1.Query != rule2.Query result = result || rule1.QueryRefId != rule2.QueryRefId result = result || rule1.Interval != rule2.Interval diff --git a/pkg/services/sqlstore/alert_rule_changes_test.go b/pkg/services/sqlstore/alert_rule_changes_test.go index 3605789b3b8..6b6b9bbc87a 100644 --- a/pkg/services/sqlstore/alert_rule_changes_test.go +++ b/pkg/services/sqlstore/alert_rule_changes_test.go @@ -22,18 +22,20 @@ func TestAlertRuleChangesDataAccess(t *testing.T) { Convey("When dashboard is removed", func() { items := []m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - Query: "Query", - QueryRefId: "A", - WarnLevel: "> 30", - CritLevel: "> 50", - Interval: "10", - Title: "Alerting title", - Description: "Alerting description", - QueryRange: "5m", - Aggregator: "avg", - OrgId: FakeOrgId, + PanelId: 1, + DashboardId: testDash.Id, + Query: "Query", + QueryRefId: "A", + WarnLevel: 30, + CritLevel: 50, + WarnOperator: ">", + CritOperator: ">", + Interval: "10", + Title: "Alerting title", + Description: "Alerting description", + QueryRange: "5m", + Aggregator: "avg", + OrgId: FakeOrgId, }, } diff --git a/pkg/services/sqlstore/alert_rule_test.go b/pkg/services/sqlstore/alert_rule_test.go index 5b41da11aac..895fb79e1e4 100644 --- a/pkg/services/sqlstore/alert_rule_test.go +++ b/pkg/services/sqlstore/alert_rule_test.go @@ -16,19 +16,21 @@ func TestAlertingDataAccess(t *testing.T) { items := []m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - OrgId: testDash.OrgId, - Query: "Query", - QueryRefId: "A", - WarnLevel: "> 30", - CritLevel: "> 50", - Interval: "10", - Title: "Alerting title", - Description: "Alerting description", - QueryRange: "5m", - Aggregator: "avg", - State: "OK", + PanelId: 1, + DashboardId: testDash.Id, + OrgId: testDash.OrgId, + Query: "Query", + QueryRefId: "A", + WarnLevel: 30, + CritLevel: 50, + WarnOperator: ">", + CritOperator: ">", + Interval: "10", + Title: "Alerting title", + Description: "Alerting description", + QueryRange: "5m", + Aggregator: "avg", + State: "OK", }, } @@ -59,8 +61,10 @@ func TestAlertingDataAccess(t *testing.T) { So(err2, ShouldBeNil) So(query.Result.Interval, ShouldEqual, "10") - So(query.Result.WarnLevel, ShouldEqual, "> 30") - So(query.Result.CritLevel, ShouldEqual, "> 50") + So(query.Result.WarnLevel, ShouldEqual, 30) + So(query.Result.CritLevel, ShouldEqual, 50) + So(query.Result.WarnOperator, ShouldEqual, ">") + So(query.Result.CritOperator, ShouldEqual, ">") So(query.Result.Query, ShouldEqual, "Query") So(query.Result.QueryRefId, ShouldEqual, "A") So(query.Result.Title, ShouldEqual, "Alerting title") @@ -177,17 +181,19 @@ func TestAlertingDataAccess(t *testing.T) { Convey("When dashboard is removed", func() { items := []m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - Query: "Query", - QueryRefId: "A", - WarnLevel: "> 30", - CritLevel: "> 50", - Interval: "10", - Title: "Alerting title", - Description: "Alerting description", - QueryRange: "5m", - Aggregator: "avg", + PanelId: 1, + DashboardId: testDash.Id, + Query: "Query", + QueryRefId: "A", + WarnLevel: 30, + CritLevel: 50, + WarnOperator: ">", + CritOperator: ">", + Interval: "10", + Title: "Alerting title", + Description: "Alerting description", + QueryRange: "5m", + Aggregator: "avg", }, } diff --git a/pkg/services/sqlstore/alert_state_test.go b/pkg/services/sqlstore/alert_state_test.go index 492de8d77aa..aba95ee100e 100644 --- a/pkg/services/sqlstore/alert_state_test.go +++ b/pkg/services/sqlstore/alert_state_test.go @@ -15,18 +15,20 @@ func TestAlertingStateAccess(t *testing.T) { items := []m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - OrgId: testDash.OrgId, - Query: "Query", - QueryRefId: "A", - WarnLevel: "> 30", - CritLevel: "> 50", - Interval: "10", - Title: "Alerting title", - Description: "Alerting description", - QueryRange: "5m", - Aggregator: "avg", + PanelId: 1, + DashboardId: testDash.Id, + OrgId: testDash.OrgId, + Query: "Query", + QueryRefId: "A", + WarnLevel: 30, + CritLevel: 50, + WarnOperator: ">", + CritOperator: ">", + Interval: "10", + Title: "Alerting title", + Description: "Alerting description", + QueryRange: "5m", + Aggregator: "avg", }, } diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index c9647c233e4..97f3d533331 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -12,8 +12,10 @@ func addAlertMigrations(mg *Migrator) { {Name: "org_id", Type: DB_BigInt, Nullable: false}, {Name: "query", Type: DB_Text, Nullable: false}, {Name: "query_ref_id", Type: DB_NVarchar, Length: 255, Nullable: false}, - {Name: "warn_level", Type: DB_NVarchar, Length: 255, Nullable: false}, - {Name: "crit_level", Type: DB_NVarchar, Length: 255, Nullable: false}, + {Name: "warn_level", Type: DB_BigInt, Nullable: false}, + {Name: "warn_operator", Type: DB_NVarchar, Length: 10, Nullable: false}, + {Name: "crit_level", Type: DB_BigInt, Nullable: false}, + {Name: "crit_operator", Type: DB_NVarchar, Length: 10, Nullable: false}, {Name: "interval", Type: DB_NVarchar, Length: 255, Nullable: false}, {Name: "title", Type: DB_NVarchar, Length: 255, Nullable: false}, {Name: "description", Type: DB_NVarchar, Length: 255, Nullable: false}, diff --git a/public/app/plugins/panel/graph/alert_tab_ctrl.ts b/public/app/plugins/panel/graph/alert_tab_ctrl.ts index 3d788da0bff..e55c9b1ceec 100644 --- a/public/app/plugins/panel/graph/alert_tab_ctrl.ts +++ b/public/app/plugins/panel/graph/alert_tab_ctrl.ts @@ -24,36 +24,25 @@ export class AlertTabCtrl { convertThresholdsToAlertThresholds() { if (this.panel.grid && this.panel.grid.threshold1) { - this.panel.alerting.warnLevel = '< ' + this.panel.grid.threshold1; + this.panel.alerting.warnOperator = '<'; + this.panel.alerting.warnLevel = this.panel.grid.threshold1; } if (this.panel.grid && this.panel.grid.threshold2) { - this.panel.alerting.critLevel = '< ' + this.panel.grid.threshold2; + this.panel.alerting.critOperator = '<'; + this.panel.alerting.critLevel = this.panel.grid.threshold2; } } thresholdsUpdated() { if (this.panel.alerting.warnLevel) { - var threshold = this.panel.alerting.warnLevel - .replace(' ', '') - .replace('>', '') - .replace('<', '') - .replace('>=', '') - .replace('<=', ''); - - this.panel.grid.threshold1 = parseInt(threshold); + this.panel.grid.threshold1 = parseInt(this.panel.alerting.warnLevel); } if (this.panel.alerting.critLevel) { - var threshold = this.panel.alerting.critLevel - .replace(' ', '') - .replace('>', '') - .replace('<', '') - .replace('>=', '') - .replace('<=', ''); - - this.panel.grid.threshold2 = parseInt(threshold); + this.panel.grid.threshold2 = parseInt(this.panel.alerting.critLevel); } + this.panelCtrl.render(); } } diff --git a/public/app/plugins/panel/graph/partials/tab_alerting.html b/public/app/plugins/panel/graph/partials/tab_alerting.html index 3d9e0040940..101a81859eb 100644 --- a/public/app/plugins/panel/graph/partials/tab_alerting.html +++ b/public/app/plugins/panel/graph/partials/tab_alerting.html @@ -16,14 +16,20 @@ Warn level - +
+ +
+
Critical level - +
+ +
+
diff --git a/public/sass/components/_alerts.scss b/public/sass/components/_alerts.scss index 4d2762e0561..7950a85bbcb 100644 --- a/public/sass/components/_alerts.scss +++ b/public/sass/components/_alerts.scss @@ -8,17 +8,14 @@ .alert-state-online { - //background-image: url('/img/online.svg'); color: $online; } .alert-state-warn { - //background-image: url('/img/warn-tiny.svg'); color: $warn; } .alert-state-critical { - //background-image: url('/img/critical.svg'); color: $critical; }