Skip to content

Commit

Permalink
Fix logger initialization on readonly filesystems
Browse files Browse the repository at this point in the history
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems.

Fixes valkey-io#2387
  • Loading branch information
vnuka-n committed Oct 4, 2024
1 parent fb51ba6 commit 0b9a0f8
Showing 1 changed file with 44 additions and 5 deletions.
49 changes: 44 additions & 5 deletions logger_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@
* Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0
*/
use once_cell::sync::OnceCell;
use std::sync::RwLock;
use std::{
path::{Path, PathBuf},
sync::RwLock,
};
use tracing::{self, event};
use tracing_appender::rolling::{RollingFileAppender, Rotation};
use tracing_appender::rolling::{RollingFileAppender, RollingWriter, Rotation};
use tracing_subscriber::{
filter::Filtered,
fmt::{
Expand All @@ -28,7 +31,7 @@ type InnerLayered = Layered<reload::Layer<InnerFiltered, Registry>, Registry>;
// A reloadable layer of subscriber to a rolling file
type FileReload = Handle<
Filtered<
Layer<InnerLayered, DefaultFields, Format, RollingFileAppender>,
Layer<InnerLayered, DefaultFields, Format, LazyRollingFileAppender>,
LevelFilter,
InnerLayered,
>,
Expand All @@ -48,6 +51,42 @@ pub static INITIATE_ONCE: InitiateOnce = InitiateOnce {
init_once: OnceCell::new(),
};

struct LazyRollingFileAppender {
file_appender: OnceCell<RollingFileAppender>,
rotation: Rotation,
directory: PathBuf,
filename_prefix: PathBuf,
}

impl LazyRollingFileAppender {
fn new(
rotation: Rotation,
directory: impl AsRef<Path>,
filename_prefix: impl AsRef<Path>,
) -> LazyRollingFileAppender {
LazyRollingFileAppender {
file_appender: OnceCell::new(),
rotation,
directory: directory.as_ref().to_path_buf(),
filename_prefix: filename_prefix.as_ref().to_path_buf(),
}
}
}

impl<'a> tracing_subscriber::fmt::writer::MakeWriter<'a> for LazyRollingFileAppender {
type Writer = RollingWriter<'a>;
fn make_writer(&'a self) -> Self::Writer {
let file_appender = self.file_appender.get_or_init(|| {
RollingFileAppender::new(
self.rotation.clone(),
self.directory.clone(),
self.filename_prefix.clone(),
)
});
file_appender.make_writer()
}
}

#[derive(Debug)]
pub enum Level {
Error = 0,
Expand Down Expand Up @@ -84,7 +123,7 @@ pub fn init(minimal_level: Option<Level>, file_name: Option<&str>) -> Level {

let (stdout_layer, stdout_reload) = reload::Layer::new(stdout_fmt);

let file_appender = RollingFileAppender::new(
let file_appender = LazyRollingFileAppender::new(
Rotation::HOURLY,
"glide-logs",
file_name.unwrap_or("output.log"),
Expand Down Expand Up @@ -130,7 +169,7 @@ pub fn init(minimal_level: Option<Level>, file_name: Option<&str>) -> Level {
});
}
Some(file) => {
let file_appender = RollingFileAppender::new(Rotation::HOURLY, "glide-logs", file);
let file_appender = LazyRollingFileAppender::new(Rotation::HOURLY, "glide-logs", file);
let _ = reloads
.file_reload
.write()
Expand Down

0 comments on commit 0b9a0f8

Please sign in to comment.