Repository: prometheus/prometheus
Pull request #12516 (2023-07-03 12:57:44)

promql: convert QueryOpts to interface
Convert QueryOpts to an interface so that downstream projects like https://github.com/thanos-community/promql-engine could extend the query options with engine specific options that are not in the original engine.

Will be used to enable query analysis per-query.



Patch:
diff --git a/promql/engine.go b/promql/engine.go
index f29db3a64..83bbdeff8 100644
--- a/promql/engine.go
+++ b/promql/engine.go
@@ -130,11 +130,35 @@ type Query interface {
 	String() string
 }
 
-type QueryOpts struct {
+type PrometheusQueryOpts struct {
 	// Enables recording per-step statistics if the engine has it enabled as well. Disabled by default.
-	EnablePerStepStats bool
+	enablePerStepStats bool
 	// Lookback delta duration for this query.
-	LookbackDelta time.Duration
+	lookbackDelta time.Duration
+}
+
+var _ QueryOpts = &PrometheusQueryOpts{}
+
+func NewPrometheusQueryOpts(enablePerStepStats bool, lookbackDelta time.Duration) QueryOpts {
+	return &PrometheusQueryOpts{
+		enablePerStepStats: enablePerStepStats,
+		lookbackDelta:      lookbackDelta,
+	}
+}
+
+func (p *PrometheusQueryOpts) EnablePerStepStats() bool {
+	return p.enablePerStepStats
+}
+
+func (p *PrometheusQueryOpts) LookbackDelta() time.Duration {
+	return p.lookbackDelta
+}
+
+type QueryOpts interface {
+	// Enables recording per-step statistics if the engine has it enabled as well. Disabled by default.
+	EnablePerStepStats() bool
+	// Lookback delta duration for this query.
+	LookbackDelta() time.Duration
 }
 
 // query implements the Query interface.
@@ -408,7 +432,7 @@ func (ng *Engine) SetQueryLogger(l QueryLogger) {
 }
 
 // NewInstantQuery returns an evaluation query for the given expression at the given time.
-func (ng *Engine) NewInstantQuery(ctx context.Context, q storage.Queryable, opts *QueryOpts, qs string, ts time.Time) (Query, error) {
+func (ng *Engine) NewInstantQuery(ctx context.Context, q storage.Queryable, opts QueryOpts, qs string, ts time.Time) (Query, error) {
 	pExpr, qry := ng.newQuery(q, qs, opts, ts, ts, 0)
 	finishQueue, err := ng.queueActive(ctx, qry)
 	if err != nil {
@@ -429,7 +453,7 @@ func (ng *Engine) NewInstantQuery(ctx context.Context, q storage.Queryable, opts
 
 // NewRangeQuery returns an evaluation query for the given time range and with
 // the resolution set by the interval.
-func (ng *Engine) NewRangeQuery(ctx context.Context, q storage.Queryable, opts *QueryOpts, qs string, start, end time.Time, interval time.Duration) (Query, error) {
+func (ng *Engine) NewRangeQuery(ctx context.Context, q storage.Queryable, opts QueryOpts, qs string, start, end time.Time, interval time.Duration) (Query, error) {
 	pExpr, qry := ng.newQuery(q, qs, opts, start, end, interval)
 	finishQueue, err := ng.queueActive(ctx, qry)
 	if err != nil {
@@ -451,13 +475,12 @@ func (ng *Engine) NewRangeQuery(ctx context.Context, q storage.Queryable, opts *
 	return qry, nil
 }
 
-func (ng *Engine) newQuery(q storage.Queryable, qs string, opts *QueryOpts, start, end time.Time, interval time.Duration) (*parser.Expr, *query) {
-	// Default to empty QueryOpts if not provided.
+func (ng *Engine) newQuery(q storage.Queryable, qs string, opts QueryOpts, start, end time.Time, interval time.Duration) (*parser.Expr, *query) {
 	if opts == nil {
-		opts = &QueryOpts{}
+		opts = NewPrometheusQueryOpts(false, 0)
 	}
 
-	lookbackDelta := opts.LookbackDelta
+	lookbackDelta := opts.LookbackDelta()
 	if lookbackDelta <= 0 {
 		lookbackDelta = ng.lookbackDelta
 	}
@@ -473,7 +496,7 @@ func (ng *Engine) newQuery(q storage.Queryable, qs string, opts *QueryOpts, star
 		stmt:        es,
 		ng:          ng,
 		stats:       stats.NewQueryTimers(),
-		sampleStats: stats.NewQuerySamples(ng.enablePerStepStats && opts.EnablePerStepStats),
+		sampleStats: stats.NewQuerySamples(ng.enablePerStepStats && opts.EnablePerStepStats()),
 		queryable:   q,
 	}
 	return &es.Expr, qry
diff --git a/web/api/v1/api.go b/web/api/v1/api.go
index ff339b09a..43ceaa6be 100644
--- a/web/api/v1/api.go
+++ b/web/api/v1/api.go
@@ -178,8 +178,13 @@ type TSDBAdminStats interface {
 // QueryEngine defines the interface for the *promql.Engine, so it can be replaced, wrapped or mocked.
 type QueryEngine interface {
 	SetQueryLogger(l promql.QueryLogger)
-	NewInstantQuery(ctx context.Context, q storage.Queryable, opts *promql.QueryOpts, qs string, ts time.Time) (promql.Query, error)
-	NewRangeQuery(ctx context.Context, q storage.Queryable, opts *promql.QueryOpts, qs string, start, end time.Time, interval time.Duration) (promql.Query, error)
+	NewInstantQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, ts time.Time) (promql.Query, error)
+	NewRangeQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, start, end time.Time, interval time.Duration) (promql.Query, error)
+}
+
+type QueryOpts interface {
+	EnablePerStepStats() bool
+	LookbackDelta() time.Duration
 }
 
 // API can register a set of endpoints in a router and handle
@@ -462,18 +467,18 @@ func (api *API) formatQuery(r *http.Request) (result apiFuncResult) {
 	return apiFuncResult{expr.Pretty(0), nil, nil, nil}
 }
 
-func extractQueryOpts(r *http.Request) (*promql.QueryOpts, error) {
-	opts := &promql.QueryOpts{
-		EnablePerStepStats: r.FormValue("stats") == "all",
-	}
+func extractQueryOpts(r *http.Request) (promql.QueryOpts, error) {
+	var duration time.Duration
+
 	if strDuration := r.FormValue("lookback_delta"); strDuration != "" {
-		duration, err := parseDuration(strDuration)
+		parsedDuration, err := parseDuration(strDuration)
 		if err != nil {
 			return nil, fmt.Errorf("error parsing lookback delta duration: %w", err)
 		}
-		opts.LookbackDelta = duration
+		duration = parsedDuration
 	}
-	return opts, nil
+
+	return promql.NewPrometheusQueryOpts(r.FormValue("stats") == "all", duration), nil
 }
 
 func (api *API) queryRange(r *http.Request) (result apiFuncResult) {


Test patch:
diff --git a/promql/engine_test.go b/promql/engine_test.go
index 72cbf9153..5ffebc202 100644
--- a/promql/engine_test.go
+++ b/promql/engine_test.go
@@ -1198,7 +1198,7 @@ load 10s
 	origMaxSamples := engine.maxSamplesPerQuery
 	for _, c := range cases {
 		t.Run(c.Query, func(t *testing.T) {
-			opts := &QueryOpts{EnablePerStepStats: true}
+			opts := NewPrometheusQueryOpts(true, 0)
 			engine.maxSamplesPerQuery = origMaxSamples
 
 			runQuery := func(expErr error) *stats.Statistics {
@@ -4626,9 +4626,7 @@ metric 0 1 2
 			if c.engineLookback != 0 {
 				eng.lookbackDelta = c.engineLookback
 			}
-			opts := &QueryOpts{
-				LookbackDelta: c.queryLookback,
-			}
+			opts := NewPrometheusQueryOpts(false, c.queryLookback)
 			qry, err := eng.NewInstantQuery(test.context, test.Queryable(), opts, query, c.ts)
 			require.NoError(t, err)
 
diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go
index 16e74071c..3aa10ee44 100644
--- a/web/api/v1/api_test.go
+++ b/web/api/v1/api_test.go
@@ -3627,7 +3627,7 @@ func TestExtractQueryOpts(t *testing.T) {
 	tests := []struct {
 		name   string
 		form   url.Values
-		expect *promql.QueryOpts
+		expect promql.QueryOpts
 		err    error
 	}{
 		{
@@ -3635,9 +3635,8 @@ func TestExtractQueryOpts(t *testing.T) {
 			form: url.Values{
 				"stats": []string{"all"},
 			},
-			expect: &promql.QueryOpts{
-				EnablePerStepStats: true,
-			},
+			expect: promql.NewPrometheusQueryOpts(true, 0),
+
 			err: nil,
 		},
 		{
@@ -3645,10 +3644,8 @@ func TestExtractQueryOpts(t *testing.T) {
 			form: url.Values{
 				"stats": []string{"none"},
 			},
-			expect: &promql.QueryOpts{
-				EnablePerStepStats: false,
-			},
-			err: nil,
+			expect: promql.NewPrometheusQueryOpts(false, 0),
+			err:    nil,
 		},
 		{
 			name: "with lookback delta",
@@ -3656,11 +3653,8 @@ func TestExtractQueryOpts(t *testing.T) {
 				"stats":          []string{"all"},
 				"lookback_delta": []string{"30s"},
 			},
-			expect: &promql.QueryOpts{
-				EnablePerStepStats: true,
-				LookbackDelta:      30 * time.Second,
-			},
-			err: nil,
+			expect: promql.NewPrometheusQueryOpts(true, 30*time.Second),
+			err:    nil,
 		},
 		{
 			name: "with invalid lookback delta",