sort_by_label: Switch to feature flag

Signed-off-by: Julien Pivotto <roidelapluie@o11y.eu>

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)

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.

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.
 

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

promql/parser/functions.go

@@ -348,16 +348,18 @@ var Functions = map[string]*Function{
 		ReturnType: ValueTypeVector,
 	},
 	"sort_by_label": {
-		Name:       "sort_by_label",
+		Name:         "sort_by_label",
-		ArgTypes:   []ValueType{ValueTypeVector, ValueTypeString},
+		ArgTypes:     []ValueType{ValueTypeVector, ValueTypeString},
-		Variadic:   -1,
+		Variadic:     -1,
-		ReturnType: ValueTypeVector,
+		ReturnType:   ValueTypeVector,
+		Experimental: true,
 	},
 	"sort_by_label_desc": {
-		Name:       "sort_by_label_desc",
+		Name:         "sort_by_label_desc",
-		ArgTypes:   []ValueType{ValueTypeVector, ValueTypeString},
+		ArgTypes:     []ValueType{ValueTypeVector, ValueTypeString},
-		Variadic:   -1,
+		Variadic:     -1,
-		ReturnType: ValueTypeVector,
+		ReturnType:   ValueTypeVector,
+		Experimental: true,
 	},
 	"sqrt": {
 		Name:       "sqrt",

promql/promql_test.go

@@ -35,7 +35,6 @@ func newTestEngine() *Engine {
 		EnableAtModifier:         true,
 		EnableNegativeOffset:     true,
 		EnablePerStepStats:       true,
-		EnableSortByLabel:        true,
 	})
 }
 

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)