diff --git a/pkg/querier/tripperware/priority.go b/pkg/querier/tripperware/priority.go index 4a8d3ae07f..a87a31888b 100644 --- a/pkg/querier/tripperware/priority.go +++ b/pkg/querier/tripperware/priority.go @@ -1,6 +1,7 @@ package tripperware import ( + "errors" "net/http" "strings" "time" @@ -11,6 +12,10 @@ import ( "github.com/cortexproject/cortex/pkg/util/validation" ) +var ( + errParseExpr = errors.New("failed to parse expr") +) + func GetPriority(r *http.Request, userID string, limits Limits, now time.Time, lookbackDelta time.Duration) (int64, error) { isQuery := strings.HasSuffix(r.URL.Path, "/query") isQueryRange := strings.HasSuffix(r.URL.Path, "/query_range") @@ -23,7 +28,9 @@ func GetPriority(r *http.Request, userID string, limits Limits, now time.Time, l expr, err := parser.ParseExpr(query) if err != nil { - return 0, err + // If query fails to be parsed, we throw a simple parse error + // and fail query later on querier. + return 0, errParseExpr } if len(queryPriority.Priorities) == 0 { diff --git a/pkg/querier/tripperware/priority_test.go b/pkg/querier/tripperware/priority_test.go index e95d9170d9..d6913e06b5 100644 --- a/pkg/querier/tripperware/priority_test.go +++ b/pkg/querier/tripperware/priority_test.go @@ -14,7 +14,7 @@ import ( "github.com/cortexproject/cortex/pkg/util/validation" ) -func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrEmptyQueryString(t *testing.T) { +func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrInvalidQueryString(t *testing.T) { now := time.Now() limits := mockLimits{queryPriority: validation.QueryPriority{ Priorities: []validation.PriorityDef{ @@ -33,6 +33,7 @@ func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrEmptyQueryString(t type testCase struct { url string queryPriorityEnabled bool + err error } tests := map[string]testCase{ @@ -43,10 +44,20 @@ func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrEmptyQueryString(t url: "/query?query=", queryPriorityEnabled: true, }, + "shouldn't return error if query is invalid": { + url: "/query?query=up[4h", + queryPriorityEnabled: true, + err: errParseExpr, + }, "should miss if query string empty - range query": { url: "/query_range?query=", queryPriorityEnabled: true, }, + "shouldn't return error if query is invalid, range query": { + url: "/query_range?query=up[4h", + queryPriorityEnabled: true, + err: errParseExpr, + }, "should miss if neither instant nor range query": { url: "/series", queryPriorityEnabled: true, @@ -58,7 +69,11 @@ func Test_GetPriorityShouldReturnDefaultPriorityIfNotEnabledOrEmptyQueryString(t limits.queryPriority.Enabled = testData.queryPriorityEnabled req, _ := http.NewRequest(http.MethodPost, testData.url, bytes.NewReader([]byte{})) priority, err := GetPriority(req, "", limits, now, 0) - assert.NoError(t, err) + if err != nil { + assert.Equal(t, testData.err, err) + } else { + assert.NoError(t, err) + } assert.Equal(t, int64(0), priority) }) } diff --git a/pkg/querier/tripperware/roundtrip.go b/pkg/querier/tripperware/roundtrip.go index 8ad3dd7fe0..f3526e3925 100644 --- a/pkg/querier/tripperware/roundtrip.go +++ b/pkg/querier/tripperware/roundtrip.go @@ -161,8 +161,10 @@ func NewQueryTripperware( if limits != nil && limits.QueryPriority(userStr).Enabled { priority, err := GetPriority(r, userStr, limits, now, lookbackDelta) - if err != nil { - return nil, err + if err != nil && err == errParseExpr { + // If query is invalid, no need to go through tripperwares + // for further splitting. + return next.RoundTrip(r) } r.Header.Set(util.QueryPriorityHeaderKey, strconv.FormatInt(priority, 10)) }