From 6fbedd62b84d50b71df04fbe7d4d24100370f3f6 Mon Sep 17 00:00:00 2001
From: Roman Khavronenko <hagen1778@gmail.com>
Date: Fri, 30 Apr 2021 08:01:05 +0100
Subject: [PATCH] vmalert: use rule's `evaluationInterval` as `step` param by
 default (#1258)

User still can override param by specifying `datasource.queryStep` flag.
---
 app/vmalert/datasource/init.go    |  3 ++-
 app/vmalert/datasource/vm.go      |  4 ++++
 app/vmalert/datasource/vm_test.go | 12 +++++++-----
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/app/vmalert/datasource/init.go b/app/vmalert/datasource/init.go
index 7d27b9d55e..62603f749b 100644
--- a/app/vmalert/datasource/init.go
+++ b/app/vmalert/datasource/init.go
@@ -23,7 +23,8 @@ var (
 
 	lookBack  = flag.Duration("datasource.lookback", 0, `Lookback defines how far into the past to look when evaluating queries. For example, if the datasource.lookback=5m then param "time" with value now()-5m will be added to every query.`)
 	queryStep = flag.Duration("datasource.queryStep", 0, "queryStep defines how far a value can fallback to when evaluating queries. "+
-		"For example, if datasource.queryStep=15s then param \"step\" with value \"15s\" will be added to every query.")
+		"For example, if datasource.queryStep=15s then param \"step\" with value \"15s\" will be added to every query."+
+		"If queryStep isn't specified, rule's evaluationInterval will be used instead.")
 	maxIdleConnections = flag.Int("datasource.maxIdleConnections", 100, `Defines the number of idle (keep-alive connections) to each configured datasource. Consider setting this value equal to the value: groups_total * group.concurrency. Too low a value may result in a high number of sockets in TIME_WAIT state.`)
 )
 
diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go
index defab53543..2c23a06dcb 100644
--- a/app/vmalert/datasource/vm.go
+++ b/app/vmalert/datasource/vm.go
@@ -209,9 +209,13 @@ func (s *VMStorage) setPrometheusReqParams(r *http.Request, query string, timest
 	if s.evaluationInterval > 0 {
 		// see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1232
 		timestamp = timestamp.Truncate(s.evaluationInterval)
+		// set step as evaluationInterval by default
+		q.Set("step", s.evaluationInterval.String())
 	}
 	q.Set("time", fmt.Sprintf("%d", timestamp.Unix()))
+
 	if s.queryStep > 0 {
+		// override step with user-specified value
 		q.Set("step", s.queryStep.String())
 	}
 	r.URL.RawQuery = q.Encode()
diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go
index f9bad36bee..db030cd5ac 100644
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@@ -213,8 +213,9 @@ func TestPrepareReq(t *testing.T) {
 				evaluationInterval: 15 * time.Second,
 			},
 			func(t *testing.T, r *http.Request) {
-				tt := timestamp.Truncate(15 * time.Second)
-				exp := fmt.Sprintf("query=%s&time=%d", query, tt.Unix())
+				evalInterval := 15 * time.Second
+				tt := timestamp.Truncate(evalInterval)
+				exp := fmt.Sprintf("query=%s&step=%v&time=%d", query, evalInterval, tt.Unix())
 				checkEqualString(t, exp, r.URL.RawQuery)
 			},
 		},
@@ -225,14 +226,15 @@ func TestPrepareReq(t *testing.T) {
 				evaluationInterval: 15 * time.Second,
 			},
 			func(t *testing.T, r *http.Request) {
+				evalInterval := 15 * time.Second
 				tt := timestamp.Add(-time.Minute)
-				tt = tt.Truncate(15 * time.Second)
-				exp := fmt.Sprintf("query=%s&time=%d", query, tt.Unix())
+				tt = tt.Truncate(evalInterval)
+				exp := fmt.Sprintf("query=%s&step=%v&time=%d", query, evalInterval, tt.Unix())
 				checkEqualString(t, exp, r.URL.RawQuery)
 			},
 		},
 		{
-			"step",
+			"step override",
 			&VMStorage{
 				queryStep: time.Minute,
 			},