From 489f087fbdbb03eebf57e2b4ec536cf6817f8370 Mon Sep 17 00:00:00 2001 From: bergquist Date: Mon, 7 Nov 2016 12:42:39 +0100 Subject: [PATCH] feat(alerting): reduce states. Make exeuction result configurable. ref #6444 --- pkg/metrics/metrics.go | 4 +- pkg/models/alert.go | 30 +++-- pkg/services/alerting/eval_context.go | 47 ++----- pkg/services/alerting/eval_context_test.go | 62 ---------- pkg/services/alerting/result_handler.go | 53 +++++--- pkg/services/alerting/result_handler_test.go | 116 +++++++++++++----- pkg/services/alerting/rule.go | 3 +- public/app/features/alerting/alert_def.ts | 10 +- .../app/features/alerting/alert_tab_ctrl.ts | 3 + .../features/alerting/partials/alert_tab.html | 11 +- 10 files changed, 181 insertions(+), 158 deletions(-) diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 1cdcc71e132accd..481818aab235577 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -40,8 +40,7 @@ var ( M_Alerting_Result_State_Ok Counter M_Alerting_Result_State_Paused Counter M_Alerting_Result_State_NoData Counter - M_Alerting_Result_State_ExecError Counter - M_Alerting_Result_State_Pending Counter + M_Alerting_Result_State_Pending Counter M_Alerting_Active_Alerts Counter M_Alerting_Notification_Sent_Slack Counter M_Alerting_Notification_Sent_Email Counter @@ -102,7 +101,6 @@ func initMetricVars(settings *MetricSettings) { M_Alerting_Result_State_Ok = RegCounter("alerting.result", "state", "ok") M_Alerting_Result_State_Paused = RegCounter("alerting.result", "state", "paused") M_Alerting_Result_State_NoData = RegCounter("alerting.result", "state", "no_data") - M_Alerting_Result_State_ExecError = RegCounter("alerting.result", "state", "exec_error") M_Alerting_Result_State_Pending = RegCounter("alerting.result", "state", "pending") M_Alerting_Active_Alerts = RegCounter("alerting.active_alerts") diff --git a/pkg/models/alert.go b/pkg/models/alert.go index ec2e8e18841bbba..67170a0dd3646b5 100644 --- a/pkg/models/alert.go +++ b/pkg/models/alert.go @@ -9,35 +9,47 @@ import ( type AlertStateType string type AlertSeverityType string type NoDataOption string +type ExecutionErrorOption string const ( - AlertStateNoData AlertStateType = "no_data" - AlertStateExecError AlertStateType = "execution_error" - AlertStatePaused AlertStateType = "paused" - AlertStateAlerting AlertStateType = "alerting" - AlertStateOK AlertStateType = "ok" - AlertStatePending AlertStateType = "pending" + AlertStateNoData AlertStateType = "no_data" + AlertStatePaused AlertStateType = "paused" + AlertStateAlerting AlertStateType = "alerting" + AlertStateOK AlertStateType = "ok" + AlertStatePending AlertStateType = "pending" ) const ( NoDataSetNoData NoDataOption = "no_data" NoDataSetAlerting NoDataOption = "alerting" - NoDataSetOK NoDataOption = "ok" NoDataKeepState NoDataOption = "keep_state" ) +const ( + ExecutionErrorSetAlerting ExecutionErrorOption = "alerting" + ExecutionErrorKeepState ExecutionErrorOption = "keep_state" +) + func (s AlertStateType) IsValid() bool { - return s == AlertStateOK || s == AlertStateNoData || s == AlertStateExecError || s == AlertStatePaused || s == AlertStatePending + return s == AlertStateOK || s == AlertStateNoData || s == AlertStatePaused || s == AlertStatePending } func (s NoDataOption) IsValid() bool { - return s == NoDataSetNoData || s == NoDataSetAlerting || s == NoDataSetOK || s == NoDataKeepState + return s == NoDataSetNoData || s == NoDataSetAlerting || s == NoDataKeepState } func (s NoDataOption) ToAlertState() AlertStateType { return AlertStateType(s) } +func (s ExecutionErrorOption) IsValid() bool { + return s == ExecutionErrorSetAlerting || s == ExecutionErrorKeepState +} + +func (s ExecutionErrorOption) ToAlertState() AlertStateType { + return AlertStateType(s) +} + type Alert struct { Id int64 Version int64 diff --git a/pkg/services/alerting/eval_context.go b/pkg/services/alerting/eval_context.go index eb11f16780f3de9..06de6332b4e9e3c 100644 --- a/pkg/services/alerting/eval_context.go +++ b/pkg/services/alerting/eval_context.go @@ -31,6 +31,17 @@ type EvalContext struct { Ctx context.Context } +func NewEvalContext(alertCtx context.Context, rule *Rule) *EvalContext { + return &EvalContext{ + Ctx: alertCtx, + StartTime: time.Now(), + Rule: rule, + Logs: make([]*ResultLogEntry, 0), + EvalMatches: make([]*EvalMatch, 0), + log: log.New("alerting.evalContext"), + } +} + type StateDescription struct { Color string Text string @@ -49,11 +60,6 @@ func (c *EvalContext) GetStateModel() *StateDescription { Color: "#888888", Text: "No Data", } - case m.AlertStateExecError: - return &StateDescription{ - Color: "#000", - Text: "Execution Error", - } case m.AlertStateAlerting: return &StateDescription{ Color: "#D63232", @@ -73,25 +79,7 @@ func (c *EvalContext) ShouldSendNotification() bool { return false } - alertState := c.Rule.State - - if c.NoDataFound { - if c.Rule.NoDataState == m.NoDataKeepState { - return false - } - - alertState = c.Rule.NoDataState.ToAlertState() - } - - if c.Error != nil { - if c.Rule.ExecutionErrorState == m.NoDataKeepState { - return false - } - - alertState = c.Rule.ExecutionErrorState.ToAlertState() - } - - return alertState != c.PrevAlertState + return true } func (a *EvalContext) GetDurationMs() float64 { @@ -128,14 +116,3 @@ func (c *EvalContext) GetRuleUrl() (string, error) { return ruleUrl, nil } } - -func NewEvalContext(alertCtx context.Context, rule *Rule) *EvalContext { - return &EvalContext{ - Ctx: alertCtx, - StartTime: time.Now(), - Rule: rule, - Logs: make([]*ResultLogEntry, 0), - EvalMatches: make([]*EvalMatch, 0), - log: log.New("alerting.evalContext"), - } -} diff --git a/pkg/services/alerting/eval_context_test.go b/pkg/services/alerting/eval_context_test.go index 976a38721b71af7..5cff2996ca679f4 100644 --- a/pkg/services/alerting/eval_context_test.go +++ b/pkg/services/alerting/eval_context_test.go @@ -2,7 +2,6 @@ package alerting import ( "context" - "fmt" "testing" "github.com/grafana/grafana/pkg/models" @@ -12,7 +11,6 @@ import ( func TestAlertingEvalContext(t *testing.T) { Convey("Eval context", t, func() { ctx := NewEvalContext(context.TODO(), &Rule{Conditions: []Condition{&conditionStub{firing: true}}}) - err := fmt.Errorf("Dummie error!") Convey("Should update alert state", func() { @@ -45,66 +43,6 @@ func TestAlertingEvalContext(t *testing.T) { So(ctx.ShouldSendNotification(), ShouldBeTrue) }) - - Convey("alerting -> ok", func() { - ctx.PrevAlertState = models.AlertStateAlerting - ctx.Rule.State = models.AlertStateOK - - So(ctx.ShouldSendNotification(), ShouldBeTrue) - }) - - Convey("ok -> no_data(alerting)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.NoDataState = models.NoDataSetAlerting - ctx.Rule.State = models.AlertStateAlerting - - So(ctx.ShouldSendNotification(), ShouldBeTrue) - }) - - Convey("ok -> no_data(ok)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.NoDataState = models.NoDataSetOK - ctx.NoDataFound = true - ctx.Rule.State = models.AlertStateNoData - - So(ctx.ShouldSendNotification(), ShouldBeFalse) - }) - - Convey("ok -> no_data(keep_last)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.NoDataState = models.NoDataKeepState - ctx.Rule.State = models.AlertStateNoData - ctx.NoDataFound = true - - So(ctx.ShouldSendNotification(), ShouldBeFalse) - }) - - Convey("ok -> execution_error(alerting)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.State = models.AlertStateExecError - ctx.Rule.ExecutionErrorState = models.NoDataSetAlerting - ctx.Error = err - - So(ctx.ShouldSendNotification(), ShouldBeTrue) - }) - - Convey("ok -> execution_error(ok)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.State = models.AlertStateExecError - ctx.Rule.ExecutionErrorState = models.NoDataSetOK - ctx.Error = err - - So(ctx.ShouldSendNotification(), ShouldBeFalse) - }) - - Convey("ok -> execution_error(keep_last)", func() { - ctx.PrevAlertState = models.AlertStateOK - ctx.Rule.State = models.AlertStateExecError - ctx.Rule.ExecutionErrorState = models.NoDataKeepState - ctx.Error = err - - So(ctx.ShouldSendNotification(), ShouldBeFalse) - }) }) }) } diff --git a/pkg/services/alerting/result_handler.go b/pkg/services/alerting/result_handler.go index ed1aaeb18a072af..c27b94839a8d36a 100644 --- a/pkg/services/alerting/result_handler.go +++ b/pkg/services/alerting/result_handler.go @@ -27,27 +27,52 @@ func NewResultHandler() *DefaultResultHandler { } } +func (handler *DefaultResultHandler) GetStateFromEvaluation(evalContext *EvalContext) m.AlertStateType { + if evalContext.Error != nil { + handler.log.Error("Alert Rule Result Error", + "ruleId", evalContext.Rule.Id, + "name", evalContext.Rule.Name, + "error", evalContext.Error, + "changing state to", evalContext.Rule.ExecutionErrorState.ToAlertState()) + + if evalContext.Rule.ExecutionErrorState == m.ExecutionErrorKeepState { + return evalContext.PrevAlertState + } else { + return evalContext.Rule.ExecutionErrorState.ToAlertState() + } + } else if evalContext.Firing { + return m.AlertStateAlerting + } else if evalContext.NoDataFound { + handler.log.Info("Alert Rule returned no data", + "ruleId", evalContext.Rule.Id, + "name", evalContext.Rule.Name, + "changing state to", evalContext.Rule.NoDataState.ToAlertState()) + + if evalContext.Rule.NoDataState == m.NoDataKeepState { + return evalContext.PrevAlertState + } else { + return evalContext.Rule.NoDataState.ToAlertState() + } + } + + return m.AlertStateOK +} + func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error { evalContext.PrevAlertState = evalContext.Rule.State executionError := "" annotationData := simplejson.New() + + evalContext.Rule.State = handler.GetStateFromEvaluation(evalContext) + if evalContext.Error != nil { - handler.log.Error("Alert Rule Result Error", "ruleId", evalContext.Rule.Id, "error", evalContext.Error) - evalContext.Rule.State = m.AlertStateExecError executionError = evalContext.Error.Error() annotationData.Set("errorMessage", executionError) - } else if evalContext.Firing { - evalContext.Rule.State = m.AlertStateAlerting + } + + if evalContext.Firing { annotationData = simplejson.NewFromAny(evalContext.EvalMatches) - } else { - if evalContext.NoDataFound { - if evalContext.Rule.NoDataState != m.NoDataKeepState { - evalContext.Rule.State = evalContext.Rule.NoDataState.ToAlertState() - } - } else { - evalContext.Rule.State = m.AlertStateOK - } } countStateResult(evalContext.Rule.State) @@ -88,8 +113,6 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error { if evalContext.ShouldSendNotification() { handler.notifier.Notify(evalContext) - } else { - handler.log.Info("Notfication not sent", "prev state", evalContext.PrevAlertState, "new state", evalContext.Rule.State) } } @@ -108,7 +131,5 @@ func countStateResult(state m.AlertStateType) { metrics.M_Alerting_Result_State_Paused.Inc(1) case m.AlertStateNoData: metrics.M_Alerting_Result_State_NoData.Inc(1) - case m.AlertStateExecError: - metrics.M_Alerting_Result_State_ExecError.Inc(1) } } diff --git a/pkg/services/alerting/result_handler_test.go b/pkg/services/alerting/result_handler_test.go index 7a1abc6d1efdfd7..321838d4f4c30ce 100644 --- a/pkg/services/alerting/result_handler_test.go +++ b/pkg/services/alerting/result_handler_test.go @@ -1,30 +1,90 @@ package alerting -// import ( -// "context" -// "testing" -// -// "github.com/grafana/grafana/pkg/models" -// . "github.com/smartystreets/goconvey/convey" -// ) -// -// func TestAlertResultHandler(t *testing.T) { -// Convey("Test result Handler", t, func() { -// -// handler := NewResultHandler() -// evalContext := NewEvalContext(context.TODO(), &Rule{}) -// -// Convey("Should update", func() { -// -// Convey("when no earlier alert state", func() { -// oldState := models.AlertStateOK -// -// evalContext.Rule.State = models.AlertStateAlerting -// evalContext.Rule.NoDataState = models.NoDataKeepState -// evalContext.NoDataFound = true -// -// So(handler.shouldUpdateAlertState(evalContext, oldState), ShouldBeFalse) -// }) -// }) -// }) -// } +import ( + "context" + "testing" + + "fmt" + + "github.com/grafana/grafana/pkg/models" + . "github.com/smartystreets/goconvey/convey" +) + +func TestAlertingResultHandler(t *testing.T) { + Convey("Result handler", t, func() { + ctx := NewEvalContext(context.TODO(), &Rule{Conditions: []Condition{&conditionStub{firing: true}}}) + dummieError := fmt.Errorf("dummie") + handler := NewResultHandler() + + Convey("Should update alert state", func() { + + Convey("ok -> alerting", func() { + ctx.PrevAlertState = models.AlertStateOK + ctx.Firing = true + + So(handler.GetStateFromEvaluation(ctx), ShouldEqual, models.AlertStateAlerting) + So(ctx.ShouldUpdateAlertState(), ShouldBeTrue) + }) + + Convey("ok -> error(alerting)", func() { + ctx.PrevAlertState = models.AlertStateOK + ctx.Error = dummieError + ctx.Rule.ExecutionErrorState = models.ExecutionErrorSetAlerting + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting) + So(ctx.ShouldUpdateAlertState(), ShouldBeTrue) + }) + + Convey("ok -> error(keep_last)", func() { + ctx.PrevAlertState = models.AlertStateOK + ctx.Error = dummieError + ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStateOK) + So(ctx.ShouldUpdateAlertState(), ShouldBeFalse) + }) + + Convey("pending -> error(keep_last)", func() { + ctx.PrevAlertState = models.AlertStatePending + ctx.Error = dummieError + ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStatePending) + So(ctx.ShouldUpdateAlertState(), ShouldBeFalse) + }) + + Convey("ok -> no_data(alerting)", func() { + ctx.PrevAlertState = models.AlertStateOK + ctx.Rule.NoDataState = models.NoDataSetAlerting + ctx.NoDataFound = true + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting) + So(ctx.ShouldUpdateAlertState(), ShouldBeTrue) + }) + + Convey("ok -> no_data(keep_last)", func() { + ctx.PrevAlertState = models.AlertStateOK + ctx.Rule.NoDataState = models.NoDataKeepState + ctx.NoDataFound = true + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStateOK) + So(ctx.ShouldUpdateAlertState(), ShouldBeFalse) + }) + + Convey("pending -> no_data(keep_last)", func() { + ctx.PrevAlertState = models.AlertStatePending + ctx.Rule.NoDataState = models.NoDataKeepState + ctx.NoDataFound = true + + ctx.Rule.State = handler.GetStateFromEvaluation(ctx) + So(ctx.Rule.State, ShouldEqual, models.AlertStatePending) + So(ctx.ShouldUpdateAlertState(), ShouldBeFalse) + }) + }) + }) +} diff --git a/pkg/services/alerting/rule.go b/pkg/services/alerting/rule.go index cc17cdb13ff2adf..809640ed4a7f6b7 100644 --- a/pkg/services/alerting/rule.go +++ b/pkg/services/alerting/rule.go @@ -19,7 +19,7 @@ type Rule struct { Name string Message string NoDataState m.NoDataOption - ExecutionErrorState m.NoDataOption + ExecutionErrorState m.ExecutionErrorOption State m.AlertStateType Conditions []Condition Notifications []int64 @@ -78,6 +78,7 @@ func NewRuleFromDBAlert(ruleDef *m.Alert) (*Rule, error) { model.Frequency = ruleDef.Frequency model.State = ruleDef.State model.NoDataState = m.NoDataOption(ruleDef.Settings.Get("noDataState").MustString("no_data")) + model.ExecutionErrorState = m.ExecutionErrorOption(ruleDef.Settings.Get("executionErrorState").MustString("alerting")) for _, v := range ruleDef.Settings.Get("notifications").MustArray() { jsonModel := simplejson.NewFromAny(v) diff --git a/public/app/features/alerting/alert_def.ts b/public/app/features/alerting/alert_def.ts index 8696e593074f26a..aaddb9e425f6737 100644 --- a/public/app/features/alerting/alert_def.ts +++ b/public/app/features/alerting/alert_def.ts @@ -37,10 +37,14 @@ var reducerTypes = [ ]; var noDataModes = [ - {text: 'OK', value: 'ok'}, {text: 'Alerting', value: 'alerting'}, {text: 'No Data', value: 'no_data'}, - {text: 'Keep Last', value: 'keep_last'}, + {text: 'Keep Last State', value: 'keep_state'}, +]; + +var executionErrorModes = [ + {text: 'Alerting', value: 'alerting'}, + {text: 'Keep Last State', value: 'keep_state'}, ]; function createReducerPart(model) { @@ -48,7 +52,6 @@ function createReducerPart(model) { return new QueryPart(model, def); } - function getStateDisplayModel(state) { switch (state) { case 'ok': { @@ -113,6 +116,7 @@ export default { conditionTypes: conditionTypes, evalFunctions: evalFunctions, noDataModes: noDataModes, + executionErrorModes: executionErrorModes, reducerTypes: reducerTypes, createReducerPart: createReducerPart, joinEvalMatches: joinEvalMatches, diff --git a/public/app/features/alerting/alert_tab_ctrl.ts b/public/app/features/alerting/alert_tab_ctrl.ts index 652a83d42e47c71..e505f9be36136cd 100644 --- a/public/app/features/alerting/alert_tab_ctrl.ts +++ b/public/app/features/alerting/alert_tab_ctrl.ts @@ -19,6 +19,7 @@ export class AlertTabCtrl { conditionModels: any; evalFunctions: any; noDataModes: any; + executionErrorModes: any; addNotificationSegment; notifications; alertNotifications; @@ -42,6 +43,7 @@ export class AlertTabCtrl { this.evalFunctions = alertDef.evalFunctions; this.conditionTypes = alertDef.conditionTypes; this.noDataModes = alertDef.noDataModes; + this.executionErrorModes = alertDef.executionErrorModes; this.appSubUrl = config.appSubUrl; } @@ -140,6 +142,7 @@ export class AlertTabCtrl { } alert.noDataState = alert.noDataState || 'no_data'; + alert.executionErrorState = alert.executionErrorState || 'alerting'; alert.frequency = alert.frequency || '60s'; alert.handler = alert.handler || 1; alert.notifications = alert.notifications || []; diff --git a/public/app/features/alerting/partials/alert_tab.html b/public/app/features/alerting/partials/alert_tab.html index 4dd5516945b5b04..468fd4ec7b263ff 100644 --- a/public/app/features/alerting/partials/alert_tab.html +++ b/public/app/features/alerting/partials/alert_tab.html @@ -82,7 +82,7 @@
Conditions
- If no data points or all values are null + If no data points or all values are null SET STATE TO
+ +
+
+