Include parameter name when returning HTTP 400 for query_range API

CHANGELOG.md

@@ -44,12 +44,13 @@
   * `/config?mode=diff`: Shows the YAML configuration with all values that differ from the defaults.
   * `/config?mode=defaults`: Shows the YAML configuration with all the default values.
 * [ENHANCEMENT] OpenStack Swift: added the following config options to OpenStack Swift backend client: #3660
-  - Chunks storage: `-swift.auth-version`, `-swift.max-retries`, `-swift.connect-timeout`, `-swift.request-timeout`.
+* [ENHANCEMENT] Query-frontend: included the parameter name failed to validate in HTTP 400 message. #3703
   - Blocks storage: ` -blocks-storage.swift.auth-version`, ` -blocks-storage.swift.max-retries`, ` -blocks-storage.swift.connect-timeout`, ` -blocks-storage.swift.request-timeout`.
   - Ruler: `-ruler.storage.swift.auth-version`, `-ruler.storage.swift.max-retries`, `-ruler.storage.swift.connect-timeout`, `-ruler.storage.swift.request-timeout`.
 * [ENHANCEMENT] Disabled in-memory shuffle-sharding subring cache in the store-gateway, ruler and compactor. This should reduce the memory utilisation in these services when shuffle-sharding is enabled, without introducing a significantly increase CPU utilisation. #3601
 * [ENHANCEMENT] Shuffle sharding: optimised subring generation used by shuffle sharding. #3601
 * [ENHANCEMENT] New /runtime_config endpoint that returns the defined runtime configuration in YAML format. The returned configuration includes overrides. #3639
+* [ENHANCEMENT] query_range API to include parameter name in HTTP 400 message. #3699
 * [ENHANCEMENT] Fail to startup Cortex if provided runtime config is invalid. #3707
 * [BUGFIX] Allow `-querier.max-query-lookback` use `y|w|d` suffix like deprecated `-store.max-look-back-period`. #3598
 * [BUGFIX] Memberlist: Entry in the ring should now not appear again after using "Forget" feature (unless it's still heartbeating). #3603

pkg/querier/queryrange/query_range.go

@@ -3,6 +3,7 @@ package queryrange
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io/ioutil"
 	"math"
 	"net/http"
@@ -13,6 +14,7 @@ import (
 	"time"
 
 	"github.com/gogo/protobuf/proto"
+	"github.com/gogo/status"
 	jsoniter "github.com/json-iterator/go"
 	"github.com/opentracing/opentracing-go"
 	otlog "github.com/opentracing/opentracing-go/log"
@@ -186,12 +188,12 @@ func (prometheusCodec) DecodeRequest(_ context.Context, r *http.Request) (Reques
 	var err error
 	result.Start, err = util.ParseTime(r.FormValue("start"))
 	if err != nil {
-		return nil, err
+		return nil, decorateWithParamName(err, "start")
 	}
 
 	result.End, err = util.ParseTime(r.FormValue("end"))
 	if err != nil {
-		return nil, err
+		return nil, decorateWithParamName(err, "end")
 	}
 
 	if result.End < result.Start {
@@ -200,7 +202,7 @@ func (prometheusCodec) DecodeRequest(_ context.Context, r *http.Request) (Reques
 
 	result.Step, err = parseDurationMs(r.FormValue("step"))
 	if err != nil {
-		return nil, err
+		return nil, decorateWithParamName(err, "step")
 	}
 
 	if result.Step <= 0 {
@@ -392,3 +394,11 @@ func encodeTime(t int64) string {
 func encodeDurationMs(d int64) string {
 	return strconv.FormatFloat(float64(d)/float64(time.Second/time.Millisecond), 'f', -1, 64)
 }
+
+func decorateWithParamName(err error, field string) error {
+	errTmpl := "invalid parameter %q; %v"
+	if status, ok := status.FromError(err); ok {
+		return httpgrpc.Errorf(int(status.Code()), errTmpl, field, status.Message())
+	}
+	return fmt.Errorf(errTmpl, field, err)
+}

pkg/querier/queryrange/query_range_test.go

@@ -29,19 +29,19 @@ func TestRequest(t *testing.T) {
 		},
 		{
 			url:         "api/v1/query_range?start=foo",
-			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "cannot parse \"foo\" to a valid timestamp"),
+			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "invalid parameter \"start\"; cannot parse \"foo\" to a valid timestamp"),
 		},
 		{
 			url:         "api/v1/query_range?start=123&end=bar",
-			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "cannot parse \"bar\" to a valid timestamp"),
+			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "invalid parameter \"end\"; cannot parse \"bar\" to a valid timestamp"),
 		},
 		{
 			url:         "api/v1/query_range?start=123&end=0",
 			expectedErr: errEndBeforeStart,
 		},
 		{
 			url:         "api/v1/query_range?start=123&end=456&step=baz",
-			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "cannot parse \"baz\" to a valid duration"),
+			expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "invalid parameter \"step\"; cannot parse \"baz\" to a valid duration"),
 		},
 		{
 			url:         "api/v1/query_range?start=123&end=456&step=-1",