refactor(connlib): don't store WorkerGuard in Session (#2125)

The various client platforms are too different in how they handle
logging. Contrary to what I suggested in the initial PR, I no longer
think that storing the guard within the session is a good idea.

For the headless client for example, we block on CTRL+C anyway and thus
can simplify have the guard stay alive for the entirety of the program.
For Apple, we can store the guard in the `WrappedSession`. For Android,
we store it in a static variable anyway.

Lastly, whilst working on the log-upload, I've encountered circular
dependencies caused by this as I would have to store more than just the
`WorkerGuard` in the `Session`.

rust/connlib/clients/android/src/lib.rs

@@ -351,7 +351,6 @@ fn connect(
         portal_url.as_str(),
         portal_token,
         device_id,
-        None,
         callback_handler,
     )
     .map_err(Into::into)

rust/connlib/clients/apple/src/lib.rs

@@ -64,6 +64,7 @@ mod ffi {
 /// This is used by the apple client to interact with our code.
 pub struct WrappedSession {
     session: Session<CallbackHandler>,
+    _guard: WorkerGuard,
 }
 
 // SAFETY: `CallbackHandler.swift` promises to be thread-safe.
@@ -157,15 +158,17 @@ impl WrappedSession {
         log_dir: String,
         callback_handler: ffi::CallbackHandler,
     ) -> Result<Self, String> {
-        Session::connect(
+        let _guard = init_logging(log_dir.into());
+
+        let session = Session::connect(
             portal_url.as_str(),
             token,
             device_id,
-            Some(init_logging(log_dir.into())),
             CallbackHandler(callback_handler.into()),
         )
-        .map(|session| Self { session })
-        .map_err(|err| err.to_string())
+        .map_err(|err| err.to_string())?;
+
+        Ok(Self { session, _guard })
     }
 
     fn disconnect(&mut self) {

rust/connlib/clients/headless/src/main.rs

@@ -101,15 +101,9 @@ fn main() -> Result<()> {
     let secret = parse_env_var::<String>(SECRET_ENV_VAR)?;
     let device_id = get_device_id();
     let log_dir = parse_env_var::<PathBuf>(LOG_DIR_ENV_VAR).unwrap_or(DEFAULT_LOG_DIR.into());
+    let _guard = init_logging(log_dir);
 
-    let mut session = Session::connect(
-        url,
-        secret,
-        device_id,
-        Some(init_logging(log_dir)),
-        CallbackHandler,
-    )
-    .unwrap();
+    let mut session = Session::connect(url, secret, device_id, CallbackHandler).unwrap();
     tracing::info!("Started new session");
 
     block_on_ctrl_c();

rust/connlib/gateway/src/main.rs

@@ -67,7 +67,7 @@ fn main() -> Result<()> {
     let url = parse_env_var::<Url>(URL_ENV_VAR)?;
     let secret = parse_env_var::<String>(SECRET_ENV_VAR)?;
     let device_id = get_device_id();
-    let mut session = Session::connect(url, secret, device_id, None, CallbackHandler).unwrap();
+    let mut session = Session::connect(url, secret, device_id, CallbackHandler).unwrap();
 
     let (tx, rx) = std::sync::mpsc::channel();
     ctrlc::set_handler(move || tx.send(()).expect("Could not send stop signal on channel."))

rust/connlib/libs/common/src/session.rs

@@ -20,7 +20,6 @@ use crate::{
     messages::{Key, ResourceDescription},
     Error, Result,
 };
-use tracing_appender::non_blocking::WorkerGuard;
 
 pub const DNS_SENTINEL: Ipv4Addr = Ipv4Addr::new(100, 100, 111, 1);
 
@@ -56,9 +55,6 @@ pub trait ControlSession<T, CB: Callbacks> {
 /// A session is created using [Session::connect], then to stop a session we use [Session::disconnect].
 pub struct Session<T, U, V, R, M, CB: Callbacks> {
     runtime_stopper: tokio::sync::mpsc::Sender<StopRuntime>,
-    // The guard must not be dropped before the runtime is dropped, otherwise logs won't get
-    // flushed to the logfile.
-    _logging_guard: Option<WorkerGuard>,
     pub callbacks: CallbackErrorFacade<CB>,
     _phantom: PhantomData<(T, U, V, R, M)>,
 }
@@ -221,7 +217,6 @@ where
         portal_url: impl TryInto<Url>,
         token: String,
         device_id: String,
-        _logging_guard: Option<WorkerGuard>,
         callbacks: CB,
     ) -> Result<Self> {
         // TODO: We could use tokio::runtime::current() to get the current runtime
@@ -234,7 +229,6 @@ where
         let (tx, mut rx) = tokio::sync::mpsc::channel(1);
         let this = Self {
             runtime_stopper: tx.clone(),
-            _logging_guard,
             callbacks,
             _phantom: PhantomData,
         };