Alerting: move getNewState to EvalContext

This fix alert state update when several evaluation attempts are needed

Signed-off-by: Thibault Chataigner <t.chataigner@criteo.com>

pkg/services/alerting/engine.go

@@ -193,6 +193,7 @@ func (e *Engine) processJob(attemptID int, attemptChan chan int, cancelChan chan
 			}
 		}
 
+		evalContext.Rule.State = evalContext.GetNewState()
 		e.resultHandler.Handle(evalContext)
 		span.Finish()
 		e.log.Debug("Job Execution completed", "timeMs", evalContext.GetDurationMs(), "alertId", evalContext.Rule.Id, "name", evalContext.Rule.Name, "firing", evalContext.Firing, "attemptID", attemptID)

pkg/services/alerting/eval_context.go

@@ -112,3 +112,34 @@ func (c *EvalContext) GetRuleUrl() (string, error) {
 		return fmt.Sprintf(urlFormat, m.GetFullDashboardUrl(ref.Uid, ref.Slug), c.Rule.PanelId, c.Rule.OrgId), nil
 	}
 }
+
+func (c *EvalContext) GetNewState() m.AlertStateType {
+	if c.Error != nil {
+		c.log.Error("Alert Rule Result Error",
+			"ruleId", c.Rule.Id,
+			"name", c.Rule.Name,
+			"error", c.Error,
+			"changing state to", c.Rule.ExecutionErrorState.ToAlertState())
+
+		if c.Rule.ExecutionErrorState == m.ExecutionErrorKeepState {
+			return c.PrevAlertState
+		}
+		return c.Rule.ExecutionErrorState.ToAlertState()
+
+	} else if c.Firing {
+		return m.AlertStateAlerting
+
+	} else if c.NoDataFound {
+		c.log.Info("Alert Rule returned no data",
+			"ruleId", c.Rule.Id,
+			"name", c.Rule.Name,
+			"changing state to", c.Rule.NoDataState.ToAlertState())
+
+		if c.Rule.NoDataState == m.NoDataKeepState {
+			return c.PrevAlertState
+		}
+		return c.Rule.NoDataState.ToAlertState()
+	}
+
+	return m.AlertStateOK
+}

pkg/services/alerting/eval_context_test.go

@@ -2,6 +2,7 @@ package alerting
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/grafana/grafana/pkg/models"
@@ -12,7 +13,7 @@ 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("Should update alert state when needed", func() {
 
 			Convey("ok -> alerting", func() {
 				ctx.PrevAlertState = models.AlertStateOK
@@ -28,5 +29,71 @@ func TestAlertingEvalContext(t *testing.T) {
 				So(ctx.ShouldUpdateAlertState(), ShouldBeFalse)
 			})
 		})
+
+		Convey("Should compute and replace properly new rule state", func() {
+			dummieError := fmt.Errorf("dummie error")
+
+			Convey("ok -> alerting", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Firing = true
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting)
+			})
+
+			Convey("ok -> error(alerting)", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Error = dummieError
+				ctx.Rule.ExecutionErrorState = models.ExecutionErrorSetAlerting
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting)
+			})
+
+			Convey("ok -> error(keep_last)", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Error = dummieError
+				ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStateOK)
+			})
+
+			Convey("pending -> error(keep_last)", func() {
+				ctx.PrevAlertState = models.AlertStatePending
+				ctx.Error = dummieError
+				ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStatePending)
+			})
+
+			Convey("ok -> no_data(alerting)", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Rule.NoDataState = models.NoDataSetAlerting
+				ctx.NoDataFound = true
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting)
+			})
+
+			Convey("ok -> no_data(keep_last)", func() {
+				ctx.PrevAlertState = models.AlertStateOK
+				ctx.Rule.NoDataState = models.NoDataKeepState
+				ctx.NoDataFound = true
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStateOK)
+			})
+
+			Convey("pending -> no_data(keep_last)", func() {
+				ctx.PrevAlertState = models.AlertStatePending
+				ctx.Rule.NoDataState = models.NoDataKeepState
+				ctx.NoDataFound = true
+
+				ctx.Rule.State = ctx.GetNewState()
+				So(ctx.Rule.State, ShouldEqual, models.AlertStatePending)
+			})
+		})
 	})
 }

pkg/services/alerting/eval_handler.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/metrics"
-	"github.com/grafana/grafana/pkg/models"
 )
 
 type DefaultEvalHandler struct {
@@ -66,40 +65,7 @@ func (e *DefaultEvalHandler) Eval(context *EvalContext) {
 	context.Firing = firing
 	context.NoDataFound = noDataFound
 	context.EndTime = time.Now()
-	context.Rule.State = e.getNewState(context)
 
 	elapsedTime := context.EndTime.Sub(context.StartTime).Nanoseconds() / int64(time.Millisecond)
 	metrics.M_Alerting_Execution_Time.Observe(float64(elapsedTime))
 }
-
-// This should be move into evalContext once its been refactored. (Carl Bergquist)
-func (handler *DefaultEvalHandler) getNewState(evalContext *EvalContext) models.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 == models.ExecutionErrorKeepState {
-			return evalContext.PrevAlertState
-		} else {
-			return evalContext.Rule.ExecutionErrorState.ToAlertState()
-		}
-	} else if evalContext.Firing {
-		return models.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 == models.NoDataKeepState {
-			return evalContext.PrevAlertState
-		} else {
-			return evalContext.Rule.NoDataState.ToAlertState()
-		}
-	}
-
-	return models.AlertStateOK
-}

pkg/services/alerting/eval_handler_test.go

@@ -2,10 +2,8 @@ package alerting
 
 import (
 	"context"
-	"fmt"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/models"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -203,73 +201,5 @@ func TestAlertingEvaluationHandler(t *testing.T) {
 			handler.Eval(context)
 			So(context.NoDataFound, ShouldBeTrue)
 		})
-
-		Convey("EvalHandler can replace alert state based for errors and no_data", func() {
-			ctx := NewEvalContext(context.TODO(), &Rule{Conditions: []Condition{&conditionStub{firing: true}}})
-			dummieError := fmt.Errorf("dummie error")
-			Convey("Should update alert state", func() {
-
-				Convey("ok -> alerting", func() {
-					ctx.PrevAlertState = models.AlertStateOK
-					ctx.Firing = true
-
-					So(handler.getNewState(ctx), ShouldEqual, models.AlertStateAlerting)
-				})
-
-				Convey("ok -> error(alerting)", func() {
-					ctx.PrevAlertState = models.AlertStateOK
-					ctx.Error = dummieError
-					ctx.Rule.ExecutionErrorState = models.ExecutionErrorSetAlerting
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting)
-				})
-
-				Convey("ok -> error(keep_last)", func() {
-					ctx.PrevAlertState = models.AlertStateOK
-					ctx.Error = dummieError
-					ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStateOK)
-				})
-
-				Convey("pending -> error(keep_last)", func() {
-					ctx.PrevAlertState = models.AlertStatePending
-					ctx.Error = dummieError
-					ctx.Rule.ExecutionErrorState = models.ExecutionErrorKeepState
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStatePending)
-				})
-
-				Convey("ok -> no_data(alerting)", func() {
-					ctx.PrevAlertState = models.AlertStateOK
-					ctx.Rule.NoDataState = models.NoDataSetAlerting
-					ctx.NoDataFound = true
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStateAlerting)
-				})
-
-				Convey("ok -> no_data(keep_last)", func() {
-					ctx.PrevAlertState = models.AlertStateOK
-					ctx.Rule.NoDataState = models.NoDataKeepState
-					ctx.NoDataFound = true
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStateOK)
-				})
-
-				Convey("pending -> no_data(keep_last)", func() {
-					ctx.PrevAlertState = models.AlertStatePending
-					ctx.Rule.NoDataState = models.NoDataKeepState
-					ctx.NoDataFound = true
-
-					ctx.Rule.State = handler.getNewState(ctx)
-					So(ctx.Rule.State, ShouldEqual, models.AlertStatePending)
-				})
-			})
-		})
 	})
 }

pkg/services/alerting/test_rule.go

@@ -53,6 +53,7 @@ func testAlertRule(rule *Rule) *EvalContext {
 	context.IsTestRun = true
 
 	handler.Eval(context)
+	context.Rule.State = context.GetNewState()
 
 	return context
 }