diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b64b0da013..57f442c2b2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * [ENHANCEMENT] Querier: Batch Iterator optimization to prevent transversing it multiple times query ranges steps does not overlap. #5237 * [BUGFIX] Catch context error in the s3 bucket client. #5240 * [BUGFIX] Fix query frontend remote read empty body. #5257 +* [BUGFIX] Fix query frontend incorrect error response format at `SplitByQuery` middleware. #5260 ## 1.15.0 in progress diff --git a/integration/query_frontend_test.go b/integration/query_frontend_test.go index 6df5d101015..5177eb1973b 100644 --- a/integration/query_frontend_test.go +++ b/integration/query_frontend_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/prompb" @@ -345,6 +346,19 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) { assert.Equal(t, model.LabelSet{labels.MetricName: "series_1"}, result[0]) } + // No need to repeat the query 400 test for each user. + if userID == 0 { + start := time.Unix(1595846748, 806*1e6) + end := time.Unix(1595846750, 806*1e6) + + _, err := c.QueryRange("up)", start, end, time.Second) + require.Error(t, err) + + apiErr, ok := err.(*v1.Error) + require.True(t, ok) + require.Equal(t, apiErr.Type, v1.ErrBadData) + } + for q := 0; q < numQueriesPerUser; q++ { go func() { defer wg.Done() @@ -359,7 +373,7 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) { wg.Wait() - extra := float64(2) + extra := float64(3) if cfg.testMissingMetricName { extra++ } diff --git a/pkg/frontend/transport/handler.go b/pkg/frontend/transport/handler.go index fdf8ae27c03..5259dd8574a 100644 --- a/pkg/frontend/transport/handler.go +++ b/pkg/frontend/transport/handler.go @@ -203,12 +203,12 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func formatGrafanaStatsFields(r *http.Request) []interface{} { - fields := make([]interface{}, 0, 2) + fields := make([]interface{}, 0, 4) if dashboardUID := r.Header.Get("X-Dashboard-Uid"); dashboardUID != "" { - fields = append(fields, dashboardUID) + fields = append(fields, "X-Dashboard-Uid", dashboardUID) } if panelID := r.Header.Get("X-Panel-Id"); panelID != "" { - fields = append(fields, panelID) + fields = append(fields, "X-Panel-Id", panelID) } return fields } diff --git a/pkg/querier/tripperware/queryrange/split_by_interval.go b/pkg/querier/tripperware/queryrange/split_by_interval.go index ab7fa2cfc47..2717fa415e6 100644 --- a/pkg/querier/tripperware/queryrange/split_by_interval.go +++ b/pkg/querier/tripperware/queryrange/split_by_interval.go @@ -47,7 +47,12 @@ func (s splitByInterval) Do(ctx context.Context, r tripperware.Request) (tripper // to line up the boundaries with step. reqs, err := splitQuery(r, s.interval(r)) if err != nil { - return nil, err + // If the query itself is bad, we don't return error but send the query + // to querier to return the expected error message. This is not very efficient + // but should be okay for now. + // TODO(yeya24): query frontend can reuse the Prometheus API handler and return + // expected error message locally without passing it to the querier through network. + return s.next.Do(ctx, r) } s.splitByCounter.Add(float64(len(reqs)))