From e80b44f19d601994a2f44cd118ef1c8ec1912ae8 Mon Sep 17 00:00:00 2001 From: Hui Wang Date: Tue, 12 Mar 2024 23:16:50 +0800 Subject: [PATCH] vmalert: deprecate cmd-line flag `-datasource.lookback` (#5877) * vmalert: deprecate cmd-line flag `-datasource.lookback` * fix lint * review fixes Signed-off-by: hagen1778 --------- Signed-off-by: hagen1778 Co-authored-by: hagen1778 --- app/vmalert/datasource/init.go | 5 ++-- app/vmalert/datasource/vm.go | 7 ++--- app/vmalert/datasource/vm_graphite_api.go | 8 +----- app/vmalert/datasource/vm_prom_api.go | 3 --- app/vmalert/datasource/vm_test.go | 33 +++-------------------- app/vmalert/remoteread/init.go | 2 +- docs/CHANGELOG.md | 2 ++ docs/vmalert.md | 4 +-- 8 files changed, 14 insertions(+), 50 deletions(-) diff --git a/app/vmalert/datasource/init.go b/app/vmalert/datasource/init.go index 786f9e174..140aff936 100644 --- 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, diff --git a/app/vmalert/datasource/vm.go b/app/vmalert/datasource/vm.go index ba8d8cd3c..7c98b0313 100644 --- 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) } diff --git a/app/vmalert/datasource/vm_graphite_api.go b/app/vmalert/datasource/vm_graphite_api.go index bb4acfeca..699919e77 100644 --- 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) diff --git a/app/vmalert/datasource/vm_prom_api.go b/app/vmalert/datasource/vm_prom_api.go index b968aa667..ab64d0729 100644 --- 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 diff --git a/app/vmalert/datasource/vm_test.go b/app/vmalert/datasource/vm_test.go index 78b38ecc1..50a03ac28 100644 --- 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) }) diff --git a/app/vmalert/remoteread/init.go b/app/vmalert/remoteread/init.go index 02836a51f..fc25ce5b9 100644 --- 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 } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 83d0bc49c..a3eb69b46 100644 --- 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). diff --git a/docs/vmalert.md b/docs/vmalert.md index dbc690c07..bc1a8a043 100644 --- 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