[cli] change default logging level for Sui CLI (Client, Move, Keytool) #13533
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
stefan-mysten
force-pushed
the
fix_logging_for_cli
branch
from
08374af
to
e7fe519
Compare
stefan-mysten
force-pushed
the
fix_logging_for_cli
branch
from
e7fe519
to
be1ad6a
Compare
| @@ -273,6 +273,10 @@ impl TelemetryConfig { | |||
| self.tokio_console = true; | |||
| } | |||
|
|
|||
| if let Ok(log_level) = env::var("RUST_LOG") { | |||
There was a problem hiding this comment.
It's an interesting find that RUST_LOG is not being read here. Couple of notes though:
RUST_LOGmay not just be a log level, it could also contain per-module levels/filters so we should check whether this does the right thing given that kind of input.- (Note, that I believe we use
tracing-rs, instead ofenv_logger, but I couldn't find its docs on how it interpretsRUST_LOGand AFAICT, it is compatible).
- (Note, that I believe we use
- From what I could tell, logging was behaving badly even when
RUST_LOGwas not set at all, which this change should not affect. Were you able to reproduce that behaviour, and if so, do these changes fix it?
There was a problem hiding this comment.
This section should be removed, RUST_LOG is already being handled elsewhere on line 304-305
There was a problem hiding this comment.
That's what happens when I code late at night (my brain stops functioning). I removed this because indeed was not needed.
From what I could tell, logging was behaving badly even when RUST_LOG was not set at all, which this change should not affect. Were you able to reproduce that behaviour, and if so, do these changes fix it?
That's a good point. I built the binaries without this change and gave it a try. The thing is that if one setsRUST_LOG, all commands will follow that log level. For example, sui start / genesis/ will all use the RUST_LOG, but so is keytool, move, client etc. It looks like in some commands we might want to useinfo!(e.g., in keytool) for easier visual debugging. Therefore, I think probably setting the log level for some commands to error is a good thing, and then the user can overwrite by setting the variable.
For some of the commands (sui start, network, genesis, etc), we should keep info level.
| @@ -273,6 +273,10 @@ impl TelemetryConfig { | |||
| self.tokio_console = true; | |||
| } | |||
|
|
|||
| if let Ok(log_level) = env::var("RUST_LOG") { | |||
There was a problem hiding this comment.
This section should be removed, RUST_LOG is already being handled elsewhere on line 304-305
stefan-mysten
force-pushed
the
fix_logging_for_cli
branch
from
be1ad6a
to
ce46b69
Compare
…nfo. This can be overwritten by setting up a RUST_LOG env variable
stefan-mysten
force-pushed
the
fix_logging_for_cli
branch
from
ce46b69
to
0421cb5
Compare
#13533) ## Description This PR changes the default logging level for Sui CLI (Client, Move, Keytool) from info to error. The logging level can be overwritten by setting the RUST_LOG env variable. ## Test Plan All tests pass, manual visual test using the Sui binary (no info logs are shown when issuing commands). --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [x] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes This PR changes the default logging level for Sui CLI (Client, Move, Keytool) from info to error. The logging level can be overwritten by setting the RUST_LOG env variable.
Description
This PR changes the default logging level for Sui CLI (Client, Move, Keytool) from info to error. The logging level can be overwritten by setting the RUST_LOG env variable.
Test Plan
All tests pass, manual visual test using the Sui binary (no info logs are shown when issuing commands).
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes
This PR changes the default logging level for Sui CLI (Client, Move, Keytool) from
infotoerror. The logging level can be overwritten by setting theRUST_LOGenv variable.