From 545b977e7afdc0b38b56bfaee0a7bb3ebeeaa467 Mon Sep 17 00:00:00 2001 From: Hui Wang Date: Tue, 4 Aug 2026 17:28:52 +0800 Subject: [PATCH] vmselect: change HTTP response code for Prometheus querying API requests when parameters are missing or incorrect --- app/vmselect/prometheus/prometheus.go | 88 ++++++++++----------- app/vmselect/promql/exec.go | 7 +- docs/victoriametrics/changelog/CHANGELOG.md | 2 + lib/httpserver/prometheus.go | 15 +++- 4 files changed, 62 insertions(+), 50 deletions(-) diff --git a/app/vmselect/prometheus/prometheus.go b/app/vmselect/prometheus/prometheus.go index 116fea3851..729a00aa70 100644 --- a/app/vmselect/prometheus/prometheus.go +++ b/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") diff --git a/app/vmselect/promql/exec.go b/app/vmselect/promql/exec.go index d2d3fd5c9c..c41a033085 100644 --- a/app/vmselect/promql/exec.go +++ b/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)) diff --git a/docs/victoriametrics/changelog/CHANGELOG.md b/docs/victoriametrics/changelog/CHANGELOG.md index fd9471a3b3..84d7461b94 100644 --- a/docs/victoriametrics/changelog/CHANGELOG.md +++ b/docs/victoriametrics/changelog/CHANGELOG.md @@ -26,6 +26,8 @@ See also [LTS releases](https://docs.victoriametrics.com/victoriametrics/lts-rel ## tip +* 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) Release candidate diff --git a/lib/httpserver/prometheus.go b/lib/httpserver/prometheus.go index 591603381d..1ca6d4656c 100644 --- a/lib/httpserver/prometheus.go +++ b/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)