windows: use settings log_filter instead of env var (#2888)

Stacked on PR #2883

There will be more changes after this PR, these features are still
broken:
- Can't change filter at runtime yet, you have to restart the client app
- connlib and GUI are sending logs to the same connlib logs files, they
should be separated

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>

rust/Cargo.lock

@@ -2040,6 +2040,7 @@ dependencies = [
  "thiserror",
  "tokio",
  "tracing",
+ "tracing-log 0.2.0",
  "tracing-subscriber",
  "url",
  "uuid",

rust/windows-client/src-tauri/Cargo.toml

@@ -25,6 +25,7 @@ serde_json = "1.0"
 thiserror = { version = "1.0", default-features = false }
 tokio = { version = "1.33.0", features = ["time"] }
 tracing = { workspace = true }
+tracing-log = "0.2"
 tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
 url = { version = "2.5.0", features = ["serde"] }
 uuid = { version = "1.5.0", features = ["v4"] }

rust/windows-client/src-tauri/src/client.rs

@@ -21,6 +21,7 @@ mod gui {
 #[cfg(target_os = "windows")]
 mod gui;
 mod local_webserver;
+mod logging;
 // Relies on some types from Tauri
 #[cfg(target_os = "windows")]
 mod settings;

rust/windows-client/src-tauri/src/client/gui.rs

@@ -4,10 +4,9 @@
 // TODO: `git grep` for unwraps before 1.0, especially this gui module
 
 use crate::client::{self, AppLocalDataDir};
-use anyhow::{anyhow, bail, Result};
+use anyhow::{anyhow, bail, Context, Result};
 use client::settings::{self, AdvancedSettings};
 use connlib_client_shared::file_logger;
-use firezone_cli_utils::setup_global_subscriber;
 use secrecy::SecretString;
 use std::{
     net::{Ipv4Addr, Ipv6Addr},
@@ -96,11 +95,24 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> {
             std::fs::create_dir_all(&cwd)?;
             std::env::set_current_dir(&cwd)?;
 
-            // Set up logger with connlib_client_shared
-            let (layer, _handle) = file_logger::layer(std::path::Path::new("logs"));
-            setup_global_subscriber(layer);
+            let advanced_settings = tokio::runtime::Handle::current()
+                .block_on(settings::load_advanced_settings(&app.handle()))
+                .unwrap_or_default();
 
-            let _ctlr_task = tokio::spawn(run_controller(app.handle(), ctlr_rx));
+            // Set up logger
+            // It's hard to set it up before Tauri's setup, because Tauri knows where all the config and data go in AppData and I don't want to replicate their logic.
+
+            let logging_handles = client::logging::setup(&advanced_settings.log_filter)?;
+            tracing::info!("started log");
+
+            let app_handle = app.handle();
+            let _ctlr_task = tokio::spawn(async move {
+                if let Err(e) =
+                    run_controller(app_handle, ctlr_rx, logging_handles, advanced_settings).await
+                {
+                    tracing::error!("run_controller returned an error: {e}");
+                }
+            });
 
             // From https://github.com/FabianLars/tauri-plugin-deep-link/blob/main/example/main.rs
             let handle = app.handle();
@@ -298,6 +310,7 @@ struct Controller {
     /// The UUIDv4 device ID persisted to disk
     /// Sent verbatim to Session::connect
     device_id: String,
+    logging_handles: client::logging::Handles,
     /// Info about currently signed-in user, if there is one
     session: Option<Session>,
 }
@@ -310,15 +323,16 @@ struct Session {
 }
 
 impl Controller {
-    async fn new(app: tauri::AppHandle) -> Result<Self> {
+    async fn new(
+        app: tauri::AppHandle,
+        logging_handles: client::logging::Handles,
+        advanced_settings: AdvancedSettings,
+    ) -> Result<Self> {
         let ctlr_tx = app
             .try_state::<Managed>()
             .ok_or_else(|| anyhow::anyhow!("can't get Managed object from Tauri"))?
             .ctlr_tx
             .clone();
-        let advanced_settings = settings::load_advanced_settings(&app)
-            .await
-            .unwrap_or_default();
 
         tracing::trace!("re-loading token");
         let session: Option<Session> = tokio::task::spawn_blocking(|| {
@@ -351,6 +365,7 @@ impl Controller {
                 ctlr_tx.clone(),
                 device_id.clone(),
                 &session.token,
+                logging_handles.logger.clone(),
             )?)
         } else {
             None
@@ -361,6 +376,7 @@ impl Controller {
             ctlr_tx,
             connlib_session,
             device_id,
+            logging_handles,
             session,
         })
     }
@@ -370,14 +386,8 @@ impl Controller {
         ctlr_tx: CtlrTx,
         device_id: String,
         token: &SecretString,
+        logger: file_logger::Handle,
     ) -> Result<connlib_client_shared::Session<CallbackHandler>> {
-        let (layer, logger) = file_logger::layer(std::path::Path::new("logs"));
-        // TODO: How can I set up the tracing subscriber if the Session isn't ready yet? Check what other clients do.
-        if false {
-            // This helps the type inference
-            setup_global_subscriber(layer);
-        }
-
         tracing::info!("Session::connect");
         Ok(connlib_client_shared::Session::connect(
             advanced_settings.api_url.clone(),
@@ -394,15 +404,12 @@ impl Controller {
 async fn run_controller(
     app: tauri::AppHandle,
     mut rx: mpsc::Receiver<ControllerRequest>,
+    logging_handles: client::logging::Handles,
+    advanced_settings: AdvancedSettings,
 ) -> Result<()> {
-    let mut controller = match Controller::new(app.clone()).await {
-        Err(e) => {
-            // TODO: There must be a shorter way to write these?
-            tracing::error!("couldn't create controller: {e}");
-            return Err(e);
-        }
-        Ok(x) => x,
-    };
+    let mut controller = Controller::new(app.clone(), logging_handles, advanced_settings)
+        .await
+        .context("couldn't create Controller")?;
 
     tracing::debug!("GUI controller main loop start");
 
@@ -424,6 +431,7 @@ async fn run_controller(
                         controller.ctlr_tx.clone(),
                         controller.device_id.clone(),
                         &auth.token,
+                        controller.logging_handles.logger.clone(),
                     )?);
                     controller.session = Some(Session {
                         actor_name: auth.actor_name,

rust/windows-client/src-tauri/src/client/logging.rs

@@ -0,0 +1,30 @@
+//! Separate module to contain all the `use` statements for setting up logging
+
+use anyhow::Result;
+use connlib_client_shared::file_logger;
+use std::{path::Path, str::FromStr};
+use tracing::subscriber::set_global_default;
+use tracing_log::LogTracer;
+use tracing_subscriber::{fmt, layer::SubscriberExt, reload, EnvFilter, Layer, Registry};
+
+pub(crate) struct Handles {
+    pub logger: file_logger::Handle,
+    pub _reloader: reload::Handle<EnvFilter, Registry>,
+}
+
+/// Set up logs for the first time.
+/// Must be called inside Tauri's `setup` callback, after the app has changed directory
+pub(crate) fn setup(log_filter: &str) -> Result<Handles> {
+    let (layer, logger) = file_logger::layer(Path::new("logs"));
+    let filter = EnvFilter::from_str(log_filter)?;
+    let (filter, reloader) = reload::Layer::new(filter);
+    let subscriber = Registry::default()
+        .with(layer.with_filter(filter))
+        .with(fmt::layer().with_filter(EnvFilter::from_str(log_filter)?));
+    set_global_default(subscriber)?;
+    LogTracer::init()?;
+    Ok(Handles {
+        logger,
+        _reloader: reloader,
+    })
+}