vmalert: deprecate cmd-line flag -datasource.lookback (#5877)

* vmalert: deprecate cmd-line flag `-datasource.lookback` * fix lint * review fixes Signed-off-by: hagen1778 <roman@victoriametrics.com> --------- Signed-off-by: hagen1778 <roman@victoriametrics.com> Co-authored-by: hagen1778 <roman@victoriametrics.com>
2024-11-21 14:44:00 +00:00 · 2024-03-12 23:16:50 +08:00 · 2024-03-12 23:16:50 +08:00 · e80b44f19d
commit e80b44f19d
parent e8bb64bad5
8 changed files with 14 additions and 50 deletions
--- a/app/vmalert/datasource/init.go
+++ b/app/vmalert/datasource/init.go
@ -46,7 +46,7 @@ var (
 	oauth2TokenURL = flag.String("datasource.oauth2.tokenUrl", "", "Optional OAuth2 tokenURL to use for -datasource.url")
 	oauth2Scopes   = flag.String("datasource.oauth2.scopes", "", "Optional OAuth2 scopes to use for -datasource.url. Scopes must be delimited by ';'")

-	lookBack = flag.Duration("datasource.lookback", 0, `Will be deprecated soon, please adjust "-search.latencyOffset"  at datasource side `+
+	lookBack = flag.Duration("datasource.lookback", 0, `Deprecated: please adjust "-search.latencyOffset" at datasource side `+
 		`or specify "latency_offset" in rule group's params. 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", 5*time.Minute, "How far a value can fallback to when evaluating queries. "+
@ -91,7 +91,7 @@ func Init(extraParams url.Values) (QuerierBuilder, error) {
 		logger.Warnf("flag `-datasource.queryTimeAlignment` is deprecated and will be removed in next releases. Please use `eval_alignment` in rule group instead.")
 	}
 	if *lookBack != 0 {
-		logger.Warnf("flag `-datasource.lookback` will be deprecated soon. Please use `-rule.evalDelay` command-line flag instead. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5155 for details.")
+		logger.Warnf("flag `-datasource.lookback` is deprecated and will be removed in next releases. Please adjust `-search.latencyOffset` at datasource side or specify `latency_offset` in rule group's params. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5155 for details.")
 	}

 	tr, err := httputils.Transport(*addr, *tlsCertFile, *tlsKeyFile, *tlsCAFile, *tlsServerName, *tlsInsecureSkipVerify)
@ -133,7 +133,6 @@ func Init(extraParams url.Values) (QuerierBuilder, error) {
 		authCfg:          authCfg,
 		datasourceURL:    strings.TrimSuffix(*addr, "/"),
 		appendTypePrefix: *appendTypePrefix,
-		lookBack:         *lookBack,
 		queryStep:        *queryStep,
 		dataSourceType:   datasourcePrometheus,
 		extraParams:      extraParams,
--- a/app/vmalert/datasource/vm.go
+++ b/app/vmalert/datasource/vm.go
@ -35,7 +35,6 @@ type VMStorage struct {
 	authCfg          *promauth.Config
 	datasourceURL    string
 	appendTypePrefix bool
-	lookBack         time.Duration
 	queryStep        time.Duration
 	dataSourceType   datasourceType

@ -63,7 +62,6 @@ func (s *VMStorage) Clone() *VMStorage {
 		authCfg:          s.authCfg,
 		datasourceURL:    s.datasourceURL,
 		appendTypePrefix: s.appendTypePrefix,
-		lookBack:         s.lookBack,
 		queryStep:        s.queryStep,

 		dataSourceType:     s.dataSourceType,
@ -122,13 +120,12 @@ func (s *VMStorage) BuildWithParams(params QuerierParams) Querier {
 }

 // NewVMStorage is a constructor for VMStorage
-func NewVMStorage(baseURL string, authCfg *promauth.Config, lookBack time.Duration, queryStep time.Duration, appendTypePrefix bool, c *http.Client) *VMStorage {
+func NewVMStorage(baseURL string, authCfg *promauth.Config, queryStep time.Duration, appendTypePrefix bool, c *http.Client) *VMStorage {
 	return &VMStorage{
 		c:                c,
 		authCfg:          authCfg,
 		datasourceURL:    strings.TrimSuffix(baseURL, "/"),
 		appendTypePrefix: appendTypePrefix,
-		lookBack:         lookBack,
 		queryStep:        queryStep,
 		dataSourceType:   datasourcePrometheus,
 		extraParams:      url.Values{},
@ -248,7 +245,7 @@ func (s *VMStorage) newQueryRequest(ctx context.Context, query string, ts time.T
 	case "", datasourcePrometheus:
 		s.setPrometheusInstantReqParams(req, query, ts)
 	case datasourceGraphite:
-		s.setGraphiteReqParams(req, query, ts)
+		s.setGraphiteReqParams(req, query)
 	default:
 		logger.Panicf("BUG: engine not found: %q", s.dataSourceType)
 	}
--- a/app/vmalert/datasource/vm_graphite_api.go
+++ b/app/vmalert/datasource/vm_graphite_api.go
@ -4,8 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"strconv"
-	"time"
 )

 type graphiteResponse []graphiteResponseTarget
@ -48,17 +46,13 @@ const (
 	graphitePrefix = "/graphite"
 )

-func (s *VMStorage) setGraphiteReqParams(r *http.Request, query string, timestamp time.Time) {
+func (s *VMStorage) setGraphiteReqParams(r *http.Request, query string) {
 	if s.appendTypePrefix {
 		r.URL.Path += graphitePrefix
 	}
 	r.URL.Path += graphitePath
 	q := r.URL.Query()
 	from := "-5min"
-	if s.lookBack > 0 {
-		lookBack := timestamp.Add(-s.lookBack)
-		from = strconv.FormatInt(lookBack.Unix(), 10)
-	}
 	q.Set("from", from)
 	q.Set("format", "json")
 	q.Set("target", query)
--- a/app/vmalert/datasource/vm_prom_api.go
+++ b/app/vmalert/datasource/vm_prom_api.go
@ -161,9 +161,6 @@ func (s *VMStorage) setPrometheusInstantReqParams(r *http.Request, query string,
 		r.URL.Path += "/api/v1/query"
 	}
 	q := r.URL.Query()
-	if s.lookBack > 0 {
-		timestamp = timestamp.Add(-s.lookBack)
-	}
 	q.Set("time", timestamp.Format(time.RFC3339))
 	if !*disableStepParam && s.evaluationInterval > 0 { // set step as evaluationInterval by default
 		// always convert to seconds to keep compatibility with older
--- a/app/vmalert/datasource/vm_test.go
+++ b/app/vmalert/datasource/vm_test.go
@ -86,7 +86,7 @@ func TestVMInstantQuery(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected: %s", err)
 	}
-	s := NewVMStorage(srv.URL, authCfg, time.Minute, 0, false, srv.Client())
+	s := NewVMStorage(srv.URL, authCfg, 0, false, srv.Client())

 	p := datasourcePrometheus
 	pq := s.BuildWithParams(QuerierParams{DataSourceType: string(p), EvaluationInterval: 15 * time.Second})
@ -225,7 +225,7 @@ func TestVMInstantQueryWithRetry(t *testing.T) {
 	srv := httptest.NewServer(mux)
 	defer srv.Close()

-	s := NewVMStorage(srv.URL, nil, time.Minute, 0, false, srv.Client())
+	s := NewVMStorage(srv.URL, nil, 0, false, srv.Client())
 	pq := s.BuildWithParams(QuerierParams{DataSourceType: string(datasourcePrometheus)})

 	expErr := func(err string) {
@ -334,7 +334,7 @@ func TestVMRangeQuery(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected: %s", err)
 	}
-	s := NewVMStorage(srv.URL, authCfg, time.Minute, *queryStep, false, srv.Client())
+	s := NewVMStorage(srv.URL, authCfg, *queryStep, false, srv.Client())

 	pq := s.BuildWithParams(QuerierParams{DataSourceType: string(datasourcePrometheus), EvaluationInterval: 15 * time.Second})

@ -487,17 +487,6 @@ func TestRequestParams(t *testing.T) {
 				checkEqualString(t, "bar", p)
 			},
 		},
-		{
-			"lookback",
-			false,
-			&VMStorage{
-				lookBack: time.Minute,
-			},
-			func(t *testing.T, r *http.Request) {
-				exp := url.Values{"query": {query}, "time": {timestamp.Add(-time.Minute).Format(time.RFC3339)}}
-				checkEqualString(t, exp.Encode(), r.URL.RawQuery)
-			},
-		},
 		{
 			"evaluation interval",
 			false,
@ -510,20 +499,6 @@ func TestRequestParams(t *testing.T) {
 				checkEqualString(t, exp.Encode(), r.URL.RawQuery)
 			},
 		},
-		{
-			"lookback + evaluation interval",
-			false,
-			&VMStorage{
-				lookBack:           time.Minute,
-				evaluationInterval: 15 * time.Second,
-			},
-			func(t *testing.T, r *http.Request) {
-				evalInterval := 15 * time.Second
-				tt := timestamp.Add(-time.Minute)
-				exp := url.Values{"query": {query}, "step": {evalInterval.String()}, "time": {tt.Format(time.RFC3339)}}
-				checkEqualString(t, exp.Encode(), r.URL.RawQuery)
-			},
-		},
 		{
 			"step override",
 			false,
@ -649,7 +624,7 @@ func TestRequestParams(t *testing.T) {
 					tc.vm.setPrometheusInstantReqParams(req, query, timestamp)
 				}
 			case datasourceGraphite:
-				tc.vm.setGraphiteReqParams(req, query, timestamp)
+				tc.vm.setGraphiteReqParams(req, query)
 			}
 			tc.checkFn(t, req)
 		})
--- a/app/vmalert/remoteread/init.go
+++ b/app/vmalert/remoteread/init.go
@ -79,5 +79,5 @@ func Init() (datasource.QuerierBuilder, error) {
 		return nil, fmt.Errorf("failed to configure auth: %w", err)
 	}
 	c := &http.Client{Transport: tr}
-	return datasource.NewVMStorage(*addr, authCfg, 0, 0, false, c), nil
+	return datasource.NewVMStorage(*addr, authCfg, 0, false, c), nil
 }
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@ -30,6 +30,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).

 ## tip

+**vmalert's cmd-line flag `-datasource.lookback` was deprecated and will have no effect anymore. It will be completely removed in next releases. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5155) for more details.**
+
 * SECURITY: upgrade Go builder from Go1.21.7 to Go1.22.1. See [the list of issues addressed in Go1.22.1](https://github.com/golang/go/issues?q=milestone%3AGo1.22.1+label%3ACherryPickApproved).

 * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth/): allow discovering ip addresses for backend instances hidden behind a shared hostname, via `discover_backend_ips: true` option. This allows evenly spreading load among backend instances. See [these docs](https://docs.victoriametrics.com/vmauth/#discovering-backend-ips) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5707).
--- a/docs/vmalert.md
+++ b/docs/vmalert.md
@ -832,7 +832,7 @@ If you use VictoriaMetrics as datasource, `[duration]` can be omitted and Victor
 * Extend `[duration]` in expr to help tolerate the delay. For example, `max_over_time(errors_total[10m]) > 0` will be active even if there is no data in datasource for last `9m`.
 * If [time series resolution](https://docs.victoriametrics.com/keyConcepts.html#time-series-resolution)
 in datasource is inconsistent or `>=5min` - try changing vmalerts `-datasource.queryStep` command-line flag to specify
-how far search query can lookback for the recent datapoint. The recommendation is to have the step 
+how far search query can look back for the recent datapoint. The recommendation is to have the step 
 at least two times bigger than the resolution.

 > Please note, data delay is inevitable in distributed systems. And it is better to account for it instead of ignoring.
@ -989,7 +989,7 @@ The shortlist of configuration flags is the following:
  -datasource.headers string
     Optional HTTP extraHeaders to send with each request to the corresponding -datasource.url. For example, -datasource.headers='My-Auth:foobar' would send 'My-Auth: foobar' HTTP header with every request to the corresponding -datasource.url. Multiple headers must be delimited by '^^': -datasource.headers='header1:value1^^header2:value2'
  -datasource.lookback duration
-     Will be deprecated soon, please adjust "-search.latencyOffset"  at datasource side or specify "latency_offset" in rule group's params. 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.
+     Deprecated: please adjust "-search.latencyOffset" at datasource side or specify "latency_offset" in rule group's params. 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.
  -datasource.maxIdleConnections int
     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. (default 100)
  -datasource.oauth2.clientID string