Repository: VictoriaMetrics/VictoriaMetrics
Pull request #5657 (2024-01-22 11:09:36)

vmalert: set `ActiveAt` to evaluation timestamp
The change is to set `ActiveAt` timestamp of created alerts to the execution timestamp instead of the actual timestamp when rule was executed. The change is justified by alert generator specification: https://github.com/prometheus/compliance/blob/main/alert_generator/specification.md#pending-state-conditions:
  "evaluation time when the alert is first created is referred to as ActiveAt for that alert".

The change also fixes flaky test `TestAlertingRule_Exec` which has dependency on the actual timestamps, which resulted into inaccurate test states:
https://github.com/VictoriaMetrics/VictoriaMetrics/actions/runs/7608452967/job/20717699688

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5657?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`df19772`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/df197723ae55ac4654ed46b0699237dc2ba9b7bd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.05% compared to head [(`8dafc09`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5657?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.00%.
> Report is 1 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5657      +/-   ##
==========================================
- Coverage   60.05%   60.00%   -0.05%     
==========================================
  Files         408      408              
  Lines       76322    76322              
==========================================
- Hits        45832    45800      -32     
- Misses      27951    27976      +25     
- Partials     2539     2546       +7     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/5657?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/app/vmalert/rule/alerting.go b/app/vmalert/rule/alerting.go
index be50ef133..da013ec10 100644
--- a/app/vmalert/rule/alerting.go
+++ b/app/vmalert/rule/alerting.go
@@ -440,14 +440,13 @@ func (ar *AlertingRule) exec(ctx context.Context, ts time.Time, limit int) ([]pr
 			a.KeepFiringSince = time.Time{}
 			continue
 		}
-		a, err := ar.newAlert(m, ls, start, qFn)
+		a, err := ar.newAlert(m, ls, ts, qFn)
 		if err != nil {
 			curState.Err = fmt.Errorf("failed to create alert: %w", err)
 			return nil, curState.Err
 		}
 		a.ID = h
 		a.State = notifier.StatePending
-		a.ActiveAt = ts
 		ar.alerts[h] = a
 		ar.logDebugf(ts, a, "created in state PENDING")
 	}
@@ -473,7 +472,7 @@ func (ar *AlertingRule) exec(ctx context.Context, ts time.Time, limit int) ([]pr
 				}
 				// alerts with ar.KeepFiringFor>0 may remain FIRING
 				// even if their expression isn't true anymore
-				if ts.Sub(a.KeepFiringSince) > ar.KeepFiringFor {
+				if ts.Sub(a.KeepFiringSince) >= ar.KeepFiringFor {
 					a.State = notifier.StateInactive
 					a.ResolvedAt = ts
 					ar.logDebugf(ts, a, "FIRING => INACTIVE: is absent in current evaluation round")


Test patch:
diff --git a/app/vmalert/rule/alerting_test.go b/app/vmalert/rule/alerting_test.go
index 5b7fb5404..95539efd4 100644
--- a/app/vmalert/rule/alerting_test.go
+++ b/app/vmalert/rule/alerting_test.go
@@ -328,14 +328,17 @@ func TestAlertingRule_Exec(t *testing.T) {
 			fq := &datasource.FakeQuerier{}
 			tc.rule.q = fq
 			tc.rule.GroupID = fakeGroup.ID()
+			ts := time.Now()
 			for i, step := range tc.steps {
 				fq.Reset()
 				fq.Add(step...)
-				if _, err := tc.rule.exec(context.TODO(), time.Now(), 0); err != nil {
+				if _, err := tc.rule.exec(context.TODO(), ts, 0); err != nil {
 					t.Fatalf("unexpected err: %s", err)
 				}
-				// artificial delay between applying steps
-				time.Sleep(defaultStep)
+
+				// shift the execution timestamp before the next iteration
+				ts = ts.Add(defaultStep)
+
 				if _, ok := tc.expAlerts[i]; !ok {
 					continue
 				}