Update logging #919
Update logging #919
Conversation
|
please explain why this change. thanks. |
|
OP updated. |
|
With current logging settings (before this PR), we need around 30GB of disk space for p2p log for first sync, and then around 200MB per hour which means 5GB per day, so need 35GB of disk space to store 7 days of log files (new default introduced in #840, was 1 day before that). In this PR I changed default log level of
|
|
Another thing changed in this PR is that console logs will be saved to files at same time by default, which I've been using in production for years. |
libraries/net/peer_connection.cpp
Outdated
| @@ -260,7 +260,7 @@ namespace graphene { namespace net | |||
| } | |||
| catch ( fc::exception& e ) | |||
| { | |||
| elog( "fatal: error connecting to peer ${remote_endpoint}: ${e}", ("remote_endpoint", remote_endpoint )("e", e.to_detail_string() ) ); | |||
| wlog( "fatal: error connecting to peer ${remote_endpoint}: ${e}", ("remote_endpoint", remote_endpoint )("e", e.to_detail_string() ) ); | |||
There was a problem hiding this comment.
Please remove "fatal" from the log message.
programs/witness_node/main.cpp
Outdated
| "filename=logs/default/default.log\n" | ||
| "# Rotate log every ? minutes, if leave out default to 60\n" | ||
| "rotation_interval=60\n" | ||
| "# how long will logs be kept (in days), if leave out default to 7\n" |
There was a problem hiding this comment.
Default limit is 1 not 7 (please also change in p2p appender). https://github.com/bitshares/bitshares-core/blob/develop/programs/witness_node/main.cpp#L367
| "# declare an appender named \"default\" that writes messages to default.log\n" | ||
| "[log.file_appender.default]\n" | ||
| "# filename can be absolute or relative to this config file\n" | ||
| "filename=logs/default/default.log\n" |
There was a problem hiding this comment.
Perhaps better call it "node.log" or "chain.log"?
There was a problem hiding this comment.
I named it default.log since it's written by the default logger in [logger.default] section.
|
@pmconrad thanks for the comments. I've made changes according to some of your comments. In regards to the dedicated delayed_node binary, IIRC we're just keeping it for backward compatibility, so IMHO no need to "fix" it. Perhaps we need to update related documents, perhaps @cedar-book can take a look. By the way, perhaps add rpc logging to |
|
Yep, adding rpc logging to the default config would be a good idea. |
|
I had created this page (https://dev.bitshares.works/en/master/bts_guide/tutorials/index.html#api) before. Probably, I'll need to add the above information (PR and explanation) also (?) |
|
@cedar-book new docs about logging is good. However we were talking about documentation about |
programs/witness_node/main.cpp
Outdated
| "appenders=p2p\n\n"; | ||
| "appenders=p2p\n\n" | ||
| "# route messages sent to the \"rpc\" logger to the \"rpc\" appender declared above,\n" | ||
| "# \"level\": \"error\" for errors only, \"warn\" for requests, \"info\" for responses.\n" |
There was a problem hiding this comment.
The level description is probably correct for the websocket api, but not for normal RPC. For simple requests with curl I didn't see responses being logged at level "info".
There was a problem hiding this comment.
Removed so far. IMHO we need a new issue to fix the inconsistency (update: here is it: #929).
This PR contains 2 changes: