Add logcli default client test #10100
Conversation
| codec := queryrange.Codec{} | ||
|
|
||
| mux.HandleFunc("/loki/api/v1/index/volume", func(w http.ResponseWriter, request *http.Request) { | ||
| volume := queryrange.LokiPromResponse{ |
There was a problem hiding this comment.
Very detailed test case, thanks your help. My only doubt here is the return value type of this test.
The return value type here should be logproto.VolumeResponse instead of queryrange.LokiPromResponse, because the code of http.go return logproto.VolumeResponse, and the test construction in http_test.go is also logproto.VolumeResponse
https://github.com/grafana/loki/blob/main/pkg/querier/http.go#L489
if resp == nil { // Some stores don't implement this
resp = &logproto.VolumeResponse{Volumes: []logproto.Volume{}}
}
if err := queryrange.WriteResponse(r, nil, resp, w); err != nil {
serverutil.WriteError(err, w)
}https://github.com/grafana/loki/blob/main/pkg/querier/http_test.go#L386
ret := &logproto.VolumeResponse{
Volumes: []logproto.Volume{
{Name: `{foo="bar"}`, Volume: 38},
},
}https://github.com/grafana/loki/blob/main/pkg/util/marshal/marshal.go#L132
func WriteVolumeResponseJSON(r *logproto.VolumeResponse, w io.Writer) error {
s := jsoniter.ConfigFastest.BorrowStream(w)
defer jsoniter.ConfigFastest.ReturnStream(s)
s.WriteVal(r)
s.WriteRaw("\n")
return s.Flush()
}There was a problem hiding this comment.
these are all inside the querier package, or in the last case, used for querier -> query frontend communication. we only use logproto.Volume for internal communication between components. The HTTP API, and response returned by the query frontend, converts the multiple logproto.Volume responses it gets from the many queriers that handle the split up query and turns it into a queryrange.LokiPromRepsonse. This is because the primary consumer of these endpoints wants the data to look like a metrics response to more easily plot it.
My test refers to the result of this test, and the test here also gets this json . https://github.com/grafana/loki/blob/main/pkg/querier/http_test.go#L419C5-L422C41 require.Equal(t,
`{"volumes":[{"name":"{foo=\"bar\"}","volume":38}]}`,
strings.TrimSpace(w.Body.String()), |
var resp loghttp.QueryResponse
if err := c.doRequest(path, params.Encode(), quiet, &resp); err != nil {
return nil, err
}can only decode mux.HandleFunc("/loki/api/v1/index/series_volume", func(w http.ResponseWriter, request *http.Request) {
seriesVolume := logproto.VolumeResponse{
Volumes: []logproto.Volume{
{Name: `{foo="bar"}`, Volume: 1024},
{Name: `{bar="baz"}`, Volume: 3350},
},
Limit: 5,
}
if err := marshal.WriteVolumeResponseJSON(&seriesVolume, w); err != nil {
serverutil.WriteError(err, w)
return
}
}) |
|
@liguozhong sorry for the long delay here. I think there might be some confusion about these endpoints, as the response type from the query frontend is different than the response type from the queriers. The We are currently using these endpoints in our production cluster, and the payload matches that of the stub in the test, so I think the bug might be in your test setup causes by an incorrect response type? That being said, I still think this test is valuable to add as is, as we are missing this type of top-level test for logcli. |
|
it may still be worthwhile to add these tests but this PR is pretty out of date without any action on it for a while |
This adds a test for the default client in
logcli. Currently it only covers happy path forvolumeandvolume_range. This was per request of @liguozhong resulting from a bug he found. @liguozhong could you add the unhappy path to recreate the bug? I'm still not sure I completely understand what it is. Thanks!