feat(gateway): add per-request logging to gateway with --request-logging #13147
Conversation
Logs POST bodies too, so probably not ideal for environments where performance is critical, or where you think there might be data leakage (although the gateway is intended for non-sensitive traffic).
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an optional request-logging middleware that captures incoming HTTP requests (including POST bodies) for debugging.
- Adds a
--request-loggingCLI flag and wires it into the gateway handler options. - Implements
LoggingHandlermiddleware that reads, optionally truncates, and logs request details. - Provides a helper to extract the remote IP address.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| gateway/handler.go | Add LoggingHandler, WithRequestLogging option, and body logging logic |
| cmd/lotus-gateway/main.go | Add request-logging CLI flag and pass it to WithRequestLogging |
Comments suppressed due to low confidence (1)
gateway/handler.go:354
- LoggingHandler lacks unit tests. Add tests to verify behavior for different HTTP methods, body sizes, and error scenarios.
// LoggingHandler logs incoming HTTP requests with details
| body, err := io.ReadAll(r.Body) | ||
| if err == nil { |
There was a problem hiding this comment.
Reading the entire request body into memory can cause high memory usage for large payloads. Consider using io.LimitedReader to cap the maximum bytes read.
| body, err := io.ReadAll(r.Body) | |
| if err == nil { | |
| const maxBodySize = 1 * 1024 * 1024 // 1 MB | |
| limitedReader := io.LimitedReader{R: r.Body, N: maxBodySize} | |
| body, err := io.ReadAll(&limitedReader) | |
| if err != nil { | |
| if err == io.EOF || limitedReader.N == 0 { | |
| http.Error(w, "Request body too large", http.StatusRequestEntityTooLarge) | |
| return | |
| } | |
| logFields = append(logFields, "body_read_error", err.Error()) | |
| } else { |
Copilot uses AI. Check for mistakes.
| // For POST requests, try to read and log the body | ||
| if r.Method == "POST" { | ||
| body, err := io.ReadAll(r.Body) | ||
| if err == nil { |
There was a problem hiding this comment.
The error from io.ReadAll is ignored when non-nil; consider logging a warning or handling errors to aid in diagnosing unexpected read failures.
| if err == nil { | |
| if err != nil { | |
| // Log a warning if reading the body fails | |
| log.Warnw("failed to read request body", "error", err) | |
| } else { |
Copilot uses AI. Check for mistakes.
| if len(bodyStr) > 1000 { | ||
| bodyStr = bodyStr[:1000] + "...[truncated]" |
There was a problem hiding this comment.
[nitpick] The truncation limit of 1000 is a magic number; extract it into a named constant to clarify its purpose and simplify future adjustments.
| if len(bodyStr) > 1000 { | |
| bodyStr = bodyStr[:1000] + "...[truncated]" | |
| if len(bodyStr) > MaxLogBodyLength { | |
| bodyStr = bodyStr[:MaxLogBodyLength] + "...[truncated]" |
Copilot uses AI. Check for mistakes.
github-project-automation
bot
moved this from 🔎 Awaiting Review
to ✔️ Approved by reviewer
in FilOz
|
some good copilot suggestions in there, I'll tweak when I have time |
|
@rvagg : are you able to land this week? |
|
Maybe |
Logs POST bodies too, so probably not ideal for environments where performance is critical, or where you think there might be data leakage (although the gateway is intended for non-sensitive traffic).
This was just a private branch feature but I found it super useful for debugging and decided there's a good chance I'm going to want this again. I also tried logging to stdout, but having it interleaved with error logs ended up being more useful. Unfortunately the JSON bodies get escaped to be reproduced as JSON output, but it's not terrible.
Another alternative I considered was providing a filename, so it ends up a bit like a classic HTTP server log,
--request-logging=access.log, but it goes a bit further than HTTP server logging with the request body capture so it may be best to not give the impression that this is the right thing to do.{"level":"info","ts":"2025-05-28T22:58:32.768+1000","logger":"gateway","caller":"gateway/handler.go:380","msg":"request","remote_ip":"192.168.1.12","method":"POST","url":"/rpc/v1","body":"{\"id\":\"0f6d72ba-f240-4ef9-83c8-9ecc42f5a635\",\"jsonrpc\":\"2.0\",\"method\":\"eth_getBalance\",\"params\":[\"0x0542c82a785fab8b53c7c59aaad7093d87265f3a\",\"0x29414a\"]}"} {"level":"info","ts":"2025-05-28T22:58:32.769+1000","logger":"gateway","caller":"gateway/handler.go:380","msg":"request","remote_ip":"192.168.1.12","method":"POST","url":"/rpc/v1","body":"{\"id\":\"e764c4b6-234c-44c0-b59c-5b75407488fe\",\"jsonrpc\":\"2.0\",\"method\":\"eth_call\",\"params\":[{\"to\":\"0x0e690d3e60b0576d01352ab03b258115eb84a047\",\"data\":\"0x01ffc9a780 ac58cd00000000000000000000000000000000000000000000000000000000\"},\"0x29414a\"]}"} {"level":"warn","ts":"2025-05-28T22:58:32.773+1000","logger":"rpc","caller":"go-jsonrpc@v0.7.0/handler.go:421","msg":"error in RPC call to 'eth_call': message execution failed (exit=[33], revert reason=[message failed with backtrace:\n00: f0162143 (method 3844450837) -- contract reverted at 86 (33)\n01: f0162143 (method 6) -- contract reverted at 349 (33)\n (RetCode=33)], vm error=[none])"}