-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce tracing #1
base: develop
Are you sure you want to change the base?
Conversation
@@ -30,3 +34,28 @@ impl PeerLogMetrics { | |||
.store(target_peer_count, Ordering::Relaxed) | |||
} | |||
} | |||
|
|||
pub fn setup_tracing() -> impl Drop { | |||
let log_path = "logs/testing.log"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: proper filename
633e593
to
e59f75d
Compare
e59f75d
to
9596fa0
Compare
info!("Eth1 RPC URLs: [{}]", eth1_rpc_urls.iter().format(", ")); | ||
info!("Eth1 RPC URLs: {:?}", eth1_rpc_urls); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is required when switching from the log
to the tracing
crate, because otherwise a panic ensues
thread 'main' panicked at /home/zarathustra/.cargo/registry/src/index.crates.io-6f17d22bba15001f/itertools-0.13.0/src/format.rs:95:21:
Format: was already formatted once
This is important to note, because this is not a compilation but a runtime error, which entails that in case of this being an issue elsewhere - meaning where log
has been exchanged for tracing
- the invocation of any of the associated macro's may cause the program to panic.
This leads me to ponder the question how can best ensure this not occurring. There are cumbersome ways of achieving this, by writing individual tests for modules, or by gathering all log messages through the execution of the test suite (i.e. file, line, level, message) and ensuring that all those present in the code are found in the logs. A better approach seems to me to assess the test coverage (as in which lines are of the code are ran during execution of the test suite) and then making sure that all lines with tracing macro's have been covered; this would mean they must have logged and hence the test would have failed in case of a panic.
and start to replace the
log
crate.Update: so far in all but three directories of the repositories the
log
crate has been replaced with thetracing
crate. The modules that remain are:features/tests/both_syntaxes_produce_correct_output.rs
p2p/src/sync_manager.rs
validator/src/validator.rs
with the reason being that the
log::log
macro andlog::Level
enum are used, which do not have a direct equivalent ontracing
. Noteworthy there is a self-implementedlog
macro as wellThese will be addressed in a subsequent PR that will be opened on top of this base PR.