Skip to content

Commit 8cdcf8d

Browse files
committed
fix query frontend incorrect error response format
Signed-off-by: Ben Ye <[email protected]>
1 parent 5e90afc commit 8cdcf8d

File tree

3 files changed

+39
-5
lines changed

3 files changed

+39
-5
lines changed

integration/query_frontend_test.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package integration
66
import (
77
"crypto/x509"
88
"crypto/x509/pkix"
9+
"encoding/json"
910
"fmt"
1011
"os"
1112
"path/filepath"
@@ -35,6 +36,14 @@ type queryFrontendTestConfig struct {
3536
setup func(t *testing.T, s *e2e.Scenario) (configFile string, flags map[string]string)
3637
}
3738

39+
type response struct {
40+
Status status `json:"status"`
41+
Data interface{} `json:"data,omitempty"`
42+
ErrorType errorType `json:"errorType,omitempty"`
43+
Error string `json:"error,omitempty"`
44+
Warnings []string `json:"warnings,omitempty"`
45+
}
46+
3847
func TestQueryFrontendWithBlocksStorageViaFlags(t *testing.T) {
3948
runQueryFrontendTest(t, queryFrontendTestConfig{
4049
testMissingMetricName: false,
@@ -345,6 +354,26 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) {
345354
assert.Equal(t, model.LabelSet{labels.MetricName: "series_1"}, result[0])
346355
}
347356

357+
// No need to repeat the query 400 test for each user.
358+
if userID == 0 {
359+
start := time.Unix(1595846748, 806*1e6)
360+
end := time.Unix(1595846750, 806*1e6)
361+
362+
_, err := c.QueryRange("up)", start, end, time.Second)
363+
require.Error(t, err)
364+
365+
// Expect the error response format to be correct.
366+
type response struct {
367+
Status string `json:"status"`
368+
ErrorType string `json:"errorType,omitempty"`
369+
Error string `json:"error,omitempty"`
370+
}
371+
var res response
372+
err = json.Unmarshal([]byte(err.Error()), &res)
373+
require.NoError(t, err)
374+
require.Equal(t, response.ErrorType, "bad_data")
375+
}
376+
348377
for q := 0; q < numQueriesPerUser; q++ {
349378
go func() {
350379
defer wg.Done()
@@ -359,7 +388,7 @@ func runQueryFrontendTest(t *testing.T, cfg queryFrontendTestConfig) {
359388

360389
wg.Wait()
361390

362-
extra := float64(2)
391+
extra := float64(3)
363392
if cfg.testMissingMetricName {
364393
extra++
365394
}

pkg/frontend/transport/handler.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,12 @@ func (f *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
203203
}
204204

205205
func formatGrafanaStatsFields(r *http.Request) []interface{} {
206-
fields := make([]interface{}, 0, 2)
206+
fields := make([]interface{}, 0, 4)
207207
if dashboardUID := r.Header.Get("X-Dashboard-Uid"); dashboardUID != "" {
208-
fields = append(fields, dashboardUID)
208+
fields = append(fields, "X-Dashboard-Uid", dashboardUID)
209209
}
210210
if panelID := r.Header.Get("X-Panel-Id"); panelID != "" {
211-
fields = append(fields, panelID)
211+
fields = append(fields, "X-Panel-Id", panelID)
212212
}
213213
return fields
214214
}

pkg/querier/tripperware/queryrange/split_by_interval.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,12 @@ func (s splitByInterval) Do(ctx context.Context, r tripperware.Request) (tripper
4747
// to line up the boundaries with step.
4848
reqs, err := splitQuery(r, s.interval(r))
4949
if err != nil {
50-
return nil, err
50+
// If the query itself is bad, we don't return error but send the query
51+
// to querier to return the expected error message. This is not very efficient
52+
// but should be okay for now.
53+
// TODO(yeya24): query frontend can reuse the Prometheus API handler and return
54+
// expected error message locally without passing it to the querier through network.
55+
return s.next.Do(ctx, r)
5156
}
5257
s.splitByCounter.Add(float64(len(reqs)))
5358

0 commit comments

Comments
 (0)