Skip to content

Commit

Permalink
Include response type in merging error message and merge already merg…
Browse files Browse the repository at this point in the history
…ed views. (grafana#10218)
  • Loading branch information
jeschkies authored Aug 10, 2023
1 parent 885c64f commit a3741d1
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 18 deletions.
8 changes: 7 additions & 1 deletion pkg/querier/queryrange/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,12 @@ func (Codec) MergeResponse(responses ...queryrangebase.Response) (queryrangebase
v.responses = append(v.responses, r.(*LokiSeriesResponseView))
}
return v, nil
case *MergedSeriesResponseView:
v := &MergedSeriesResponseView{}
for _, r := range responses {
v.responses = append(v.responses, r.(*MergedSeriesResponseView).responses...)
}
return v, nil
case *LokiLabelNamesResponse:
labelNameRes := responses[0].(*LokiLabelNamesResponse)
uniqueNames := make(map[string]struct{})
Expand Down Expand Up @@ -942,7 +948,7 @@ func (Codec) MergeResponse(responses ...queryrangebase.Response) (queryrangebase
Headers: headers,
}, nil
default:
return nil, errors.New("unknown response in merging responses")
return nil, fmt.Errorf("unknown response type (%T) in merging responses", responses[0])
}
}

Expand Down
41 changes: 25 additions & 16 deletions pkg/querier/queryrange/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,13 +847,23 @@ func Test_codec_EncodeResponse(t *testing.T) {

func Test_codec_MergeResponse(t *testing.T) {
tests := []struct {
name string
responses []queryrangebase.Response
want queryrangebase.Response
wantErr bool
name string
responses []queryrangebase.Response
want queryrangebase.Response
errorMessage string
}{
{"empty", []queryrangebase.Response{}, nil, true},
{"unknown response", []queryrangebase.Response{&badResponse{}}, nil, true},
{
"empty",
[]queryrangebase.Response{},
nil,
"merging responses requires at least one response",
},
{
"unknown response",
[]queryrangebase.Response{&badResponse{}},
nil,
"unknown response type (*queryrange.badResponse) in merging responses",
},
{
"prom",
[]queryrangebase.Response{
Expand All @@ -877,7 +887,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki backward",
Expand Down Expand Up @@ -965,7 +975,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki backward limited",
Expand Down Expand Up @@ -1050,7 +1060,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki forward",
Expand Down Expand Up @@ -1138,7 +1148,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki forward limited",
Expand Down Expand Up @@ -1222,7 +1232,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki series",
Expand Down Expand Up @@ -1268,7 +1278,7 @@ func Test_codec_MergeResponse(t *testing.T) {
},
},
},
false,
"",
},
{
"loki labels",
Expand All @@ -1295,15 +1305,14 @@ func Test_codec_MergeResponse(t *testing.T) {
Version: 1,
Data: []string{"foo", "bar", "buzz", "blip", "blop"},
},
false,
"",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := DefaultCodec.MergeResponse(tt.responses...)
if (err != nil) != tt.wantErr {
t.Errorf("codec.MergeResponse() error = %v, wantErr %v", err, tt.wantErr)
return
if tt.errorMessage != "" {
require.ErrorContains(t, err, tt.errorMessage)
}
require.Equal(t, tt.want, got)
})
Expand Down
1 change: 0 additions & 1 deletion pkg/querier/queryrange/views_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ func TestMergedViewMaterialize(t *testing.T) {
}
expected := []string{`{baz="woof", i="1"}`, `{baz="woof", i="3"}`, `{foo="bar", i="2"}`}
require.ElementsMatch(t, series, expected)

}

func TestMergedViewJSON(t *testing.T) {
Expand Down

0 comments on commit a3741d1

Please sign in to comment.