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]>

Author: yeya24

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
 

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++
 	}

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
 }

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)))