From 67b94747144a9b056263be48a31767724cd3327b Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Mon, 18 Jul 2022 12:21:25 -0700 Subject: [PATCH] appender: add a builder for constructing `RollingFileAppender`s (#2227) ## Motivation Several currently open PRs, such as #2225 and #2221, add new configuration parameters to the rolling file appender in `tracing-appender`. The best way to add new optional configuration settings without breaking existing APIs or creating a very large number of new constructors is to add a builder interface. ## Solution Since a number of PRs would all need to add the builder API, introducing potential conflicts, this branch _just_ adds the builder interface without adding any new configuration options. Once this merges, the existing in-flight PRs can be rebased onto this branch to use the builder interface without conflicting with each other. Also, the `Builder::build` method is fallible and returns a `Result`, rather than panicking. This is a relatively common pattern in Rust --- for example, `std::thread::Builder::spawn` returns a `Result` if a new thread cannot be spawned, while `std::thread::spawn` simply panics. This allows users to handle appender initialization errors gracefully without breaking the API of the existing `new` constructor. Fixes #1953 Signed-off-by: Eliza Weisman --- tracing-appender/Cargo.toml | 1 + tracing-appender/src/rolling.rs | 160 ++++++++++++++++------- tracing-appender/src/rolling/builder.rs | 167 ++++++++++++++++++++++++ 3 files changed, 280 insertions(+), 48 deletions(-) create mode 100644 tracing-appender/src/rolling/builder.rs diff --git a/tracing-appender/Cargo.toml b/tracing-appender/Cargo.toml index 2d02b38e95..9f4e52139b 100644 --- a/tracing-appender/Cargo.toml +++ b/tracing-appender/Cargo.toml @@ -24,6 +24,7 @@ rust-version = "1.53.0" crossbeam-channel = "0.5.0" time = { version = "0.3", default-features = false, features = ["formatting"] } parking_lot = { optional = true, version = "0.12.0" } +thiserror = "1" [dependencies.tracing-subscriber] path = "../tracing-subscriber" diff --git a/tracing-appender/src/rolling.rs b/tracing-appender/src/rolling.rs index db1099403f..d4b2ee6fcf 100644 --- a/tracing-appender/src/rolling.rs +++ b/tracing-appender/src/rolling.rs @@ -31,11 +31,14 @@ use std::{ fmt::{self, Debug}, fs::{self, File, OpenOptions}, io::{self, Write}, - path::Path, + path::{Path, PathBuf}, sync::atomic::{AtomicUsize, Ordering}, }; use time::{format_description, Duration, OffsetDateTime, Time}; +mod builder; +pub use builder::{Builder, InitError}; + /// A file appender with the ability to rotate log files at a fixed schedule. /// /// `RollingFileAppender` implements the [`std:io::Write` trait][write] and will @@ -98,8 +101,8 @@ pub struct RollingWriter<'a>(RwLockReadGuard<'a, File>); #[derive(Debug)] struct Inner { - log_directory: String, - log_filename_prefix: String, + log_directory: PathBuf, + log_filename_prefix: Option, rotation: Rotation, next_date: AtomicUsize, } @@ -122,8 +125,10 @@ impl RollingFileAppender { /// - [`Rotation::daily()`][daily], /// - [`Rotation::never()`][never()] /// + /// Additional parameters can be configured using [`RollingFileAppender::builder`]. /// /// # Examples + /// /// ```rust /// # fn docs() { /// use tracing_appender::rolling::{RollingFileAppender, Rotation}; @@ -133,16 +138,63 @@ impl RollingFileAppender { pub fn new( rotation: Rotation, directory: impl AsRef, - file_name_prefix: impl AsRef, + filename_prefix: impl AsRef, ) -> RollingFileAppender { + let filename_prefix = filename_prefix + .as_ref() + .to_str() + .expect("filename prefix must be a valid UTF-8 string"); + Self::builder() + .rotation(rotation) + .filename_prefix(filename_prefix) + .build(directory) + .expect("initializing rolling file appender failed") + } + + /// Returns a new [`Builder`] for configuring a `RollingFileAppender`. + /// + /// The builder interface can be used to set additional configuration + /// parameters when constructing a new appender. + /// + /// Unlike [`RollingFileAppender::new`], the [`Builder::build`] method + /// returns a `Result` rather than panicking when the appender cannot be + /// initialized. Therefore, the builder interface can also be used when + /// appender initialization errors should be handled gracefully. + /// + /// # Examples + /// + /// ```rust + /// # fn docs() { + /// use tracing_appender::rolling::{RollingFileAppender, Rotation}; + /// + /// let file_appender = RollingFileAppender::builder() + /// .rotation(Rotation::HOURLY) // rotate log files once every hour + /// .filename_prefix("myapp") // log file names will be prefixed with `myapp.` + /// .build("/var/log") // try to build an appender that stores log files in `/var/log` + /// .expect("initializing rolling file appender failed"); + /// # drop(file_appender); + /// # } + /// ``` + #[must_use] + pub fn builder() -> Builder { + Builder::new() + } + + fn from_builder(builder: &Builder, directory: impl AsRef) -> Result { + let Builder { + ref rotation, + ref prefix, + } = builder; + let filename_prefix = prefix.clone(); + let directory = directory.as_ref().to_path_buf(); let now = OffsetDateTime::now_utc(); - let (state, writer) = Inner::new(now, rotation, directory, file_name_prefix); - Self { + let (state, writer) = Inner::new(now, rotation.clone(), directory, filename_prefix)?; + Ok(Self { state, writer, #[cfg(test)] now: Box::new(OffsetDateTime::now_utc), - } + }) } #[inline] @@ -428,35 +480,42 @@ impl Rotation { } } - pub(crate) fn join_date(&self, filename: &str, date: &OffsetDateTime) -> String { - match *self { + pub(crate) fn join_date(&self, filename: Option<&str>, date: &OffsetDateTime) -> String { + let date = match *self { Rotation::MINUTELY => { let format = format_description::parse("[year]-[month]-[day]-[hour]-[minute]") .expect("Unable to create a formatter; this is a bug in tracing-appender"); - - let date = date - .format(&format) - .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); - format!("{}.{}", filename, date) + date.format(&format) + .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender") } Rotation::HOURLY => { let format = format_description::parse("[year]-[month]-[day]-[hour]") .expect("Unable to create a formatter; this is a bug in tracing-appender"); - - let date = date - .format(&format) - .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); - format!("{}.{}", filename, date) + date.format(&format) + .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender") } Rotation::DAILY => { let format = format_description::parse("[year]-[month]-[day]") .expect("Unable to create a formatter; this is a bug in tracing-appender"); - let date = date - .format(&format) - .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender"); - format!("{}.{}", filename, date) + date.format(&format) + .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender") + } + Rotation::NEVER => { + // If there's a name prefix, use that. + if let Some(filename) = filename { + return filename.to_owned(); + } + + // Otherwise, just use the date. + let format = format_description::parse("[year]-[month]-[day]") + .expect("Unable to create a formatter; this is a bug in tracing-appender"); + date.format(&format) + .expect("Unable to format OffsetDateTime; this is a bug in tracing-appender") } - Rotation::NEVER => filename.to_string(), + }; + match filename { + Some(filename) => format!("{}.{}", filename, date), + None => date, } } } @@ -480,20 +539,16 @@ impl Inner { now: OffsetDateTime, rotation: Rotation, directory: impl AsRef, - file_name_prefix: impl AsRef, - ) -> (Self, RwLock) { - let log_directory = directory.as_ref().to_str().unwrap(); - let log_filename_prefix = file_name_prefix.as_ref().to_str().unwrap(); - - let filename = rotation.join_date(log_filename_prefix, &now); + log_filename_prefix: Option, + ) -> Result<(Self, RwLock), builder::InitError> { + let log_directory = directory.as_ref().to_path_buf(); + let filename = rotation.join_date(log_filename_prefix.as_deref(), &now); let next_date = rotation.next_date(&now); - let writer = RwLock::new( - create_writer(log_directory, &filename).expect("failed to create appender"), - ); + let writer = RwLock::new(create_writer(log_directory.as_ref(), &filename)?); let inner = Inner { - log_directory: log_directory.to_string(), - log_filename_prefix: log_filename_prefix.to_string(), + log_directory, + log_filename_prefix, next_date: AtomicUsize::new( next_date .map(|date| date.unix_timestamp() as usize) @@ -501,11 +556,13 @@ impl Inner { ), rotation, }; - (inner, writer) + Ok((inner, writer)) } fn refresh_writer(&self, now: OffsetDateTime, file: &mut File) { - let filename = self.rotation.join_date(&self.log_filename_prefix, &now); + let filename = self + .rotation + .join_date(self.log_filename_prefix.as_deref(), &now); match create_writer(&self.log_directory, &filename) { Ok(new_file) => { @@ -552,20 +609,22 @@ impl Inner { } } -fn create_writer(directory: &str, filename: &str) -> io::Result { - let path = Path::new(directory).join(filename); +fn create_writer(directory: &Path, filename: &str) -> Result { + let path = directory.join(filename); let mut open_options = OpenOptions::new(); open_options.append(true).create(true); let new_file = open_options.open(path.as_path()); if new_file.is_err() { if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - return open_options.open(path); + fs::create_dir_all(parent).map_err(InitError::ctx("failed to create log directory"))?; + return open_options + .open(path) + .map_err(InitError::ctx("failed to create initial log file")); } } - new_file + new_file.map_err(InitError::ctx("failed to create initial log file")) } #[cfg(test)] @@ -673,19 +732,19 @@ mod test { let now = OffsetDateTime::parse("2020-02-01 10:01:00 +00:00:00", &format).unwrap(); // per-minute - let path = Rotation::MINUTELY.join_date("app.log", &now); + let path = Rotation::MINUTELY.join_date(Some("app.log"), &now); assert_eq!("app.log.2020-02-01-10-01", path); // per-hour - let path = Rotation::HOURLY.join_date("app.log", &now); + let path = Rotation::HOURLY.join_date(Some("app.log"), &now); assert_eq!("app.log.2020-02-01-10", path); // per-day - let path = Rotation::DAILY.join_date("app.log", &now); + let path = Rotation::DAILY.join_date(Some("app.log"), &now); assert_eq!("app.log.2020-02-01", path); // never - let path = Rotation::NEVER.join_date("app.log", &now); + let path = Rotation::NEVER.join_date(Some("app.log"), &now); assert_eq!("app.log", path); } @@ -702,8 +761,13 @@ mod test { let now = OffsetDateTime::parse("2020-02-01 10:01:00 +00:00:00", &format).unwrap(); let directory = tempfile::tempdir().expect("failed to create tempdir"); - let (state, writer) = - Inner::new(now, Rotation::HOURLY, directory.path(), "test_make_writer"); + let (state, writer) = Inner::new( + now, + Rotation::HOURLY, + directory.path(), + Some("test_make_writer".to_string()), + ) + .unwrap(); let clock = Arc::new(Mutex::new(now)); let now = { diff --git a/tracing-appender/src/rolling/builder.rs b/tracing-appender/src/rolling/builder.rs new file mode 100644 index 0000000000..82161f0cc6 --- /dev/null +++ b/tracing-appender/src/rolling/builder.rs @@ -0,0 +1,167 @@ +use super::{RollingFileAppender, Rotation}; +use std::{io, path::Path}; +use thiserror::Error; + +/// A [builder] for configuring [`RollingFileAppender`]s. +/// +/// [builder]: https://rust-unofficial.github.io/patterns/patterns/creational/builder.html +#[derive(Debug)] +pub struct Builder { + pub(super) rotation: Rotation, + pub(super) prefix: Option, +} + +/// Errors returned by [`Builder::build`]. +#[derive(Error, Debug)] +#[error("{context}: {source}")] +pub struct InitError { + context: &'static str, + #[source] + source: io::Error, +} + +impl InitError { + pub(crate) fn ctx(context: &'static str) -> impl FnOnce(io::Error) -> Self { + move |source| Self { context, source } + } +} + +impl Builder { + /// Returns a new `Builder` for configuring a [`RollingFileAppender`], with + /// the default parameters. + /// + /// # Default Values + /// + /// The default values for the builder are: + /// + /// | Parameter | Default Value | Notes | + /// | :-------- | :------------ | :---- | + /// | [`rotation`] | [`Rotation::NEVER`] | By default, log files will never be rotated. | + /// | [`filename_prefix`] | `""` | By default, log file names will not have a prefix. | + /// + /// [`rotation`]: Self::rotation + /// [`filename_prefix`]: Self::filename_prefix + #[must_use] + pub const fn new() -> Self { + Self { + rotation: Rotation::NEVER, + prefix: None, + } + } + + /// Sets the [rotation strategy] for log files. + /// + /// By default, this is [`Rotation::NEVER`]. + /// + /// # Examples + /// + /// ``` + /// # fn docs() { + /// use tracing_appender::rolling::{Rotation, RollingFileAppender}; + /// + /// let appender = RollingFileAppender::builder() + /// .rotation(Rotation::HOURLY) // rotate log files once every hour + /// // ... + /// .build("/var/log") + /// .expect("failed to initialize rolling file appender"); + /// + /// # drop(appender) + /// # } + /// ``` + /// + /// [rotation strategy]: Rotation + #[must_use] + pub fn rotation(self, rotation: Rotation) -> Self { + Self { rotation, ..self } + } + + /// Sets the prefix for log filenames. The prefix is output before the + /// timestamp in the file name, and if it is non-empty, it is followed by a + /// dot (`.`). + /// + /// By default, log files do not have a prefix. + /// + /// # Examples + /// + /// Setting a prefix: + /// + /// ``` + /// use tracing_appender::rolling::RollingFileAppender; + /// + /// # fn docs() { + /// let appender = RollingFileAppender::builder() + /// .filename_prefix("myapp.log") // log files will have names like "myapp.log.2019-01-01" + /// // ... + /// .build("/var/log") + /// .expect("failed to initialize rolling file appender"); + /// # drop(appender) + /// # } + /// ``` + /// + /// No prefix: + /// + /// ``` + /// use tracing_appender::rolling::RollingFileAppender; + /// + /// # fn docs() { + /// let appender = RollingFileAppender::builder() + /// .filename_prefix("") // log files will have names like "2019-01-01" + /// // ... + /// .build("/var/log") + /// .expect("failed to initialize rolling file appender"); + /// # drop(appender) + /// # } + /// ``` + /// + /// [rotation strategy]: Rotation + #[must_use] + pub fn filename_prefix(self, prefix: impl Into) -> Self { + let prefix = prefix.into(); + // If the configured prefix is the empty string, then don't include a + // separator character. + let prefix = if prefix.is_empty() { + None + } else { + Some(prefix) + }; + Self { prefix, ..self } + } + + /// Builds a new [`RollingFileAppender`] with the configured parameters, + /// emitting log files to the provided directory. + /// + /// Unlike [`RollingFileAppender::new`], this returns a `Result` rather than + /// panicking when the appender cannot be initialized. + /// + /// # Examples + /// + /// ``` + /// use tracing_appender::rolling::{Rotation, RollingFileAppender}; + /// + /// # fn docs() { + /// let appender = RollingFileAppender::builder() + /// .rotation(Rotation::DAILY) // rotate log files once per day + /// .filename_prefix("myapp.log") // log files will have names like "myapp.log.2019-01-01" + /// .build("/var/log/myapp") // write log files to the '/var/log/myapp' directory + /// .expect("failed to initialize rolling file appender"); + /// # drop(appender); + /// # } + /// ``` + /// + /// This is equivalent to + /// ``` + /// # fn docs() { + /// let appender = tracing_appender::rolling::daily("myapp.log", "/var/log/myapp"); + /// # drop(appender); + /// # } + /// ``` + pub fn build(&self, directory: impl AsRef) -> Result { + RollingFileAppender::from_builder(self, directory) + } +} + +impl Default for Builder { + fn default() -> Self { + Self::new() + } +}