From fdf051ad5aa1e3dce3d53cb20963466b6a6566e0 Mon Sep 17 00:00:00 2001 From: bergquist Date: Thu, 9 Jun 2016 22:21:28 +0200 Subject: [PATCH] feat(alerting): begin alert rule storage refactoring --- pkg/api/alerting.go | 7 - pkg/models/alerts.go | 55 ++++---- pkg/models/alerts_test.go | 39 ++++++ pkg/services/alerting/dashboard_parser.go | 110 +++++++++++----- pkg/services/alerting/executor.go | 27 ++-- pkg/services/alerting/executor_test.go | 16 +-- pkg/services/alerting/models.go | 46 ++++--- pkg/services/alerting/rule_reader.go | 13 +- pkg/services/sqlstore/alert_rule.go | 2 +- .../sqlstore/alert_rule_changes_test.go | 19 +-- pkg/services/sqlstore/alert_rule_test.go | 60 +++------ pkg/services/sqlstore/alert_state_test.go | 19 +-- .../sqlstore/dashboard_parser_test.go | 122 +++++++++++------- pkg/services/sqlstore/migrations/alert_mig.go | 11 +- pkg/tsdb/graphite/graphite.go | 8 +- .../app/plugins/panel/graph/alert_tab_ctrl.ts | 3 +- 16 files changed, 300 insertions(+), 257 deletions(-) create mode 100644 pkg/models/alerts_test.go diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index 297a4b5fe59..4106dc0eae3 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -64,15 +64,8 @@ func GetAlerts(c *middleware.Context) Response { Id: alert.Id, DashboardId: alert.DashboardId, PanelId: alert.PanelId, - Query: alert.Query, - QueryRefId: alert.QueryRefId, - WarnLevel: alert.WarnLevel, - CritLevel: alert.CritLevel, - Frequency: alert.Frequency, Name: alert.Name, Description: alert.Description, - QueryRange: alert.QueryRange, - Aggregator: alert.Aggregator, State: alert.State, }) } diff --git a/pkg/models/alerts.go b/pkg/models/alerts.go index 030aabea1b9..e069f7e81c3 100644 --- a/pkg/models/alerts.go +++ b/pkg/models/alerts.go @@ -2,49 +2,44 @@ package models import ( "time" + + "github.com/grafana/grafana/pkg/components/simplejson" ) type AlertRule struct { - Id int64 `json:"id"` - OrgId int64 `json:"-"` - DatasourceId int64 `json:"datasourceId"` - DashboardId int64 `json:"dashboardId"` - PanelId int64 `json:"panelId"` - Query string `json:"query"` - QueryRefId string `json:"queryRefId"` - WarnLevel float64 `json:"warnLevel"` - CritLevel float64 `json:"critLevel"` - WarnOperator string `json:"warnOperator"` - CritOperator string `json:"critOperator"` - Frequency int64 `json:"frequency"` - Name string `json:"name"` - Description string `json:"description"` - QueryRange int `json:"queryRange"` - Aggregator string `json:"aggregator"` - State string `json:"state"` + Id int64 + OrgId int64 + DashboardId int64 + PanelId int64 + Name string + Description string + State string - Created time.Time `json:"created"` - Updated time.Time `json:"updated"` + Created time.Time + Updated time.Time + + Expression *simplejson.Json } func (alertRule *AlertRule) ValidToSave() bool { - return alertRule.Query != "" && alertRule.Frequency != 0 && alertRule.QueryRange != 0 && alertRule.Name != "" + return true } -func (this *AlertRule) Equals(other *AlertRule) bool { +func (this *AlertRule) ContainsUpdates(other *AlertRule) bool { result := false - result = result || this.Aggregator != other.Aggregator - result = result || this.CritLevel != other.CritLevel - result = result || this.WarnLevel != other.WarnLevel - result = result || this.WarnOperator != other.WarnOperator - result = result || this.CritOperator != other.CritOperator - result = result || this.Query != other.Query - result = result || this.QueryRefId != other.QueryRefId - result = result || this.Frequency != other.Frequency result = result || this.Name != other.Name result = result || this.Description != other.Description - result = result || this.QueryRange != other.QueryRange + + json1, err1 := this.Expression.MarshalJSON() + json2, err2 := other.Expression.MarshalJSON() + + if err1 != nil || err2 != nil { + return false + } + + result = result || string(json1) != string(json2) + //don't compare .State! That would be insane. return result diff --git a/pkg/models/alerts_test.go b/pkg/models/alerts_test.go new file mode 100644 index 00000000000..33a4937f5ad --- /dev/null +++ b/pkg/models/alerts_test.go @@ -0,0 +1,39 @@ +package models + +import ( + "testing" + + "github.com/grafana/grafana/pkg/components/simplejson" + . "github.com/smartystreets/goconvey/convey" +) + +func TestAlertingModelTest(t *testing.T) { + Convey("Testing Alerting model", t, func() { + + json1, _ := simplejson.NewJson([]byte(`{ "field": "value" }`)) + json2, _ := simplejson.NewJson([]byte(`{ "field": "value" }`)) + + rule1 := &AlertRule{ + Expression: json1, + Name: "Namn", + Description: "Description", + } + + rule2 := &AlertRule{ + Expression: json2, + Name: "Namn", + Description: "Description", + } + + Convey("Testing AlertRule equals", func() { + + So(rule1.ContainsUpdates(rule2), ShouldBeFalse) + }) + + Convey("Changing the expression should contain update", func() { + json2, _ := simplejson.NewJson([]byte(`{ "field": "newValue" }`)) + rule1.Expression = json2 + So(rule1.ContainsUpdates(rule2), ShouldBeTrue) + }) + }) +} diff --git a/pkg/services/alerting/dashboard_parser.go b/pkg/services/alerting/dashboard_parser.go index 66a719373db..c6df8e53a10 100644 --- a/pkg/services/alerting/dashboard_parser.go +++ b/pkg/services/alerting/dashboard_parser.go @@ -18,54 +18,63 @@ func ParseAlertsFromDashboard(cmd *m.SaveDashboardCommand) []*m.AlertRule { alerting := panel.Get("alerting") alert := &m.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").MustFloat64(), - CritLevel: alerting.Get("critLevel").MustFloat64(), - WarnOperator: alerting.Get("warnOperator").MustString(), - CritOperator: alerting.Get("critOperator").MustString(), - Frequency: alerting.Get("frequency").MustInt64(), - Name: alerting.Get("name").MustString(), - Description: alerting.Get("description").MustString(), - QueryRange: alerting.Get("queryRange").MustInt(), - Aggregator: alerting.Get("aggregator").MustString(), + DashboardId: cmd.Result.Id, + OrgId: cmd.Result.OrgId, + PanelId: panel.Get("id").MustInt64(), + Id: alerting.Get("id").MustInt64(), + Name: alerting.Get("name").MustString(), + Description: alerting.Get("description").MustString(), } log.Info("Alertrule: %v", alert.Name) + + expression := alerting + valueQuery := expression.Get("valueQuery") + valueQueryRef := valueQuery.Get("queryRefId").MustString() for _, targetsObj := range panel.Get("targets").MustArray() { target := simplejson.NewFromAny(targetsObj) - if target.Get("refId").MustString() == alert.QueryRefId { - targetJson, err := target.MarshalJSON() - if err == nil { - alert.Query = string(targetJson) + if target.Get("refId").MustString() == valueQueryRef { + datsourceName := "" + if target.Get("datasource").MustString() != "" { + datsourceName = target.Get("datasource").MustString() + } else if panel.Get("datasource").MustString() != "" { + datsourceName = panel.Get("datasource").MustString() } - continue - } - } - if panel.Get("datasource").MustString() == "" { - query := &m.GetDataSourcesQuery{OrgId: cmd.OrgId} - if err := bus.Dispatch(query); err == nil { - for _, ds := range query.Result { - if ds.IsDefault { - alert.DatasourceId = ds.Id + if datsourceName == "" { + query := &m.GetDataSourcesQuery{OrgId: cmd.OrgId} + if err := bus.Dispatch(query); err == nil { + for _, ds := range query.Result { + if ds.IsDefault { + valueQuery.Set("datasourceId", ds.Id) + } + } } + } else { + query := &m.GetDataSourceByNameQuery{ + Name: panel.Get("datasource").MustString(), + OrgId: cmd.OrgId, + } + bus.Dispatch(query) + valueQuery.Set("datasourceId", query.Result.Id) + } + + targetQuery := target.Get("target").MustString() + if targetQuery != "" { + valueQuery.Set("query", targetQuery) } } - } else { - query := &m.GetDataSourceByNameQuery{ - Name: panel.Get("datasource").MustString(), - OrgId: cmd.OrgId, - } - bus.Dispatch(query) - alert.DatasourceId = query.Result.Id } - if alert.ValidToSave() { + expression.Set("valueQuery", valueQuery) + alert.Expression = expression + + alertRule := &AlertRule{} + + ParseAlertRulesFromAlertModel(alert, alertRule) + + if alert.ValidToSave() && alertRule.IsValid() { alerts = append(alerts, alert) } } @@ -73,3 +82,34 @@ func ParseAlertsFromDashboard(cmd *m.SaveDashboardCommand) []*m.AlertRule { return alerts } + +func (rule *AlertRule) IsValid() bool { + return rule.ValueQuery.Query != "" +} + +func ParseAlertRulesFromAlertModel(ruleDef *m.AlertRule, model *AlertRule) error { + critical := ruleDef.Expression.Get("critical") + model.Critical = Level{ + Operator: critical.Get("operator").MustString(), + Level: critical.Get("level").MustFloat64(), + } + + warning := ruleDef.Expression.Get("warning") + model.Warning = Level{ + Operator: warning.Get("operator").MustString(), + Level: warning.Get("level").MustFloat64(), + } + + model.Frequency = ruleDef.Expression.Get("frequency").MustInt64() + + valueQuery := ruleDef.Expression.Get("valueQuery") + model.ValueQuery = AlertQuery{ + Query: valueQuery.Get("query").MustString(), + DatasourceId: valueQuery.Get("datasourceId").MustInt64(), + From: valueQuery.Get("From").MustInt64(), + Until: valueQuery.Get("until").MustInt64(), + Aggregator: valueQuery.Get("aggregator").MustString(), + } + + return nil +} diff --git a/pkg/services/alerting/executor.go b/pkg/services/alerting/executor.go index 08a2125bc4f..e676c318e92 100644 --- a/pkg/services/alerting/executor.go +++ b/pkg/services/alerting/executor.go @@ -102,12 +102,12 @@ func (e *ExecutorImpl) Execute(job *AlertJob, resultQueue chan *AlertResult) { func (e *ExecutorImpl) executeQuery(job *AlertJob) (tsdb.TimeSeriesSlice, error) { getDsInfo := &m.GetDataSourceByIdQuery{ - Id: job.Rule.DatasourceId, + Id: 1, OrgId: job.Rule.OrgId, } if err := bus.Dispatch(getDsInfo); err != nil { - return nil, fmt.Errorf("Could not find datasource for %d", job.Rule.DatasourceId) + return nil, fmt.Errorf("Could not find datasource") } req := e.GetRequestForAlertRule(job.Rule, getDsInfo.Result) @@ -130,16 +130,15 @@ func (e *ExecutorImpl) executeQuery(job *AlertJob) (tsdb.TimeSeriesSlice, error) } func (e *ExecutorImpl) GetRequestForAlertRule(rule *AlertRule, datasource *m.DataSource) *tsdb.Request { - req := &tsdb.Request{ TimeRange: tsdb.TimeRange{ - From: "-" + strconv.Itoa(rule.QueryRange) + "s", + From: "-" + strconv.Itoa(int(rule.ValueQuery.From)) + "s", To: "now", }, Queries: []*tsdb.Query{ { - RefId: rule.QueryRefId, - Query: rule.Query, + RefId: "A", + Query: "apps.fakesite.*.counters.requests.count", DataSource: &tsdb.DataSourceInfo{ Id: datasource.Id, Name: datasource.Name, @@ -159,15 +158,15 @@ func (e *ExecutorImpl) evaluateRule(rule *AlertRule, series tsdb.TimeSeriesSlice for _, serie := range series { log.Debug("Evaluating series", "series", serie.Name) - if aggregator[rule.Aggregator] == nil { + if aggregator["avg"] == nil { continue } - var aggValue = aggregator[rule.Aggregator](serie) - var critOperartor = operators[rule.CritOperator] - var critResult = critOperartor(aggValue, rule.CritLevel) + var aggValue = aggregator["avg"](serie) + var critOperartor = operators[rule.Critical.Operator] + var critResult = critOperartor(aggValue, rule.Critical.Level) - log.Trace(resultLogFmt, "Crit", serie.Name, aggValue, rule.CritOperator, rule.CritLevel, critResult) + log.Trace(resultLogFmt, "Crit", serie.Name, aggValue, rule.Critical.Operator, rule.Critical.Level, critResult) if critResult { return &AlertResult{ State: alertstates.Critical, @@ -176,9 +175,9 @@ func (e *ExecutorImpl) evaluateRule(rule *AlertRule, series tsdb.TimeSeriesSlice } } - var warnOperartor = operators[rule.CritOperator] - var warnResult = warnOperartor(aggValue, rule.CritLevel) - log.Trace(resultLogFmt, "Warn", serie.Name, aggValue, rule.WarnOperator, rule.WarnLevel, warnResult) + var warnOperartor = operators[rule.Warning.Operator] + var warnResult = warnOperartor(aggValue, rule.Warning.Level) + log.Trace(resultLogFmt, "Warn", serie.Name, aggValue, rule.Warning.Operator, rule.Warning.Level, warnResult) if warnResult { return &AlertResult{ State: alertstates.Warn, diff --git a/pkg/services/alerting/executor_test.go b/pkg/services/alerting/executor_test.go index 2ccbf18ad74..ccee2b80ba1 100644 --- a/pkg/services/alerting/executor_test.go +++ b/pkg/services/alerting/executor_test.go @@ -14,7 +14,7 @@ func TestAlertingExecutor(t *testing.T) { Convey("single time serie", func() { Convey("Show return ok since avg is above 2", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "sum"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{2, 0}}), @@ -25,7 +25,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("Show return critical since below 2", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: "<", Aggregator: "sum"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: "<"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{2, 0}}), @@ -36,7 +36,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("Show return critical since sum is above 10", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "sum"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{9, 0}, {9, 0}}), @@ -47,7 +47,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("Show return ok since avg is below 10", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "avg"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{9, 0}, {9, 0}}), @@ -58,7 +58,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("Show return ok since min is below 10", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "min"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{11, 0}, {9, 0}}), @@ -69,7 +69,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("Show return ok since max is above 10", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "max"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{1, 0}, {11, 0}}), @@ -82,7 +82,7 @@ func TestAlertingExecutor(t *testing.T) { Convey("muliple time series", func() { Convey("both are ok", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "sum"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{2, 0}}), @@ -94,7 +94,7 @@ func TestAlertingExecutor(t *testing.T) { }) Convey("first serie is good, second is critical", func() { - rule := &AlertRule{CritLevel: 10, CritOperator: ">", Aggregator: "sum"} + rule := &AlertRule{Critical: Level{Level: 10, Operator: ">"}} timeSeries := []*tsdb.TimeSeries{ tsdb.NewTimeSeries("test1", [][2]float64{{2, 0}}), diff --git a/pkg/services/alerting/models.go b/pkg/services/alerting/models.go index 7b0fb616a1c..afbefac3658 100644 --- a/pkg/services/alerting/models.go +++ b/pkg/services/alerting/models.go @@ -18,21 +18,37 @@ type AlertResult struct { } type AlertRule struct { - Id int64 - OrgId int64 + Id int64 + OrgId int64 + DashboardId int64 + PanelId int64 + //WarnLevel float64 + //CritLevel float64 + //WarnOperator string + //CritOperator string + Frequency int64 + Name string + Description string + State string + + Warning Level + Critical Level + + ValueQuery AlertQuery + EvalFunc string + EvalQuery AlertQuery + EvalParam string +} + +type Level struct { + Operator string + Level float64 +} + +type AlertQuery struct { + Query string DatasourceId int64 - DashboardId int64 - PanelId int64 - Query string - QueryRefId string - WarnLevel float64 - CritLevel float64 - WarnOperator string - CritOperator string - Frequency int64 - Name string - Description string - QueryRange int Aggregator string - State string + From int64 + Until int64 } diff --git a/pkg/services/alerting/rule_reader.go b/pkg/services/alerting/rule_reader.go index 49b781a01c6..c8c3a73cc55 100644 --- a/pkg/services/alerting/rule_reader.go +++ b/pkg/services/alerting/rule_reader.go @@ -52,19 +52,12 @@ func (arr *AlertRuleReader) Fetch() []*AlertRule { model := &AlertRule{} model.Id = ruleDef.Id model.OrgId = ruleDef.OrgId - model.DatasourceId = ruleDef.DatasourceId - model.Query = ruleDef.Query - model.QueryRefId = ruleDef.QueryRefId - model.WarnLevel = ruleDef.WarnLevel - model.WarnOperator = ruleDef.WarnOperator - model.CritLevel = ruleDef.CritLevel - model.CritOperator = ruleDef.CritOperator - model.Frequency = ruleDef.Frequency model.Name = ruleDef.Name model.Description = ruleDef.Description - model.Aggregator = ruleDef.Aggregator model.State = ruleDef.State - model.QueryRange = ruleDef.QueryRange + + ParseAlertRulesFromAlertModel(ruleDef, model) + res[i] = model } diff --git a/pkg/services/sqlstore/alert_rule.go b/pkg/services/sqlstore/alert_rule.go index 36f142df7bb..1228e4cfbff 100644 --- a/pkg/services/sqlstore/alert_rule.go +++ b/pkg/services/sqlstore/alert_rule.go @@ -186,7 +186,7 @@ func upsertAlerts(alerts []*m.AlertRule, posted []*m.AlertRule, sess *xorm.Sessi } if update { - if alertToUpdate.Equals(alert) { + if alertToUpdate.ContainsUpdates(alert) { alert.Updated = time.Now() alert.State = alertToUpdate.State _, err := sess.Id(alert.Id).Update(alert) diff --git a/pkg/services/sqlstore/alert_rule_changes_test.go b/pkg/services/sqlstore/alert_rule_changes_test.go index e04d46b7fe2..cbd4ce93448 100644 --- a/pkg/services/sqlstore/alert_rule_changes_test.go +++ b/pkg/services/sqlstore/alert_rule_changes_test.go @@ -22,20 +22,11 @@ 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, - WarnOperator: ">", - CritOperator: ">", - Frequency: 10, - Name: "Alerting title", - Description: "Alerting description", - QueryRange: 3600, - Aggregator: "avg", - OrgId: FakeOrgId, + PanelId: 1, + DashboardId: testDash.Id, + Name: "Alerting title", + Description: "Alerting description", + OrgId: FakeOrgId, }, } diff --git a/pkg/services/sqlstore/alert_rule_test.go b/pkg/services/sqlstore/alert_rule_test.go index 9a3b0088b5c..9edcf20882a 100644 --- a/pkg/services/sqlstore/alert_rule_test.go +++ b/pkg/services/sqlstore/alert_rule_test.go @@ -8,7 +8,6 @@ import ( ) func TestAlertingDataAccess(t *testing.T) { - Convey("Testing Alerting data access", t, func() { InitTestDB(t) @@ -16,21 +15,11 @@ func TestAlertingDataAccess(t *testing.T) { items := []*m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - OrgId: testDash.OrgId, - Query: "Query", - QueryRefId: "A", - WarnLevel: 30, - CritLevel: 50, - WarnOperator: ">", - CritOperator: ">", - Frequency: 10, - Name: "Alerting title", - Description: "Alerting description", - QueryRange: 3600, - Aggregator: "avg", - DatasourceId: 42, + PanelId: 1, + DashboardId: testDash.Id, + OrgId: testDash.OrgId, + Name: "Alerting title", + Description: "Alerting description", }, } @@ -58,25 +47,15 @@ func TestAlertingDataAccess(t *testing.T) { alert := alertQuery.Result[0] So(err2, ShouldBeNil) - So(alert.Frequency, ShouldEqual, 10) - So(alert.WarnLevel, ShouldEqual, 30) - So(alert.CritLevel, ShouldEqual, 50) - So(alert.WarnOperator, ShouldEqual, ">") - So(alert.CritOperator, ShouldEqual, ">") - So(alert.Query, ShouldEqual, "Query") - So(alert.QueryRefId, ShouldEqual, "A") So(alert.Name, ShouldEqual, "Alerting title") So(alert.Description, ShouldEqual, "Alerting description") - So(alert.QueryRange, ShouldEqual, 3600) - So(alert.Aggregator, ShouldEqual, "avg") So(alert.State, ShouldEqual, "OK") - So(alert.DatasourceId, ShouldEqual, 42) }) Convey("Alerts with same dashboard id and panel id should update", func() { modifiedItems := items - modifiedItems[0].Query = "Updated Query" - modifiedItems[0].State = "ALERT" + modifiedItems[0].Name = "New name" + //modifiedItems[0].State = "ALERT" modifiedCmd := m.SaveAlertsCommand{ DashboardId: testDash.Id, @@ -97,7 +76,7 @@ func TestAlertingDataAccess(t *testing.T) { So(err2, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) - So(query.Result[0].Query, ShouldEqual, "Updated Query") + So(query.Result[0].Name, ShouldEqual, "Name") Convey("Alert state should not be updated", func() { So(query.Result[0].State, ShouldEqual, "OK") @@ -120,19 +99,19 @@ func TestAlertingDataAccess(t *testing.T) { { DashboardId: testDash.Id, PanelId: 1, - Query: "1", + Name: "1", OrgId: 1, }, { DashboardId: testDash.Id, PanelId: 2, - Query: "2", + Name: "2", OrgId: 1, }, { DashboardId: testDash.Id, PanelId: 3, - Query: "3", + Name: "3", OrgId: 1, }, } @@ -180,19 +159,10 @@ 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, - WarnOperator: ">", - CritOperator: ">", - Frequency: 10, - Name: "Alerting title", - Description: "Alerting description", - QueryRange: 3600, - Aggregator: "avg", + PanelId: 1, + DashboardId: testDash.Id, + Name: "Alerting title", + Description: "Alerting description", }, } diff --git a/pkg/services/sqlstore/alert_state_test.go b/pkg/services/sqlstore/alert_state_test.go index 97a785eceac..c112db9725d 100644 --- a/pkg/services/sqlstore/alert_state_test.go +++ b/pkg/services/sqlstore/alert_state_test.go @@ -15,20 +15,11 @@ func TestAlertingStateAccess(t *testing.T) { items := []*m.AlertRule{ { - PanelId: 1, - DashboardId: testDash.Id, - OrgId: testDash.OrgId, - Query: "Query", - QueryRefId: "A", - WarnLevel: 30, - CritLevel: 50, - WarnOperator: ">", - CritOperator: ">", - Frequency: 10, - Name: "Alerting title", - Description: "Alerting description", - QueryRange: 3600, - Aggregator: "avg", + PanelId: 1, + DashboardId: testDash.Id, + OrgId: testDash.OrgId, + Name: "Alerting title", + Description: "Alerting description", }, } diff --git a/pkg/services/sqlstore/dashboard_parser_test.go b/pkg/services/sqlstore/dashboard_parser_test.go index 7dc508c50f2..315ee203576 100644 --- a/pkg/services/sqlstore/dashboard_parser_test.go +++ b/pkg/services/sqlstore/dashboard_parser_test.go @@ -101,17 +101,45 @@ func TestAlertModel(t *testing.T) { "timeShift": null, "aliasColors": {}, "seriesOverrides": [], + + "alerting": { - "queryRef": "A", - "warnLevel": 30, - "critLevel": 50, - "warnOperator": ">", - "critOperator": ">", - "aggregator": "sum", - "queryRange": 3600, "frequency": 10, - "name": "active desktop users", - "description": "restart webservers" + "warning": { + "op": ">", + "level": 10 + }, + "critical": { + "op": ">", + "level": 20 + }, + "function": "static", + "valueQuery": { + "queryRefId": "A", + "from": "5m", + "to": "now", + "agg": "avg", + "params": [ + "#A", + "5m", + "now", + "avg" + ] + }, + "evalQuery": { + "queryRefId": "A", + "from": "5m", + "to": "now", + "agg": "avg", + "params": [ + "#A", + "5m", + "now", + "avg" + ] + }, + "evalStringParam1": "", + "name": "Alerting Panel Title alert" }, "links": [] }, @@ -189,16 +217,42 @@ func TestAlertModel(t *testing.T) { }, "seriesOverrides": [], "alerting": { - "queryRef": "A", - "warnOperator": ">", - "critOperator": ">", - "warnLevel": 300, - "critLevel": 500, - "aggregator": "avg", - "queryRange": 3600, "frequency": 10, - "name": "active mobile users", - "description": "restart itunes" + "warning": { + "op": ">", + "level": 10 + }, + "critical": { + "op": ">", + "level": 20 + }, + "function": "static", + "valueQuery": { + "queryRefId": "A", + "from": "5m", + "to": "now", + "agg": "avg", + "params": [ + "#A", + "5m", + "now", + "avg" + ] + }, + "evalQuery": { + "queryRefId": "A", + "from": "5m", + "to": "now", + "agg": "avg", + "params": [ + "#A", + "5m", + "now", + "avg" + ] + }, + "evalStringParam1": "", + "name": "Alerting Panel Title alert" }, "links": [] } @@ -379,37 +433,13 @@ func TestAlertModel(t *testing.T) { So(v.DashboardId, ShouldEqual, 1) So(v.PanelId, ShouldNotEqual, 0) - So(v.WarnLevel, ShouldNotBeEmpty) - So(v.CritLevel, ShouldNotBeEmpty) - - So(v.Aggregator, ShouldNotBeEmpty) - So(v.Query, ShouldNotBeEmpty) - So(v.QueryRefId, ShouldNotBeEmpty) - So(v.QueryRange, ShouldNotEqual, 0) - So(v.Frequency, ShouldNotEqual, 0) So(v.Name, ShouldNotBeEmpty) So(v.Description, ShouldNotBeEmpty) + + expr := simplejson.NewFromAny(v.Expression) + So(expr.Get("valueQuery").Get("query").MustString(), ShouldNotEqual, "") + So(expr.Get("valueQuery").Get("datsourceId").MustInt64(), ShouldNotEqual, 0) } - - So(alerts[0].WarnLevel, ShouldEqual, 30) - So(alerts[1].WarnLevel, ShouldEqual, 300) - - So(alerts[0].Frequency, ShouldEqual, 10) - So(alerts[1].Frequency, ShouldEqual, 10) - - 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)"}`) - - So(alerts[0].DatasourceId, ShouldEqual, 2) - So(alerts[1].DatasourceId, ShouldEqual, 1) }) }) } diff --git a/pkg/services/sqlstore/migrations/alert_mig.go b/pkg/services/sqlstore/migrations/alert_mig.go index e5b5d783886..42cb9b78d39 100644 --- a/pkg/services/sqlstore/migrations/alert_mig.go +++ b/pkg/services/sqlstore/migrations/alert_mig.go @@ -11,21 +11,12 @@ func addAlertMigrations(mg *Migrator) { Columns: []*Column{ {Name: "id", Type: DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true}, {Name: "dashboard_id", Type: DB_BigInt, Nullable: false}, - {Name: "datasource_id", Type: DB_BigInt, Nullable: false}, {Name: "panel_id", Type: DB_BigInt, Nullable: false}, {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_Float, Nullable: false}, - {Name: "warn_operator", Type: DB_NVarchar, Length: 10, Nullable: false}, - {Name: "crit_level", Type: DB_Float, Nullable: false}, - {Name: "crit_operator", Type: DB_NVarchar, Length: 10, Nullable: false}, - {Name: "frequency", Type: DB_BigInt, Nullable: false}, {Name: "name", Type: DB_NVarchar, Length: 255, Nullable: false}, {Name: "description", Type: DB_NVarchar, Length: 255, Nullable: false}, - {Name: "query_range", Type: DB_Int, Nullable: false}, - {Name: "aggregator", Type: DB_NVarchar, Length: 255, Nullable: false}, {Name: "state", Type: DB_NVarchar, Length: 255, Nullable: false}, + {Name: "expression", Type: DB_Text, Nullable: false}, {Name: "created", Type: DB_DateTime, Nullable: false}, {Name: "updated", Type: DB_DateTime, Nullable: false}, }, diff --git a/pkg/tsdb/graphite/graphite.go b/pkg/tsdb/graphite/graphite.go index b7c09715a8d..36e03288879 100644 --- a/pkg/tsdb/graphite/graphite.go +++ b/pkg/tsdb/graphite/graphite.go @@ -7,7 +7,6 @@ import ( "net/url" "time" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/tsdb" ) @@ -39,7 +38,7 @@ func (e *GraphiteExecutor) Execute(queries tsdb.QuerySlice, context *tsdb.QueryC for _, query := range queries { params["target"] = []string{ - getTargetFromQuery(query.Query), + query.Query, } } @@ -77,8 +76,3 @@ func (e *GraphiteExecutor) Execute(queries tsdb.QuerySlice, context *tsdb.QueryC result.QueryResults["A"] = queryRes return result } - -func getTargetFromQuery(query string) string { - json, _ := simplejson.NewJson([]byte(query)) - return json.Get("target").MustString() -} diff --git a/public/app/plugins/panel/graph/alert_tab_ctrl.ts b/public/app/plugins/panel/graph/alert_tab_ctrl.ts index df2516a15ec..1b1f052632c 100644 --- a/public/app/plugins/panel/graph/alert_tab_ctrl.ts +++ b/public/app/plugins/panel/graph/alert_tab_ctrl.ts @@ -81,6 +81,7 @@ export class AlertTabCtrl { var defaultName = (this.panelCtrl.dashboard.title + ' ' + this.panel.title + ' alert'); this.panel.alerting.name = this.panel.alerting.name || defaultName; + this.panel.alerting.description = this.panel.alerting.description || defaultName; this.panel.targets.map(target => { this.metricTargets.push(target); @@ -92,7 +93,7 @@ export class AlertTabCtrl { } evalFuncChanged() { - var evalFuncDef = _.findWhere(this.evalFuncs, { value: this.rule.expression.evalFunc }); + var evalFuncDef = _.findWhere(this.evalFuncs, { value: this.rule.evalFunc }); console.log(evalFuncDef); this.secondParam = evalFuncDef.secondParam; }