Fix/403/log debug sources #405
Closed
+56
−22
Conversation
This file contains hidden or 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
- parse RUST_LOG env var and normalize to lowercase
- detect debug mode by checking for any substring matching `=debug`
- construct EnvFilter from RUST_LOG with fallback to "info" if unset
- configure tracing_subscriber::fmt::Layer with conditional toggles:
* with_file(true/false)
* with_line_number(true/false)
* with_target(true/false)
- registry initialized with fmt_layer to apply formatting globally
Implements enhancement requested in flashbots#403
- add temp-env as a dev-dependency
- add #[cfg(test)] module in crates/cli/src/main.rs
- use temp_env::with_var to set RUST_LOG temporarily
- verify debug mode detection logic:
* RUST_LOG="contender=debug" → assert contains("=debug")
* RUST_LOG="contender=info" → assert !contains("=debug")
- ensures environment isolation between tests without global state
leakage
Adds test case for the enhancement implemented in flashbots#403
- add entry under Unreleased in crates/cli/CHANGELOG.md - add corresponding summary under Unreleased in root CHANGELOG.md Adds changelog entries for enhancement implemented in flashbots#403
zeroXbrock
reviewed
Dec 31, 2025
Member
zeroXbrock
left a comment
zeroXbrock
left a comment
There was a problem hiding this comment.
thanks for the quick turnaround @LilSuperUser, left a few comments
| #[cfg(not(feature = "async-tracing"))] | ||
| { | ||
| contender_core::util::init_core_tracing(filter); | ||
| #[test] |
Member
There was a problem hiding this comment.
same as below, we don't need to test the functionality of an external crate ourselves
|
|
||
| #[test] | ||
| fn debug_mode_detected_from_rust_log() { | ||
| with_var("RUST_LOG", Some("contender=debug"), || { |
Member
There was a problem hiding this comment.
I don't think this test is really necessary, all it does is test the with_var function.
|
|
||
| fn init_tracing() { | ||
| let filter = EnvFilter::try_from_default_env().ok(); // fallback if RUST_LOG is unset | ||
| #[cfg(feature = "async-tracing")] |
Member
There was a problem hiding this comment.
we still need the async-tracing conditional, looks like it was just removed...
Member
|
closing in favor of #406 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 (#403)
The CLI’s tracing output was previously verbose, always including file, line, and target information regardless of log level. This added noise for normal usage at
infolevel.We wanted to improve UX by showing full source context only when explicitly running in debug mode.
Additionally, the detection logic for debug mode (
contains("=debug")) is custom, so it requires test coverage to ensure correctness.Finally, changelogs needed to be updated to reflect the enhancement and its associated tests.
Solution
Logging enhancement
RUST_LOGenv var and normalize to lowercase.=debug.EnvFilterfromRUST_logwith fallback to"info"if unset.tracing_subscriber::fmt::Layerwith conditional toggles:with_file(true/false)with_line_number(true/false)with_target(true/false)Scoped test case
temp-envas a dev-dependency.moduele incrates/cli/src/main.rs`.temp_env::with_varto setRUST_LOGtemp in each test scope.RUST_LOG="contender=debug"→debug_modeistrue.RUST_LOG="contender=info"→debug_modeisfalse.PR Checklist
cargo +nightly clippy --workspace --lib --examples --tests --benches --all-features --locked --fixcargo fmt --allCHANGELOG.mdin each affected crate