Alerting: recalculate EndsAt (#35830)

* setEndsAt

* one more test case

* add should clause to tests

pkg/services/ngalert/state/manager.go

@@ -83,13 +83,13 @@ func (st *Manager) setNextState(alertRule *ngModels.AlertRule, result eval.Resul
 	st.Log.Debug("setting alert state", "uid", alertRule.UID)
 	switch result.State {
 	case eval.Normal:
-		currentState = currentState.resultNormal(result)
+		currentState.resultNormal(result)
 	case eval.Alerting:
-		currentState = currentState.resultAlerting(alertRule, result)
+		currentState.resultAlerting(alertRule, result)
 	case eval.Error:
-		currentState = currentState.resultError(alertRule, result)
+		currentState.resultError(alertRule, result)
 	case eval.NoData:
-		currentState = currentState.resultNoData(alertRule, result)
+		currentState.resultNoData(alertRule, result)
 	case eval.Pending: // we do not emit results with this state
 	}
 

pkg/services/ngalert/state/state.go

@@ -30,76 +30,54 @@ type Evaluation struct {
 	EvaluationString string
 }
 
-func (a *State) resultNormal(result eval.Result) *State {
+func (a *State) resultNormal(result eval.Result) {
 	if a.State != eval.Normal {
 		a.EndsAt = result.EvaluatedAt
 		a.StartsAt = result.EvaluatedAt
 	}
 	a.Error = result.Error // should be nil since state is not error
 	a.State = eval.Normal
-	return a
 }
 
-func (a *State) resultAlerting(alertRule *ngModels.AlertRule, result eval.Result) *State {
+func (a *State) resultAlerting(alertRule *ngModels.AlertRule, result eval.Result) {
 	switch a.State {
 	case eval.Alerting:
-		if !(alertRule.For > 0) {
+		a.setEndsAt(alertRule, result)
-			// If there is not For set, we will set EndsAt to be twice the evaluation interval
-			// to avoid flapping with every evaluation
-			a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second)
-			return a
-		}
-		a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
 	case eval.Pending:
 		if result.EvaluatedAt.Sub(a.StartsAt) > alertRule.For {
 			a.State = eval.Alerting
 			a.StartsAt = result.EvaluatedAt
-			a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
+			a.setEndsAt(alertRule, result)
 		}
 	default:
 		a.StartsAt = result.EvaluatedAt
+		a.setEndsAt(alertRule, result)
 		if !(alertRule.For > 0) {
-			a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second)
+			// If For is 0, immediately set Alerting
 			a.State = eval.Alerting
 		} else {
-			a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
+			a.State = eval.Pending
-			if result.EvaluatedAt.Sub(a.StartsAt) > alertRule.For {
-				a.State = eval.Alerting
-			} else {
-				a.State = eval.Pending
-			}
 		}
 	}
-	return a
 }
 
-func (a *State) resultError(alertRule *ngModels.AlertRule, result eval.Result) *State {
+func (a *State) resultError(alertRule *ngModels.AlertRule, result eval.Result) {
 	a.Error = result.Error
 	if a.StartsAt.IsZero() {
 		a.StartsAt = result.EvaluatedAt
 	}
-	if !(alertRule.For > 0) {
+	a.setEndsAt(alertRule, result)
-		a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second)
-	} else {
-		a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
-	}
 
-	switch alertRule.ExecErrState {
+	if alertRule.ExecErrState == ngModels.AlertingErrState {
-	case ngModels.AlertingErrState:
 		a.State = eval.Alerting
 	}
-	return a
 }
 
-func (a *State) resultNoData(alertRule *ngModels.AlertRule, result eval.Result) *State {
+func (a *State) resultNoData(alertRule *ngModels.AlertRule, result eval.Result) {
 	if a.StartsAt.IsZero() {
 		a.StartsAt = result.EvaluatedAt
 	}
-	if !(alertRule.For > 0) {
+	a.setEndsAt(alertRule, result)
-		a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second)
-	} else {
-		a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
-	}
 
 	switch alertRule.NoDataState {
 	case ngModels.Alerting:
@@ -109,7 +87,6 @@ func (a *State) resultNoData(alertRule *ngModels.AlertRule, result eval.Result)
 	case ngModels.OK:
 		a.State = eval.Normal
 	}
-	return a
 }
 
 func (a *State) NeedsSending(resendDelay time.Duration) bool {
@@ -147,3 +124,13 @@ func (a *State) TrimResults(alertRule *ngModels.AlertRule) {
 	copy(newResults, a.Results[len(a.Results)-int(numBuckets):])
 	a.Results = newResults
 }
+
+func (a *State) setEndsAt(alertRule *ngModels.AlertRule, result eval.Result) {
+	if int64(alertRule.For.Seconds()) > alertRule.IntervalSeconds {
+		// For is set and longer than IntervalSeconds
+		a.EndsAt = result.EvaluatedAt.Add(alertRule.For)
+	} else {
+		// For is not set or is less than or equal to IntervalSeconds
+		a.EndsAt = result.EvaluatedAt.Add(time.Duration(alertRule.IntervalSeconds*2) * time.Second)
+	}
+}

pkg/services/ngalert/state/state_test.go

@@ -4,6 +4,8 @@ import (
 	"testing"
 	"time"
 
+	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
+
 	"github.com/grafana/grafana/pkg/services/ngalert/eval"
 
 	"github.com/stretchr/testify/assert"
@@ -73,3 +75,93 @@ func TestNeedsSending(t *testing.T) {
 		})
 	}
 }
+
+func TestSetEndsAt(t *testing.T) {
+	evaluationTime, _ := time.Parse("2006-01-02", "2021-03-25")
+	testCases := []struct {
+		name       string
+		expected   time.Time
+		testState  *State
+		testRule   *ngmodels.AlertRule
+		testResult eval.Result
+	}{
+		{
+			name:      "For: unset Interval: 10s EndsAt should be evaluation time + 2X IntervalSeconds",
+			expected:  evaluationTime.Add(20 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+		{
+			name:      "For: 0s Interval: 10s EndsAt should be evaluation time + 2X IntervalSeconds",
+			expected:  evaluationTime.Add(20 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				For:             0 * time.Second,
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+		{
+			name:      "For: 1s Interval: 10s EndsAt should be evaluation time + 2X IntervalSeconds",
+			expected:  evaluationTime.Add(20 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				For:             0 * time.Second,
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+		{
+			name:      "For: 10s Interval: 10s EndsAt should be evaluation time + 2X IntervalSeconds",
+			expected:  evaluationTime.Add(20 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				For:             10 * time.Second,
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+		{
+			name:      "For: 11s Interval: 10s EndsAt should be evaluation time + For duration",
+			expected:  evaluationTime.Add(11 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				For:             11 * time.Second,
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+		{
+			name:      "For: 20s Interval: 10s EndsAt should be evaluation time + For duration",
+			expected:  evaluationTime.Add(20 * time.Second),
+			testState: &State{},
+			testRule: &ngmodels.AlertRule{
+				For:             20 * time.Second,
+				IntervalSeconds: 10,
+			},
+			testResult: eval.Result{
+				EvaluatedAt: evaluationTime,
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			tc.testState.setEndsAt(tc.testRule, tc.testResult)
+			assert.Equal(t, tc.expected, tc.testState.EndsAt)
+		})
+	}
+}