Alerting: Fix panic in backtesting API when the testing interval is not times of evaluation interval (#68727)

* add test for the bug
* update backtesting evaluators to accept a number of evaluations instead of `to` to have control over the number evaluations in one place

pkg/services/ngalert/backtesting/engine.go

@@ -25,10 +25,10 @@ var (
 	backtestingEvaluatorFactory = newBacktestingEvaluator
 )
 
-type callbackFunc = func(now time.Time, results eval.Results) error
+type callbackFunc = func(evaluationIndex int, now time.Time, results eval.Results) error
 
 type backtestingEvaluator interface {
-	Eval(ctx context.Context, from, to time.Time, interval time.Duration, callback callbackFunc) error
+	Eval(ctx context.Context, from time.Time, interval time.Duration, evaluations int, callback callbackFunc) error
 }
 
 type stateManager interface {
@@ -84,8 +84,11 @@ func (e *Engine) Test(ctx context.Context, user *user.SignedInUser, rule *models
 	tsField := data.NewField("Time", nil, make([]time.Time, length))
 	valueFields := make(map[string]*data.Field)
 
-	err = evaluator.Eval(ruleCtx, from, to, time.Duration(rule.IntervalSeconds)*time.Second, func(currentTime time.Time, results eval.Results) error {
-		idx := int(currentTime.Sub(from).Seconds()) / int(rule.IntervalSeconds)
+	err = evaluator.Eval(ruleCtx, from, time.Duration(rule.IntervalSeconds)*time.Second, length, func(idx int, currentTime time.Time, results eval.Results) error {
+		if idx >= length {
+			logger.Info("Unexpected evaluation. Skipping", "from", from, "to", to, "interval", rule.IntervalSeconds, "evaluationTime", currentTime, "evaluationIndex", idx, "expectedEvaluations", length)
+			return nil
+		}
 		states := stateManager.ProcessEvalResults(ruleCtx, currentTime, rule, results, nil)
 		tsField.Set(idx, currentTime)
 		for _, s := range states {
@@ -110,7 +113,7 @@ func (e *Engine) Test(ctx context.Context, user *user.SignedInUser, rule *models
 	for _, f := range valueFields {
 		fields = append(fields, f)
 	}
-	result := data.NewFrame("Backtesting results", fields...)
+	result := data.NewFrame("Testing results", fields...)
 
 	if err != nil {
 		return nil, err

pkg/services/ngalert/backtesting/engine_test.go

@@ -263,6 +263,36 @@ func TestEvaluatorTest(t *testing.T) {
 		})
 	})
 
+	t.Run("should not fail if 'to-from' is not times of interval", func(t *testing.T) {
+		from := time.Unix(0, 0)
+		to := from.Add(5 * ruleInterval)
+
+		states := []state.StateTransition{
+			{
+				State: &state.State{
+					CacheID:     "state-1",
+					Labels:      models.GenerateAlertLabels(rand.Intn(5)+1, "test-"),
+					State:       eval.Normal,
+					StateReason: util.GenerateShortUID(),
+				},
+			},
+		}
+
+		manager.stateCallback = func(now time.Time) []state.StateTransition {
+			return states
+		}
+
+		frame, err := engine.Test(context.Background(), nil, rule, from, to)
+		require.NoError(t, err)
+		expectedLen := frame.Rows()
+		for i := 0; i < 100; i++ {
+			jitter := time.Duration(rand.Int63n(ruleInterval.Milliseconds())) * time.Millisecond
+			frame, err = engine.Test(context.Background(), nil, rule, from, to.Add(jitter))
+			require.NoError(t, err)
+			require.Equalf(t, expectedLen, frame.Rows(), "jitter %v caused result to be different that base-line", jitter)
+		}
+	})
+
 	t.Run("should backfill field with nulls if a new dimension created in the middle", func(t *testing.T) {
 		from := time.Unix(0, 0)
 
@@ -359,18 +389,16 @@ type fakeBacktestingEvaluator struct {
 	evalCallback func(now time.Time) (eval.Results, error)
 }
 
-func (f *fakeBacktestingEvaluator) Eval(_ context.Context, from, to time.Time, interval time.Duration, callback callbackFunc) error {
-	idx := 0
-	for now := from; now.Before(to); now = now.Add(interval) {
+func (f *fakeBacktestingEvaluator) Eval(_ context.Context, from time.Time, interval time.Duration, evaluations int, callback callbackFunc) error {
+	for idx, now := 0, from; idx < evaluations; idx, now = idx+1, now.Add(interval) {
 		results, err := f.evalCallback(now)
 		if err != nil {
 			return err
 		}
-		err = callback(now, results)
+		err = callback(idx, now, results)
 		if err != nil {
 			return err
 		}
-		idx++
 	}
 	return nil
 }

pkg/services/ngalert/backtesting/eval_data.go

@@ -37,10 +37,9 @@ func newDataEvaluator(refID string, frame *data.Frame) (*dataEvaluator, error) {
 	}, nil
 }
 
-func (d *dataEvaluator) Eval(_ context.Context, from, to time.Time, interval time.Duration, callback callbackFunc) error {
+func (d *dataEvaluator) Eval(_ context.Context, from time.Time, interval time.Duration, evaluations int, callback callbackFunc) error {
 	var resampled = make([]mathexp.Series, 0, len(d.data))
-
-	iterations := 0
+	to := from.Add(time.Duration(evaluations) * interval)
 	for _, s := range d.data {
 		// making sure the input data frame is aligned with the interval
 		r, err := s.Resample(d.refID, interval, d.downsampleFunction, d.upsampleFunction, from, to.Add(-interval)) // we want to query [from,to)
@@ -48,10 +47,9 @@ func (d *dataEvaluator) Eval(_ context.Context, from, to time.Time, interval tim
 			return err
 		}
 		resampled = append(resampled, r)
-		iterations = r.Len()
 	}
 
-	for i := 0; i < iterations; i++ {
+	for i := 0; i < evaluations; i++ {
 		result := make([]eval.Result, 0, len(resampled))
 		var now time.Time
 		for _, series := range resampled {
@@ -87,7 +85,7 @@ func (d *dataEvaluator) Eval(_ context.Context, from, to time.Time, interval tim
 				EvaluatedAt: now,
 			})
 		}
-		err := callback(now, result)
+		err := callback(i, now, result)
 		if err != nil {
 			return err
 		}

pkg/services/ngalert/backtesting/eval_data_test.go

@@ -96,11 +96,11 @@ func TestDataEvaluator_Eval(t *testing.T) {
 	t.Run("should use data points when frame resolution matches evaluation interval", func(t *testing.T) {
 		r := make([]results, 0, frame.Rows())
 
-		invterval := time.Second
+		interval := time.Second
 
-		resultsCount := int(to.Sub(from).Seconds() / invterval.Seconds())
+		resultsCount := int(to.Sub(from).Seconds() / interval.Seconds())
 
-		err = evaluator.Eval(context.Background(), from, to, time.Second, func(now time.Time, res eval.Results) error {
+		err = evaluator.Eval(context.Background(), from, time.Second, resultsCount, func(idx int, now time.Time, res eval.Results) error {
 			r = append(r, results{
 				now, res,
 			})
@@ -164,7 +164,7 @@ func TestDataEvaluator_Eval(t *testing.T) {
 			size := to.Sub(from).Milliseconds() / interval.Milliseconds()
 			r := make([]results, 0, size)
 
-			err = evaluator.Eval(context.Background(), from, to, interval, func(now time.Time, res eval.Results) error {
+			err = evaluator.Eval(context.Background(), from, interval, int(size), func(idx int, now time.Time, res eval.Results) error {
 				r = append(r, results{
 					now, res,
 				})
@@ -195,7 +195,7 @@ func TestDataEvaluator_Eval(t *testing.T) {
 			size := int(to.Sub(from).Seconds() / interval.Seconds())
 			r := make([]results, 0, size)
 
-			err = evaluator.Eval(context.Background(), from, to, interval, func(now time.Time, res eval.Results) error {
+			err = evaluator.Eval(context.Background(), from, interval, size, func(idx int, now time.Time, res eval.Results) error {
 				r = append(r, results{
 					now, res,
 				})
@@ -230,7 +230,7 @@ func TestDataEvaluator_Eval(t *testing.T) {
 		t.Run("should be noData until the frame interval", func(t *testing.T) {
 			newFrom := from.Add(-10 * time.Second)
 			r := make([]results, 0, int(to.Sub(newFrom).Seconds()))
-			err = evaluator.Eval(context.Background(), newFrom, to, time.Second, func(now time.Time, res eval.Results) error {
+			err = evaluator.Eval(context.Background(), newFrom, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) error {
 				r = append(r, results{
 					now, res,
 				})
@@ -258,7 +258,7 @@ func TestDataEvaluator_Eval(t *testing.T) {
 		t.Run("should be the last value after the frame interval", func(t *testing.T) {
 			newTo := to.Add(10 * time.Second)
 			r := make([]results, 0, int(newTo.Sub(from).Seconds()))
-			err = evaluator.Eval(context.Background(), from, newTo, time.Second, func(now time.Time, res eval.Results) error {
+			err = evaluator.Eval(context.Background(), from, time.Second, cap(r), func(idx int, now time.Time, res eval.Results) error {
 				r = append(r, results{
 					now, res,
 				})
@@ -282,12 +282,10 @@ func TestDataEvaluator_Eval(t *testing.T) {
 	})
 	t.Run("should stop if callback error", func(t *testing.T) {
 		expectedError := errors.New("error")
-		evals := 0
-		err = evaluator.Eval(context.Background(), from, to, time.Second, func(now time.Time, res eval.Results) error {
-			if evals > 5 {
+		err = evaluator.Eval(context.Background(), from, time.Second, 6, func(idx int, now time.Time, res eval.Results) error {
+			if idx == 5 {
 				return expectedError
 			}
-			evals++
 			return nil
 		})
 		require.ErrorIs(t, err, expectedError)

pkg/services/ngalert/backtesting/eval_query.go

@@ -12,13 +12,13 @@ type queryEvaluator struct {
 	eval eval.ConditionEvaluator
 }
 
-func (d *queryEvaluator) Eval(ctx context.Context, from, to time.Time, interval time.Duration, callback callbackFunc) error {
-	for now := from; now.Before(to); now = now.Add(interval) {
+func (d *queryEvaluator) Eval(ctx context.Context, from time.Time, interval time.Duration, evaluations int, callback callbackFunc) error {
+	for idx, now := 0, from; idx < evaluations; idx, now = idx+1, now.Add(interval) {
 		results, err := d.eval.Evaluate(ctx, now)
 		if err != nil {
 			return err
 		}
-		err = callback(now, results)
+		err = callback(idx, now, results)
 		if err != nil {
 			return err
 		}

pkg/services/ngalert/backtesting/eval_query_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
@@ -18,8 +19,7 @@ func TestQueryEvaluator_Eval(t *testing.T) {
 	ctx := context.Background()
 	interval := time.Duration(rand.Int63n(9)+1) * time.Second
 	times := rand.Intn(11) + 5
-	to := time.Now()
-	from := to.Add(-time.Duration(times) * interval)
+	from := time.Now().Add(-time.Duration(times) * interval)
 
 	t.Run("should evaluate query", func(t *testing.T) {
 		m := &eval_mocks.ConditionEvaluatorMock{}
@@ -29,15 +29,20 @@ func TestQueryEvaluator_Eval(t *testing.T) {
 			eval: m,
 		}
 
-		intervals := make([]time.Time, 0, times)
+		intervals := make([]time.Time, times)
 
-		err := evaluator.Eval(ctx, from, to, interval, func(now time.Time, results eval.Results) error {
-			intervals = append(intervals, now)
+		err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error {
+			intervals[idx] = now
 			return nil
 		})
 		require.NoError(t, err)
 		require.Len(t, intervals, times)
 
+		expected := from
+		for idx, actual := range intervals {
+			assert.Equalf(t, expected, actual, "item at index %d is not times of interval %v", idx, interval)
+			expected = expected.Add(interval)
+		}
 		m.AssertNumberOfCalls(t, "Evaluate", times)
 		for _, now := range intervals {
 			m.AssertCalled(t, "Evaluate", ctx, now)
@@ -57,7 +62,7 @@ func TestQueryEvaluator_Eval(t *testing.T) {
 
 			intervals := make([]time.Time, 0, times)
 
-			err := evaluator.Eval(ctx, from, to, interval, func(now time.Time, results eval.Results) error {
+			err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error {
 				intervals = append(intervals, now)
 				return nil
 			})
@@ -76,7 +81,7 @@ func TestQueryEvaluator_Eval(t *testing.T) {
 
 			intervals := make([]time.Time, 0, times)
 
-			err := evaluator.Eval(ctx, from, to, interval, func(now time.Time, results eval.Results) error {
+			err := evaluator.Eval(ctx, from, interval, times, func(idx int, now time.Time, results eval.Results) error {
 				if len(intervals) > 3 {
 					return expectedError
 				}