Skip to content

Logging updates prior to supporting configuration through the API #246

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 3, 2018
Merged

Logging updates prior to supporting configuration through the API #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions logger/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use std::fmt;
pub enum LoggerError {
/// First attempt at initialization failed.
NeverInitialized(String),
/// The logger does not allow reinitialization.
AlreadyInitialized,
/// Initialization has previously failed and can not be retried.
Poisoned(String),
/// Creating log file fails.
Expand All @@ -24,6 +26,7 @@ impl fmt::Display for LoggerError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let printable = match *self {
LoggerError::NeverInitialized(ref e) => e,
LoggerError::AlreadyInitialized => "Reinitialization of logger not allowed",
LoggerError::Poisoned(ref e) => e,
LoggerError::CreateLogFile(ref e) => e.description(),
LoggerError::FileLogWrite(ref e) => e.description(),
Expand All @@ -47,6 +50,7 @@ mod tests {
LoggerError::NeverInitialized(String::from("Bad Log Path Provided"))
).contains("NeverInitialized")
);
assert!(format!("{:?}", LoggerError::AlreadyInitialized).contains("AlreadyInitialized"));
assert!(
format!(
"{:?}",
Expand Down
45 changes: 25 additions & 20 deletions logger/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,11 @@ pub const DEFAULT_LEVEL: Level = Level::Warn;

/// Synchronization primitives used to run a one-time global initialization.
///
const UNINITIALIZED: usize = 0;
const INITIALIZED: usize = 1;

static INIT: Once = ONCE_INIT;
static mut INIT_RES: Result<()> = Ok(());
static mut INIT_RES: Result<usize> = Ok(UNINITIALIZED);

/// Logger representing the logging subsystem.
///
Expand Down Expand Up @@ -132,9 +135,8 @@ impl Logger {
/// warn!("This will print 'WARN' surrounded by square brackets followed by log message");
/// }
/// ```
pub fn set_include_level(mut self, option: bool) -> Self {
pub fn set_include_level(&mut self, option: bool) {
self.show_level = option;
self
}

/// Enables or disables including the file path and the line numbers in the tag of the log message.
Expand All @@ -156,16 +158,14 @@ impl Logger {
/// warn!("This will print '[WARN:file_path.rs:155]' followed by log message");
/// }
/// ```
pub fn set_include_origin(mut self, file_path: bool, line_numbers: bool) -> Self {
pub fn set_include_origin(&mut self, file_path: bool, line_numbers: bool) {
self.show_file_path = file_path;
self.show_line_numbers = line_numbers;

//buut if the file path is not shown, do not show line numbers either
if !self.show_file_path {
self.show_line_numbers = false;
}

self
}

/// Explicitly sets the log level for the Logger.
Expand All @@ -190,12 +190,11 @@ impl Logger {
/// .unwrap();
/// }
/// ```
pub fn set_level(mut self, level: Level) -> Self {
pub fn set_level(&mut self, level: Level) {
self.level_info.code = level;
if self.level_info.writer != Destination::File {
self.level_info.writer = get_default_destination(level);
}
self
}

/// Creates the first portion (to the left of the separator)
Expand Down Expand Up @@ -259,7 +258,10 @@ impl Logger {
"{}",
INIT_RES.as_ref().err().unwrap()
)));
} else if INIT_RES.is_ok() && INIT_RES.as_ref().unwrap() != &UNINITIALIZED {
return Err(LoggerError::AlreadyInitialized);
}

INIT.call_once(|| {
if let Some(path) = log_path.as_ref() {
match FileLogWriter::new(path) {
Expand All @@ -283,6 +285,8 @@ impl Logger {
"{}",
INIT_RES.as_ref().err().unwrap()
)));
} else {
INIT_RES = Ok(INITIALIZED);
}
}
Ok(())
Expand Down Expand Up @@ -382,13 +386,14 @@ mod tests {

#[test]
fn test_init() {
let l = Logger::new().set_include_origin(false, true);
let mut l = Logger::new();
l.set_include_origin(false, true);
assert_eq!(l.show_line_numbers, false);

let l = Logger::new()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to be able to do the configuration of the logger (i.e. call the set functions) progressively. In the first version where the functions would return self I would only be able to do it from one call (Logger::new.set_bla.set_bla2 ...). But in the case of the API I need to be able to parse the config that I receive and call the set functions separately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

.set_include_origin(true, true)
.set_include_level(true)
.set_level(log::Level::Info);
let mut l = Logger::new();
l.set_include_origin(true, true);
l.set_include_level(true);
l.set_level(log::Level::Info);
assert_eq!(l.show_line_numbers, true);
assert_eq!(l.show_file_path, true);
assert_eq!(l.show_level, true);
Expand All @@ -398,7 +403,7 @@ mod tests {
warn!("warning");

let l = Logger::new();
assert!(l.init(None).is_ok());
assert!(l.init(None).is_err());

info!("info");
warn!("warning");
Expand Down Expand Up @@ -431,15 +436,15 @@ mod tests {
assert!(format!("{:?}", INIT_RES).contains("Poisoned"));
}

let l = Logger::new()
.set_include_level(true)
.set_include_origin(false, false);
let mut l = Logger::new();
l.set_include_level(true);
l.set_include_origin(false, false);
let error_metadata = MetadataBuilder::new().level(Level::Error).build();
let log_record = log::Record::builder().metadata(error_metadata).build();
Logger::log(&l, &log_record);
let l = Logger::new()
.set_include_level(false)
.set_include_origin(true, true);
let mut l = Logger::new();
l.set_include_level(false);
l.set_include_origin(true, true);
Logger::log(&l, &log_record);
}

Expand Down
16 changes: 9 additions & 7 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,22 +163,24 @@ fn vmm_no_api_handler(
).expect("cannot create VMM");

// this is temporary; user will be able to customize logging subsystem through API
let mut l = Logger::new();
l.set_level(logger::Level::Info);
if cmd_arguments.is_present("log_file") {
if let Err(e) = Logger::new()
.set_level(logger::Level::Info)
.init(Some(String::from(
cmd_arguments.value_of("log_file").unwrap(),
))) {
if let Err(e) = l.init(Some(String::from(
cmd_arguments.value_of("log_file").unwrap(),
))) {
eprintln!(
"main: Failed to initialize logging subsystem: {:?}. Trying without a file",
e
);
if let Err(e) = Logger::new().set_level(logger::Level::Info).init(None) {
l = Logger::new();
l.set_level(logger::Level::Info);
if let Err(e) = l.init(None) {
panic!("main: Failed to initialize logging subsystem: {:?}", e);
}
}
} else {
if let Err(e) = Logger::new().set_level(logger::Level::Info).init(None) {
if let Err(e) = l.init(None) {
panic!("main: Failed to initialize logging subsystem: {:?}", e);
}
}
Expand Down