Merge pull request #6489 from bergquist/reduce_notification_states

Reduce alerting states

pkg/metrics/metrics.go

@@ -40,7 +40,6 @@ 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_Active_Alerts             Counter
 	M_Alerting_Notification_Sent_Slack   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")

pkg/models/alert.go

@@ -9,10 +9,10 @@ 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"
@@ -22,22 +22,34 @@ const (
 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

pkg/services/alerting/eval_context.go

@@ -26,10 +26,23 @@ type EvalContext struct {
 	ImagePublicUrl  string
 	ImageOnDiskPath string
 	NoDataFound     bool
+	PrevAlertState  m.AlertStateType
 
 	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"),
+		PrevAlertState: rule.State,
+	}
+}
+
 type StateDescription struct {
 	Color string
 	Text  string
@@ -48,11 +61,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",
@@ -63,6 +71,18 @@ func (c *EvalContext) GetStateModel() *StateDescription {
 	}
 }
 
+func (c *EvalContext) ShouldUpdateAlertState() bool {
+	return c.Rule.State != c.PrevAlertState
+}
+
+func (c *EvalContext) ShouldSendNotification() bool {
+	if (c.PrevAlertState == m.AlertStatePending) && (c.Rule.State == m.AlertStateOK) {
+		return false
+	}
+
+	return true
+}
+
 func (a *EvalContext) GetDurationMs() float64 {
 	return float64(a.EndTime.Nanosecond()-a.StartTime.Nanosecond()) / float64(1000000)
 }
@@ -97,14 +117,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"),
-	}
-}

pkg/services/alerting/eval_context_test.go

@@ -0,0 +1,48 @@
+package alerting
+
+import (
+	"context"
+	"testing"
+
+	"github.com/grafana/grafana/pkg/models"
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func TestAlertingEvalContext(t *testing.T) {
+	Convey("Eval context", t, func() {
+		ctx := NewEvalContext(context.TODO(), &Rule{Conditions: []Condition{&conditionStub{firing: true}}})
+
+		Convey("Should update alert state", func() {
+
+			Convey("ok -> alerting", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Rule.State = models.AlertStateAlerting
+
+				So(ctx.ShouldUpdateAlertState(), ShouldBeTrue)
+			})
+
+			Convey("ok -> ok", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Rule.State = models.AlertStateOK
+
+				So(ctx.ShouldUpdateAlertState(), ShouldBeFalse)
+			})
+		})
+
+		Convey("Should send notifications", func() {
+			Convey("pending -> ok", func() {
+				ctx.PrevAlertState = models.AlertStatePending
+				ctx.Rule.State = models.AlertStateOK
+
+				So(ctx.ShouldSendNotification(), ShouldBeFalse)
+			})
+
+			Convey("ok -> alerting", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Rule.State = models.AlertStateAlerting
+
+				So(ctx.ShouldSendNotification(), ShouldBeTrue)
+			})
+		})
+	})
+}

pkg/services/alerting/result_handler.go

@@ -27,32 +27,55 @@ func NewResultHandler() *DefaultResultHandler {
 	}
 }
 
-func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error {
-	oldState := evalContext.Rule.State
+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 {
 	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)
-	if handler.shouldUpdateAlertState(evalContext, oldState) {
-		handler.log.Info("New state change", "alertId", evalContext.Rule.Id, "newState", evalContext.Rule.State, "oldState", oldState)
+	if evalContext.ShouldUpdateAlertState() {
+		handler.log.Info("New state change", "alertId", evalContext.Rule.Id, "newState", evalContext.Rule.State, "prev state", evalContext.PrevAlertState)
 
 		cmd := &m.SetAlertStateCommand{
 			AlertId:  evalContext.Rule.Id,
@@ -76,7 +99,7 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error {
 			Title:       evalContext.Rule.Name,
 			Text:        evalContext.GetStateModel().Text,
 			NewState:    string(evalContext.Rule.State),
-			PrevState:   string(oldState),
+			PrevState:   string(evalContext.PrevAlertState),
 			Epoch:       time.Now().Unix(),
 			Data:        annotationData,
 		}
@@ -86,21 +109,14 @@ func (handler *DefaultResultHandler) Handle(evalContext *EvalContext) error {
 			handler.log.Error("Failed to save annotation for new alert state", "error", err)
 		}
 
-		if (oldState == m.AlertStatePending) && (evalContext.Rule.State == m.AlertStateOK) {
-			handler.log.Info("Notfication not sent", "oldState", oldState, "newState", evalContext.Rule.State)
-		} else {
+		if evalContext.ShouldSendNotification() {
 			handler.notifier.Notify(evalContext)
 		}
-
 	}
 
 	return nil
 }
 
-func (handler *DefaultResultHandler) shouldUpdateAlertState(evalContext *EvalContext, oldState m.AlertStateType) bool {
-	return evalContext.Rule.State != oldState
-}
-
 func countStateResult(state m.AlertStateType) {
 	switch state {
 	case m.AlertStatePending:
@@ -113,7 +129,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)
 	}
 }

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)
+			})
+		})
+	})
+}

pkg/services/alerting/rule.go

@@ -19,6 +19,7 @@ type Rule struct {
 	Name                string
 	Message             string
 	NoDataState         m.NoDataOption
+	ExecutionErrorState m.ExecutionErrorOption
 	State               m.AlertStateType
 	Conditions          []Condition
 	Notifications       []int64
@@ -77,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)

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,

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 || [];

public/app/features/alerting/partials/alert_tab.html

@@ -82,7 +82,7 @@
 
 			<div class="gf-form-group">
 				<div class="gf-form">
-          <span class="gf-form-label">If no data points or all values are null</span>
+          <span class="gf-form-label width-18">If no data points or all values are null</span>
           <span class="gf-form-label query-keyword">SET STATE TO</span>
 					<div class="gf-form-select-wrapper">
 						<select class="gf-form-input" ng-model="ctrl.alert.noDataState" ng-options="f.value as f.text for f in ctrl.noDataModes">
@@ -90,6 +90,15 @@
 					</div>
 				</div>
 
+				<div class="gf-form">
+          <span class="gf-form-label width-18">On execution error or timeout</span>
+          <span class="gf-form-label query-keyword">SET STATE TO</span>
+					<div class="gf-form-select-wrapper">
+						<select class="gf-form-input" ng-model="ctrl.alert.executionErrorState" ng-options="f.value as f.text for f in ctrl.executionErrorModes">
+						</select>
+					</div>
+				</div>
+
 				<div class="gf-form-button-row">
 					<button class="btn btn-inverse" ng-click="ctrl.test()">
 						Test Rule