Merge pull request #12516 from vinted/convert_queryopts_to_interface

promql: convert QueryOpts to interface

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) {
+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.
 	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

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{
+			opts := NewPrometheusQueryOpts(false, c.queryLookback)
-				LookbackDelta: c.queryLookback,
-			}
 			qry, err := eng.NewInstantQuery(test.context, test.Queryable(), opts, query, c.ts)
 			require.NoError(t, err)
 

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)
+	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)
+	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) {
+func extractQueryOpts(r *http.Request) (promql.QueryOpts, error) {
-	opts := &promql.QueryOpts{
+	var duration time.Duration
-		EnablePerStepStats: r.FormValue("stats") == "all",
+
-	}
 	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) {

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{
+			expect: promql.NewPrometheusQueryOpts(true, 0),
-				EnablePerStepStats: true,
+
-			},
 			err: nil,
 		},
 		{
@@ -3645,10 +3644,8 @@ func TestExtractQueryOpts(t *testing.T) {
 			form: url.Values{
 				"stats": []string{"none"},
 			},
-			expect: &promql.QueryOpts{
+			expect: promql.NewPrometheusQueryOpts(false, 0),
-				EnablePerStepStats: false,
+			err:    nil,
-			},
-			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{
+			expect: promql.NewPrometheusQueryOpts(true, 30*time.Second),
-				EnablePerStepStats: true,
+			err:    nil,
-				LookbackDelta:      30 * time.Second,
-			},
-			err: nil,
 		},
 		{
 			name: "with invalid lookback delta",