vmselect: change HTTP response code for Prometheus querying API requests when parameters are missing or incorrect

app/vmalert/config/testdata/rules/rules-replay-good.rules

@@ -18,7 +18,7 @@ groups:
     concurrency: 2
     rules:
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning

app/vmselect/prometheus/prometheus.go

@@ -516,7 +516,7 @@ func DeleteHandler(startTime time.Time, r *http.Request) error {
 	cp.deadline = searchutil.GetDeadlineForDelete(r, startTime)
 
 	if !cp.IsDefaultTimeRange() {
-		return fmt.Errorf("start=%d and end=%d args aren't supported. Remove these args from the query in order to delete all the matching metrics", cp.start, cp.end)
+		return fmt.Errorf("delete API does not support specific time ranges using start and end args, the series can only be deleted completely")
 	}
 	sq := storage.NewSearchQuery(cp.start, cp.end, cp.filterss, *maxDeleteSeries)
 	deletedCount, err := netstorage.DeleteSeries(nil, sq, cp.deadline)
@@ -540,11 +540,11 @@ func LabelValuesHandler(qt *querytracer.Tracer, startTime time.Time, labelName s
 
 	cp, err := getCommonParamsForLabelsAPI(r, startTime, false)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	limit, err := httputil.GetInt(r, "limit")
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	sq := storage.NewSearchQuery(cp.start, cp.end, cp.filterss, *maxLabelsAPISeries)
 
@@ -584,7 +584,7 @@ func TSDBStatusHandler(qt *querytracer.Tracer, startTime time.Time, w http.Respo
 
 	cp, err := getCommonParams(r, startTime, false)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	cp.deadline = searchutil.GetDeadlineForStatusRequest(r, startTime)
 
@@ -596,7 +596,7 @@ func TSDBStatusHandler(qt *querytracer.Tracer, startTime time.Time, w http.Respo
 		} else {
 			t, err := time.Parse("2006-01-02", dateStr)
 			if err != nil {
-				return fmt.Errorf("cannot parse `date` arg %q: %w", dateStr, err)
+				return httpserver.InvalidParamError(fmt.Errorf("cannot parse `date` arg %q: %w", dateStr, err))
 			}
 			date = uint64(t.Unix()) / secsPerDay
 		}
@@ -607,7 +607,7 @@ func TSDBStatusHandler(qt *querytracer.Tracer, startTime time.Time, w http.Respo
 	if len(topNStr) > 0 {
 		n, err := strconv.Atoi(topNStr)
 		if err != nil {
-			return fmt.Errorf("cannot parse `topN` arg %q: %w", topNStr, err)
+			return httpserver.InvalidParamError(fmt.Errorf("cannot parse `topN` arg %q: %w", topNStr, err))
 		}
 		if n <= 0 {
 			n = 1
@@ -645,11 +645,11 @@ func LabelsHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseW
 
 	cp, err := getCommonParamsForLabelsAPI(r, startTime, false)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	limit, err := httputil.GetInt(r, "limit")
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	sq := storage.NewSearchQuery(cp.start, cp.end, cp.filterss, *maxLabelsAPISeries)
 	labels, err := netstorage.LabelNames(qt, sq, limit, cp.deadline)
@@ -671,10 +671,9 @@ func LabelsHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseW
 //
 // See https://prometheus.io/docs/prometheus/latest/querying/api/#querying-metric-metadata
 func MetadataHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWriter, r *http.Request) error {
-
 	limit, err := httputil.GetInt(r, "limit")
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	if limit < 0 {
 		limit = 0
@@ -734,11 +733,11 @@ func SeriesHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseW
 	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/91
 	cp, err := getCommonParamsForLabelsAPI(r, startTime, true)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	limit, err := httputil.GetInt(r, "limit")
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 
 	sq := storage.NewSearchQuery(cp.start, cp.end, cp.filterss, *maxSeriesLimit)
@@ -772,19 +771,19 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr
 	mayCache := !httputil.GetBool(r, "nocache")
 	query := r.FormValue("query")
 	if len(query) == 0 {
-		return fmt.Errorf("missing `query` arg")
+		return httpserver.InvalidParamError(fmt.Errorf("missing `query` arg"))
 	}
 	start, err := httputil.GetTime(r, "time", ct)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	lookbackDelta, err := getMaxLookback(r)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	step, err := httputil.GetDuration(r, "step", lookbackDelta)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	if step <= 0 {
 		step = defaultStep
@@ -792,16 +791,16 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr
 
 	maxLen := searchutil.GetMaxQueryLen()
 	if len(query) > maxLen {
-		return fmt.Errorf("too long query; got %d bytes; mustn't exceed `-search.maxQueryLen=%d` bytes", len(query), maxLen)
+		return httpserver.InvalidParamError(fmt.Errorf("too long query; got %d bytes; mustn't exceed `-search.maxQueryLen=%d` bytes", len(query), maxLen))
 	}
 	etfs, err := searchutil.GetExtraTagFilters(r)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	if childQuery, windowExpr, offsetExpr := promql.IsMetricSelectorWithRollup(query); childQuery != "" {
 		window, err := windowExpr.NonNegativeDuration(step)
 		if err != nil {
-			return fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err)
+			return httpserver.InvalidParamError(fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err))
 		}
 		offset := offsetExpr.Duration(step)
 		start -= offset
@@ -815,7 +814,7 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr
 
 		tagFilterss, err := getTagFilterssFromMatches([]string{childQuery})
 		if err != nil {
-			return err
+			return httpserver.InvalidParamError(err)
 		}
 		filterss := searchutil.JoinTagFilterss(tagFilterss, etfs)
 
@@ -831,22 +830,25 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr
 		return nil
 	}
 	if childQuery, windowExpr, stepExpr, offsetExpr := promql.IsRollup(query); childQuery != "" {
+		if len(childQuery) > maxLen {
+			return httpserver.InvalidParamError(fmt.Errorf("too long query; got %d bytes; mustn't exceed `-search.maxQueryLen=%d` bytes", len(childQuery), maxLen))
+		}
 		newStep, err := stepExpr.NonNegativeDuration(step)
 		if err != nil {
-			return fmt.Errorf("cannot parse step in square brackets at %s: %w", query, err)
+			return httpserver.InvalidParamError(fmt.Errorf("cannot parse step in square brackets at %s: %w", query, err))
 		}
 		if newStep > 0 {
 			step = newStep
 		}
 		window, err := windowExpr.NonNegativeDuration(step)
 		if err != nil {
-			return fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err)
+			return httpserver.InvalidParamError(fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err))
 		}
 		offset := offsetExpr.Duration(step)
 		start -= offset
 		end := start
 		start = end - window
-		if err := queryRangeHandler(qt, startTime, w, childQuery, start, end, step, r, ct, etfs); err != nil {
+		if err := queryRangeHandler(qt, startTime, w, childQuery, start, end, step, lookbackDelta, r, ct, etfs); err != nil {
 			return fmt.Errorf("error when executing query=%q on the time range (start=%d, end=%d, step=%d): %w", childQuery, start, end, step, err)
 		}
 		return nil
@@ -854,7 +856,7 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr
 
 	queryOffset, err := getLatencyOffsetMilliseconds(r)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	if !httputil.GetBool(r, "nocache") && ct-start < queryOffset && start-ct < queryOffset {
 		// Adjust start time only if `nocache` arg isn't set.
@@ -928,45 +930,43 @@ func QueryRangeHandler(qt *querytracer.Tracer, startTime time.Time, w http.Respo
 	ct := startTime.UnixNano() / 1e6
 	query := r.FormValue("query")
 	if len(query) == 0 {
-		return fmt.Errorf("missing `query` arg")
+		return httpserver.InvalidParamError(fmt.Errorf("missing `query` arg"))
+	}
+	maxLen := searchutil.GetMaxQueryLen()
+	if len(query) > maxLen {
+		return httpserver.InvalidParamError(fmt.Errorf("too long query; got %d bytes; mustn't exceed `-search.maxQueryLen=%d` bytes", len(query), maxLen))
 	}
 	start, err := httputil.GetTime(r, "start", ct-defaultStep)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	end, err := httputil.GetTime(r, "end", ct)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	step, err := httputil.GetDuration(r, "step", defaultStep)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
 	etfs, err := searchutil.GetExtraTagFilters(r)
 	if err != nil {
-		return err
+		return httpserver.InvalidParamError(err)
 	}
-	if err := queryRangeHandler(qt, startTime, w, query, start, end, step, r, ct, etfs); err != nil {
+	lookbackDelta, err := getMaxLookback(r)
+	if err != nil {
+		return httpserver.InvalidParamError(err)
+	}
+	if err := queryRangeHandler(qt, startTime, w, query, start, end, step, lookbackDelta, r, ct, etfs); err != nil {
 		return fmt.Errorf("error when executing query=%q on the time range (start=%d, end=%d, step=%d): %w", query, start, end, step, err)
 	}
 	return nil
 }
 
 func queryRangeHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWriter, query string,
-	start, end, step int64, r *http.Request, ct int64, etfs [][]storage.TagFilter) error {
+	start, end, step, lookbackDelta int64, r *http.Request, ct int64, etfs [][]storage.TagFilter) error {
 	deadline := searchutil.GetDeadlineForQuery(r, startTime)
 	mayCache := !httputil.GetBool(r, "nocache")
 	optimizeRepeatedBinaryOpSubexprs := httputil.GetBool(r, "optimize_repeated_binary_op_subexprs")
-	lookbackDelta, err := getMaxLookback(r)
-	if err != nil {
-		return err
-	}
-
-	// Validate input args.
-	maxLen := searchutil.GetMaxQueryLen()
-	if len(query) > maxLen {
-		return fmt.Errorf("too long query; got %d bytes; mustn't exceed `-search.maxQueryLen=%d` bytes", len(query), maxLen)
-	}
 	if start > end {
 		end = start + defaultStep
 	}
@@ -1005,7 +1005,7 @@ func queryRangeHandler(qt *querytracer.Tracer, startTime time.Time, w http.Respo
 	if step < maxStepForPointsAdjustment.Milliseconds() {
 		queryOffset, err := getLatencyOffsetMilliseconds(r)
 		if err != nil {
-			return err
+			return httpserver.InvalidParamError(err)
 		}
 		if ct-queryOffset < end {
 			result = adjustLastPoints(result, ct-queryOffset, ct+step)
@@ -1156,13 +1156,13 @@ func QueryStatsHandler(w http.ResponseWriter, r *http.Request) error {
 	if len(topNStr) > 0 {
 		n, err := strconv.Atoi(topNStr)
 		if err != nil {
-			return fmt.Errorf("cannot parse `topN` arg %q: %w", topNStr, err)
+			return httpserver.InvalidParamError(fmt.Errorf("cannot parse `topN` arg %q: %w", topNStr, err))
 		}
 		topN = n
 	}
 	maxLifetimeMsecs, err := httputil.GetDuration(r, "maxLifetime", 10*60*1000)
 	if err != nil {
-		return fmt.Errorf("cannot parse `maxLifetime` arg: %w", err)
+		return httpserver.InvalidParamError(fmt.Errorf("cannot parse `maxLifetime` arg: %w", err))
 	}
 	maxLifetime := time.Duration(maxLifetimeMsecs) * time.Millisecond
 	w.Header().Set("Content-Type", "application/json")

app/vmselect/promql/exec.go

@@ -11,6 +11,7 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/netstorage"
 	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/querystats"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/httpserver"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/querytracer"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
@@ -46,15 +47,15 @@ func Exec(qt *querytracer.Tracer, ec *EvalConfig, q string, isFirstPointOnly boo
 
 	e, err := parsePromQLWithCache(q)
 	if err != nil {
-		return nil, err
+		return nil, httpserver.InvalidParamError(err)
 	}
 
 	if *disableImplicitConversion || *logImplicitConversion {
 		isInvalid := metricsql.IsLikelyInvalid(e)
 		if isInvalid && *disableImplicitConversion {
 			// we don't add query=%q to err message as it will be added by the caller
-			return nil, fmt.Errorf("query requires implicit conversion and is rejected according to -search.disableImplicitConversion command-line flag. " +
-				"See https://docs.victoriametrics.com/victoriametrics/metricsql/#implicit-query-conversions for details")
+			return nil, httpserver.InvalidParamError(fmt.Errorf("query requires implicit conversion and is rejected according to -search.disableImplicitConversion command-line flag. " +
+				"See https://docs.victoriametrics.com/victoriametrics/metricsql/#implicit-query-conversions for details"))
 		}
 		if isInvalid && *logImplicitConversion {
 			logger.Warnf("query=%q requires implicit conversion, see https://docs.victoriametrics.com/victoriametrics/metricsql/#implicit-query-conversions for details", e.AppendString(nil))

deployment/docker/rules/alerts-cluster.yml

@@ -75,7 +75,7 @@ groups:
             Consider to limit the ingestion rate, decrease retention or scale the disk space if possible."
 
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+", path!="*"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning
@@ -83,24 +83,8 @@ groups:
         annotations:
           dashboard: "{{ $externalURL }}/d/oS7Bi_0Wz?viewPanel=52&var-instance={{ $labels.instance }}"
           summary: "Too many errors served for {{ $labels.job }} path {{ $labels.path }} (instance {{ $labels.instance }})"
-          description: |
-              Requests to path {{ $labels.path }} are receiving errors.
-              Please verify if clients are sending correct requests.
-
-      # Auth errors and unknown paths should be handled by a different alert
-      # See https://github.com/VictoriaMetrics/VictoriaMetrics/blob/fdd9a221df835daa378ae2e6c9f12e4e3be79c76/lib/httpserver/httpserver.go#L589-L591
-      - alert: RequestErrorsToUnknownPaths
-        expr: sum(increase(vm_http_request_errors_total{path=~"^(\*|)$"}[5m])) by(job, instance, reason) > 0
-        for: 15m
-        labels:
-          severity: warning
-          show_at: dashboard
-        annotations:
-          dashboard: "{{ $externalURL }}/d/oS7Bi_0Wz?viewPanel=52&var-instance={{ $labels.instance }}"
-          summary: "Too many errors served for {{ $labels.job }} with reason {{ $labels.reason }} (instance {{ $labels.instance }})"
-          description: |
-              Requests are failing with reason {{ $labels.reason }}.
-              Please verify if clients are sending correct requests.
+          description: "Requests to path {{ $labels.path }} are receiving errors.
+            Please verify if clients are sending correct requests."
 
       - alert: RPCErrors
         expr: |

deployment/docker/rules/alerts-single-node.yml

@@ -75,7 +75,7 @@ groups:
             Consider to limit the ingestion rate, decrease retention or scale the disk space if possible."
 
       - alert: RequestErrorsToAPI
-        expr: increase(vm_http_request_errors_total{path=~".+"}[5m]) > 0
+        expr: increase(vm_http_request_errors_total[5m]) > 0
         for: 15m
         labels:
           severity: warning

docs/victoriametrics/changelog/CHANGELOG.md

@@ -26,7 +26,7 @@ See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-rel
 
 ## tip
 
-* FEATURE: [alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/rules): update `RequestErrorsToAPI` alerting rule and add `RequestErrorsToUnknownPaths` to [cluster alerts](https://github.com/VictoriaMetrics/VictoriaMetrics/blob/master/deployment/docker/rules/alerts-cluster.yml). The new rule notifies when authentication fails or unknown paths are requested. Previously both cases were treated as errors to the API.
+* FEATURE: [vmsingle](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/) and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/): change the HTTP response code for Prometheus querying API requests from `422 Unprocessable Entity` to `400 Bad Request` when request parameters are missing or incorrect. See [#11330](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/11330).
 
 ## [v1.149.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.149.0)
 

lib/httpserver/prometheus.go

@@ -5,9 +5,18 @@ import (
 	"net/http"
 )
 
-// SendPrometheusError sends err to w in Prometheus querying API response format.
-//
-// See https://prometheus.io/docs/prometheus/latest/querying/api/#format-overview for more details
+// InvalidParamError sets HTTP status code to 400 Bad Request for Prometheus querying APIs when parameters are missing or incorrect,
+// see https://prometheus.io/docs/prometheus/latest/querying/api/#format-overview.
+func InvalidParamError(err error) *ErrorWithStatusCode {
+	return &ErrorWithStatusCode{
+		Err:        err,
+		StatusCode: http.StatusBadRequest,
+	}
+}
+
+// SendPrometheusError sends err to w in Prometheus querying API response format,
+// and sets HTTP status code to 422 Unprocessable Entity when code is not set,
+// see https://prometheus.io/docs/prometheus/latest/querying/api/#format-overview for more details.
 func SendPrometheusError(w http.ResponseWriter, r *http.Request, err error) {
 	errStr := err.Error()
 	logHTTPError(r, errStr)