rpc service and stageloop logger updates #7696
Conversation
cmd/rpcdaemon/cli/config.go
Outdated
| httpEndpoint := fmt.Sprintf("%s:%d", cfg.HttpListenAddress, cfg.HttpPort) | ||
|
|
||
| logger.Trace("TraceRequests = %t\n", cfg.TraceRequests) | ||
| logger.Trace(fmt.Sprintf("TraceRequests = %t", cfg.TraceRequests)) |
There was a problem hiding this comment.
@mh0lt hi.
- add prefix like [rpc] - because erigon has many components
- logs formatting is expensive and this is reason why usually "lazy". For example: if log-level is "Info" then no reason to format all Trace messages. Our logger support "lazy" formatting - just change it to logger.Trace("[rpc] config", "TraceRequests", cfg.TraceRequests")
- rpcdaemon doesn't print much of logs at startup (same as Erigon) - so, probably no much reasons to hide this log line (i mean - maybe can increase to Info) level. And maybe even print many other cfg fields in this log line.
There was a problem hiding this comment.
I would just use logger.Trace("TraceRequests", "on", cfg.TraceRequests) or remove these lines altogether
There was a problem hiding this comment.
Hi, happy to remove the lines.
I have a couple of questions though:
Isn't something like [rpc] supported by the logger context ? Then you don't need to add it for every log message.
Also is there a way of checking the current log level in order to just avoid the execution cost of preparing the log output in the first place, even if the logger is not formatting it will still incur the execution time of any parameter preparation ?
There was a problem hiding this comment.
Yes, I think we need to start using logger context instead of prefixes.
Not sure about the way of checking the level, I think it is not possible because it is ultimately decided by the log handles that filter out the log messages before they are directed to a specific place (console, file, etc.)
This is another update to logging to replace the root logger with a contextual logger