From 22cd48868ad3038387287f3e02fd6406aef21a91 Mon Sep 17 00:00:00 2001 From: schou Date: Sun, 21 Feb 2021 13:03:58 -0500 Subject: [PATCH] adding feature flag, promql-negative-offset Signed-off-by: schou --- cmd/prometheus/main.go | 7 ++++++- docs/disabled_features.md | 6 ++++++ docs/querying/basics.md | 8 +++++++- promql/engine.go | 29 +++++++++++++++++++++++++---- promql/engine_test.go | 12 ++++++++++++ web/api/v1/api.go | 4 ++++ 6 files changed, 60 insertions(+), 6 deletions(-) diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 964b0165c..c7309c9bd 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -117,7 +117,8 @@ type flagConfig struct { featureList []string // These options are extracted from featureList // for ease of use. - enablePromQLAtModifier bool + enablePromQLAtModifier bool + enablePromQLNegativeOffset bool prometheusURL string corsRegexString string @@ -134,6 +135,9 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error { case "promql-at-modifier": c.enablePromQLAtModifier = true level.Info(logger).Log("msg", "Experimental promql-at-modifier enabled") + case "promql-negative-offset": + c.enablePromQLNegativeOffset = true + level.Info(logger).Log("msg", "Experimental promql-negative-offset enabled") case "remote-write-receiver": c.web.RemoteWriteReceiver = true level.Info(logger).Log("msg", "Experimental remote-write-receiver enabled") @@ -441,6 +445,7 @@ func main() { LookbackDelta: time.Duration(cfg.lookbackDelta), NoStepSubqueryIntervalFn: noStepSubqueryInterval.Get, EnableAtModifier: cfg.enablePromQLAtModifier, + EnableNegativeOffset: cfg.enablePromQLNegativeOffset, } queryEngine = promql.NewEngine(opts) diff --git a/docs/disabled_features.md b/docs/disabled_features.md index ec6363451..e4d1376f6 100644 --- a/docs/disabled_features.md +++ b/docs/disabled_features.md @@ -18,6 +18,12 @@ They may be enabled by default in future versions. The `@` modifier lets you specify the evaluation time for instant vector selectors, range vector selectors, and subqueries. More details can be found [here](querying/basics.md#modifier). +## Negative offset in PromQL + +`--enable-feature=promql-negative-offset` + +In contrast to the positive offset modifier, the negative offset modifier lets one shift a vector into the future. An example in which one may want to use a negative offset is: when reviewing past data and making temporal comparisons with earlier data and later data. More details can be found [here](querying/basics.md#offset-modifier). + ## Remote Write Receiver `--enable-feature=remote-write-receiver` diff --git a/docs/querying/basics.md b/docs/querying/basics.md index 1465a0a40..90287b998 100644 --- a/docs/querying/basics.md +++ b/docs/querying/basics.md @@ -204,11 +204,17 @@ The same works for range vectors. This returns the 5-minute rate that rate(http_requests_total[5m] offset 1w) -For comparisons with temporal shifts forward and backward, a negative offset +For comparisons with temporal shifts forward in time, a negative offset can be specified: rate(http_requests_total[5m] offset -1w) +This negative offset is disabled by default since it breaks the invariant +that PromQL does not look ahead of the evaluation time for samples. This +feature is enabled by setting `--enable-feature=promql-negative-offset` +flag. See [disabled features](../disabled_features.md) for more details +about this flag. + ### @ modifier The `@` modifier allows changing the evaluation time for individual instant diff --git a/promql/engine.go b/promql/engine.go index 2ed101441..06defd2fe 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -211,6 +211,9 @@ type EngineOpts struct { // EnableAtModifier if true enables @ modifier. Disabled otherwise. EnableAtModifier bool + + // EnableNegativeOffset if true enables negative (-) offset values. Disabled otherwise. + EnableNegativeOffset bool } // Engine handles the lifetime of queries from beginning to end. @@ -226,6 +229,7 @@ type Engine struct { lookbackDelta time.Duration noStepSubqueryIntervalFn func(rangeMillis int64) int64 enableAtModifier bool + enableNegativeOffset bool } // NewEngine returns a new engine. @@ -307,6 +311,7 @@ func NewEngine(opts EngineOpts) *Engine { lookbackDelta: opts.LookbackDelta, noStepSubqueryIntervalFn: opts.NoStepSubqueryIntervalFn, enableAtModifier: opts.EnableAtModifier, + enableNegativeOffset: opts.EnableNegativeOffset, } } @@ -388,9 +393,10 @@ func (ng *Engine) newQuery(q storage.Queryable, expr parser.Expr, start, end tim } var ErrValidationAtModifierDisabled = errors.New("@ modifier is disabled") +var ErrValidationNegativeOffsetDisabled = errors.New("negative offset is disabled") func (ng *Engine) validateOpts(expr parser.Expr) error { - if ng.enableAtModifier { + if ng.enableAtModifier && ng.enableNegativeOffset { return nil } @@ -398,23 +404,38 @@ func (ng *Engine) validateOpts(expr parser.Expr) error { parser.Inspect(expr, func(node parser.Node, path []parser.Node) error { switch n := node.(type) { case *parser.VectorSelector: - if n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END { + if !ng.enableAtModifier && + (n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END) { validationErr = ErrValidationAtModifierDisabled return validationErr } + if !ng.enableNegativeOffset && n.OriginalOffset < 0 { + validationErr = ErrValidationNegativeOffsetDisabled + return validationErr + } case *parser.MatrixSelector: vs := n.VectorSelector.(*parser.VectorSelector) - if vs.Timestamp != nil || vs.StartOrEnd == parser.START || vs.StartOrEnd == parser.END { + if !ng.enableAtModifier && + (vs.Timestamp != nil || vs.StartOrEnd == parser.START || vs.StartOrEnd == parser.END) { validationErr = ErrValidationAtModifierDisabled return validationErr } + if !ng.enableNegativeOffset && vs.OriginalOffset < 0 { + validationErr = ErrValidationNegativeOffsetDisabled + return validationErr + } case *parser.SubqueryExpr: - if n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END { + if !ng.enableAtModifier && + (n.Timestamp != nil || n.StartOrEnd == parser.START || n.StartOrEnd == parser.END) { validationErr = ErrValidationAtModifierDisabled return validationErr } + if !ng.enableNegativeOffset && n.OriginalOffset < 0 { + validationErr = ErrValidationNegativeOffsetDisabled + return validationErr + } } return nil }) diff --git a/promql/engine_test.go b/promql/engine_test.go index c3a96a9c2..9dea17315 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -2299,6 +2299,18 @@ func TestEngineOptsValidation(t *testing.T) { }, { opts: EngineOpts{EnableAtModifier: true}, query: "rate(metric[1h:1m] @ end())", + }, { + opts: EngineOpts{EnableNegativeOffset: false}, + query: "metric offset -1s", fail: true, expError: ErrValidationNegativeOffsetDisabled, + }, { + opts: EngineOpts{EnableNegativeOffset: true}, + query: "metric offset -1s", + }, { + opts: EngineOpts{EnableAtModifier: true, EnableNegativeOffset: true}, + query: "metric @ 100 offset -2m", + }, { + opts: EngineOpts{EnableAtModifier: true, EnableNegativeOffset: true}, + query: "metric offset -2m @ 100", }, } diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 81980ae47..b283948c3 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -370,6 +370,8 @@ func (api *API) query(r *http.Request) (result apiFuncResult) { qry, err := api.QueryEngine.NewInstantQuery(api.Queryable, r.FormValue("query"), ts) if err == promql.ErrValidationAtModifierDisabled { err = errors.New("@ modifier is disabled, use --enable-feature=promql-at-modifier to enable it") + } else if err == promql.ErrValidationNegativeOffsetDisabled { + err = errors.New("negative offset is disabled, use --enable-feature=promql-negative-offset to enable it") } if err != nil { return invalidParamError(err, "query") @@ -448,6 +450,8 @@ func (api *API) queryRange(r *http.Request) (result apiFuncResult) { qry, err := api.QueryEngine.NewRangeQuery(api.Queryable, r.FormValue("query"), start, end, step) if err == promql.ErrValidationAtModifierDisabled { err = errors.New("@ modifier is disabled, use --enable-feature=promql-at-modifier to enable it") + } else if err == promql.ErrValidationNegativeOffsetDisabled { + err = errors.New("negative offset is disabled, use --enable-feature=promql-negative-offset to enable it") } if err != nil { return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil}