Logging #26
@ -16,92 +16,105 @@ use getset::Setters;
|
|||||||
* Represents a valid `WANessa` configuration. Intended as a read-only singleton.
|
* Represents a valid `WANessa` configuration. Intended as a read-only singleton.
|
||||||
* See [`DEFAULTS`]
|
* See [`DEFAULTS`]
|
||||||
*/
|
*/
|
||||||
#[derive(Clone,PartialEq, Eq)]
|
#[derive(Clone, PartialEq, Eq, Getters, Setters)]
|
||||||
#[derive(Getters, Setters)]
|
#[getset(get = "pub", set = "pub")]
|
||||||
#[getset(get = "pub")]
|
|
||||||
#[allow(clippy::struct_excessive_bools)] // False positive, since it is a config struct.
|
#[allow(clippy::struct_excessive_bools)] // False positive, since it is a config struct.
|
||||||
pub struct Config {
|
pub struct Config {
|
||||||
/// See [`LogVerbosity`].<br>
|
/// See [`LogVerbosity`].<br>
|
||||||
|
|||||||
/// Default: [`Warning`]
|
/// Default: [`Warning`]
|
||||||
pub log_verbosity: LogVerbosity,
|
log_verbosity: LogVerbosity,
|
||||||
/// Logs UTC time and date of message, if true.<br>
|
/// Logs UTC time and date of message, if true.<br>
|
||||||
/// Default: `false`
|
/// Default: `false`
|
||||||
pub log_time: bool,
|
log_time: bool,
|
||||||
/// Time and date format.<br>
|
/// Time and date format.<br>
|
||||||
/// Defaults to `%F/%T:%f`.<br>
|
/// Defaults to `%F/%T:%f`.<br>
|
||||||
/// See [chrono](https://docs.rs/chrono/latest/chrono/format/strftime/index.html).<br>
|
/// See [chrono](https://docs.rs/chrono/latest/chrono/format/strftime/index.html).<br>
|
||||||
/// Default example : `2001-12-31/23:59:33:26490000`
|
/// Default example : `2001-12-31/23:59:33:26490000`
|
||||||
#[getset(skip)]
|
#[getset(skip)]
|
||||||
pub log_time_format: Option<String>,
|
log_time_format: Option<String>,
|
||||||
/// Logs location in code, where the message was logged, if true.<br>
|
/// Logs location in code, where the message was logged, if true.<br>
|
||||||
/// Default: `false`
|
/// Default: `false`
|
||||||
pub log_location: bool,
|
log_location: bool,
|
||||||
/// If `Some(path)` tries to also write the log to `path` in addition to stderr/stderr.<br>
|
/// If `Some(path)` tries to also write the log to `path` in addition to stderr/stderr.<br>
|
||||||
/// Default: [None]
|
/// Default: [None]
|
||||||
pub log_path: Option<PathBuf>,
|
log_path: Option<PathBuf>,
|
||||||
/// Logs to standard out, if true.<br>
|
/// Logs to standard out, if true.<br>
|
||||||
/// Default: `true`
|
/// Default: `true`
|
||||||
pub log_stdout: bool,
|
log_stdout: bool,
|
||||||
/// Logs to standard err, if true.<br>
|
/// Logs to standard err, if true.<br>
|
||||||
/// Default: `true`
|
/// Default: `true`
|
||||||
pub log_stderr: bool,
|
log_stderr: bool,
|
||||||
/// Database connection address.
|
/// Database connection address.
|
||||||
/// Is an option to allow constructing a default config during compile time.<br>
|
/// Is an option to allow constructing a default config during compile time.<br>
|
||||||
/// Default: `localhost`.
|
/// Default: `localhost`.
|
||||||
#[getset(skip)]
|
#[getset(skip)]
|
||||||
pub db_addr: Option<String>,
|
db_addr: Option<String>,
|
||||||
/// Database connection port.<br>
|
/// Database connection port.<br>
|
||||||
/// Default: `6969` (nice).
|
/// Default: `6969` (nice).
|
||||||
pub db_port: u16,
|
db_port: u16,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Config {
|
impl Config {
|
||||||
/// Getter for [`Self::db_addr`].
|
/// Getter for [`Self::db_addr`].
|
||||||
|
#[must_use]
|
||||||
pub fn db_addr(&self) -> &str {
|
pub fn db_addr(&self) -> &str {
|
||||||
self.db_addr.as_ref().map_or("localhost", |addr| addr)
|
self.db_addr.as_ref().map_or("localhost", |addr| addr)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Getter for the [`Self::log_time_format`].
|
/// Setter for [`Self::db_addr`].
|
||||||
pub fn log_time_format(&self) -> &str
|
pub fn set_db_addr(&mut self, db_addr: impl Into<String>) {
|
||||||
{
|
self.db_addr = Some(db_addr.into());
|
||||||
self.log_time_format.as_ref().map_or("%F-%T:%f", |fmt| fmt)
|
}
|
||||||
}
|
|
||||||
|
/// Getter for [`Self::log_time_format`].
|
||||||
|
#[must_use]
|
||||||
|
pub fn log_time_format(&self) -> &str {
|
||||||
|
todo!();
|
||||||
|
// self.log_time_format.as_ref().map_or("%F-%T:%f", |fmt| fmt)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Setter for [`Self::log_time_format`].
|
||||||
|
pub fn set_log_time_format(&mut self, format: impl Into<String>) {
|
||||||
|
self.log_time_format = Some(format.into());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// See [`DEFAULTS`].
|
/// See [`DEFAULTS`].
|
||||||
impl Default for Config {
|
impl Default for Config {
|
||||||
/// See [`DEFAULTS`].
|
/// See [`DEFAULTS`].
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
DEFAULTS
|
DEFAULTS
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Default configuration.
|
/**
|
||||||
/// ```rust
|
* Default configuration.
|
||||||
/// # use config::Config;
|
* ```rust
|
||||||
/// # use config::LogVerbosity::Warning;
|
* # use config::Config;
|
||||||
/// let DEFAULTS = Config
|
* # use config::LogVerbosity::Warning;
|
||||||
/// {
|
* let DEFAULTS = Config
|
||||||
/// log_verbosity: Warning,
|
* {
|
||||||
/// log_time: false,
|
* log_verbosity: Warning,
|
||||||
/// log_time_format: None,
|
* log_time: false,
|
||||||
/// log_location: false,
|
* log_time_format: None,
|
||||||
/// log_stdout: true,
|
* log_location: false,
|
||||||
/// log_stderr: true,
|
* log_stdout: true,
|
||||||
/// log_path: None,
|
* log_stderr: true,
|
||||||
/// db_addr: None,
|
* log_path: None,
|
||||||
/// db_port: 6969,
|
* db_addr: None,
|
||||||
/// };
|
* db_port: 6969,
|
||||||
/// # assert!(DEFAULTS == config::DEFAULTS)
|
* };
|
||||||
/// ```
|
* # assert!(DEFAULTS == config::DEFAULTS)
|
||||||
|
* ```
|
||||||
|
*/
|
||||||
pub const DEFAULTS: Config = Config {
|
pub const DEFAULTS: Config = Config {
|
||||||
log_verbosity: Warning,
|
log_verbosity: Warning,
|
||||||
log_time: false,
|
log_time: false,
|
||||||
log_time_format: None,
|
log_time_format: None,
|
||||||
log_location: false,
|
log_location: false,
|
||||||
log_stdout: true,
|
log_stdout: true,
|
||||||
log_stderr: true,
|
log_stderr: true,
|
||||||
log_path: None,
|
log_path: None,
|
||||||
db_addr: None,
|
db_addr: None,
|
||||||
db_port: 6969,
|
db_port: 6969,
|
||||||
};
|
};
|
||||||
@ -113,36 +126,31 @@ pub static CONFIG: RwLock<Config> = RwLock::new(DEFAULTS);
|
|||||||
* Each level includes the previous ones.
|
* Each level includes the previous ones.
|
||||||
*/
|
*/
|
||||||
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
|
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
|
||||||
pub enum LogVerbosity
|
pub enum LogVerbosity {
|
||||||
{
|
/// Critical Errors, may lead to crashes or deactivating certain features.
|
||||||
/// Critical Errors, may lead to crashes or deactivating certain features.
|
Error,
|
||||||
Error,
|
/// Very minor and recovered errors, such as invalid configs.
|
||||||
/// Very minor and recovered errors, such as invalid configs.
|
Warning,
|
||||||
Warning,
|
/// Very verbose and detailed. Basically gives a step-by-step instruction on what is currently done.
|
||||||
/// Very verbose and detailed. Basically gives a step-by-step instruction on what is currently done.
|
Information,
|
||||||
Information,
|
/// Very technical and even more verbose.
|
||||||
/// Very technical and even more verbose.
|
/// May contain secrets and private information.
|
||||||
/// May contain secrets and private information.
|
/// **Do not use in production environments!**
|
||||||
hendrik marked this conversation as resolved
Outdated
leon
commented
It is mentioned here, that each level includes the previous ones, but including explicit ordinals is still a good idea. > LogVerbosity doesn't specify values, but logging/src/lib.rs:75 assumes order
> -> Accidental reordering of the enum would lead to errors
It is mentioned here, that each level includes the previous ones, but including explicit ordinals is still a good idea.
leon
commented
@hendrik
|
|||||||
/// **Do not use in production environments!**
|
Debugging,
|
||||||
Debugging,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl PartialOrd for LogVerbosity
|
impl PartialOrd for LogVerbosity {
|
||||||
{
|
/// Some operator overloading of comparison symbols (==, <,>=, etc.) as syntactic sugar.
|
||||||
/// Some operator overloading of comparison symbols (==, <,>=, etc.) as syntactic sugar.
|
/// See [`PartialOrd`].
|
||||||
/// See [`PartialOrd`].
|
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
|
||||||
fn partial_cmp(&self, other: &Self) -> Option<Ordering>
|
Some(self.cmp(other))
|
||||||
{
|
}
|
||||||
(*self as usize).partial_cmp(&(*other as usize))
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Ord for LogVerbosity
|
impl Ord for LogVerbosity {
|
||||||
{
|
/// Some operator overloading of comparison symbols (==, <,>=, etc.) as syntactic sugar.
|
||||||
/// Some operator overloading of comparison symbols (==, <,>=, etc.) as syntactic sugar.
|
/// See [`Ord`].
|
||||||
/// See [`Ord`].
|
fn cmp(&self, other: &Self) -> Ordering {
|
||||||
fn cmp(&self, other: &Self) -> Ordering
|
(*self as usize).cmp(&(*other as usize))
|
||||||
{
|
}
|
||||||
(*self as usize).cmp(&(*other as usize))
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
@ -11,13 +11,13 @@ use config::LogVerbosity;
|
|||||||
|
|
||||||
use chrono::Utc;
|
use chrono::Utc;
|
||||||
|
|
||||||
use LogVerbosity::Warning;
|
|
||||||
use LogVerbosity::Information;
|
use LogVerbosity::Information;
|
||||||
|
use LogVerbosity::Warning;
|
||||||
|
|
||||||
use LogMessageType::GenericErr;
|
|
||||||
use LogMessageType::GenericWarn;
|
|
||||||
use LogMessageType::GenericInfo;
|
|
||||||
use LogMessageType::GenericDebug;
|
use LogMessageType::GenericDebug;
|
||||||
|
use LogMessageType::GenericErr;
|
||||||
|
use LogMessageType::GenericInfo;
|
||||||
|
use LogMessageType::GenericWarn;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Logs the given message.
|
* Logs the given message.
|
||||||
@ -27,55 +27,57 @@ use LogMessageType::GenericDebug;
|
|||||||
* or if another error occurs, such as a full disk.
|
* or if another error occurs, such as a full disk.
|
||||||
*/
|
*/
|
||||||
// TODO: Create macro to make last three parameters optional.
|
// TODO: Create macro to make last three parameters optional.
|
||||||
pub fn log_message(msg: &LogMessage, file: &str, line: &str, column: &str)
|
pub fn log_message(msg: &LogMessage, file: &str, line: &str, column: &str) {
|
||||||
{
|
let conf = config::CONFIG.read().unwrap_or_else(|_| {
|
||||||
let conf = config::CONFIG.read().expect(&format!("Failed aqcuire read lock on config\nTried to log message: {msg}"));
|
panic!("Failed aqcuire read lock on config\nTried to log message: {msg}")
|
||||||
|
});
|
||||||
|
|
||||||
if conf.log_verbosity() < &msg.1 { return; }
|
if conf.log_verbosity() < &msg.1 {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
let mut log_line = String::new();
|
let mut log_line = String::new();
|
||||||
|
|
||||||
// add time substring
|
// add time substring
|
||||||
if *conf.log_time()
|
if *conf.log_time() {
|
||||||
{
|
log_line += &format!("{} ", Utc::now().format(conf.log_time_format()));
|
||||||
log_line += &format!("{} ", Utc::now().format(conf.log_time_format()));
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// add code location substring
|
// add code location substring
|
||||||
if *conf.log_location()
|
if *conf.log_location() {
|
||||||
{
|
log_line += &format!("{file}:{line},{column} ");
|
||||||
leon
commented
While logging is not a dependency of the program's main function, it is a crucial part of administrating it. If the program cannot log to stdout because the feature was disabled by the admin or due to some problem, it could lead to an edge case-where the program has no logging and cannot inform the sysadmin about errors. Although I see how panicking is not the best solution. A controlled shutdown is preferable and a config Option allowing the sysadmin to set a limit on how many log-messages may be dropped within x time, without the service shutting down. > logging/src/lib.rs:46 and logging/src/lib.rs:48 will panic if an IO error occurs. Maybe consider to not panic and print another error to the console, informing about the writing problems, rather than crashing the program as logging is not a crucial feature for the program's functioning
While logging is not a dependency of the program's main function, it is a crucial part of administrating it. If the program cannot log to stdout because the feature was disabled by the admin or due to some problem, it could lead to an edge case-where the program has no logging and cannot inform the sysadmin about errors.
Although I see how panicking is not *the best* solution. A controlled shutdown is preferable and a config Option allowing the sysadmin to set a limit on how many log-messages may be dropped within x time, without the service shutting down.
@hendrik
hendrik
commented
If the stdout was disabled, panicking wouldn't tell the admin about the error either. The whole program shutting down is not a good indicator for a failure, if not shutting down is a safe alternative. The program could also notify the admin via CLI, Internet, Email or many other ways that something went wrong while still keeping up its full functionality (as from the perspective of the user). If the stdout was disabled, panicking wouldn't tell the admin about the error either. The whole program shutting down is not a good indicator for a failure, if not shutting down is a safe alternative. The program could also notify the admin via CLI, Internet, Email or many other ways that something went wrong while still keeping up its full functionality (as from the perspective of the user).
leon
commented
The error in this case is the inability to report about warnings. This edge-case happens, when there is absolutely no way of informing the sysadmin of other errors, including CLI and E-Mail, etc. I would suggest the following:
But until then, I will remove the panic and spit something into stderr, ignoring the stderr setting. The error in this case is the inability to report about warnings. This edge-case happens, when there is absolutely no way of informing the sysadmin of other errors, including CLI and E-Mail, etc.
Let's assume the program detects something which could to serious data-loss in the future. The program should warn the sysadmin about it, but not yet shut down, unless the error is possibly imminent. But if all configured ways of logging and communicating with the sysadmin fail, the service should try to shut down to prevent data loss or other unintended behavior.
I would suggest the following:
- Introduce a serious-warning verbosity between error and warning for soon-to-be errors
- implementing more log channels, such as E-Mail
- implementing some sort of unwind&shutdown macro/function as a better alternative to panic!(), which would generally be useful
- introduce a config-attribute, which determines if the service may be shut down due to errors in logging:
- What verbosity must the log at least have?
- How many log failures may happen?
- In what amount of time?
But until then, I will remove the panic and spit something into stderr, ignoring the stderr setting.
leon
commented
See See e4baaa5f45
|
|||||||
log_line += &format!("{file}:{line},{column} ");
|
}
|
||||||
}
|
|
||||||
|
|
||||||
log_line += &msg.to_string();
|
log_line += &msg.to_string();
|
||||||
|
|
||||||
// May panic if file cannot be opened or written to.
|
// May panic if file cannot be opened or written to.
|
||||||
conf.log_path().as_ref().map_or_else(
|
conf.log_path().as_ref().map_or_else(
|
||||||
|| {}, |path|
|
|| {},
|
||||||
{
|
|path| {
|
||||||
let mut file = OpenOptions::new()
|
let mut file = OpenOptions::new()
|
||||||
.write(true)
|
.write(true)
|
||||||
.append(true)
|
.append(true)
|
||||||
.open(path)
|
.open(path)
|
||||||
.expect(
|
.unwrap_or_else(|_| {
|
||||||
&format!("Could not open log file: {path:#?}")
|
panic!(
|
||||||
);
|
"Could not open log fil
|
||||||
writeln!(file, "{log_line}").expect(&format!("Could not write log to file: {path:#?}"));
|
e: {path:#?}"
|
||||||
}
|
)
|
||||||
);
|
});
|
||||||
|
writeln!(file, "{log_line}")
|
||||||
|
.unwrap_or_else(|_| panic!("Could not write log to file: {path:#?}"));
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
if msg.1 <= Warning && *conf.log_stderr()
|
if msg.1 <= Warning && *conf.log_stderr() {
|
||||||
{
|
// May panic if writing to stderr fails.
|
||||||
// May panic if writing to stderr fails.
|
eprintln!("{log_line}");
|
||||||
eprintln!("{log_line}");
|
} else if msg.1 >= Information && *conf.log_stdout() {
|
||||||
}
|
// May panic if writing to stdout fails.
|
||||||
else if msg.1 >= Information && *conf.log_stdout()
|
println!("{log_line}");
|
||||||
{
|
}
|
||||||
// May panic if writing to stdout fails.
|
|
||||||
println!("{log_line}");
|
|
||||||
}
|
|
||||||
|
|
||||||
drop(conf); // remove read lock
|
drop(conf); // remove read lock
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -83,12 +85,10 @@ pub fn log_message(msg: &LogMessage, file: &str, line: &str, column: &str)
|
|||||||
*/
|
*/
|
||||||
pub struct LogMessage(LogMessageType, LogVerbosity);
|
pub struct LogMessage(LogMessageType, LogVerbosity);
|
||||||
|
|
||||||
impl Display for LogMessage
|
impl Display for LogMessage {
|
||||||
{
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
|
||||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>
|
self.0.fmt(f) // just display LogMessageType
|
||||||
{
|
}
|
||||||
self.0.fmt(f) // just display LogMessageType
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -118,32 +118,36 @@ pub enum LogMessageType {
|
|||||||
* respectively and (hopefully) detailed messages.
|
* respectively and (hopefully) detailed messages.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
impl LogMessageType
|
impl LogMessageType {
|
||||||
{
|
/// Returns a new [`LogMessage`] based on the default verbosity of the given [`LogMessageType`].
|
||||||
/// Returns a new [`LogMessage`] based on the default verbosity of the given [`LogMessageType`].
|
#[must_use]
|
||||||
pub fn log_message(&self) -> LogMessage
|
pub fn log_message(&self) -> LogMessage {
|
||||||
{
|
let verbosity = match self {
|
||||||
let verbosity = match self
|
GenericErr(_) => LogVerbosity::Error,
|
||||||
{
|
GenericWarn(_) => LogVerbosity::Warning,
|
||||||
GenericErr(_) => LogVerbosity::Error,
|
GenericInfo(_) => LogVerbosity::Information,
|
||||||
GenericWarn(_) => LogVerbosity::Warning,
|
GenericDebug(_) => LogVerbosity::Debugging,
|
||||||
GenericInfo(_) => LogVerbosity::Information,
|
};
|
||||||
GenericDebug(_) => LogVerbosity::Debugging,
|
|
||||||
};
|
|
||||||
|
|
||||||
LogMessage(self.clone(), verbosity)
|
LogMessage(self.clone(), verbosity)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Display for LogMessageType {
|
impl Display for LogMessageType {
|
||||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error>
|
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
|
||||||
{
|
match self {
|
||||||
match self
|
GenericErr(err) => {
|
||||||
{
|
write!(f, "Generic Error: {err}")
|
||||||
GenericErr(err) => { write!(f, "Generic Error: {err}") },
|
}
|
||||||
GenericWarn(warn) => { write!(f, "Generic Warning: {warn}") },
|
GenericWarn(warn) => {
|
||||||
GenericInfo(info) => { write!(f, "Generic Information: {info}") },
|
write!(f, "Generic Warning: {warn}")
|
||||||
GenericDebug(debug) => { write!(f, "Generic Debug Message: {debug}") },
|
}
|
||||||
}
|
GenericInfo(info) => {
|
||||||
}
|
write!(f, "Generic Information: {info}")
|
||||||
|
}
|
||||||
|
GenericDebug(debug) => {
|
||||||
|
write!(f, "Generic Debug Message: {debug}")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
I considered this.
I just thought that setters are more convenient than using
mem::swap
or*config = Config::new(...)
. I also was not sure, if the config is truly read-only since we never specified if hot-reloading the config is a desired feature. Depending on how the CLI is implemented, hot-applying config-changes might also be a necessity to disable stdout logging, which would pollute the CLIs visuals.@hendrik
I assumed the config would be read only, because some attribute changes after initialization could easily cause unexpected problems. For example, changing the database server at runtime is not something the current code is designed for, nor would it be a sensible feature to implement. If the log-settings should be mutable after the initialization phase, we should add a mutable reference to some "LogPreferences" structure to our Config instead of generally supporting hot-reload for the whole Config
@leon
Seems sensible. It is also probably a good idea to split the config struct into multiple parts.
See
a787dd93e5