Address comments

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>

Author: harry671003

docs/blocks-storage/querier.md

@@ -257,7 +257,7 @@ querier:
     # https://github.com/thanos-io/promql-engine rather than the Prometheus
     # promql engine.
     # CLI flag: -querier.thanos-engine
-    [enable_thanos_engine: <boolean> | default = false]
+    [enabled: <boolean> | default = false]
 
     # Enable xincrease, xdelta, xrate etc from Thanos engine.
     # CLI flag: -querier.enable-x-functions

docs/configuration/config-file-reference.md

@@ -4307,7 +4307,7 @@ thanos_engine:
   # https://github.com/thanos-io/promql-engine rather than the Prometheus promql
   # engine.
   # CLI flag: -querier.thanos-engine
-  [enable_thanos_engine: <boolean> | default = false]
+  [enabled: <boolean> | default = false]
 
   # Enable xincrease, xdelta, xrate etc from Thanos engine.
   # CLI flag: -querier.enable-x-functions
@@ -5034,6 +5034,23 @@ ring:
 # ruler.enable-ha-evaluation is true.
 # CLI flag: -ruler.liveness-check-timeout
 [liveness_check_timeout: <duration> | default = 1s]
+
+thanos_engine:
+  # Experimental. Use Thanos promql engine
+  # https://github.com/thanos-io/promql-engine rather than the Prometheus promql
+  # engine.
+  # CLI flag: -ruler.thanos-engine
+  [enabled: <boolean> | default = false]
+
+  # Enable xincrease, xdelta, xrate etc from Thanos engine.
+  # CLI flag: -ruler.enable-x-functions
+  [enable_x_functions: <boolean> | default = false]
+
+  # Logical plan optimizers. Multiple optimizers can be provided as a
+  # comma-separated list. Supported values: default, all, propagate-matchers,
+  # sort-matchers, merge-selects, detect-histogram-stats
+  # CLI flag: -ruler.optimizers
+  [optimizers: <string> | default = "default"]
 ```
 
 ### `ruler_storage_config`

pkg/configs/userconfig/config.go

@@ -9,10 +9,10 @@ import (
 	"github.com/pkg/errors"
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/model/rulefmt"
-	"github.com/prometheus/prometheus/promql/parser"
 	"github.com/prometheus/prometheus/rules"
 	"gopkg.in/yaml.v3"
 
+	"github.com/cortexproject/cortex/pkg/parser"
 	util_log "github.com/cortexproject/cortex/pkg/util/log"
 )
 

pkg/configs/userconfig/config_test.go

@@ -12,12 +12,12 @@ import (
 	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/model/labels"
 	"github.com/prometheus/prometheus/model/rulefmt"
-	"github.com/prometheus/prometheus/promql/parser"
 	"github.com/prometheus/prometheus/rules"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/yaml.v3"
 
+	"github.com/cortexproject/cortex/pkg/parser"
 	util_log "github.com/cortexproject/cortex/pkg/util/log"
 )
 

pkg/cortex/cortex.go

@@ -379,6 +379,7 @@ func New(cfg Config) (*Cortex, error) {
 		return nil, err
 	}
 
+	cortex.setupPromQLFunctions()
 	return cortex, nil
 }
 
@@ -537,3 +538,9 @@ func (t *Cortex) readyHandler(sm *services.Manager) http.HandlerFunc {
 		util.WriteTextResponse(w, "ready")
 	}
 }
+
+func (t *Cortex) setupPromQLFunctions() {
+	// The holt_winters function is renamed to double_exponential_smoothing and has been experimental since Prometheus v3. (https://github.com/prometheus/prometheus/pull/14930)
+	// The cortex supports holt_winters for users using this function.
+	querier.EnableExperimentalPromQLFunctions(t.Cfg.Querier.EnablePromQLExperimentalFunctions, true)
+}

pkg/cortex/modules.go

@@ -564,7 +564,6 @@ func (t *Cortex) initQueryFrontendTripperware() (serv services.Service, err erro
 		t.Cfg.Querier.DefaultEvaluationInterval,
 		t.Cfg.Querier.MaxSubQuerySteps,
 		t.Cfg.Querier.LookbackDelta,
-		t.Cfg.Querier.EnablePromQLExperimentalFunctions,
 	)
 
 	return services.NewIdleService(nil, func(_ error) error {
@@ -657,7 +656,7 @@ func (t *Cortex) initRuler() (serv services.Service, err error) {
 				return t.Cfg.Querier.DefaultEvaluationInterval.Milliseconds()
 			},
 		}
-		queryEngine := engine.New(opts, t.Cfg.Querier.Engine, rulerRegisterer)
+		queryEngine := engine.New(opts, t.Cfg.Ruler.ThanosEngine, rulerRegisterer)
 
 		managerFactory := ruler.DefaultTenantManagerFactory(t.Cfg.Ruler, t.Cfg.ExternalPusher, t.Cfg.ExternalQueryable, queryEngine, t.Overrides, metrics, prometheus.DefaultRegisterer)
 		manager, err = ruler.NewDefaultMultiTenantManager(t.Cfg.Ruler, t.Overrides, managerFactory, metrics, prometheus.DefaultRegisterer, util_log.Logger)

pkg/engine/config.go

@@ -10,22 +10,21 @@ import (
 
 var supportedOptimizers = []string{"default", "all", "propagate-matchers", "sort-matchers", "merge-selects", "detect-histogram-stats"}
 
-// Config contains the configuration to create engine
-type Config struct {
-	EnableThanosEngine bool                    `yaml:"enable_thanos_engine"`
-	EnableXFunctions   bool                    `yaml:"enable_x_functions"`
-	Optimizers         string                  `yaml:"optimizers"`
-	LogicalOptimizers  []logicalplan.Optimizer `yaml:"-"`
+// ThanosEngineConfig contains the configuration to create engine
+type ThanosEngineConfig struct {
+	Enabled           bool                    `yaml:"enabled"`
+	EnableXFunctions  bool                    `yaml:"enable_x_functions"`
+	Optimizers        string                  `yaml:"optimizers"`
+	LogicalOptimizers []logicalplan.Optimizer `yaml:"-"`
 }
 
-// RegisterFlags adds the flags required to config this to the given FlagSet.
-func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
-	f.BoolVar(&cfg.EnableThanosEngine, "querier.thanos-engine", false, "Experimental. Use Thanos promql engine https://github.com/thanos-io/promql-engine rather than the Prometheus promql engine.")
-	f.BoolVar(&cfg.EnableXFunctions, "querier.enable-x-functions", false, "Enable xincrease, xdelta, xrate etc from Thanos engine.")
-	f.StringVar(&cfg.Optimizers, "querier.optimizers", "default", "Logical plan optimizers. Multiple optimizers can be provided as a comma-separated list. Supported values: "+strings.Join(supportedOptimizers, ", "))
+func (cfg *ThanosEngineConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
+	f.BoolVar(&cfg.Enabled, prefix+"thanos-engine", false, "Experimental. Use Thanos promql engine https://github.com/thanos-io/promql-engine rather than the Prometheus promql engine.")
+	f.BoolVar(&cfg.EnableXFunctions, prefix+"enable-x-functions", false, "Enable xincrease, xdelta, xrate etc from Thanos engine.")
+	f.StringVar(&cfg.Optimizers, prefix+"optimizers", "default", "Logical plan optimizers. Multiple optimizers can be provided as a comma-separated list. Supported values: "+strings.Join(supportedOptimizers, ", "))
 }
 
-func (cfg *Config) Validate() error {
+func (cfg *ThanosEngineConfig) Validate() error {
 	splitOptimizers := strings.Split(cfg.Optimizers, ",")
 
 	for _, optimizer := range splitOptimizers {

pkg/engine/engine.go

@@ -51,16 +51,16 @@ type Engine struct {
 	engineSwitchQueriesTotal *prometheus.CounterVec
 }
 
-func New(opts promql.EngineOpts, engineCfg Config, reg prometheus.Registerer) *Engine {
+func New(opts promql.EngineOpts, thanosEngineCfg ThanosEngineConfig, reg prometheus.Registerer) *Engine {
 	prometheusEngine := promql.NewEngine(opts)
 
 	var thanosEngine *thanosengine.Engine
-	if engineCfg.EnableThanosEngine {
+	if thanosEngineCfg.Enabled {
 		thanosEngine = thanosengine.New(thanosengine.Opts{
 			EngineOpts:        opts,
-			LogicalOptimizers: engineCfg.LogicalOptimizers,
+			LogicalOptimizers: thanosEngineCfg.LogicalOptimizers,
 			EnableAnalysis:    true,
-			EnableXFunctions:  engineCfg.EnableXFunctions,
+			EnableXFunctions:  thanosEngineCfg.EnableXFunctions,
 		})
 	}
 

pkg/engine/engine_test.go

@@ -39,7 +39,7 @@ func TestEngine_Fallback(t *testing.T) {
 		Logger: utillog.GoKitLogToSlog(log.NewNopLogger()),
 		Reg:    reg,
 	}
-	queryEngine := New(opts, Config{EnableThanosEngine: true}, reg)
+	queryEngine := New(opts, ThanosEngineConfig{Enabled: true}, reg)
 
 	// instant query, should go to fallback
 	_, _ = queryEngine.NewInstantQuery(ctx, queryable, nil, "unimplemented(foo)", now)
@@ -70,7 +70,7 @@ func TestEngine_Switch(t *testing.T) {
 		Logger: utillog.GoKitLogToSlog(log.NewNopLogger()),
 		Reg:    reg,
 	}
-	queryEngine := New(opts, Config{EnableThanosEngine: true}, reg)
+	queryEngine := New(opts, ThanosEngineConfig{Enabled: true}, reg)
 
 	// Query Prometheus engine
 	r := &http.Request{Header: http.Header{}}
@@ -111,7 +111,7 @@ func TestEngine_XFunctions(t *testing.T) {
 		Logger: utillog.GoKitLogToSlog(log.NewNopLogger()),
 		Reg:    reg,
 	}
-	queryEngine := New(opts, Config{EnableThanosEngine: true, EnableXFunctions: true}, reg)
+	queryEngine := New(opts, ThanosEngineConfig{Enabled: true, EnableXFunctions: true}, reg)
 
 	for name := range parse.XFunctions {
 		t.Run(name, func(t *testing.T) {

pkg/parser/parser.go

@@ -0,0 +1,21 @@
+package parser
+
+import (
+	"maps"
+
+	promqlparser "github.com/prometheus/prometheus/promql/parser"
+	"github.com/thanos-io/promql-engine/execution/parse"
+)
+
+var functions = buildFunctions()
+
+func buildFunctions() map[string]*promqlparser.Function {
+	fns := make(map[string]*promqlparser.Function, len(promqlparser.Functions))
+	maps.Copy(fns, promqlparser.Functions)
+	maps.Copy(fns, parse.XFunctions)
+	return fns
+}
+
+func ParseExpr(qs string) (promqlparser.Expr, error) {
+	return promqlparser.NewParser(qs, promqlparser.WithFunctions(functions)).ParseExpr()
+}