-
Notifications
You must be signed in to change notification settings - Fork 44
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
dev: move from log
to tracing
#782
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #782 +/- ##
===========================================
+ Coverage 78.16% 78.25% +0.09%
===========================================
Files 159 159
Lines 8863 8891 +28
===========================================
+ Hits 6928 6958 +30
+ Misses 1935 1933 -2 ☔ View full report in Codecov by Sentry. |
@josecelano your |
Hi, @da2ce7, I guess it's because the tests are now printing to the console even if, by default, they should not capture the output. The Anyway, I will check it. I'm not sure if we use only standard output. If that is an error output, we should discard those lines. I have to review your code to check how you are printing those lines. |
Hi @da2ce7, this is the patch (fie //! Setup for the application tracing.
//!
//! It redirects the tracing info to the standard output with the tracing level defined in the configuration.
//!
//! - `Off` (i.e. don't load any subscriber...)
//! - `Error`
//! - `Warn`
//! - `Info`
//! - `Debug`
//! - `Trace`
//!
//! Refer to the [configuration crate documentation](https://docs.rs/torrust-tracker-configuration) to know how to change tracing settings.
use std::sync::Once;
use torrust_tracker_configuration::{Configuration, TraceLevel};
use tracing::info;
use tracing::level_filters::LevelFilter;
static INIT: Once = Once::new();
/// It redirects the tracing info to the standard output with the tracing level defined in the configuration
pub fn setup(cfg: &Configuration) {
let level = config_level_or_default(&cfg.tracing_max_verbosity_level);
if level == LevelFilter::OFF {
return;
}
INIT.call_once(|| {
stdout_config(level);
});
}
/// Option `tracing_max_verbosity_level` in configuration is mandatory but it can
/// be empty. The values are:
///
/// ```text
/// trace_level = "" -> Default is Info
/// trace_level = "Off"
/// trace_level = "Error"
/// trace_level = "Warn"
/// trace_level = "Info"
/// trace_level = "Debug"
/// trace_level = "Trace"
/// ```
fn config_level_or_default(trace_level: &TraceLevel) -> LevelFilter {
if trace_level.to_string().is_empty() {
return LevelFilter::OFF;
}
trace_level
.to_string()
.parse()
.unwrap_or_else(|_| panic!("Tracing level filter is not valid: {trace_level}"))
}
fn stdout_config(filter: LevelFilter) {
let () = tracing_subscriber::fmt().pretty().with_max_level(filter).init();
info!("tracing initialized.");
} The problem was the filter level running tests was not "Off" but the default one "Info". By the way, good PR! UPDATEMaybe, we should consider not accepting an empty string for the filter level in the configuration. It would make sense to default to "Info" if the whole option is not present in the config file. |
@da2ce7 one question, can we disable the location in the output for production:
I would add the line UPDATE That's also making some E2E tests fail.
However, I have yet to check if the problem is parsing the output. |
fd9f45a
to
fe15f24
Compare
fe15f24
to
9f71c47
Compare
Replaces log with fern. Better async support.
9f71c47
to
a086cff
Compare
Relates to: #727 |
Transition to new modern library, with proper support for async instrumentation.