Skip to content

Commit

Permalink
[antlir2] tracing improvements
Browse files Browse the repository at this point in the history
Summary:
A collection of improvements to make antlir2 logs more useful

- remove unnecessary context that massively bloats logs (eg dumping all eden
 redirections on every line)
- add more `err, ret` instrumentation to get traces even if the feature prints
 nothing

Test Plan: testhard

Reviewed By: epilatow

Differential Revision: D49834806

fbshipit-source-id: 63542f1cc611e82631d43f9d3bc8b256caefa45a
  • Loading branch information
vmagro authored and facebook-github-bot committed Oct 3, 2023
1 parent 873f8f1 commit 7458e93
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 22 deletions.
1 change: 0 additions & 1 deletion antlir/antlir2/antlir2/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ rust_binary(
"serde_json",
"thiserror",
"tracing",
"tracing-glog",
"tracing-subscriber",
"//antlir:find_root",
"//antlir/antlir2/antlir2_btrfs:antlir2_btrfs",
Expand Down
2 changes: 1 addition & 1 deletion antlir/antlir2/antlir2/src/cmd/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl Compile {
}

impl Compile {
#[tracing::instrument(name = "compile", skip(self))]
#[tracing::instrument(name = "compile", skip(self), ret, err)]
pub(crate) fn run(self) -> Result<()> {
let ctx = self
.compileish
Expand Down
4 changes: 2 additions & 2 deletions antlir/antlir2/antlir2/src/cmd/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Subcommand {

impl Map {
/// Create a new mutable subvolume based on the [SetupArgs].
#[tracing::instrument(skip(self), ret, err)]
#[tracing::instrument(skip(self, rootless), ret, err)]
fn create_new_subvol(
&self,
working_volume: &WorkingVolume,
Expand All @@ -136,7 +136,7 @@ impl Map {
Ok(subvol)
}

#[tracing::instrument(name = "map", skip(self))]
#[tracing::instrument(name = "map", skip_all, ret, err)]
pub(crate) fn run(
self,
log_path: Option<&Path>,
Expand Down
17 changes: 2 additions & 15 deletions antlir/antlir2/antlir2/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,10 @@ fn main() -> Result<()> {
let args = Args::parse();

tracing_subscriber::registry()
.with(
tracing_subscriber::fmt::Layer::default()
.event_format(
tracing_glog::Glog::default()
.with_span_context(true)
.with_timer(tracing_glog::LocalTime::default()),
)
.fmt_fields(tracing_glog::GlogFields::default()),
)
.with(tracing_subscriber::fmt::Layer::default().with_ansi(false))
.with(args.log.file()?.map(|file| {
tracing_subscriber::fmt::Layer::default()
.event_format(
tracing_glog::Glog::default()
.with_span_context(true)
.with_timer(tracing_glog::LocalTime::default()),
)
.fmt_fields(tracing_glog::GlogFields::default())
.with_ansi(false)
.with_writer(file)
}))
.init();
Expand Down
21 changes: 18 additions & 3 deletions antlir/antlir2/antlir2_working_volume/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

use std::fmt::Debug;
use std::fs::File;
use std::io::ErrorKind;
use std::os::fd::AsRawFd;
Expand All @@ -25,6 +26,7 @@ use nix::fcntl::OFlag;
use nix::sys::stat::Mode;
use serde::Deserialize;
use tracing::trace;
use tracing::trace_span;
use tracing::warn;
use uuid::Uuid;

Expand Down Expand Up @@ -53,12 +55,20 @@ pub struct WorkingVolume {
eden: Option<EdenInfo>,
}

#[derive(Debug, Clone)]
#[derive(Clone)]
struct EdenInfo {
repo_root: PathBuf,
redirections: Vec<EdenRedirection>,
}

impl Debug for EdenInfo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("EdenInfo")
.field("repo_root", &self.repo_root)
.finish_non_exhaustive()
}
}

#[derive(Debug, Clone, Deserialize)]
struct EdenRedirection {
repo_path: PathBuf,
Expand Down Expand Up @@ -210,6 +220,9 @@ impl WorkingVolume {
for entry in std::fs::read_dir(&self.path)? {
let entry = entry?;
if entry.path().is_dir() {
let span =
trace_span!("collect_garbage", path = entry.path().display().to_string());
let _guard = span.enter();
// skip outputs that are very new to avoid any race condition
// between creating the output and setting the keepalive
if let Ok(mtime) = entry.metadata().and_then(|meta| meta.modified()) {
Expand All @@ -228,10 +241,12 @@ impl WorkingVolume {
},
}?;
if delete {
trace!("{} is no longer referenced", entry.path().display());
trace!("subvol is no longer referenced");
try_delete_subvol(&entry.path());
if let Err(e) = std::fs::remove_file(&keepalive_path) {
warn!("failed to remove keepalive file: {e}");
if e.kind() != std::io::ErrorKind::NotFound {
warn!("failed to remove keepalive file: {e}");
}
}
}
}
Expand Down

0 comments on commit 7458e93

Please sign in to comment.