Add trace-level logging of RPC requests #1441
Conversation
cla-bot
bot
added
the
s
Codecov Report
@@ Coverage Diff @@
## master #1441 +/- ##
==========================================
- Coverage 52.32% 48.52% -3.81%
==========================================
Files 257 245 -12
Lines 34003 29915 -4088
Branches 555 555
==========================================
- Hits 17793 14515 -3278
+ Misses 14134 13314 -820
- Partials 2076 2086 +10 |
cmd/nitro/nitro.go
Outdated
| func (l RpcResultLogger) OnResult(response interface{}, err error) { | ||
| if err != nil { | ||
| // The request might not've been logged if the log level is debug not trace, so we log it again here | ||
| log.Debug("received error response from L1 RPC", "request", l.request, "response", response, "err", err) |
There was a problem hiding this comment.
A warning or at least info seems in place?
There was a problem hiding this comment.
I'm just not sure under what conditions this might appear, and don't want to concern users unnecessarily. But I'll make it an info 👍
cmd/nitro/nitro.go
Outdated
| log.Debug("received error response from L1 RPC", "request", l.request, "response", response, "err", err) | ||
| } else { | ||
| // The request was already logged and can be cross-referenced by JSON-RPC id | ||
| log.Trace("received response from L1 RPC", "response", response, "err", err) |
There was a problem hiding this comment.
err is certainly nil here, no need to print.
Maybe this should be debug and OnRequest should stay trace?
(the amount of logs should be reasonable for what we get in Debug).
There was a problem hiding this comment.
The err thing was a leftover from a previous refactor, I'll remove that
The responses for querying blocks, logs, and txs can be really large so I'd prefer to keep this hidden from users unless they need it
| // The request was already logged and can be cross-referenced by JSON-RPC id | ||
| log.Trace("received response from L1 RPC", "response", response, "err", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
To log all l1 - we should also add a print in HeaderReader when receiving events from SubscribeNewHead, same loglevel as OnResult. I think that's the only subscribe in nitro node.
We could add a hook in client for subscriptions - but since subscription is already a hook it seems redundant.
There was a problem hiding this comment.
I've added a log line there
Uses OffchainLabs/go-ethereum#192 to log all requests and responses at the trace level. The performance impact of this should be negligible compared to the network overhead of an RPC request in the first place, especially considering that the request/response will only be serialized for logging if that log level is enabled.
You can also live-reload the log level config to enable trace logging and see the L1 RPC requests and responses on an already running node.