From c1ec6ae851c284713881518794eef9a924f7b3b7 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Tue, 28 Nov 2023 15:10:12 +0100 Subject: [PATCH] sort_by_label: Switch to feature flag Signed-off-by: Julien Pivotto --- cmd/prometheus/main.go | 4 ---- docs/feature_flags.md | 7 ------- docs/querying/functions.md | 4 ++-- promql/engine.go | 11 ----------- promql/parser/functions.go | 18 ++++++++++-------- promql/promql_test.go | 1 - promql/test.go | 3 +++ 7 files changed, 15 insertions(+), 33 deletions(-) diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index e0b32d7c8..4112cd842 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -154,7 +154,6 @@ type flagConfig struct { enableNewSDManager bool enablePerStepStats bool enableAutoGOMAXPROCS bool - enablePromQLSortByLabel bool prometheusURL string corsRegexString string @@ -210,8 +209,6 @@ func (c *flagConfig) setFeatureListOptions(logger log.Logger) error { config.DefaultConfig.GlobalConfig.ScrapeProtocols = config.DefaultNativeHistogramScrapeProtocols config.DefaultGlobalConfig.ScrapeProtocols = config.DefaultNativeHistogramScrapeProtocols level.Info(logger).Log("msg", "Experimental native histogram support enabled. Changed default scrape_protocols to prefer PrometheusProto format.", "global.scrape_protocols", fmt.Sprintf("%v", config.DefaultGlobalConfig.ScrapeProtocols)) - case "promql-sort-by-label": - c.enablePromQLSortByLabel = true case "": continue case "promql-at-modifier", "promql-negative-offset": @@ -668,7 +665,6 @@ func main() { EnableAtModifier: true, EnableNegativeOffset: true, EnablePerStepStats: cfg.enablePerStepStats, - EnableSortByLabel: cfg.enablePromQLSortByLabel, } queryEngine = promql.NewEngine(opts) diff --git a/docs/feature_flags.md b/docs/feature_flags.md index 2689de0a6..d57763af0 100644 --- a/docs/feature_flags.md +++ b/docs/feature_flags.md @@ -195,10 +195,3 @@ won't work when you push OTLP metrics. Enables PromQL functions that are considered experimental and whose name or semantics could change. - -## PromQL: Sort By Label Function - -`--enable-feature=promql-sort-by-label` - -When enabled, the `sort_by_label` and `sort_by_label_desc` functions can be used -to sort instant query results by their label values. diff --git a/docs/querying/functions.md b/docs/querying/functions.md index 67a32b995..c730ac110 100644 --- a/docs/querying/functions.md +++ b/docs/querying/functions.md @@ -588,7 +588,7 @@ Same as `sort`, but sorts in descending order. ## `sort_by_label()` -**This function has to be enabled via a [feature flag](../feature_flags/#promql-sort-by-label-function).** +**This function has to be enabled via the [feature flag](../feature_flags/) `--enable-feature=promql-experimental-functions`.** `sort_by_label(v instant-vector, label string, ...)` returns vector elements sorted by their label values and sample value in case of label values being equal, in ascending order. @@ -596,7 +596,7 @@ Please note that the sort by label functions only affect the results of instant ## `sort_by_label_desc()` -**This function has to be enabled via a [feature flag](../feature_flags/#promql-sort-by-label-function).** +**This function has to be enabled via the [feature flag](../feature_flags/) `--enable-feature=promql-experimental-functions`.** Same as `sort_by_label`, but sorts in descending order. diff --git a/promql/engine.go b/promql/engine.go index 0240f4848..37057d209 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -304,9 +304,6 @@ type EngineOpts struct { // EnablePerStepStats if true allows for per-step stats to be computed on request. Disabled otherwise. EnablePerStepStats bool - - // EnableSortByLabel if true enables sort_by_label/sort_by_label_desc query functions. - EnableSortByLabel bool } // Engine handles the lifetime of queries from beginning to end. @@ -324,7 +321,6 @@ type Engine struct { enableAtModifier bool enableNegativeOffset bool enablePerStepStats bool - enableSortByLabel bool } // NewEngine returns a new engine. @@ -415,7 +411,6 @@ func NewEngine(opts EngineOpts) *Engine { enableAtModifier: opts.EnableAtModifier, enableNegativeOffset: opts.EnableNegativeOffset, enablePerStepStats: opts.EnablePerStepStats, - enableSortByLabel: opts.EnableSortByLabel, } } @@ -707,7 +702,6 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval lookbackDelta: s.LookbackDelta, samplesStats: query.sampleStats, noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn, - enableSortByLabel: ng.enableSortByLabel, } query.sampleStats.InitStepTracking(start, start, 1) @@ -765,7 +759,6 @@ func (ng *Engine) execEvalStmt(ctx context.Context, query *query, s *parser.Eval lookbackDelta: s.LookbackDelta, samplesStats: query.sampleStats, noStepSubqueryIntervalFn: ng.noStepSubqueryIntervalFn, - enableSortByLabel: ng.enableSortByLabel, } query.sampleStats.InitStepTracking(evaluator.startTimestamp, evaluator.endTimestamp, evaluator.interval) val, warnings, err := evaluator.Eval(s.Expr) @@ -1019,7 +1012,6 @@ type evaluator struct { lookbackDelta time.Duration samplesStats *stats.QuerySamples noStepSubqueryIntervalFn func(rangeMillis int64) int64 - enableSortByLabel bool } // errorf causes a panic with the input formatted into an error. @@ -1395,8 +1387,6 @@ func (ev *evaluator) eval(expr parser.Expr) (parser.Value, annotations.Annotatio if ok { return ev.rangeEvalTimestampFunctionOverVectorSelector(vs, call, e) } - } else if (e.Func.Name == "sort_by_label" || e.Func.Name == "sort_by_label_desc") && !ev.enableSortByLabel { - ev.error(fmt.Errorf("sort_by_label() and sort_by_label_desc() require the \"promql-sort-by-label\" feature flag to be set")) } // Check if the function has a matrix argument. @@ -1790,7 +1780,6 @@ func (ev *evaluator) eval(expr parser.Expr) (parser.Value, annotations.Annotatio lookbackDelta: ev.lookbackDelta, samplesStats: ev.samplesStats.NewChild(), noStepSubqueryIntervalFn: ev.noStepSubqueryIntervalFn, - enableSortByLabel: ev.enableSortByLabel, } res, ws := newEv.eval(e.Expr) ev.currentSamples = newEv.currentSamples diff --git a/promql/parser/functions.go b/promql/parser/functions.go index 775480a19..ee2e90c55 100644 --- a/promql/parser/functions.go +++ b/promql/parser/functions.go @@ -348,16 +348,18 @@ var Functions = map[string]*Function{ ReturnType: ValueTypeVector, }, "sort_by_label": { - Name: "sort_by_label", - ArgTypes: []ValueType{ValueTypeVector, ValueTypeString}, - Variadic: -1, - ReturnType: ValueTypeVector, + Name: "sort_by_label", + ArgTypes: []ValueType{ValueTypeVector, ValueTypeString}, + Variadic: -1, + ReturnType: ValueTypeVector, + Experimental: true, }, "sort_by_label_desc": { - Name: "sort_by_label_desc", - ArgTypes: []ValueType{ValueTypeVector, ValueTypeString}, - Variadic: -1, - ReturnType: ValueTypeVector, + Name: "sort_by_label_desc", + ArgTypes: []ValueType{ValueTypeVector, ValueTypeString}, + Variadic: -1, + ReturnType: ValueTypeVector, + Experimental: true, }, "sqrt": { Name: "sqrt", diff --git a/promql/promql_test.go b/promql/promql_test.go index 82a9235de..05821b1c1 100644 --- a/promql/promql_test.go +++ b/promql/promql_test.go @@ -35,7 +35,6 @@ func newTestEngine() *Engine { EnableAtModifier: true, EnableNegativeOffset: true, EnablePerStepStats: true, - EnableSortByLabel: true, }) } diff --git a/promql/test.go b/promql/test.go index 7274a8f0d..589b1e5b6 100644 --- a/promql/test.go +++ b/promql/test.go @@ -73,6 +73,9 @@ func LoadedStorage(t testutil.T, input string) *teststorage.TestStorage { // RunBuiltinTests runs an acceptance test suite against the provided engine. func RunBuiltinTests(t *testing.T, engine engineQuerier) { + t.Cleanup(func() { parser.EnableExperimentalFunctions = false }) + parser.EnableExperimentalFunctions = true + files, err := fs.Glob(testsFs, "*/*.test") require.NoError(t, err)