Add more log details #474
Add more log details #474
Conversation
There was a problem hiding this comment.
Overall: Very big fan of utilizing tracing's fields more in the messages, thanks for doing this!
I think all the problems you mentioned in the description go away if you just use a command-line flag for the log output.
I don't think we should be too smart about when we enable / disable JSON. If the app is just running within systemd, JSON logs will be pretty annoying :)
| Err(e) => { | ||
| tracing::warn!(%peer, "Failed to make State0 for execution setup: {:#}", e); | ||
| Err(error) => { | ||
| tracing::warn!(%peer, %error, "Failed to make State0 for execution setup."); |
There was a problem hiding this comment.
This change is not quite functionally equivalent. {:#} will log the whole causality chain for an anyhow::Error whereas % will just invoke regular Display. See this open ticket here: tokio-rs/tracing#1311
It is not yet resolved and I think we should really keep the causality chain otherwise debugging might be painful.
There was a problem hiding this comment.
It is not yet resolved and I think we should really keep the causality chain otherwise debugging might be painful.
Good catch, I did not know that.
There was a problem hiding this comment.
I revert and print the error message inside the message and unified somehow across all log messages.
bonomat
force-pushed
the
448-more-logging
branch
4 times, most recently
from
bc7261d to
90dd500
Compare
| let identity = seed.derive_libp2p_identity(); | ||
| let transport = transport::build_clear_net(&identity)?; | ||
| let peer_id = identity.public().into_peer_id(); | ||
| tracing::debug!("Our peer-id: {}", peer_id); | ||
| tracing::debug!(%peer_id, "Our peer-id"); |
There was a problem hiding this comment.
Same here - I think the output will repeat itself Our peer-id peer_id=...
Not really a big deal, but eventually we might want to think about some best practices for our log statements.
Co-authored-by: Daniel Karzel <[email protected]>
|
bors r+ |
474: Add more log details r=bonomat a=bonomat Resolves #448 1. The first commit adds an additional log statement of the exchange rate for each state-update. This is useful because it allows us to measure profitability easily, i.e. by knowing what was the exchange rate when the swap was started and what was it when it was finalized. 2. The second commit changes a bunch of log messages. 3. The third commit is adds a new commandline flag to toggle json format. Co-authored-by: Philipp Hoenisch <[email protected]> Co-authored-by: Philipp Hoenisch <[email protected]>
|
Build failed: |
|
docker failure :/ |
|
bors retry |
|
Already running a review |
Resolves #448