Clean up verbosity 0 logging messages and display one-line block sync #3328
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s to maximize user perception of block sync progress of a working, healthy node.
joske
approved these changes
Jun 24, 2024
raychu86
suggested changes
Jul 2, 2024
| @@ -352,7 +352,7 @@ impl<N: Network, C: ConsensusStorage<N>> LedgerService<N> for CoreLedgerService< | |||
| metrics::update_block_metrics(block); | |||
| } | |||
|
|
|||
| tracing::info!("\n\nAdvanced to block {} at round {} - {}\n", block.height(), block.round(), block.hash()); | |||
| tracing::info!("[{}] {}", block.height(), block.hash()); | |||
There was a problem hiding this comment.
While more verbose, the previous message of Advanced to lock {} at round {} has been very valuable for debugging and parsing logs.
| @@ -175,7 +175,7 @@ impl<N: Network> Gateway<N> { | |||
| worker_senders: IndexMap<u8, WorkerSender<N>>, | |||
| sync_sender: Option<SyncSender<N>>, | |||
| ) { | |||
| debug!("Starting the gateway for the memory pool..."); | |||
| trace!("Starting the gateway for the memory pool..."); | |||
There was a problem hiding this comment.
Suggested change
| trace!("Starting the gateway for the memory pool..."); | |
| info!("Starting the gateway for the memory pool..."); |
This should be an info log like all the other module bootup messages.
| @@ -524,7 +524,7 @@ impl<N: Network> Gateway<N> { | |||
| // If the event was unable to be sent, disconnect. | |||
| if let Err(e) = &result { | |||
| warn!("{CONTEXT} Failed to send '{name}' to '{peer_ip}': {e}"); | |||
| debug!("{CONTEXT} Disconnecting from '{peer_ip}' (unable to send)"); | |||
| trace!("{CONTEXT} Disconnecting from '{peer_ip}' (unable to send)"); | |||
There was a problem hiding this comment.
Suggested change
| trace!("{CONTEXT} Disconnecting from '{peer_ip}' (unable to send)"); | |
| warn!("{CONTEXT} Disconnecting from '{peer_ip}' - Failed to send '{name}': {e}"); |
We can unify the 2 log messages to a single one that matches the other disconnect warn messages.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Currently verbosity settings of the client are mixed with info, debug, and trace log levels. When a user starts the node, the mix of messages is confusing and it's not clear if a node is connecting, syncing blocks, and working properly. It's important to give immediate feedback to users that their node is syncing as quickly as possible without distractions. This patch does two important things:
--verbosity 0it only displaysinfomessages that give meaningful information to the user about: peer connections (initial, drops, and refreshes) and block sync progress.\n\n's from the block advance and instead only prints: [{BLOCK_NUM}] {BLOCK_HASH} which gives a clear message to the user every time a block is added to the ledger. When run with verbosity 0, the primary information a user sees is blocks being added to the ledger. This gives immediate and clear feedback to the user that their client node is syncing properly and it shows the performance of the node.Here's an example of
--verbosity 0output using this PR:Several files where changed from
info!todebug!and vice versa. Somedebug!s were changed totrace!as some of the modules override the usual log levels which causes trace and debug logs to be produced at verbosity 0.Test Plan
Compiled and run using
--verbosity 0and the client output matches the above screenshot.