Skip to content

Commit

Permalink
Add jaeger-query HTTP handler diagnostic logging (#2906)
Browse files Browse the repository at this point in the history
* Add diagnostic logging to jaeger-query's HTTP handler

Signed-off-by: albertteoh <albert.teoh@logz.io>

* More descriptive log message

Signed-off-by: albertteoh <albert.teoh@logz.io>

* Add tests

Signed-off-by: albertteoh <albert.teoh@logz.io>

* Address review comments

Signed-off-by: albertteoh <albert.teoh@logz.io>
  • Loading branch information
albertteoh authored Mar 27, 2021
1 parent ecd3bbb commit f49c6df
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 13 deletions.
10 changes: 8 additions & 2 deletions cmd/query/app/http_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,13 @@ func (aH *APIHandler) writeJSON(w http.ResponseWriter, r *http.Request, response
return json.MarshalIndent(v, "", " ")
}
}
resp, _ := marshall(response)
resp, err := marshall(response)
if err != nil {
aH.handleError(w, fmt.Errorf("failed marshalling HTTP response to JSON: %w", err), http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
w.Write(resp)
if _, err := w.Write(resp); err != nil {
aH.handleError(w, fmt.Errorf("failed writing HTTP response: %w", err), http.StatusInternalServerError)
}
}
65 changes: 54 additions & 11 deletions cmd/query/app/http_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"fmt"
"io"
"io/ioutil"
"math"
"net/http"
"net/http/httptest"
"testing"
Expand Down Expand Up @@ -185,16 +186,51 @@ func TestLogOnServerError(t *testing.T) {
assert.Equal(t, e, (*l.logs)[0].f[0].Interface)
}

func TestPrettyPrint(t *testing.T) {
data := struct{ Data string }{Data: "Bender"}
// httpResponseErrWriter implements the http.ResponseWriter interface that returns an error on Write.
type httpResponseErrWriter struct{}

func (h *httpResponseErrWriter) Write([]byte) (int, error) {
return 0, fmt.Errorf("failed to write")
}
func (h *httpResponseErrWriter) WriteHeader(statusCode int) {}
func (h *httpResponseErrWriter) Header() http.Header {
return http.Header{}
}
func TestWriteJSON(t *testing.T) {
testCases := []struct {
param string
output string
name string
data interface{}
param string
output string
httpWriteErr bool
}{
{output: `{"Data":"Bender"}`},
{param: "?prettyPrint=false", output: `{"Data":"Bender"}`},
{param: "?prettyPrint=x", output: "{\n \"Data\": \"Bender\"\n}"},
{
name: "no pretty print param passed",
data: struct{ Data string }{Data: "Bender"},
output: `{"Data":"Bender"}`,
},
{
name: "pretty print explicitly disabled",
data: struct{ Data string }{Data: "Bender"},
param: "?prettyPrint=false",
output: `{"Data":"Bender"}`,
},
{
name: "pretty print enabled",
data: struct{ Data string }{Data: "Bender"},
param: "?prettyPrint=x",
output: "{\n \"Data\": \"Bender\"\n}",
},
{
name: "fail JSON marshal",
data: struct{ Data float64 }{Data: math.Inf(1)},
output: "{\"data\":null,\"total\":0,\"limit\":0,\"offset\":0,\"errors\":[{\"code\":500,\"msg\":\"failed marshalling HTTP response to JSON: json: unsupported value: +Inf\"}]}\n",
},
{
name: "fail http write",
data: struct{ Data string }{Data: "Bender"},
httpWriteErr: true,
},
}

get := func(url string) string {
Expand All @@ -206,10 +242,17 @@ func TestPrettyPrint(t *testing.T) {
}

for _, testCase := range testCases {
t.Run(testCase.param, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
new(APIHandler).writeJSON(w, r, &data)
}))
t.Run(testCase.name, func(t *testing.T) {
apiHandler := &APIHandler{
logger: zap.NewNop(),
}
httpHandler := func(w http.ResponseWriter, r *http.Request) {
if testCase.httpWriteErr {
w = &httpResponseErrWriter{}
}
apiHandler.writeJSON(w, r, &testCase.data)
}
server := httptest.NewServer(http.HandlerFunc(httpHandler))
defer server.Close()

out := get(server.URL + testCase.param)
Expand Down

0 comments on commit f49c6df

Please sign in to comment.