github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(connlib): recreate log file if it got deleted (#8926)

Browse Source 
Currently, when `connlib`'s log file gets deleted, we write logs into
nirvana until the corresponding process gets restarted. This is painful
for users to do because they need to restart the IPC service or Network
Extension. Instead, we can simply check if the log file exists prior to
writing to it and re-create it if it doesn't.

Resolves: #6850
Related: #7569
This commit is contained in:
Thomas Eizinger
2025-04-29 23:05:02 +10:00
committed by GitHub
parent 6114bb274f
commit 122d84cfa2
7 changed files with 91 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
rust/Cargo.lock generated
View File

@@ -2391,6 +2391,7 @@ dependencies = [
"rand 0.8.5",
"sentry-tracing",
"supports-color",
"tempfile",
"thiserror 1.0.69",
"time",
"tracing",

115
rust/headless-client/src/clear_logs.rs
View File

@@ -1,11 +1,7 @@
use anyhow::{Context, Result};
use std::{
ffi::OsStr,
io::ErrorKind::NotFound,
path::{Path, PathBuf},
};
use std::{io::ErrorKind::NotFound, path::Path};
/// Deletes all `.log` and `.jsonl` files in `path` except the most recent
/// Deletes all `.log` files in `path`.
pub async fn clear_logs(path: &Path) -> Result<()> {
let mut dir = match tokio::fs::read_dir(path).await {
Ok(x) => x,
@@ -18,96 +14,49 @@ pub async fn clear_logs(path: &Path) -> Result<()> {
return Err(error.into());
}
};
let mut paths = vec![];
while let Some(entry) = dir.next_entry().await? {
paths.push(entry.path());
}
let mut result = Ok(());
// If we can't delete some files due to permission errors, just keep going
// and delete as much as we can, then return the most recent error
let mut result = Ok(());
let to_delete = choose_logs_to_delete(&paths);
for path in &to_delete {
if let Err(e) = tokio::fs::remove_file(path).await {
while let Some(entry) = dir
.next_entry()
.await
.context("Failed to read next dir entry")?
{
if entry
.file_name()
.to_str()
.is_none_or(|name| !name.ends_with("log"))
{
continue;
}
if let Err(e) = tokio::fs::remove_file(entry.path()).await {
result = Err(e);
}
}
result.context("Failed to delete at least one file")
}
fn choose_logs_to_delete(paths: &[PathBuf]) -> Vec<&Path> {
let mut most_recent_stem = None;
for path in paths {
if path.extension() != Some(OsStr::new("log")) {
continue;
}
let Some(stem) = path.file_stem() else {
continue;
};
match most_recent_stem {
None => most_recent_stem = Some(stem),
Some(most_recent) if stem > most_recent => most_recent_stem = Some(stem),
Some(_) => {}
}
}
let Some(most_recent_stem) = most_recent_stem else {
tracing::warn!(
"Nothing to delete, should be impossible since both processes always write logs"
);
return vec![];
};
let Some(most_recent_stem) = most_recent_stem.to_str() else {
tracing::warn!("Most recent log file does not have a UTF-8 path");
return vec![];
};
paths
.iter()
.filter_map(|path| {
// Don't delete files if we can't parse their stems as UTF-8.
let stem = path.file_stem()?.to_str()?;
(stem < most_recent_stem).then_some(path.as_path())
})
.collect()
}
#[cfg(test)]
mod tests {
use std::path::Path;
use super::*;
#[tokio::test]
async fn only_deletes_log_files() {
let dir = tempfile::tempdir().unwrap();
std::fs::write(dir.path().join("first.log"), "log file 1").unwrap();
std::fs::write(dir.path().join("second.log"), "log file 1").unwrap();
std::fs::write(dir.path().join("not_a_logfile.tmp"), "something important").unwrap();
clear_logs(dir.path()).await.unwrap();
#[test]
fn clear_logs_logic() {
// These are out of order just to make sure it works anyway
let paths: Vec<_> = [
"connlib.2024-08-05-19-41-46.jsonl",
"connlib.2024-08-05-19-41-46.log",
"connlib.2024-08-07-14-17-56.jsonl",
"connlib.2024-08-07-14-17-56.log",
"connlib.2024-08-06-14-21-13.jsonl",
"connlib.2024-08-06-14-21-13.log",
"connlib.2024-08-06-14-51-19.jsonl",
"connlib.2024-08-06-14-51-19.log",
"crash.2024-07-22-21-16-20.dmp",
"last_crash.dmp",
]
.into_iter()
.map(|x| Path::new("/bogus").join(x))
.collect();
let to_delete = super::choose_logs_to_delete(&paths);
assert_eq!(
to_delete,
[
"/bogus/connlib.2024-08-05-19-41-46.jsonl",
"/bogus/connlib.2024-08-05-19-41-46.log",
"/bogus/connlib.2024-08-06-14-21-13.jsonl",
"/bogus/connlib.2024-08-06-14-21-13.log",
"/bogus/connlib.2024-08-06-14-51-19.jsonl",
"/bogus/connlib.2024-08-06-14-51-19.log",
]
.into_iter()
.map(Path::new)
.collect::<Vec<_>>()
std::fs::read_to_string(dir.path().join("not_a_logfile.tmp")).unwrap(),
"something important"
);
}
}

1
rust/logging/Cargo.toml
View File

@@ -21,6 +21,7 @@ tracing-log = { workspace = true }
tracing-subscriber = { workspace = true, features = ["env-filter"] }
[dev-dependencies]
tempfile = { workspace = true }
thiserror = { workspace = true }
[lints]

48
rust/logging/src/file.rs
View File

@@ -119,6 +119,17 @@ impl Appender {
ret
}
Some((_, filename))
if !std::fs::exists(self.directory.join(&filename)).unwrap_or_default() =>
{
let (mut file, name) = self.create_new_writer()?;
let ret = cb(&mut file);
self.current = Some((file, name));
ret
}
Some((file, _)) => cb(file),
}
}
@@ -197,3 +208,40 @@ impl io::Write for Appender {
self.with_current_file(|f| f.flush())
}
}
#[cfg(test)]
mod tests {
use std::time::Duration;
use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt};
use super::*;
#[test]
fn deleting_log_file_creates_new_one() {
let dir = tempfile::tempdir().unwrap();
let (layer, _handle) = layer(dir.path(), "connlib");
let _guard = tracing_subscriber::registry()
.with(layer)
.with(tracing_subscriber::EnvFilter::from("info"))
.set_default();
tracing::info!("This is a test");
std::thread::sleep(Duration::from_millis(1000)); // Wait a bit until background thread has flushed the log.
for dir in std::fs::read_dir(dir.path()).unwrap() {
let dir = dir.unwrap();
std::fs::remove_file(dir.path()).unwrap();
}
tracing::info!("Write after delete");
std::thread::sleep(Duration::from_millis(1000)); // Wait a bit until background thread has flushed the log.
let content = std::fs::read_to_string(dir.path().join("latest")).unwrap();
assert!(content.contains("Write after delete"))
}
}

3
website/src/components/Changelog/Android.tsx
View File

@@ -24,6 +24,9 @@ export default function Android() {
Improves connection reliability by maintaining the order of IP packets
across GSO batches.
</ChangeItem>
<ChangeItem pull="8926">
Rolls over to a new log-file as soon as logs are cleared.
</ChangeItem>
</Unreleased>
<Entry version="1.4.7" date={new Date("2025-04-21")}>
<ChangeItem pull="8798">

3
website/src/components/Changelog/Apple.tsx
View File

@@ -34,6 +34,9 @@ export default function Apple() {
Improves connection reliability by maintaining the order of IP packets
across GSO batches.
</ChangeItem>
<ChangeItem pull="8926">
Rolls over to a new log-file as soon as logs are cleared.
</ChangeItem>
</Unreleased>
<Entry version="1.4.12" date={new Date("2025-04-21")}>
<ChangeItem pull="8798">

3
website/src/components/Changelog/GUI.tsx
View File

@@ -34,6 +34,9 @@ export default function GUI({ os }: { os: OS }) {
Improves connection reliability by maintaining the order of IP packets.
</ChangeItem>
)}
<ChangeItem pull="8926">
Rolls over to a new log-file as soon as logs are cleared.
</ChangeItem>
</Unreleased>
<Entry version="1.4.11" date={new Date("2025-04-21")}>
<ChangeItem pull="8798">