PromQL: Always enable negative offset and @ modifier

This follows the line of argument that the invariant of not looking
ahead of the query time was merely emerging behavior and not a
documented stable feature. Any query that looks ahead of the query
time was simply invalid before the introduction of the negative offset
and the @ modifier.

Signed-off-by: beorn7 <beorn@grafana.com>

cmd/prometheus/main.go

@@ -149,8 +149,6 @@ type flagConfig struct {
 	featureList []string
 	// These options are extracted from featureList
 	// for ease of use.
-	enablePromQLAtModifier     bool
-	enablePromQLNegativeOffset bool
 	enableExpandExternalLabels bool
 	enableNewSDManager         bool
 
@@ -166,12 +164,6 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error {
 		opts := strings.Split(f, ",")
 		for _, o := range opts {
 			switch o {
-			case "promql-at-modifier":
-				c.enablePromQLAtModifier = true
-				level.Info(logger).Log("msg", "promql-at-modifier enabled")
-			case "promql-negative-offset":
-				c.enablePromQLNegativeOffset = true
-				level.Info(logger).Log("msg", "promql-negative-offset enabled")
 			case "remote-write-receiver":
 				c.web.EnableRemoteWriteReceiver = true
 				level.Warn(logger).Log("msg", "Remote write receiver enabled via feature flag remote-write-receiver. This is DEPRECATED. Use --web.enable-remote-write-receiver.")
@@ -195,6 +187,8 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error {
 				level.Info(logger).Log("msg", "Experimental agent mode enabled.")
 			case "":
 				continue
+			case "promql-at-modifier", "promql-negative-offset":
+				level.Warn(logger).Log("msg", "This option for --enable-feature is now permanently enabled and therefore a no-op.", "option", o)
 			default:
 				level.Warn(logger).Log("msg", "Unknown option for --enable-feature", "option", o)
 			}
@@ -570,8 +564,10 @@ func main() {
 			ActiveQueryTracker:       promql.NewActiveQueryTracker(localStoragePath, cfg.queryConcurrency, log.With(logger, "component", "activeQueryTracker")),
 			LookbackDelta:            time.Duration(cfg.lookbackDelta),
 			NoStepSubqueryIntervalFn: noStepSubqueryInterval.Get,
-			EnableAtModifier:         cfg.enablePromQLAtModifier,
-			EnableNegativeOffset:     cfg.enablePromQLNegativeOffset,
+			// EnableAtModifier and EnableNegativeOffset have to be
+			// always on for regular PromQL as of Prometheus v2.33.
+			EnableAtModifier:     true,
+			EnableNegativeOffset: true,
 		}
 
 		queryEngine = promql.NewEngine(opts)

cmd/promtool/main.go

@@ -203,17 +203,14 @@ func main() {
 		p = &promqlPrinter{}
 	}
 
-	var queryOpts promql.LazyLoaderOpts
 	for _, f := range *featureList {
 		opts := strings.Split(f, ",")
 		for _, o := range opts {
 			switch o {
-			case "promql-at-modifier":
-				queryOpts.EnableAtModifier = true
-			case "promql-negative-offset":
-				queryOpts.EnableNegativeOffset = true
 			case "":
 				continue
+			case "promql-at-modifier", "promql-negative-offset":
+				fmt.Printf("  WARNING: Option for --enable-feature is a no-op after promotion to a stable feature: %q\n", o)
 			default:
 				fmt.Printf("  WARNING: Unknown option for --enable-feature: %q\n", o)
 			}
@@ -258,7 +255,13 @@ func main() {
 		os.Exit(QueryLabels(*queryLabelsServer, *queryLabelsName, *queryLabelsBegin, *queryLabelsEnd, p))
 
 	case testRulesCmd.FullCommand():
-		os.Exit(RulesUnitTest(queryOpts, *testRulesFiles...))
+		os.Exit(RulesUnitTest(
+			promql.LazyLoaderOpts{
+				EnableAtModifier:     true,
+				EnableNegativeOffset: true,
+			},
+			*testRulesFiles...),
+		)
 
 	case tsdbBenchWriteCmd.FullCommand():
 		os.Exit(checkErr(benchmarkWrite(*benchWriteOutPath, *benchSamplesFile, *benchWriteNumMetrics, *benchWriteNumScrapes)))

docs/feature_flags.md

@@ -11,18 +11,6 @@ Their behaviour can change in future releases which will be communicated via the
 You can enable them using the `--enable-feature` flag with a comma separated list of features.
 They may be enabled by default in future versions.
 
-## `@` Modifier in PromQL
-
-`--enable-feature=promql-at-modifier`
-
-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).
-
-This feature is considered stable, but since it breaks the invariant that
-PromQL does not look ahead of the evaluation time, it still needs to be enabled
-via this feature flag. The feature will be permanently enabled (and the feature
-flag ignored) upon major release v3.
-
 ## Expand environment variables in external labels
 
 `--enable-feature=expand-external-labels`
@@ -31,25 +19,6 @@ Replace `${var}` or `$var` in the [`external_labels`](configuration/configuratio
 values according to the values of the current environment variables. References
 to undefined variables are replaced by the empty string.
 
-## Negative offset in PromQL
-
-This negative offset is disabled by default since it breaks the invariant
-that PromQL does not look ahead of the evaluation time for samples.
-
-`--enable-feature=promql-negative-offset`
-
-In contrast to the positive offset modifier, the negative offset modifier lets
-one shift a vector selector into the future. An example in which one may want
-to use a negative offset is reviewing past data and making temporal comparisons
-with more recent data.
-
-More details can be found [here](querying/basics.md#offset-modifier).
-
-This feature is considered stable, but since it breaks the invariant that
-PromQL does not look ahead of the evaluation time, it still needs to be enabled
-via this feature flag. The feature will be permanently enabled (and the feature
-flag ignored) upon major release v3.
-
 ## Remote Write Receiver
 
 `--enable-feature=remote-write-receiver`

docs/querying/basics.md

@@ -209,9 +209,7 @@ can be specified:
 
     rate(http_requests_total[5m] offset -1w)
 
-This feature is enabled by setting `--enable-feature=promql-negative-offset`
-flag. See [feature flags](../feature_flags.md) for more details about
-this flag.
+Note that this allows a query to look ahead of its evaluation time.
 
 ### @ modifier
 
@@ -249,10 +247,6 @@ These 2 queries will produce the same result.
     # offset before @
     http_requests_total offset 5m @ 1609746000
 
-This modifier is disabled by default since it breaks the invariant that PromQL
-does not look ahead of the evaluation time for samples. It can be enabled by setting
-`--enable-feature=promql-at-modifier` flag. See [feature flags](../feature_flags.md) for more details about this flag.
-
 Additionally, `start()` and `end()` can also be used as values for the `@` modifier as special values.
 
 For a range query, they resolve to the start and end of the range query respectively and remain the same for all steps.
@@ -262,6 +256,8 @@ For an instant query, `start()` and `end()` both resolve to the evaluation time.
     http_requests_total @ start()
     rate(http_requests_total[5m] @ end())
 
+Note that the `@` modifier allows a query to look ahead of its evaluation time.
+
 ## Subquery
 
 Subquery allows you to run an instant query for a given range and resolution. The result of a subquery is a range vector.

go.mod

@@ -64,7 +64,7 @@ require (
 	golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
 	golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e
 	golang.org/x/time v0.0.0-20211116232009-f0f3c7e86c11
-	golang.org/x/tools v0.1.8
+	golang.org/x/tools v0.1.9-0.20211209172050-90a85b2969be
 	google.golang.org/api v0.64.0
 	google.golang.org/genproto v0.0.0-20211223182754-3ac035c7e7cb
 	google.golang.org/protobuf v1.27.1

go.sum

@@ -1785,8 +1785,8 @@ golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
 golang.org/x/tools v0.1.6-0.20210726203631-07bc1bf47fb2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
-golang.org/x/tools v0.1.8 h1:P1HhGGuLW4aAclzjtmJdf0mJOjVUZUzOTqkAkWL+l6w=
-golang.org/x/tools v0.1.8/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
+golang.org/x/tools v0.1.9-0.20211209172050-90a85b2969be h1:JRBiPXZpZ1FsceyPRRme0vX394zXC3xlhqu705k9nzM=
+golang.org/x/tools v0.1.9-0.20211209172050-90a85b2969be/go.mod h1:nABZi5QlRsZVlzPpHl034qft6wpY4eDcsTt5AaioBiU=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

promql/engine.go

@@ -222,10 +222,17 @@ type EngineOpts struct {
 	// a subquery in milliseconds if no step in range vector was specified `[30m:<step>]`.
 	NoStepSubqueryIntervalFn func(rangeMillis int64) int64
 
-	// EnableAtModifier if true enables @ modifier. Disabled otherwise.
+	// EnableAtModifier if true enables @ modifier. Disabled otherwise. This
+	// is supposed to be enabled for regular PromQL (as of Prometehus v2.33)
+	// but the option to disable it is still provided here for those using
+	// the Engine outside of Prometheus.
 	EnableAtModifier bool
 
-	// EnableNegativeOffset if true enables negative (-) offset values. Disabled otherwise.
+	// EnableNegativeOffset if true enables negative (-) offset
+	// values. Disabled otherwise. This is supposed to be enabled for
+	// regular PromQL (as of Prometehus v2.33) but the option to disable it
+	// is still provided here for those using the Engine outside of
+	// Prometheus.
 	EnableNegativeOffset bool
 }
 

promql/test.go

@@ -680,7 +680,9 @@ type LazyLoader struct {
 
 // LazyLoaderOpts are options for the lazy loader.
 type LazyLoaderOpts struct {
-	// Disabled PromQL engine features.
+	// Both of these must be set to true for regular PromQL (as of
+	// Prometheus v2.33). They can still be disabled here for legacy and
+	// other uses.
 	EnableAtModifier, EnableNegativeOffset bool
 }
 

web/api/v1/api.go

@@ -378,11 +378,6 @@ 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")
 	}
@@ -458,11 +453,6 @@ 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}
 	}