tech(alerting): refactor how evalhandler uses conditions

pkg/services/alerting/conditions/query.go

@@ -33,16 +33,17 @@ type AlertQuery struct {
 	To           string
 }
 
-func (c *QueryCondition) Eval(context *alerting.EvalContext) {
+func (c *QueryCondition) Eval(context *alerting.EvalContext) (*alerting.ConditionResult, error) {
 	timeRange := tsdb.NewTimeRange(c.Query.From, c.Query.To)
+
 	seriesList, err := c.executeQuery(context, timeRange)
 	if err != nil {
-		context.Error = err
-		return
+		return nil, err
 	}
 
 	emptySerieCount := 0
 	evalMatchCount := 0
+	var matches []*alerting.EvalMatch
 	for _, series := range seriesList {
 		reducedValue := c.Reducer.Reduce(series)
 		evalMatch := c.Evaluator.Eval(reducedValue)
@@ -60,15 +61,19 @@ func (c *QueryCondition) Eval(context *alerting.EvalContext) {
 
 		if evalMatch {
 			evalMatchCount++
-			context.EvalMatches = append(context.EvalMatches, &alerting.EvalMatch{
+
+			matches = append(matches, &alerting.EvalMatch{
 				Metric: series.Name,
 				Value:  reducedValue.Float64,
 			})
 		}
 	}
 
-	context.NoDataFound = emptySerieCount == len(seriesList)
-	context.Firing = evalMatchCount > 0
+	return &alerting.ConditionResult{
+		Firing:      evalMatchCount > 0,
+		NoDataFound: emptySerieCount == len(seriesList),
+		EvalMatches: matches,
+	}, nil
 }
 
 func (c *QueryCondition) executeQuery(context *alerting.EvalContext, timeRange *tsdb.TimeRange) (tsdb.TimeSeriesSlice, error) {

pkg/services/alerting/conditions/query_test.go

@@ -46,19 +46,19 @@ func TestQueryCondition(t *testing.T) {
 			Convey("should fire when avg is above 100", func() {
 				points := tsdb.NewTimeSeriesPointsFromArgs(120, 0)
 				ctx.series = tsdb.TimeSeriesSlice{tsdb.NewTimeSeries("test1", points)}
-				ctx.exec()
+				cr, err := ctx.exec()
 
-				So(ctx.result.Error, ShouldBeNil)
-				So(ctx.result.Firing, ShouldBeTrue)
+				So(err, ShouldBeNil)
+				So(cr.Firing, ShouldBeTrue)
 			})
 
 			Convey("Should not fire when avg is below 100", func() {
 				points := tsdb.NewTimeSeriesPointsFromArgs(90, 0)
 				ctx.series = tsdb.TimeSeriesSlice{tsdb.NewTimeSeries("test1", points)}
-				ctx.exec()
+				cr, err := ctx.exec()
 
-				So(ctx.result.Error, ShouldBeNil)
-				So(ctx.result.Firing, ShouldBeFalse)
+				So(err, ShouldBeNil)
+				So(cr.Firing, ShouldBeFalse)
 			})
 
 			Convey("Should fire if only first serie matches", func() {
@@ -66,10 +66,10 @@ func TestQueryCondition(t *testing.T) {
 					tsdb.NewTimeSeries("test1", tsdb.NewTimeSeriesPointsFromArgs(120, 0)),
 					tsdb.NewTimeSeries("test2", tsdb.NewTimeSeriesPointsFromArgs(0, 0)),
 				}
-				ctx.exec()
+				cr, err := ctx.exec()
 
-				So(ctx.result.Error, ShouldBeNil)
-				So(ctx.result.Firing, ShouldBeTrue)
+				So(err, ShouldBeNil)
+				So(cr.Firing, ShouldBeTrue)
 			})
 
 			Convey("Empty series", func() {
@@ -78,10 +78,10 @@ func TestQueryCondition(t *testing.T) {
 						tsdb.NewTimeSeries("test1", tsdb.NewTimeSeriesPointsFromArgs()),
 						tsdb.NewTimeSeries("test2", tsdb.NewTimeSeriesPointsFromArgs()),
 					}
-					ctx.exec()
+					cr, err := ctx.exec()
 
-					So(ctx.result.Error, ShouldBeNil)
-					So(ctx.result.NoDataFound, ShouldBeTrue)
+					So(err, ShouldBeNil)
+					So(cr.NoDataFound, ShouldBeTrue)
 				})
 
 				Convey("Should set NoDataFound both series contains null", func() {
@@ -89,10 +89,10 @@ func TestQueryCondition(t *testing.T) {
 						tsdb.NewTimeSeries("test1", tsdb.TimeSeriesPoints{tsdb.TimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}),
 						tsdb.NewTimeSeries("test2", tsdb.TimeSeriesPoints{tsdb.TimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}),
 					}
-					ctx.exec()
+					cr, err := ctx.exec()
 
-					So(ctx.result.Error, ShouldBeNil)
-					So(ctx.result.NoDataFound, ShouldBeTrue)
+					So(err, ShouldBeNil)
+					So(cr.NoDataFound, ShouldBeTrue)
 				})
 
 				Convey("Should not set NoDataFound if one serie is empty", func() {
@@ -100,10 +100,10 @@ func TestQueryCondition(t *testing.T) {
 						tsdb.NewTimeSeries("test1", tsdb.NewTimeSeriesPointsFromArgs()),
 						tsdb.NewTimeSeries("test2", tsdb.NewTimeSeriesPointsFromArgs(120, 0)),
 					}
-					ctx.exec()
+					cr, err := ctx.exec()
 
-					So(ctx.result.Error, ShouldBeNil)
-					So(ctx.result.NoDataFound, ShouldBeFalse)
+					So(err, ShouldBeNil)
+					So(cr.NoDataFound, ShouldBeFalse)
 				})
 			})
 		})
@@ -120,7 +120,7 @@ type queryConditionTestContext struct {
 
 type queryConditionScenarioFunc func(c *queryConditionTestContext)
 
-func (ctx *queryConditionTestContext) exec() {
+func (ctx *queryConditionTestContext) exec() (*alerting.ConditionResult, error) {
 	jsonModel, err := simplejson.NewJson([]byte(`{
             "type": "query",
             "query":  {
@@ -146,7 +146,7 @@ func (ctx *queryConditionTestContext) exec() {
 		}, nil
 	}
 
-	condition.Eval(ctx.result)
+	return condition.Eval(ctx.result)
 }
 
 func queryConditionScenario(desc string, fn queryConditionScenarioFunc) {

pkg/services/alerting/eval_handler.go

@@ -20,8 +20,12 @@ func NewEvalHandler() *DefaultEvalHandler {
 }
 
 func (e *DefaultEvalHandler) Eval(context *EvalContext) {
+	firing := true
 	for _, condition := range context.Rule.Conditions {
-		condition.Eval(context)
+		cr, err := condition.Eval(context)
+		if err != nil {
+			context.Error = err
+		}
 
 		// break if condition could not be evaluated
 		if context.Error != nil {
@@ -29,11 +33,15 @@ func (e *DefaultEvalHandler) Eval(context *EvalContext) {
 		}
 
 		// break if result has not triggered yet
-		if context.Firing == false {
+		if cr.Firing == false {
+			firing = false
 			break
 		}
+
+		context.EvalMatches = append(context.EvalMatches, cr.EvalMatches...)
 	}
 
+	context.Firing = firing
 	context.EndTime = time.Now()
 	elapsedTime := context.EndTime.Sub(context.StartTime) / time.Millisecond
 	metrics.M_Alerting_Exeuction_Time.Update(elapsedTime)

pkg/services/alerting/eval_handler_test.go

@@ -9,10 +9,11 @@ import (
 
 type conditionStub struct {
 	firing  bool
+	matches []*EvalMatch
 }
 
-func (c *conditionStub) Eval(context *EvalContext) {
-	context.Firing = c.firing
+func (c *conditionStub) Eval(context *EvalContext) (*ConditionResult, error) {
+	return &ConditionResult{Firing: c.firing, EvalMatches: c.matches}, nil
 }
 
 func TestAlertingExecutor(t *testing.T) {
@@ -30,10 +31,10 @@ func TestAlertingExecutor(t *testing.T) {
 			So(context.Firing, ShouldEqual, true)
 		})
 
-		Convey("Show return false with not passing condition", func() {
+		Convey("Show return false with not passing asdf", func() {
 			context := NewEvalContext(context.TODO(), &Rule{
 				Conditions: []Condition{
-					&conditionStub{firing: true},
+					&conditionStub{firing: true, matches: []*EvalMatch{&EvalMatch{}, &EvalMatch{}}},
 					&conditionStub{firing: false},
 				},
 			})

pkg/services/alerting/interfaces.go

@@ -21,6 +21,12 @@ type Notifier interface {
 	GetIsDefault() bool
 }
 
+type ConditionResult struct {
+	Firing      bool
+	NoDataFound bool
+	EvalMatches []*EvalMatch
+}
+
 type Condition interface {
-	Eval(result *EvalContext)
+	Eval(result *EvalContext) (*ConditionResult, error)
 }

pkg/services/alerting/rule_test.go

@@ -10,7 +10,9 @@ import (
 
 type FakeCondition struct{}
 
-func (f *FakeCondition) Eval(context *EvalContext) {}
+func (f *FakeCondition) Eval(context *EvalContext) (*ConditionResult, error) {
+	return &ConditionResult{}, nil
+}
 
 func TestAlertRuleModel(t *testing.T) {
 	Convey("Testing alert rule", t, func() {