3 files changed, 73 insertions, 40 deletions

diff --git a/mullvad-daemon/src/logging.rs b/mullvad-daemon/src/logging.rs
index c1aa521680..8ad461e4bf 100644
--- a/mullvad-daemon/src/logging.rs
+++ b/mullvad-daemon/src/logging.rs
@@ -46,7 +46,6 @@ impl<'a, T: Clone + io::Write> MakeWriter<'a> for OptionalMakeWriter<T> {
 }
 
 pub const WARNING_SILENCED_CRATES: &[&str] = &["netlink_proto", "quinn_udp"];
-const DAEMON_LOG_FILENAME: &str = "daemon.log";
 pub const SILENCED_CRATES: &[&str] = &[
     "h2",
     "tokio_core",
@@ -92,6 +91,27 @@ pub struct LogHandle {
     _file_appender_guard: Option<non_blocking::WorkerGuard>,
 }
 
+/// A location to put logs.
+///
+/// It is necessary to logically separate the directory from the absolute path of the log file due
+/// to the API of [`tracing_appender::rolling`].
+#[derive(Clone, Debug, PartialEq)]
+pub struct LogLocation {
+    /// The directory where the logs will be recorded.
+    pub directory: PathBuf,
+    /// The filename where the logs will be recorded (relative to [Self::directory]).
+    pub filename: PathBuf,
+}
+
+impl LogLocation {
+    /// Construct the final path of the log file made up by the components of this [`LogLocation`].
+    ///
+    /// `self.directory/self.filename`
+    pub fn log_path(&self) -> PathBuf {
+        self.directory.join(&self.filename)
+    }
+}
+
 /// A simple, asynchronous log sink.
 ///
 /// To read from a [`LogStreamer`] sink, check out the associated [`LogHandle`] and [`LogHandle::get_log_stream`].
@@ -142,9 +162,14 @@ impl LogHandle {
     }
 }
 
