Fix splitByInterval incorrect error response format (#5260) (#5261)

* fix query frontend incorrect error response format * update changelog * fix integration test --------- Signed-off-by: Ben Ye <[email protected]>
cortexproject · Apr 12, 2023 · 5371089 · 5371089
1 parent 66948fd
commit 5371089
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,7 @@
 * [BUGFIX] Query Frontend: Disable `absent`, `absent_over_time` and `scalar` for vertical sharding. #5221
 * [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.14.0 2022-12-02
 

diff --git 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
@@ -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
@@ -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)))