Got rid of 2 fields in readHandler.

m3db · Nov 24, 2020 · 7db2913 · 7db2913
1 parent 2314219
commit 7db2913
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 54 deletions.
diff --git a/src/query/api/v1/handler/prom/prom.go b/src/query/api/v1/handler/prom/prom.go
@@ -24,6 +24,8 @@ import (
 	"net/http"
 	"time"
 
+	promstorage "github.com/prometheus/prometheus/storage"
+
 	"github.com/m3db/m3/src/query/api/v1/options"
 	"github.com/m3db/m3/src/query/block"
 	"github.com/m3db/m3/src/query/graphite/errors"
@@ -43,6 +45,7 @@ func init() {
 type opts struct {
 	promQLEngine *promql.Engine
 	instant      bool
+	queryable    promstorage.Queryable
 	newQueryFn   NewQueryFn
 }
 
@@ -66,31 +69,37 @@ func withEngine(promQLEngine *promql.Engine, instant bool) Option {
 		}
 		o.instant = instant
 		o.promQLEngine = promQLEngine
-		o.newQueryFn = newRangeQueryFn
+		o.newQueryFn = newRangeQueryFn(promQLEngine, o.queryable)
 		if instant {
-			o.newQueryFn = newInstantQueryFn
+			o.newQueryFn = newInstantQueryFn(promQLEngine, o.queryable)
 		}
 		return nil
 	}
 }
 
 func newDefaultOptions(hOpts options.HandlerOptions) opts {
+	queryable := prometheus.NewPrometheusQueryable(
+		prometheus.PrometheusOptions{
+			Storage:           hOpts.Storage(),
+			InstrumentOptions: hOpts.InstrumentOpts(),
+		})
 	return opts{
 		promQLEngine: hOpts.PrometheusEngine(),
+		queryable:    queryable,
 		instant:      false,
-		newQueryFn:   newRangeQueryFn,
+		newQueryFn:   newRangeQueryFn(hOpts.PrometheusEngine(), queryable),
 	}
 }
 
 // NewReadHandler creates a handler to handle PromQL requests.
 func NewReadHandler(hOpts options.HandlerOptions, options ...Option) (http.Handler, error) {
-	queryable := prometheus.NewPrometheusQueryable(
-		prometheus.PrometheusOptions{
-			Storage:           hOpts.Storage(),
-			InstrumentOptions: hOpts.InstrumentOpts(),
-		})
-
-	return newReadHandler(hOpts, queryable, options...)
+	opts := newDefaultOptions(hOpts)
+	for _, optionFn := range options {
+		if err := optionFn(&opts); err != nil {
+			return nil, err
+		}
+	}
+	return newReadHandler(hOpts, opts)
 }
 
 // ApplyRangeWarnings applies warnings encountered during execution.

diff --git a/src/query/api/v1/handler/prom/read.go b/src/query/api/v1/handler/prom/read.go
@@ -42,69 +42,55 @@ import (
 )
 
 // NewQueryFn creates a new promql Query.
-type NewQueryFn func(
-	engine *promql.Engine,
-	queryable promstorage.Queryable,
-	params models.RequestParams,
-) (promql.Query, error)
+type NewQueryFn func(params models.RequestParams) (promql.Query, error)
 
 var (
 	newRangeQueryFn = func(
 		engine *promql.Engine,
 		queryable promstorage.Queryable,
-		params models.RequestParams,
-	) (promql.Query, error) {
-		return engine.NewRangeQuery(
-			queryable,
-			params.Query,
-			params.Start,
-			params.End,
-			params.Step)
+	) NewQueryFn {
+		return func(params models.RequestParams) (promql.Query, error) {
+			return engine.NewRangeQuery(
+				queryable,
+				params.Query,
+				params.Start,
+				params.End,
+				params.Step)
+		}
 	}
 
 	newInstantQueryFn = func(
 		engine *promql.Engine,
 		queryable promstorage.Queryable,
-		params models.RequestParams,
-	) (promql.Query, error) {
-		return engine.NewInstantQuery(
-			queryable,
-			params.Query,
-			params.Now)
+	) NewQueryFn {
+		return func(params models.RequestParams) (promql.Query, error) {
+			return engine.NewInstantQuery(
+				queryable,
+				params.Query,
+				params.Now)
+		}
 	}
 )
 
 type readHandler struct {
-	engine     *promql.Engine
-	queryable  promstorage.Queryable
-	hOpts      options.HandlerOptions
-	scope      tally.Scope
-	logger     *zap.Logger
-	opts       opts
+	hOpts  options.HandlerOptions
+	scope  tally.Scope
+	logger *zap.Logger
+	opts   opts
 }
 
 func newReadHandler(
 	hOpts options.HandlerOptions,
-	queryable promstorage.Queryable,
-	options ...Option,
+	options opts,
 ) (http.Handler, error) {
 	scope := hOpts.InstrumentOpts().MetricsScope().Tagged(
 		map[string]string{"handler": "prometheus-read"},
 	)
-	opts := newDefaultOptions(hOpts)
-	for _, optionFn := range options {
-		if err := optionFn(&opts); err != nil {
-			return nil, err
-		}
-	}
-
 	return &readHandler{
-		engine:     opts.promQLEngine,
-		queryable:  queryable,
-		hOpts:      hOpts,
-		opts:       opts,
-		scope:      scope,
-		logger:     hOpts.InstrumentOpts().Logger(),
+		hOpts:  hOpts,
+		opts:   options,
+		scope:  scope,
+		logger: hOpts.InstrumentOpts().Logger(),
 	}, nil
 }
 
@@ -138,7 +124,7 @@ func (h *readHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		defer cancel()
 	}
 
-	qry, err := h.opts.newQueryFn(h.engine, h.queryable, params)
+	qry, err := h.opts.newQueryFn(params)
 	if err != nil {
 		h.logger.Error("error creating query",
 			zap.Error(err), zap.String("query", params.Query),

diff --git a/src/query/api/v1/handler/prom/read_test.go b/src/query/api/v1/handler/prom/read_test.go
@@ -72,10 +72,19 @@ func setupTest(t *testing.T) testHandlers {
 		SetFetchOptionsBuilder(fetchOptsBuilder).
 		SetEngine(engine)
 	queryable := &mockQueryable{}
-	readHandler, err := newReadHandler(hOpts, queryable, WithEngine(testPromQLEngine))
+	readHandler, err := newReadHandler(hOpts, opts{
+		promQLEngine: testPromQLEngine,
+		queryable:    queryable,
+		instant:      false,
+		newQueryFn:   newRangeQueryFn(testPromQLEngine, queryable),
+	})
 	require.NoError(t, err)
-	readInstantHandler, err := newReadHandler(hOpts,
-		queryable, WithInstantEngine(testPromQLEngine))
+	readInstantHandler, err := newReadHandler(hOpts, opts{
+		promQLEngine: testPromQLEngine,
+		queryable:    queryable,
+		instant:      true,
+		newQueryFn:   newInstantQueryFn(testPromQLEngine, queryable),
+	})
 	require.NoError(t, err)
 	return testHandlers{
 		queryable:          queryable,