Skip to content

Commit abf8236

Browse files
rubywtlrubywtl
committed
add base engine interface and edit unit test
Signed-off-by: rubywtl <[email protected]>
1 parent df1ff95 commit abf8236

File tree

8 files changed

+55
-26
lines changed

8 files changed

+55
-26
lines changed

‎pkg/api/handlers.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func NewQuerierHandler(
163163
cfg Config,
164164
queryable storage.SampleAndChunkQueryable,
165165
exemplarQueryable storage.ExemplarQueryable,
166-
engine engine.Engine,
166+
engine engine.BaseEngine,
167167
metadataQuerier querier.MetadataQuerier,
168168
reg prometheus.Registerer,
169169
logger log.Logger,
@@ -200,7 +200,7 @@ func NewQuerierHandler(
200200
corsOrigin := regexp.MustCompile(".*")
201201
translateSampleAndChunkQueryable := querier.NewErrorTranslateSampleAndChunkQueryable(queryable)
202202
api := v1.NewAPI(
203-
&engine,
203+
engine,
204204
translateSampleAndChunkQueryable, // Translate errors to errors expected by API.
205205
nil, // No remote write support.
206206
exemplarQueryable,

‎pkg/api/handlers_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"strings"
1010
"testing"
1111

12-
"github.com/cortexproject/cortex/pkg/engine"
1312
"github.com/prometheus/common/version"
1413
v1 "github.com/prometheus/prometheus/web/api/v1"
1514
"github.com/stretchr/testify/assert"
@@ -233,7 +232,7 @@ func TestBuildInfoAPI(t *testing.T) {
233232
version.Version = tc.version
234233
version.Branch = tc.branch
235234
version.Revision = tc.revision
236-
handler := NewQuerierHandler(cfg, nil, nil, engine.Engine{}, nil, nil, &FakeLogger{})
235+
handler := NewQuerierHandler(cfg, nil, nil, nil, nil, nil, &FakeLogger{})
237236
writer := httptest.NewRecorder()
238237
req := httptest.NewRequest("GET", "/api/v1/status/buildinfo", nil)
239238
req = req.WithContext(user.InjectOrgID(req.Context(), "test"))

‎pkg/api/queryapi/query_api.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,23 @@ import (
1111
"github.com/go-kit/log/level"
1212
"github.com/grafana/regexp"
1313
"github.com/munnerz/goautoneg"
14+
"github.com/prometheus/prometheus/promql"
1415
"github.com/prometheus/prometheus/storage"
1516
"github.com/prometheus/prometheus/util/annotations"
1617
"github.com/prometheus/prometheus/util/httputil"
1718
v1 "github.com/prometheus/prometheus/web/api/v1"
19+
"github.com/thanos-io/promql-engine/logicalplan"
1820
"github.com/weaveworks/common/httpgrpc"
1921

2022
"github.com/cortexproject/cortex/pkg/engine"
2123
"github.com/cortexproject/cortex/pkg/querier"
2224
"github.com/cortexproject/cortex/pkg/util"
2325
"github.com/cortexproject/cortex/pkg/util/api"
24-
"github.com/prometheus/prometheus/promql"
25-
"github.com/thanos-io/promql-engine/logicalplan"
2626
)
2727

2828
type QueryAPI struct {
2929
queryable storage.SampleAndChunkQueryable
30-
queryEngine engine.Engine
30+
queryEngine engine.BaseEngine
3131
now func() time.Time
3232
statsRenderer v1.StatsRenderer
3333
logger log.Logger
@@ -36,7 +36,7 @@ type QueryAPI struct {
3636
}
3737

3838
func NewQueryAPI(
39-
qe engine.Engine,
39+
qe engine.BaseEngine,
4040
q storage.SampleAndChunkQueryable,
4141
statsRenderer v1.StatsRenderer,
4242
logger log.Logger,

‎pkg/api/queryapi/query_api_test.go

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,23 @@ import (
1414
"github.com/go-kit/log"
1515
"github.com/gorilla/mux"
1616
"github.com/grafana/regexp"
17+
"github.com/prometheus/client_golang/prometheus"
1718
"github.com/prometheus/common/model"
1819
"github.com/prometheus/prometheus/model/labels"
1920
"github.com/prometheus/prometheus/promql"
21+
"github.com/prometheus/prometheus/promql/parser"
2022
"github.com/prometheus/prometheus/storage"
2123
"github.com/prometheus/prometheus/util/annotations"
2224
v1 "github.com/prometheus/prometheus/web/api/v1"
2325
"github.com/stretchr/testify/require"
26+
"github.com/thanos-io/promql-engine/logicalplan"
27+
"github.com/thanos-io/promql-engine/query"
2428
"github.com/weaveworks/common/user"
2529

2630
engine2 "github.com/cortexproject/cortex/pkg/engine"
2731
"github.com/cortexproject/cortex/pkg/querier"
2832
"github.com/cortexproject/cortex/pkg/querier/series"
2933
"github.com/cortexproject/cortex/pkg/querier/stats"
30-
"github.com/prometheus/client_golang/prometheus"
31-
"github.com/prometheus/prometheus/promql/parser"
32-
"github.com/thanos-io/promql-engine/logicalplan"
33-
"github.com/thanos-io/promql-engine/query"
3434
)
3535

3636
type mockSampleAndChunkQueryable struct {
@@ -182,7 +182,7 @@ func Test_CustomAPI(t *testing.T) {
182182

183183
for _, test := range tests {
184184
t.Run(test.name, func(t *testing.T) {
185-
c := NewQueryAPI(*engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
185+
c := NewQueryAPI(engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
186186

187187
router := mux.NewRouter()
188188
router.Path("/api/v1/query").Methods("POST").Handler(c.Wrap(c.InstantQueryHandler))
@@ -243,7 +243,7 @@ func Test_InvalidCodec(t *testing.T) {
243243
},
244244
}
245245

246-
queryAPI := NewQueryAPI(*engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{&mockCodec{}}, regexp.MustCompile(".*"))
246+
queryAPI := NewQueryAPI(engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{&mockCodec{}}, regexp.MustCompile(".*"))
247247
router := mux.NewRouter()
248248
router.Path("/api/v1/query").Methods("POST").Handler(queryAPI.Wrap(queryAPI.InstantQueryHandler))
249249

@@ -284,7 +284,7 @@ func Test_CustomAPI_StatsRenderer(t *testing.T) {
284284
},
285285
}
286286

287-
queryAPI := NewQueryAPI(*engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
287+
queryAPI := NewQueryAPI(engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
288288

289289
router := mux.NewRouter()
290290
router.Path("/api/v1/query_range").Methods("POST").Handler(queryAPI.Wrap(queryAPI.RangeQueryHandler))
@@ -314,7 +314,6 @@ func Test_Logicalplan_Requests(t *testing.T) {
314314
prometheus.NewRegistry(),
315315
)
316316

317-
// TODO: find a way to include two different-metric queryables
318317
mockMatrix := model.Matrix{
319318
{
320319
Metric: model.Metric{"__name__": "test", "foo": "bar"},
@@ -342,8 +341,8 @@ func Test_Logicalplan_Requests(t *testing.T) {
342341
expectedBody string
343342
}{
344343
{
345-
name: "[Range Query] with valid logical plan",
346-
path: "/api/v1/query_range?end=1536673680&query=test&start=1536673665&step=5",
344+
name: "[Range Query] with valid logical plan and empty query string",
345+
path: "/api/v1/query_range?end=1536673680&query=&start=1536673665&step=5",
347346
start: 1536673665,
348347
end: 1536673680,
349348
stepDuration: 5,
@@ -366,7 +365,7 @@ func Test_Logicalplan_Requests(t *testing.T) {
366365
expectedBody: `{"status":"error","errorType":"server_error","error":"invalid character 'r' after top-level value"}`,
367366
},
368367
{
369-
name: "[Range Query] with empty body", // will fall back promql query execution
368+
name: "[Range Query] with empty body and non-empty query string", // fall back to promql query execution
370369
path: "/api/v1/query_range?end=1536673680&query=test&start=1536673665&step=5",
371370
start: 1536673665,
372371
end: 1536673680,
@@ -378,7 +377,19 @@ func Test_Logicalplan_Requests(t *testing.T) {
378377
expectedBody: `{"status":"success","data":{"resultType":"matrix","result":[{"metric":{"__name__":"test","foo":"bar"},"values":[[1536673665,"0"],[1536673670,"1"],[1536673675,"1"],[1536673680,"1"]]}]}}`,
379378
},
380379
{
381-
name: "[Instant Query] with valid logical plan",
380+
name: "[Range Query] with empty body and empty query string", // fall back to promql query execution, but will have error because of empty query string
381+
path: "/api/v1/query_range?end=1536673680&query=&start=1536673665&step=5",
382+
start: 1536673665,
383+
end: 1536673680,
384+
stepDuration: 5,
385+
requestBody: func(t *testing.T) []byte {
386+
return []byte{}
387+
},
388+
expectedCode: http.StatusBadRequest,
389+
expectedBody: "{\"status\":\"error\",\"errorType\":\"bad_data\",\"error\":\"invalid parameter \\\"query\\\"; unknown position: parse error: no expression found in input\"}",
390+
},
391+
{
392+
name: "[Instant Query] with valid logical plan and empty query string",
382393
path: "/api/v1/query?query=test&time=1536673670",
383394
start: 1536673670,
384395
end: 1536673670,
@@ -402,7 +413,7 @@ func Test_Logicalplan_Requests(t *testing.T) {
402413
expectedBody: `{"status":"error","errorType":"server_error","error":"invalid character 'r' after top-level value"}`,
403414
},
404415
{
405-
name: "[Instant Query] with empty body",
416+
name: "[Instant Query] with empty body and non-empty query string",
406417
path: "/api/v1/query?query=test&time=1536673670",
407418
start: 1536673670,
408419
end: 1536673670,
@@ -413,11 +424,23 @@ func Test_Logicalplan_Requests(t *testing.T) {
413424
expectedCode: http.StatusOK,
414425
expectedBody: `{"status":"success","data":{"resultType":"vector","result":[{"metric":{"__name__":"test","foo":"bar"},"value":[1536673670,"1"]}]}}`,
415426
},
427+
{
428+
name: "[Instant Query] with empty body and empty query string",
429+
path: "/api/v1/query?query=&time=1536673670",
430+
start: 1536673670,
431+
end: 1536673670,
432+
stepDuration: 0,
433+
requestBody: func(t *testing.T) []byte {
434+
return []byte{}
435+
},
436+
expectedCode: http.StatusBadRequest,
437+
expectedBody: "{\"status\":\"error\",\"errorType\":\"bad_data\",\"error\":\"invalid parameter \\\"query\\\"; unknown position: parse error: no expression found in input\"}",
438+
},
416439
}
417440

418441
for _, tt := range tests {
419442
t.Run(tt.name, func(t *testing.T) {
420-
c := NewQueryAPI(*engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
443+
c := NewQueryAPI(engine, mockQueryable, querier.StatsRenderer, log.NewNopLogger(), []v1.Codec{v1.JSONCodec{}}, regexp.MustCompile(".*"))
421444
router := mux.NewRouter()
422445
router.Path("/api/v1/query").Methods("POST").Handler(c.Wrap(c.InstantQueryHandler))
423446
router.Path("/api/v1/query_range").Methods("POST").Handler(c.Wrap(c.RangeQueryHandler))

‎pkg/cortex/cortex.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ type Cortex struct {
322322
QuerierQueryable prom_storage.SampleAndChunkQueryable
323323
ExemplarQueryable prom_storage.ExemplarQueryable
324324
MetadataQuerier querier.MetadataQuerier
325-
QuerierEngine engine.Engine
325+
QuerierEngine engine.BaseEngine
326326
QueryFrontendTripperware tripperware.Tripperware
327327
ResourceMonitor *resource.Monitor
328328

‎pkg/cortex/modules.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -666,7 +666,7 @@ func (t *Cortex) initRuler() (serv services.Service, err error) {
666666
// TODO: Consider wrapping logger to differentiate from querier module logger
667667
queryable, _, engine := querier.New(t.Cfg.Querier, t.Overrides, t.Distributor, t.StoreQueryables, rulerRegisterer, util_log.Logger, t.Overrides.RulesPartialData)
668668

669-
managerFactory := ruler.DefaultTenantManagerFactory(t.Cfg.Ruler, t.Distributor, queryable, &engine, t.Overrides, metrics, prometheus.DefaultRegisterer)
669+
managerFactory := ruler.DefaultTenantManagerFactory(t.Cfg.Ruler, t.Distributor, queryable, engine, t.Overrides, metrics, prometheus.DefaultRegisterer)
670670
manager, err = ruler.NewDefaultMultiTenantManager(t.Cfg.Ruler, t.Overrides, managerFactory, metrics, prometheus.DefaultRegisterer, util_log.Logger)
671671
}
672672

‎pkg/engine/engine.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ func GetEngineType(ctx context.Context) Type {
4444
return None
4545
}
4646

47+
type BaseEngine interface {
48+
NewInstantQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, ts time.Time) (promql.Query, error)
49+
NewRangeQuery(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, qs string, start, end time.Time, interval time.Duration) (promql.Query, error)
50+
MakeInstantQueryFromPlan(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, root logicalplan.Node, ts time.Time, qs string) (promql.Query, error)
51+
MakeRangeQueryFromPlan(ctx context.Context, q storage.Queryable, opts promql.QueryOpts, root logicalplan.Node, start time.Time, end time.Time, interval time.Duration, qs string) (promql.Query, error)
52+
}
53+
4754
type Engine struct {
4855
prometheusEngine *promql.Engine
4956
thanosEngine *thanosengine.Engine

‎pkg/querier/querier.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func getChunksIteratorFunction(_ Config) chunkIteratorFunc {
197197
}
198198

199199
// New builds a queryable and promql engine.
200-
func New(cfg Config, limits *validation.Overrides, distributor Distributor, stores []QueryableWithFilter, reg prometheus.Registerer, logger log.Logger, isPartialDataEnabled partialdata.IsCfgEnabledFunc) (storage.SampleAndChunkQueryable, storage.ExemplarQueryable, engine.Engine) {
200+
func New(cfg Config, limits *validation.Overrides, distributor Distributor, stores []QueryableWithFilter, reg prometheus.Registerer, logger log.Logger, isPartialDataEnabled partialdata.IsCfgEnabledFunc) (storage.SampleAndChunkQueryable, storage.ExemplarQueryable, engine.BaseEngine) {
201201
iteratorFunc := getChunksIteratorFunction(cfg)
202202

203203
distributorQueryable := newDistributorQueryable(distributor, cfg.IngesterMetadataStreaming, cfg.IngesterLabelNamesWithMatchers, iteratorFunc, cfg.QueryIngestersWithin, isPartialDataEnabled, cfg.IngesterQueryMaxAttempts)
@@ -247,7 +247,7 @@ func New(cfg Config, limits *validation.Overrides, distributor Distributor, stor
247247
},
248248
}
249249
queryEngine := engine.New(opts, cfg.ThanosEngine, reg)
250-
return NewSampleAndChunkQueryable(lazyQueryable), exemplarQueryable, *queryEngine
250+
return NewSampleAndChunkQueryable(lazyQueryable), exemplarQueryable, queryEngine
251251
}
252252

253253
// NewSampleAndChunkQueryable creates a SampleAndChunkQueryable from a

0 commit comments

Comments
 (0)