Add jaeger-query HTTP handler diagnostic logging #2906
Conversation
Signed-off-by: albertteoh <[email protected]>
Signed-off-by: albertteoh <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2906 +/- ##
==========================================
+ Coverage 95.92% 95.94% +0.01%
==========================================
Files 223 223
Lines 9704 9708 +4
==========================================
+ Hits 9309 9314 +5
+ Misses 326 325 -1
Partials 69 69
Continue to review full report at Codecov.
|
Signed-off-by: albertteoh <[email protected]>
albertteoh
force-pushed
the
query-diagnostic-logging
branch
2 times, most recently
from
d49f172 to
f3eef71
Compare
Yup, sloppiness on my part. I had a go at writing tests for it, please let me know if there's a more idiomatic approach. |
cmd/query/app/http_handler.go
Outdated
| if b, err := w.Write(resp); err != nil { | ||
| aH.handleError(w, fmt.Errorf("failed writing HTTP response: %w", err), http.StatusInternalServerError) | ||
| } else { | ||
| aH.logger.Debug("Successfully wrote HTTP response", zap.Int("bytes", b)) |
There was a problem hiding this comment.
I'm not a fan of this debug logging, is it necessary? It should be observable from the client side already.
cmd/query/app/http_handler_test.go
Outdated
| } | ||
|
|
||
| // ErrHandlerFunc injects the httpResponseErrWriter into the HTTP handler. | ||
| type ErrHandlerFunc func(http.ResponseWriter, *http.Request) |
There was a problem hiding this comment.
This type isn't necessary, you can inline in L262 using http.HandlerFunc
albertteoh
force-pushed
the
query-diagnostic-logging
branch
from
ec55c5c to
9b216a0
Compare
Signed-off-by: albertteoh <[email protected]>
albertteoh
force-pushed
the
query-diagnostic-logging
branch
from
9b216a0 to
0ea8674
Compare
|
Thanks for the review! |
Signed-off-by: albertteoh [email protected]
Which problem is this PR solving?
/metricsAPI endpoint).Short description of the changes
json.MarshalandResponseWriter.Writeerrors and log them. Log success at debug level as well.