+/// Initialize a global logger.
+///
+/// * log_level: Base log level, used if `RUST_LOG` is not set.
+/// * log_location: Path to the log file, see [`LogLocation`].
+/// * output_timestamp: Whether timestamps should be included in the log output.
 pub fn init_logger(
     log_level: log::LevelFilter,
-    log_dir: Option<&PathBuf>,
+    log_location: Option<LogLocation>,
     output_timestamp: bool,
 ) -> Result<LogHandle, Error> {
     let level_filter = match log_level {
@@ -162,13 +187,16 @@ pub fn init_logger(
     let default_filter = silence_crates(env_filter);
 
     // TODO: Switch this to a rolling appender, likely daily or hourly
-    let (_file_appender_guard, non_blocking_file_appender) = if let Some(log_dir) = log_dir {
-        let file_appender = tracing_appender::rolling::never(log_dir, DAEMON_LOG_FILENAME);
-        let (appender, guard) = non_blocking(file_appender);
-        (Some(guard), OptionalMakeWriter(Some(appender)))
-    } else {
-        (None, OptionalMakeWriter(None))
-    };
+    let (_file_appender_guard, non_blocking_file_appender) =
+        if let Some(log_location) = log_location.as_ref() {
+            rotate_log(&log_location.log_path()).map_err(Error::RotateLog)?;
+            let file_appender =
+                tracing_appender::rolling::never(&log_location.directory, &log_location.filename);
+            let (appender, guard) = non_blocking(file_appender);
+            (Some(guard), OptionalMakeWriter(Some(appender)))
+        } else {
+            (None, OptionalMakeWriter(None))
+        };
 
     let (tx, _) = tokio::sync::broadcast::channel(128);
     let log_stream = LogStreamer { tx };
@@ -185,10 +213,6 @@ pub fn init_logger(
         .with_ansi(true)
         .with_span_events(FmtSpan::NEW | FmtSpan::CLOSE);
 
-    if let Some(log_dir) = log_dir {
-        rotate_log(&log_dir.join(DAEMON_LOG_FILENAME)).map_err(Error::RotateLog)?;
-    }
-
     #[cfg(all(target_os = "android", debug_assertions))]
     let reg = {
         let android_layer = paranoid_android::layer("mullvad-daemon");
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index ea0e9d395a..bbc99ca7ee 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -3,8 +3,9 @@ use std::{path::PathBuf, thread, time::Duration};
 #[cfg(not(windows))]
 use mullvad_daemon::cleanup_old_rpc_socket;
 use mullvad_daemon::{
-    Daemon, DaemonCommandChannel, DaemonConfig, exception_logging, logging, rpc_uniqueness_check,
-    runtime, version,
+    Daemon, DaemonCommandChannel, DaemonConfig, exception_logging,
+    logging::{self, LogLocation},
+    rpc_uniqueness_check, runtime, version,
 };
 use talpid_types::ErrorExt;
 
@@ -59,9 +60,10 @@ async fn run() -> Result<(), String> {
             // uniqueness check must happen before logging initializaton,
             // as initializing logs will rotate any existing log file.
             assert_unique().await?;
-            let (log_dir, reload_handle) = init_daemon_logging(config)?;
+            let (log_location, reload_handle) = init_daemon_logging(config)?;
             log::trace!("Using configuration: {:?}", config);
 
+            let log_dir = log_location.map(|l| l.directory);
             run_standalone(log_dir, reload_handle).await
         }
 
@@ -110,29 +112,36 @@ async fn assert_unique() -> Result<(), &'static str> {
 /// Initialize logging to stderr and to file (if configured).
 fn init_daemon_logging(
     config: &cli::Config,
-) -> Result<(Option<PathBuf>, logging::LogHandle), String> {
-    let log_dir = get_log_dir(config)?;
+) -> Result<(Option<LogLocation>, logging::LogHandle), String> {
+    let log_location = get_log_dir(config)?.map(|directory| LogLocation {
+        directory,
+        filename: PathBuf::from("daemon.log"),
+    });
 
-    let reload_handle = init_logger(config, log_dir.clone())?;
+    let reload_handle = init_logger(config, log_location.clone())?;
 
-    if let Some(ref log_dir) = log_dir {
-        log::info!("Logging to {}", log_dir.display());
+    if let Some(log_location) = log_location.as_ref() {
+        log::info!("Logging to {}", log_location.log_path().display());
     }
-    Ok((log_dir, reload_handle))
+    Ok((log_location, reload_handle))
 }
 
 /// Initialize logging to stder and to the [`EARLY_BOOT_LOG_FILENAME`]
 #[cfg(target_os = "linux")]
 fn init_early_boot_logging(config: &cli::Config) {
+    let logging = get_log_dir(config)
+        .ok()
+        .flatten()
+        .map(|log_dir| LogLocation {
+            directory: log_dir,
+            filename: PathBuf::from(EARLY_BOOT_LOG_FILENAME),
+        });
+
     // If it's possible to log to the filesystem - attempt to do so, but failing that mustn't stop
     // the daemon from starting here.
-    if let Ok(Some(log_dir)) = get_log_dir(config)
-        && init_logger(config, Some(log_dir.join(EARLY_BOOT_LOG_FILENAME))).is_ok()
-    {
-        return;
-    }
-
-    let _ = init_logger(config, None);
+    if init_logger(config, logging).is_err() {
+        let _ = init_logger(config, None);
+    };
 }
 
 /// Initialize logging to stderr and to file (if provided).
@@ -140,26 +149,23 @@ fn init_early_boot_logging(config: &cli::Config) {
 /// Also install the [exception_logging] signal handler to log faults.
 fn init_logger(
     config: &cli::Config,
-    log_file: Option<PathBuf>,
+    log_location: Option<LogLocation>,
 ) -> Result<logging::LogHandle, String> {
     #[cfg(unix)]
-    if let Some(log_file) = &log_file {
+    if let Some(log_location) = log_location.as_ref() {
         use std::os::unix::ffi::OsStrExt;
 
         exception_logging::set_log_file(
-            std::ffi::CString::new(log_file.as_os_str().as_bytes())
+            std::ffi::CString::new(log_location.log_path().as_os_str().as_bytes())
                 .map_err(|_| "Log file path contains null-bytes".to_string())?,
         );
     }
 
     exception_logging::enable();
 
-    let reload_handle = logging::init_logger(
-        config.log_level,
-        log_file.as_ref(),
-        config.log_stdout_timestamps,
-    )
-    .map_err(|e| e.display_chain_with_msg("Unable to initialize logger"))?;
+    let reload_handle =
+        logging::init_logger(config.log_level, log_location, config.log_stdout_timestamps)
+            .map_err(|e| e.display_chain_with_msg("Unable to initialize logger"))?;
     log_panics::init();
     version::log_version();
     Ok(reload_handle)
diff --git a/mullvad-daemon/src/system_service.rs b/mullvad-daemon/src/system_service.rs
index a45fd01ebb..6425779d6f 100644
--- a/mullvad-daemon/src/system_service.rs
+++ b/mullvad-daemon/src/system_service.rs
@@ -62,7 +62,7 @@ windows_service::define_windows_service!(service_main, handle_service_main);
 
 pub fn handle_service_main(_arguments: Vec<OsString>) {
     let config = cli::get_config();
-    let (log_dir, reload_handle) =
+    let (log_location, reload_handle) =
         init_daemon_logging(config).expect("Failed to initialize logging");
     log::info!("Service started.");
 
@@ -115,7 +115,10 @@ pub fn handle_service_main(_arguments: Vec<OsString>) {
         Ok(runtime) => runtime,
     };
 
-    let result = runtime.block_on(crate::create_daemon(log_dir, reload_handle));
+    let result = runtime.block_on(crate::create_daemon(
+        log_location.map(|l| l.directory),
+        reload_handle,
+    ));
     let result = if let Ok(daemon) = result {
         let shutdown_handle = daemon.shutdown_handle();