Expose log-to-std, fix log timestamp.[CPP-109] #171
Conversation
| pub fn from_filtered_cli() -> CliOptions { | ||
| let args = std::env::args(); | ||
| debug!("args {:?}", args); | ||
| eprintln!("args {:?}", args); |
There was a problem hiding this comment.
Seemed overkill to have this and filtered args display in the console log so this only goes to standard error.
| filtered_args.push(arg); | ||
| } | ||
| debug!("filtered_args: {:?}", filtered_args); | ||
| debug!("filtered_args: {:?}", &filtered_args[1..]); |
There was a problem hiding this comment.
Remove the first argument which equates to the python3 to only show arguments passed to console.
console_backend/src/server.rs
Outdated
| } | ||
| } else { | ||
| eprintln!("no client receive endpoint"); | ||
| debug!("no client receive endpoint"); |
There was a problem hiding this comment.
Not entirely sure what to do here. These debug statements never show, I think there is an issue with logging from within a PyObject. Either we keep them as eprintln or remove them entirely. What are your thoughts @silverjam?
There was a problem hiding this comment.
Can we try with the headless console and see if they show? That's using env_logger not our custom logger.
There was a problem hiding this comment.
Our headless console does use our custom logger.
https://github.com/swift-nav/console_pp/blob/71b465c13f570583b1f3003f1a76e52a4912c5a7/console_backend/src/bin/headless-console.rs#L31
There was a problem hiding this comment.
I was thinking of the fileio module: https://github.com/swift-nav/console_pp/blob/f8366907c72f55ce5c6d1b40e23d255d09afaf5c/console_backend/src/bin/fileio.rs#L51 -- might try enabling that here too and see what happens
There was a problem hiding this comment.
@silverjam Looking at this again, no we cannot try this in headless console because these messages are in server.rs. These two functions fetch_message and start are the only things we do not run in headless-console. The headless-console is more or less a duplicate of start but tailored for lifetime testing.
There was a problem hiding this comment.
We could abstract these functions outside of server.rs and call them from the external location. The headless runner would still need to be separate though to add the message counting. Maybe in a separate PR.
There was a problem hiding this comment.
I'm fine with pushing fix-ups to another PR, but I was suggesting just throwing in a call to env_logger::init(); to see if it caught the log messages we're missing. If it does then it means we'll need to rework how we're setting up our custom log destination so that it happens earlier.
There was a problem hiding this comment.
Let's put these back to eprintln! for now -- see https://pyo3.rs/v0.12.4/logging.html -- they log through the Python logging system, but the logger is setup during module initialization
| reset_device: false, | ||
| advanced_networking_update: None, | ||
| auto_survey_data: AutoSurveyData::new(), | ||
| log_to_std: ArcBool::new_with(true), |
There was a problem hiding this comment.
Start with true so the initial "args" will go to the terminal. Then once the command line options are parsed, if log-to-std is not enabled it will switch back to false.
.
john-michaelburke
force-pushed
the
john-michaelburke/debug-cli
branch
from
1736b7c to
ce1b1d0
Compare
Co-authored-by: Jason Mobarak <[email protected]>
Co-authored-by: Jason Mobarak <[email protected]>
There was a problem hiding this comment.
Currently solution is an improvement but we need to move logging around so that it happens earlier and (maybe) re-work logging so that it happens via Python logging module: https://pyo3.rs/v0.12.4/logging.html -- @john-michaelburke can you log a Jira for this please?
console_backend/src/server.rs
Outdated
| } | ||
| } else { | ||
| eprintln!("no client receive endpoint"); | ||
| debug!("no client receive endpoint"); |
There was a problem hiding this comment.
Let's put these back to eprintln! for now -- see https://pyo3.rs/v0.12.4/logging.html -- they log through the Python logging system, but the logger is setup during module initialization
|
3 participants
cargo make run -- --log-to-std