From 3f230fc9f894d76cccc2dc75ea11909004c28c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Date: Mon, 3 Jul 2023 15:56:06 +0300 Subject: [PATCH] promql: convert QueryOpts to interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Giedrius Statkevičius --- promql/engine.go | 43 ++++++++++++++++++++++++++++++++---------- promql/engine_test.go | 6 ++---- web/api/v1/api.go | 23 +++++++++++++--------- web/api/v1/api_test.go | 20 +++++++------------- 4 files changed, 56 insertions(+), 36 deletions(-) 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/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.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) { 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",