From 652fe67dcec7d6c8e65fc9de77b2a55c06e54f92 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 17 May 2024 00:02:40 +0000 Subject: [PATCH 1/5] add tracing macros Signed-off-by: jiaxiao zhou --- crates/shim/Cargo.toml | 1 + crates/shim/src/args.rs | 3 +++ crates/shim/src/asynchronous/mod.rs | 10 ++++++++++ crates/shim/src/cgroup.rs | 11 +++++++++++ crates/shim/src/lib.rs | 2 ++ crates/shim/src/synchronous/mod.rs | 10 ++++++++++ 6 files changed, 37 insertions(+) diff --git a/crates/shim/Cargo.toml b/crates/shim/Cargo.toml index d1b89543..62cbad70 100644 --- a/crates/shim/Cargo.toml +++ b/crates/shim/Cargo.toml @@ -53,6 +53,7 @@ serde.workspace = true serde_json.workspace = true thiserror.workspace = true time.workspace = true +tracing = "0.1" # Async dependencies async-trait = { workspace = true, optional = true } diff --git a/crates/shim/src/args.rs b/crates/shim/src/args.rs index 325f4792..8f0ff77c 100644 --- a/crates/shim/src/args.rs +++ b/crates/shim/src/args.rs @@ -16,6 +16,8 @@ use std::ffi::OsStr; +use tracing::{instrument, Span}; + use crate::error::{Error, Result}; /// Flags to be passed from containerd daemon to a shim binary. @@ -44,6 +46,7 @@ pub struct Flags { } /// Parses command line arguments passed to the shim. +#[instrument(skip_all, parent = Span::current(), level= "Info")] pub fn parse>(args: &[S]) -> Result { let mut flags = Flags::default(); diff --git a/crates/shim/src/asynchronous/mod.rs b/crates/shim/src/asynchronous/mod.rs index 8189edda..841cc7bf 100644 --- a/crates/shim/src/asynchronous/mod.rs +++ b/crates/shim/src/asynchronous/mod.rs @@ -48,6 +48,7 @@ use nix::{ }; use signal_hook_tokio::Signals; use tokio::{io::AsyncWriteExt, sync::Notify}; +use tracing::{instrument, Span}; use crate::{ args, @@ -99,6 +100,7 @@ pub trait Shim { } /// Async Shim entry point that must be invoked from tokio `main`. +#[instrument(parent = Span::current(), level= "Info")] pub async fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -109,6 +111,7 @@ where } } +#[instrument(parent = Span::current(), level= "Info")] async fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -239,6 +242,7 @@ impl ExitSignal { /// Spawn is a helper func to launch shim process asynchronously. /// Typically this expected to be called from `StartShim`. +#[instrument(parent = Span::current(), level= "Info")] pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; @@ -299,6 +303,7 @@ pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Ok(address) } +#[instrument(skip_all, parent = Span::current(), level= "Info")] fn setup_signals_tokio(config: &Config) -> Signals { if config.no_reaper { Signals::new([SIGTERM, SIGINT, SIGPIPE]).expect("new signal failed") @@ -307,6 +312,7 @@ fn setup_signals_tokio(config: &Config) -> Signals { } } +#[instrument(skip_all, parent = Span::current(), level= "Info")] async fn handle_signals(signals: Signals) { let mut signals = signals.fuse(); while let Some(sig) = signals.next().await { @@ -360,12 +366,14 @@ async fn handle_signals(signals: Signals) { } } +#[instrument(parent = Span::current(), level= "Info")] async fn remove_socket_silently(address: &str) { remove_socket(address) .await .unwrap_or_else(|e| warn!("failed to remove socket: {}", e)) } +#[instrument(parent = Span::current(), level= "Info")] async fn remove_socket(address: &str) -> Result<()> { let path = parse_sockaddr(address); if let Ok(md) = Path::new(path).metadata() { @@ -380,6 +388,7 @@ async fn remove_socket(address: &str) -> Result<()> { Ok(()) } +#[instrument(skip_all, parent = Span::current(), level= "Info")] async fn start_listener(address: &str) -> Result { let addr = address.to_string(); asyncify(move || -> Result { @@ -391,6 +400,7 @@ async fn start_listener(address: &str) -> Result { .await } +#[instrument(parent = Span::current(), level= "Info")] async fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { diff --git a/crates/shim/src/cgroup.rs b/crates/shim/src/cgroup.rs index 5e4222c0..d7ba3c67 100644 --- a/crates/shim/src/cgroup.rs +++ b/crates/shim/src/cgroup.rs @@ -31,12 +31,14 @@ use containerd_shim_protos::{ shim::oci::Options, }; use oci_spec::runtime::LinuxResources; +use tracing::{instrument, Span}; use crate::error::{Error, Result}; // OOM_SCORE_ADJ_MAX is from https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h#L10 const OOM_SCORE_ADJ_MAX: i64 = 1000; +#[instrument(parent = Span::current(), level= "Info")] pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { if pid == 0 { return Ok(()); @@ -62,6 +64,7 @@ pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { } /// Add a process to the given relative cgroup path +#[instrument(parent = Span::current(), level= "Info")] pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { let h = hierarchies::auto(); // use relative path here, need to trim prefix '/' @@ -74,6 +77,7 @@ pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { /// Sets the OOM score for the process to the parents OOM score + 1 /// to ensure that they parent has a lower score than the shim +#[instrument(parent = Span::current(), level= "Info")] pub fn adjust_oom_score(pid: u32) -> Result<()> { let score = read_process_oom_score(std::os::unix::process::parent_id())?; if score < OOM_SCORE_ADJ_MAX { @@ -82,6 +86,7 @@ pub fn adjust_oom_score(pid: u32) -> Result<()> { Ok(()) } +#[instrument(parent = Span::current(), level= "Info")] fn read_process_oom_score(pid: u32) -> Result { let content = fs::read_to_string(format!("/proc/{}/oom_score_adj", pid)) .map_err(io_error!(e, "read oom score"))?; @@ -92,12 +97,14 @@ fn read_process_oom_score(pid: u32) -> Result { Ok(score) } +#[instrument(parent = Span::current(), level= "Info")] fn write_process_oom_score(pid: u32, score: i64) -> Result<()> { fs::write(format!("/proc/{}/oom_score_adj", pid), score.to_string()) .map_err(io_error!(e, "write oom score")) } /// Collect process cgroup stats, return only necessary parts of it +#[instrument(parent = Span::current(), level= "Info")] pub fn collect_metrics(pid: u32) -> Result { let mut metrics = Metrics::new(); @@ -179,6 +186,7 @@ pub fn collect_metrics(pid: u32) -> Result { } // get_cgroup will return either cgroup v1 or v2 depending on system configuration +#[instrument(parent = Span::current(), level= "Info")] fn get_cgroup(pid: u32) -> Result { let hierarchies = hierarchies::auto(); let cgroup = if hierarchies.v2() { @@ -194,6 +202,7 @@ fn get_cgroup(pid: u32) -> Result { } /// Get the cgroups v2 path given a PID +#[instrument(parent = Span::current(), level= "Info")] pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { // todo: should upstream to cgroups-rs let path = format!("/proc/{}/cgroup", pid); @@ -207,6 +216,7 @@ pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { } // https://github.com/opencontainers/runc/blob/1950892f69597aa844cbf000fbdf77610dda3a44/libcontainer/cgroups/fs2/defaultpath.go#L83 +#[instrument(parent = Span::current(), level= "Info")] fn parse_cgroups_v2_path(content: &str) -> Result { // the entry for cgroup v2 is always in the format like `0::$PATH` // where 0 is the hierarchy ID, the controller name is omitted in cgroup v2 @@ -222,6 +232,7 @@ fn parse_cgroups_v2_path(content: &str) -> Result { } /// Update process cgroup limits +#[instrument(parent = Span::current(), level= "Info")] pub fn update_resources(pid: u32, resources: &LinuxResources) -> Result<()> { // get container main process cgroup let cgroup = get_cgroup(pid)?; diff --git a/crates/shim/src/lib.rs b/crates/shim/src/lib.rs index 2575ff6e..f2b34fde 100644 --- a/crates/shim/src/lib.rs +++ b/crates/shim/src/lib.rs @@ -30,6 +30,7 @@ pub use protos::{ shim::shim::DeleteResponse, ttrpc::{context::Context, Result as TtrpcResult}, }; +use tracing::{instrument, Span}; #[cfg(unix)] ioctl_write_ptr_bad!(ioctl_set_winsz, libc::TIOCSWINSZ, libc::winsize); @@ -167,6 +168,7 @@ pub const SOCKET_ROOT: &str = "/var/run/containerd"; pub const SOCKET_ROOT: &str = r"\\.\pipe\containerd-containerd"; /// Make socket path from containerd socket path, namespace and id. +#[instrument(parent = Span::current(), level= "Info")] pub fn socket_address(socket_path: &str, namespace: &str, id: &str) -> String { let path = PathBuf::from(socket_path) .join(namespace) diff --git a/crates/shim/src/synchronous/mod.rs b/crates/shim/src/synchronous/mod.rs index 8acb556b..d55ee8a8 100644 --- a/crates/shim/src/synchronous/mod.rs +++ b/crates/shim/src/synchronous/mod.rs @@ -58,6 +58,7 @@ use std::{ }; pub use log::{debug, error, info, warn}; +use tracing::{instrument, Span}; use util::{read_address, write_address}; use crate::{ @@ -182,6 +183,7 @@ pub trait Shim { } /// Shim entry point that must be invoked from `main`. +#[instrument(parent = Span::current(), level= "Info")] pub fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -192,6 +194,7 @@ where } } +#[instrument(parent = Span::current(), level= "Info")] fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -289,6 +292,7 @@ where } } +#[instrument(skip_all, parent = Span::current(), level= "Info")] fn create_server(_flags: args::Flags) -> Result { let mut server = Server::new(); @@ -306,6 +310,7 @@ fn create_server(_flags: args::Flags) -> Result { Ok(server) } +#[instrument(skip_all, parent = Span::current(), level= "Info")] fn setup_signals(_config: &Config) -> Option { #[cfg(unix)] { @@ -341,6 +346,7 @@ unsafe extern "system" fn signal_handler(_: u32) -> i32 { 1 } +#[instrument(skip_all, parent = Span::current(), level= "Info")] fn handle_signals(mut _signals: Option) { #[cfg(unix)] { @@ -402,6 +408,7 @@ fn handle_signals(mut _signals: Option) { } } +#[instrument(parent = Span::current(), level= "Info")] fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { @@ -416,10 +423,12 @@ fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result Err(other!("time out waiting for socket {}", address)) } +#[instrument(parent = Span::current(), level= "Info")] fn remove_socket_silently(address: &str) { remove_socket(address).unwrap_or_else(|e| warn!("failed to remove file {} {:?}", address, e)) } +#[instrument(parent = Span::current(), level= "Info")] fn remove_socket(address: &str) -> Result<()> { #[cfg(unix)] { @@ -448,6 +457,7 @@ fn remove_socket(address: &str) -> Result<()> { /// Spawn is a helper func to launch shim process. /// Typically this expected to be called from `StartShim`. +#[instrument(parent = Span::current(), level= "Info")] pub fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result<(u32, String)> { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; From 7a043d5f64550102adba3048b6a338b826195973 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 17 May 2024 23:27:46 +0000 Subject: [PATCH 2/5] conditionally compile tracing behind a feature Signed-off-by: jiaxiao zhou --- Cargo.toml | 1 + crates/shim/Cargo.toml | 6 ++- crates/shim/src/args.rs | 4 +- crates/shim/src/asynchronous/mod.rs | 20 +++++----- crates/shim/src/cgroup.rs | 22 +++++------ crates/shim/src/lib.rs | 4 +- crates/shim/src/synchronous/mod.rs | 20 +++++----- crates/shim_instrument/Cargo.toml | 19 ++++++++++ crates/shim_instrument/README.md | 31 +++++++++++++++ crates/shim_instrument/src/lib.rs | 58 +++++++++++++++++++++++++++++ 10 files changed, 149 insertions(+), 36 deletions(-) create mode 100644 crates/shim_instrument/Cargo.toml create mode 100644 crates/shim_instrument/README.md create mode 100644 crates/shim_instrument/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 7bd69216..65e0f6ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "crates/shim", "crates/shim-protos", "crates/snapshots", + "crates/shim_instrument", ] [profile.release] diff --git a/crates/shim/Cargo.toml b/crates/shim/Cargo.toml index 62cbad70..25026b4f 100644 --- a/crates/shim/Cargo.toml +++ b/crates/shim/Cargo.toml @@ -22,6 +22,7 @@ async = [ "signal-hook-tokio", "tokio", ] +tracing = ["dep:tracing", "shim_instrument/tracing"] docs = [] [[example]] @@ -53,7 +54,10 @@ serde.workspace = true serde_json.workspace = true thiserror.workspace = true time.workspace = true -tracing = "0.1" + +# tracing +tracing = { version = "0.1", optional = true } +shim_instrument = { path = "../shim_instrument" } # Async dependencies async-trait = { workspace = true, optional = true } diff --git a/crates/shim/src/args.rs b/crates/shim/src/args.rs index 8f0ff77c..6f7c8f05 100644 --- a/crates/shim/src/args.rs +++ b/crates/shim/src/args.rs @@ -16,7 +16,7 @@ use std::ffi::OsStr; -use tracing::{instrument, Span}; +use shim_instrument::shim_instrument as instrument; use crate::error::{Error, Result}; @@ -46,7 +46,7 @@ pub struct Flags { } /// Parses command line arguments passed to the shim. -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] pub fn parse>(args: &[S]) -> Result { let mut flags = Flags::default(); diff --git a/crates/shim/src/asynchronous/mod.rs b/crates/shim/src/asynchronous/mod.rs index 841cc7bf..9ac47686 100644 --- a/crates/shim/src/asynchronous/mod.rs +++ b/crates/shim/src/asynchronous/mod.rs @@ -46,9 +46,9 @@ use nix::{ }, unistd::Pid, }; +use shim_instrument::shim_instrument as instrument; use signal_hook_tokio::Signals; use tokio::{io::AsyncWriteExt, sync::Notify}; -use tracing::{instrument, Span}; use crate::{ args, @@ -100,7 +100,7 @@ pub trait Shim { } /// Async Shim entry point that must be invoked from tokio `main`. -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub async fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -111,7 +111,7 @@ where } } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] async fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -242,7 +242,7 @@ impl ExitSignal { /// Spawn is a helper func to launch shim process asynchronously. /// Typically this expected to be called from `StartShim`. -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; @@ -303,7 +303,7 @@ pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Ok(address) } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] fn setup_signals_tokio(config: &Config) -> Signals { if config.no_reaper { Signals::new([SIGTERM, SIGINT, SIGPIPE]).expect("new signal failed") @@ -312,7 +312,7 @@ fn setup_signals_tokio(config: &Config) -> Signals { } } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] async fn handle_signals(signals: Signals) { let mut signals = signals.fuse(); while let Some(sig) = signals.next().await { @@ -366,14 +366,14 @@ async fn handle_signals(signals: Signals) { } } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] async fn remove_socket_silently(address: &str) { remove_socket(address) .await .unwrap_or_else(|e| warn!("failed to remove socket: {}", e)) } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] async fn remove_socket(address: &str) -> Result<()> { let path = parse_sockaddr(address); if let Ok(md) = Path::new(path).metadata() { @@ -388,7 +388,7 @@ async fn remove_socket(address: &str) -> Result<()> { Ok(()) } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] async fn start_listener(address: &str) -> Result { let addr = address.to_string(); asyncify(move || -> Result { @@ -400,7 +400,7 @@ async fn start_listener(address: &str) -> Result { .await } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] async fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { diff --git a/crates/shim/src/cgroup.rs b/crates/shim/src/cgroup.rs index d7ba3c67..06857987 100644 --- a/crates/shim/src/cgroup.rs +++ b/crates/shim/src/cgroup.rs @@ -31,14 +31,14 @@ use containerd_shim_protos::{ shim::oci::Options, }; use oci_spec::runtime::LinuxResources; -use tracing::{instrument, Span}; +use shim_instrument::shim_instrument as instrument; use crate::error::{Error, Result}; // OOM_SCORE_ADJ_MAX is from https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h#L10 const OOM_SCORE_ADJ_MAX: i64 = 1000; -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { if pid == 0 { return Ok(()); @@ -64,7 +64,7 @@ pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { } /// Add a process to the given relative cgroup path -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { let h = hierarchies::auto(); // use relative path here, need to trim prefix '/' @@ -77,7 +77,7 @@ pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { /// Sets the OOM score for the process to the parents OOM score + 1 /// to ensure that they parent has a lower score than the shim -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn adjust_oom_score(pid: u32) -> Result<()> { let score = read_process_oom_score(std::os::unix::process::parent_id())?; if score < OOM_SCORE_ADJ_MAX { @@ -86,7 +86,7 @@ pub fn adjust_oom_score(pid: u32) -> Result<()> { Ok(()) } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn read_process_oom_score(pid: u32) -> Result { let content = fs::read_to_string(format!("/proc/{}/oom_score_adj", pid)) .map_err(io_error!(e, "read oom score"))?; @@ -97,14 +97,14 @@ fn read_process_oom_score(pid: u32) -> Result { Ok(score) } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn write_process_oom_score(pid: u32, score: i64) -> Result<()> { fs::write(format!("/proc/{}/oom_score_adj", pid), score.to_string()) .map_err(io_error!(e, "write oom score")) } /// Collect process cgroup stats, return only necessary parts of it -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn collect_metrics(pid: u32) -> Result { let mut metrics = Metrics::new(); @@ -186,7 +186,7 @@ pub fn collect_metrics(pid: u32) -> Result { } // get_cgroup will return either cgroup v1 or v2 depending on system configuration -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn get_cgroup(pid: u32) -> Result { let hierarchies = hierarchies::auto(); let cgroup = if hierarchies.v2() { @@ -202,7 +202,7 @@ fn get_cgroup(pid: u32) -> Result { } /// Get the cgroups v2 path given a PID -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { // todo: should upstream to cgroups-rs let path = format!("/proc/{}/cgroup", pid); @@ -216,7 +216,7 @@ pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { } // https://github.com/opencontainers/runc/blob/1950892f69597aa844cbf000fbdf77610dda3a44/libcontainer/cgroups/fs2/defaultpath.go#L83 -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn parse_cgroups_v2_path(content: &str) -> Result { // the entry for cgroup v2 is always in the format like `0::$PATH` // where 0 is the hierarchy ID, the controller name is omitted in cgroup v2 @@ -232,7 +232,7 @@ fn parse_cgroups_v2_path(content: &str) -> Result { } /// Update process cgroup limits -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn update_resources(pid: u32, resources: &LinuxResources) -> Result<()> { // get container main process cgroup let cgroup = get_cgroup(pid)?; diff --git a/crates/shim/src/lib.rs b/crates/shim/src/lib.rs index f2b34fde..3f8a1f66 100644 --- a/crates/shim/src/lib.rs +++ b/crates/shim/src/lib.rs @@ -30,7 +30,7 @@ pub use protos::{ shim::shim::DeleteResponse, ttrpc::{context::Context, Result as TtrpcResult}, }; -use tracing::{instrument, Span}; +use shim_instrument::shim_instrument as instrument; #[cfg(unix)] ioctl_write_ptr_bad!(ioctl_set_winsz, libc::TIOCSWINSZ, libc::winsize); @@ -168,7 +168,7 @@ pub const SOCKET_ROOT: &str = "/var/run/containerd"; pub const SOCKET_ROOT: &str = r"\\.\pipe\containerd-containerd"; /// Make socket path from containerd socket path, namespace and id. -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn socket_address(socket_path: &str, namespace: &str, id: &str) -> String { let path = PathBuf::from(socket_path) .join(namespace) diff --git a/crates/shim/src/synchronous/mod.rs b/crates/shim/src/synchronous/mod.rs index d55ee8a8..bae331e3 100644 --- a/crates/shim/src/synchronous/mod.rs +++ b/crates/shim/src/synchronous/mod.rs @@ -58,7 +58,7 @@ use std::{ }; pub use log::{debug, error, info, warn}; -use tracing::{instrument, Span}; +use shim_instrument::shim_instrument as instrument; use util::{read_address, write_address}; use crate::{ @@ -183,7 +183,7 @@ pub trait Shim { } /// Shim entry point that must be invoked from `main`. -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -194,7 +194,7 @@ where } } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -292,7 +292,7 @@ where } } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] fn create_server(_flags: args::Flags) -> Result { let mut server = Server::new(); @@ -310,7 +310,7 @@ fn create_server(_flags: args::Flags) -> Result { Ok(server) } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] fn setup_signals(_config: &Config) -> Option { #[cfg(unix)] { @@ -346,7 +346,7 @@ unsafe extern "system" fn signal_handler(_: u32) -> i32 { 1 } -#[instrument(skip_all, parent = Span::current(), level= "Info")] +#[instrument(skip_all, level = "Info")] fn handle_signals(mut _signals: Option) { #[cfg(unix)] { @@ -408,7 +408,7 @@ fn handle_signals(mut _signals: Option) { } } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { @@ -423,12 +423,12 @@ fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result Err(other!("time out waiting for socket {}", address)) } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn remove_socket_silently(address: &str) { remove_socket(address).unwrap_or_else(|e| warn!("failed to remove file {} {:?}", address, e)) } -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] fn remove_socket(address: &str) -> Result<()> { #[cfg(unix)] { @@ -457,7 +457,7 @@ fn remove_socket(address: &str) -> Result<()> { /// Spawn is a helper func to launch shim process. /// Typically this expected to be called from `StartShim`. -#[instrument(parent = Span::current(), level= "Info")] +#[instrument(level = "Info")] pub fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result<(u32, String)> { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; diff --git a/crates/shim_instrument/Cargo.toml b/crates/shim_instrument/Cargo.toml new file mode 100644 index 00000000..e25270bc --- /dev/null +++ b/crates/shim_instrument/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "shim_instrument" +version = "0.1.0" +license.workspace = true +repository.workspace = true +homepage.workspace = true +edition.workspace = true + +[dependencies] +proc-macro2 = "1.0" +quote = "1.0" +syn = { version = "1.0", features = ["full"] } +tracing = { version = "0.1", optional = true } + +[features] +tracing = ["dep:tracing"] + +[lib] +proc-macro = true \ No newline at end of file diff --git a/crates/shim_instrument/README.md b/crates/shim_instrument/README.md new file mode 100644 index 00000000..ead9a166 --- /dev/null +++ b/crates/shim_instrument/README.md @@ -0,0 +1,31 @@ +# shim-instrument + +Provides a way to conditionally compile `tracing` instrumentation into the code based on a feature flag "tracing". This can be useful in scenarios where you want to avoid the overhead of tracing in production or in builds where tracing is not required. + +## Usage + +In `Cargo.toml`: + +```toml +[dependencies] +shim-instrument = { path = "../../shim-instrument" } +tracing = { version = "0.1", optional = true } + +[features] +tracing = ["dep:tracing", "shim-instrument/tracing"] +``` + +In code: + +```rust +#[shim_instrument(skip_all, level = "Info")] +fn f1(a: i32) -> i32 { + a + 1 +} +``` + +To enable tracing, build with the `tracing` feature: + +```sh +cargo build --features tracing +``` \ No newline at end of file diff --git a/crates/shim_instrument/src/lib.rs b/crates/shim_instrument/src/lib.rs new file mode 100644 index 00000000..d79d81a1 --- /dev/null +++ b/crates/shim_instrument/src/lib.rs @@ -0,0 +1,58 @@ +extern crate proc_macro; + +use proc_macro::TokenStream; +use quote::quote; +use syn::{parse_macro_input, ItemFn}; +#[cfg(feature = "tracing")] +use syn::{AttributeArgs, Meta, NestedMeta}; + +/// proc macro to instrument functions with tracing. +/// +/// # Arguments +/// +/// - `args`: Attributes to pass to the `#[tracing::instrument]` macro. +/// - `input`: The function to be instrumented. +/// +/// # Example +/// +/// ``` +/// #[shim_instrument(level = "Info")] +/// fn my_function() { +/// // function body +/// } +/// ``` +#[proc_macro_attribute] +pub fn shim_instrument(args: TokenStream, input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as ItemFn); + + #[cfg(feature = "tracing")] + let output = { + let args = parse_macro_input!(args as AttributeArgs); + let span_parent = quote! { parent = tracing::Span::current(), }; + let attrs = args.iter().map(|arg| match arg { + NestedMeta::Meta(Meta::NameValue(nv)) => quote! { #nv }, + NestedMeta::Meta(Meta::Path(path)) => quote! { #path }, + _ => quote! {}, + }); + let attrs = quote! { #span_parent #(, #attrs)* }; + let fn_block = &input.block; + let fn_attrs = &input.attrs; + let fn_vis = &input.vis; + let fn_sig = &input.sig; + + quote! { + #[tracing::instrument(#attrs)] + #(#fn_attrs)* + #fn_vis #fn_sig #fn_block + } + }; + + #[cfg(not(feature = "tracing"))] + let output = { + quote! { + #input + } + }; + + TokenStream::from(output) +} From 3fc3687a7498939212275d4521fc96436978c831 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Fri, 17 May 2024 23:32:35 +0000 Subject: [PATCH 3/5] make clippy happy Signed-off-by: jiaxiao zhou --- crates/shim_instrument/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/shim_instrument/src/lib.rs b/crates/shim_instrument/src/lib.rs index d79d81a1..6575cfd7 100644 --- a/crates/shim_instrument/src/lib.rs +++ b/crates/shim_instrument/src/lib.rs @@ -22,12 +22,12 @@ use syn::{AttributeArgs, Meta, NestedMeta}; /// } /// ``` #[proc_macro_attribute] -pub fn shim_instrument(args: TokenStream, input: TokenStream) -> TokenStream { +pub fn shim_instrument(_args: TokenStream, input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as ItemFn); #[cfg(feature = "tracing")] let output = { - let args = parse_macro_input!(args as AttributeArgs); + let args = parse_macro_input!(_args as AttributeArgs); let span_parent = quote! { parent = tracing::Span::current(), }; let attrs = args.iter().map(|arg| match arg { NestedMeta::Meta(Meta::NameValue(nv)) => quote! { #nv }, From 5b30b66936a7ee5f836c29b212a0fc88e0844c4c Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Mon, 20 May 2024 18:21:01 +0000 Subject: [PATCH 4/5] remove instrument proc-macro and use cfg_attr instead Signed-off-by: jiaxiao zhou --- Cargo.toml | 1 - crates/shim/Cargo.toml | 3 +- crates/shim/src/args.rs | 4 +- crates/shim/src/asynchronous/mod.rs | 19 +++++----- crates/shim/src/cgroup.rs | 21 +++++------ crates/shim/src/lib.rs | 4 +- crates/shim/src/synchronous/mod.rs | 19 +++++----- crates/shim_instrument/Cargo.toml | 19 ---------- crates/shim_instrument/README.md | 31 --------------- crates/shim_instrument/src/lib.rs | 58 ----------------------------- 10 files changed, 32 insertions(+), 147 deletions(-) delete mode 100644 crates/shim_instrument/Cargo.toml delete mode 100644 crates/shim_instrument/README.md delete mode 100644 crates/shim_instrument/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 65e0f6ee..7bd69216 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,6 @@ members = [ "crates/shim", "crates/shim-protos", "crates/snapshots", - "crates/shim_instrument", ] [profile.release] diff --git a/crates/shim/Cargo.toml b/crates/shim/Cargo.toml index 25026b4f..ff2f1d6b 100644 --- a/crates/shim/Cargo.toml +++ b/crates/shim/Cargo.toml @@ -22,7 +22,7 @@ async = [ "signal-hook-tokio", "tokio", ] -tracing = ["dep:tracing", "shim_instrument/tracing"] +tracing = ["dep:tracing"] docs = [] [[example]] @@ -57,7 +57,6 @@ time.workspace = true # tracing tracing = { version = "0.1", optional = true } -shim_instrument = { path = "../shim_instrument" } # Async dependencies async-trait = { workspace = true, optional = true } diff --git a/crates/shim/src/args.rs b/crates/shim/src/args.rs index 6f7c8f05..8be030bd 100644 --- a/crates/shim/src/args.rs +++ b/crates/shim/src/args.rs @@ -16,8 +16,6 @@ use std::ffi::OsStr; -use shim_instrument::shim_instrument as instrument; - use crate::error::{Error, Result}; /// Flags to be passed from containerd daemon to a shim binary. @@ -46,7 +44,7 @@ pub struct Flags { } /// Parses command line arguments passed to the shim. -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))] pub fn parse>(args: &[S]) -> Result { let mut flags = Flags::default(); diff --git a/crates/shim/src/asynchronous/mod.rs b/crates/shim/src/asynchronous/mod.rs index 9ac47686..a61e380d 100644 --- a/crates/shim/src/asynchronous/mod.rs +++ b/crates/shim/src/asynchronous/mod.rs @@ -46,7 +46,6 @@ use nix::{ }, unistd::Pid, }; -use shim_instrument::shim_instrument as instrument; use signal_hook_tokio::Signals; use tokio::{io::AsyncWriteExt, sync::Notify}; @@ -100,7 +99,7 @@ pub trait Shim { } /// Async Shim entry point that must be invoked from tokio `main`. -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub async fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -111,7 +110,7 @@ where } } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] async fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -242,7 +241,7 @@ impl ExitSignal { /// Spawn is a helper func to launch shim process asynchronously. /// Typically this expected to be called from `StartShim`. -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; @@ -303,7 +302,7 @@ pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Ok(address) } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] fn setup_signals_tokio(config: &Config) -> Signals { if config.no_reaper { Signals::new([SIGTERM, SIGINT, SIGPIPE]).expect("new signal failed") @@ -312,7 +311,7 @@ fn setup_signals_tokio(config: &Config) -> Signals { } } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] async fn handle_signals(signals: Signals) { let mut signals = signals.fuse(); while let Some(sig) = signals.next().await { @@ -366,14 +365,14 @@ async fn handle_signals(signals: Signals) { } } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] async fn remove_socket_silently(address: &str) { remove_socket(address) .await .unwrap_or_else(|e| warn!("failed to remove socket: {}", e)) } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] async fn remove_socket(address: &str) -> Result<()> { let path = parse_sockaddr(address); if let Ok(md) = Path::new(path).metadata() { @@ -388,7 +387,7 @@ async fn remove_socket(address: &str) -> Result<()> { Ok(()) } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] async fn start_listener(address: &str) -> Result { let addr = address.to_string(); asyncify(move || -> Result { @@ -400,7 +399,7 @@ async fn start_listener(address: &str) -> Result { .await } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] async fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { diff --git a/crates/shim/src/cgroup.rs b/crates/shim/src/cgroup.rs index 06857987..bb330704 100644 --- a/crates/shim/src/cgroup.rs +++ b/crates/shim/src/cgroup.rs @@ -31,14 +31,13 @@ use containerd_shim_protos::{ shim::oci::Options, }; use oci_spec::runtime::LinuxResources; -use shim_instrument::shim_instrument as instrument; use crate::error::{Error, Result}; // OOM_SCORE_ADJ_MAX is from https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h#L10 const OOM_SCORE_ADJ_MAX: i64 = 1000; -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { if pid == 0 { return Ok(()); @@ -64,7 +63,7 @@ pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { } /// Add a process to the given relative cgroup path -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { let h = hierarchies::auto(); // use relative path here, need to trim prefix '/' @@ -77,7 +76,7 @@ pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { /// Sets the OOM score for the process to the parents OOM score + 1 /// to ensure that they parent has a lower score than the shim -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] pub fn adjust_oom_score(pid: u32) -> Result<()> { let score = read_process_oom_score(std::os::unix::process::parent_id())?; if score < OOM_SCORE_ADJ_MAX { @@ -86,7 +85,7 @@ pub fn adjust_oom_score(pid: u32) -> Result<()> { Ok(()) } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn read_process_oom_score(pid: u32) -> Result { let content = fs::read_to_string(format!("/proc/{}/oom_score_adj", pid)) .map_err(io_error!(e, "read oom score"))?; @@ -97,14 +96,14 @@ fn read_process_oom_score(pid: u32) -> Result { Ok(score) } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn write_process_oom_score(pid: u32, score: i64) -> Result<()> { fs::write(format!("/proc/{}/oom_score_adj", pid), score.to_string()) .map_err(io_error!(e, "write oom score")) } /// Collect process cgroup stats, return only necessary parts of it -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub fn collect_metrics(pid: u32) -> Result { let mut metrics = Metrics::new(); @@ -186,7 +185,7 @@ pub fn collect_metrics(pid: u32) -> Result { } // get_cgroup will return either cgroup v1 or v2 depending on system configuration -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn get_cgroup(pid: u32) -> Result { let hierarchies = hierarchies::auto(); let cgroup = if hierarchies.v2() { @@ -202,7 +201,7 @@ fn get_cgroup(pid: u32) -> Result { } /// Get the cgroups v2 path given a PID -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { // todo: should upstream to cgroups-rs let path = format!("/proc/{}/cgroup", pid); @@ -216,7 +215,7 @@ pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { } // https://github.com/opencontainers/runc/blob/1950892f69597aa844cbf000fbdf77610dda3a44/libcontainer/cgroups/fs2/defaultpath.go#L83 -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn parse_cgroups_v2_path(content: &str) -> Result { // the entry for cgroup v2 is always in the format like `0::$PATH` // where 0 is the hierarchy ID, the controller name is omitted in cgroup v2 @@ -232,7 +231,7 @@ fn parse_cgroups_v2_path(content: &str) -> Result { } /// Update process cgroup limits -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub fn update_resources(pid: u32, resources: &LinuxResources) -> Result<()> { // get container main process cgroup let cgroup = get_cgroup(pid)?; diff --git a/crates/shim/src/lib.rs b/crates/shim/src/lib.rs index 3f8a1f66..2077fb9f 100644 --- a/crates/shim/src/lib.rs +++ b/crates/shim/src/lib.rs @@ -30,7 +30,7 @@ pub use protos::{ shim::shim::DeleteResponse, ttrpc::{context::Context, Result as TtrpcResult}, }; -use shim_instrument::shim_instrument as instrument; + #[cfg(unix)] ioctl_write_ptr_bad!(ioctl_set_winsz, libc::TIOCSWINSZ, libc::winsize); @@ -168,7 +168,7 @@ pub const SOCKET_ROOT: &str = "/var/run/containerd"; pub const SOCKET_ROOT: &str = r"\\.\pipe\containerd-containerd"; /// Make socket path from containerd socket path, namespace and id. -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] pub fn socket_address(socket_path: &str, namespace: &str, id: &str) -> String { let path = PathBuf::from(socket_path) .join(namespace) diff --git a/crates/shim/src/synchronous/mod.rs b/crates/shim/src/synchronous/mod.rs index bae331e3..4c8a9fbb 100644 --- a/crates/shim/src/synchronous/mod.rs +++ b/crates/shim/src/synchronous/mod.rs @@ -58,7 +58,6 @@ use std::{ }; pub use log::{debug, error, info, warn}; -use shim_instrument::shim_instrument as instrument; use util::{read_address, write_address}; use crate::{ @@ -183,7 +182,7 @@ pub trait Shim { } /// Shim entry point that must be invoked from `main`. -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -194,7 +193,7 @@ where } } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -292,7 +291,7 @@ where } } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] fn create_server(_flags: args::Flags) -> Result { let mut server = Server::new(); @@ -310,7 +309,7 @@ fn create_server(_flags: args::Flags) -> Result { Ok(server) } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] fn setup_signals(_config: &Config) -> Option { #[cfg(unix)] { @@ -346,7 +345,7 @@ unsafe extern "system" fn signal_handler(_: u32) -> i32 { 1 } -#[instrument(skip_all, level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] fn handle_signals(mut _signals: Option) { #[cfg(unix)] { @@ -408,7 +407,7 @@ fn handle_signals(mut _signals: Option) { } } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { @@ -423,12 +422,12 @@ fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result Err(other!("time out waiting for socket {}", address)) } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn remove_socket_silently(address: &str) { remove_socket(address).unwrap_or_else(|e| warn!("failed to remove file {} {:?}", address, e)) } -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] fn remove_socket(address: &str) -> Result<()> { #[cfg(unix)] { @@ -457,7 +456,7 @@ fn remove_socket(address: &str) -> Result<()> { /// Spawn is a helper func to launch shim process. /// Typically this expected to be called from `StartShim`. -#[instrument(level = "Info")] +#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] pub fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result<(u32, String)> { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; diff --git a/crates/shim_instrument/Cargo.toml b/crates/shim_instrument/Cargo.toml deleted file mode 100644 index e25270bc..00000000 --- a/crates/shim_instrument/Cargo.toml +++ /dev/null @@ -1,19 +0,0 @@ -[package] -name = "shim_instrument" -version = "0.1.0" -license.workspace = true -repository.workspace = true -homepage.workspace = true -edition.workspace = true - -[dependencies] -proc-macro2 = "1.0" -quote = "1.0" -syn = { version = "1.0", features = ["full"] } -tracing = { version = "0.1", optional = true } - -[features] -tracing = ["dep:tracing"] - -[lib] -proc-macro = true \ No newline at end of file diff --git a/crates/shim_instrument/README.md b/crates/shim_instrument/README.md deleted file mode 100644 index ead9a166..00000000 --- a/crates/shim_instrument/README.md +++ /dev/null @@ -1,31 +0,0 @@ -# shim-instrument - -Provides a way to conditionally compile `tracing` instrumentation into the code based on a feature flag "tracing". This can be useful in scenarios where you want to avoid the overhead of tracing in production or in builds where tracing is not required. - -## Usage - -In `Cargo.toml`: - -```toml -[dependencies] -shim-instrument = { path = "../../shim-instrument" } -tracing = { version = "0.1", optional = true } - -[features] -tracing = ["dep:tracing", "shim-instrument/tracing"] -``` - -In code: - -```rust -#[shim_instrument(skip_all, level = "Info")] -fn f1(a: i32) -> i32 { - a + 1 -} -``` - -To enable tracing, build with the `tracing` feature: - -```sh -cargo build --features tracing -``` \ No newline at end of file diff --git a/crates/shim_instrument/src/lib.rs b/crates/shim_instrument/src/lib.rs deleted file mode 100644 index 6575cfd7..00000000 --- a/crates/shim_instrument/src/lib.rs +++ /dev/null @@ -1,58 +0,0 @@ -extern crate proc_macro; - -use proc_macro::TokenStream; -use quote::quote; -use syn::{parse_macro_input, ItemFn}; -#[cfg(feature = "tracing")] -use syn::{AttributeArgs, Meta, NestedMeta}; - -/// proc macro to instrument functions with tracing. -/// -/// # Arguments -/// -/// - `args`: Attributes to pass to the `#[tracing::instrument]` macro. -/// - `input`: The function to be instrumented. -/// -/// # Example -/// -/// ``` -/// #[shim_instrument(level = "Info")] -/// fn my_function() { -/// // function body -/// } -/// ``` -#[proc_macro_attribute] -pub fn shim_instrument(_args: TokenStream, input: TokenStream) -> TokenStream { - let input = parse_macro_input!(input as ItemFn); - - #[cfg(feature = "tracing")] - let output = { - let args = parse_macro_input!(_args as AttributeArgs); - let span_parent = quote! { parent = tracing::Span::current(), }; - let attrs = args.iter().map(|arg| match arg { - NestedMeta::Meta(Meta::NameValue(nv)) => quote! { #nv }, - NestedMeta::Meta(Meta::Path(path)) => quote! { #path }, - _ => quote! {}, - }); - let attrs = quote! { #span_parent #(, #attrs)* }; - let fn_block = &input.block; - let fn_attrs = &input.attrs; - let fn_vis = &input.vis; - let fn_sig = &input.sig; - - quote! { - #[tracing::instrument(#attrs)] - #(#fn_attrs)* - #fn_vis #fn_sig #fn_block - } - }; - - #[cfg(not(feature = "tracing"))] - let output = { - quote! { - #input - } - }; - - TokenStream::from(output) -} From 781f83205134c26f3607b1cd47ad06bd585f23c6 Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Mon, 20 May 2024 18:35:56 +0000 Subject: [PATCH 5/5] remove parent attribute in tracing macro Signed-off-by: jiaxiao zhou --- crates/shim/src/args.rs | 2 +- crates/shim/src/asynchronous/mod.rs | 18 +++++++++--------- crates/shim/src/cgroup.rs | 20 ++++++++++---------- crates/shim/src/lib.rs | 2 +- crates/shim/src/synchronous/mod.rs | 18 +++++++++--------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/shim/src/args.rs b/crates/shim/src/args.rs index 8be030bd..989eda1d 100644 --- a/crates/shim/src/args.rs +++ b/crates/shim/src/args.rs @@ -44,7 +44,7 @@ pub struct Flags { } /// Parses command line arguments passed to the shim. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "Info"))] pub fn parse>(args: &[S]) -> Result { let mut flags = Flags::default(); diff --git a/crates/shim/src/asynchronous/mod.rs b/crates/shim/src/asynchronous/mod.rs index a61e380d..4a145310 100644 --- a/crates/shim/src/asynchronous/mod.rs +++ b/crates/shim/src/asynchronous/mod.rs @@ -99,7 +99,7 @@ pub trait Shim { } /// Async Shim entry point that must be invoked from tokio `main`. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub async fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -110,7 +110,7 @@ where } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] async fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -241,7 +241,7 @@ impl ExitSignal { /// Spawn is a helper func to launch shim process asynchronously. /// Typically this expected to be called from `StartShim`. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?; @@ -302,7 +302,7 @@ pub async fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Ok(address) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] fn setup_signals_tokio(config: &Config) -> Signals { if config.no_reaper { Signals::new([SIGTERM, SIGINT, SIGPIPE]).expect("new signal failed") @@ -311,7 +311,7 @@ fn setup_signals_tokio(config: &Config) -> Signals { } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] async fn handle_signals(signals: Signals) { let mut signals = signals.fuse(); while let Some(sig) = signals.next().await { @@ -365,14 +365,14 @@ async fn handle_signals(signals: Signals) { } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] async fn remove_socket_silently(address: &str) { remove_socket(address) .await .unwrap_or_else(|e| warn!("failed to remove socket: {}", e)) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] async fn remove_socket(address: &str) -> Result<()> { let path = parse_sockaddr(address); if let Ok(md) = Path::new(path).metadata() { @@ -387,7 +387,7 @@ async fn remove_socket(address: &str) -> Result<()> { Ok(()) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] async fn start_listener(address: &str) -> Result { let addr = address.to_string(); asyncify(move || -> Result { @@ -399,7 +399,7 @@ async fn start_listener(address: &str) -> Result { .await } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] async fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { diff --git a/crates/shim/src/cgroup.rs b/crates/shim/src/cgroup.rs index bb330704..1df260f3 100644 --- a/crates/shim/src/cgroup.rs +++ b/crates/shim/src/cgroup.rs @@ -37,7 +37,7 @@ use crate::error::{Error, Result}; // OOM_SCORE_ADJ_MAX is from https://github.com/torvalds/linux/blob/master/include/uapi/linux/oom.h#L10 const OOM_SCORE_ADJ_MAX: i64 = 1000; -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))] pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { if pid == 0 { return Ok(()); @@ -63,7 +63,7 @@ pub fn set_cgroup_and_oom_score(pid: u32) -> Result<()> { } /// Add a process to the given relative cgroup path -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))] pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { let h = hierarchies::auto(); // use relative path here, need to trim prefix '/' @@ -76,7 +76,7 @@ pub fn add_task_to_cgroup(path: &str, pid: u32) -> Result<()> { /// Sets the OOM score for the process to the parents OOM score + 1 /// to ensure that they parent has a lower score than the shim -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))] pub fn adjust_oom_score(pid: u32) -> Result<()> { let score = read_process_oom_score(std::os::unix::process::parent_id())?; if score < OOM_SCORE_ADJ_MAX { @@ -85,7 +85,7 @@ pub fn adjust_oom_score(pid: u32) -> Result<()> { Ok(()) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn read_process_oom_score(pid: u32) -> Result { let content = fs::read_to_string(format!("/proc/{}/oom_score_adj", pid)) .map_err(io_error!(e, "read oom score"))?; @@ -96,14 +96,14 @@ fn read_process_oom_score(pid: u32) -> Result { Ok(score) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn write_process_oom_score(pid: u32, score: i64) -> Result<()> { fs::write(format!("/proc/{}/oom_score_adj", pid), score.to_string()) .map_err(io_error!(e, "write oom score")) } /// Collect process cgroup stats, return only necessary parts of it -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub fn collect_metrics(pid: u32) -> Result { let mut metrics = Metrics::new(); @@ -185,7 +185,7 @@ pub fn collect_metrics(pid: u32) -> Result { } // get_cgroup will return either cgroup v1 or v2 depending on system configuration -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn get_cgroup(pid: u32) -> Result { let hierarchies = hierarchies::auto(); let cgroup = if hierarchies.v2() { @@ -201,7 +201,7 @@ fn get_cgroup(pid: u32) -> Result { } /// Get the cgroups v2 path given a PID -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { // todo: should upstream to cgroups-rs let path = format!("/proc/{}/cgroup", pid); @@ -215,7 +215,7 @@ pub fn get_cgroups_v2_path_by_pid(pid: u32) -> Result { } // https://github.com/opencontainers/runc/blob/1950892f69597aa844cbf000fbdf77610dda3a44/libcontainer/cgroups/fs2/defaultpath.go#L83 -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn parse_cgroups_v2_path(content: &str) -> Result { // the entry for cgroup v2 is always in the format like `0::$PATH` // where 0 is the hierarchy ID, the controller name is omitted in cgroup v2 @@ -231,7 +231,7 @@ fn parse_cgroups_v2_path(content: &str) -> Result { } /// Update process cgroup limits -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub fn update_resources(pid: u32, resources: &LinuxResources) -> Result<()> { // get container main process cgroup let cgroup = get_cgroup(pid)?; diff --git a/crates/shim/src/lib.rs b/crates/shim/src/lib.rs index 2077fb9f..5f447bd7 100644 --- a/crates/shim/src/lib.rs +++ b/crates/shim/src/lib.rs @@ -168,7 +168,7 @@ pub const SOCKET_ROOT: &str = "/var/run/containerd"; pub const SOCKET_ROOT: &str = r"\\.\pipe\containerd-containerd"; /// Make socket path from containerd socket path, namespace and id. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "Info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))] pub fn socket_address(socket_path: &str, namespace: &str, id: &str) -> String { let path = PathBuf::from(socket_path) .join(namespace) diff --git a/crates/shim/src/synchronous/mod.rs b/crates/shim/src/synchronous/mod.rs index 4c8a9fbb..d1cfcc38 100644 --- a/crates/shim/src/synchronous/mod.rs +++ b/crates/shim/src/synchronous/mod.rs @@ -182,7 +182,7 @@ pub trait Shim { } /// Shim entry point that must be invoked from `main`. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub fn run(runtime_id: &str, opts: Option) where T: Shim + Send + Sync + 'static, @@ -193,7 +193,7 @@ where } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn bootstrap(runtime_id: &str, opts: Option) -> Result<()> where T: Shim + Send + Sync + 'static, @@ -291,7 +291,7 @@ where } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] fn create_server(_flags: args::Flags) -> Result { let mut server = Server::new(); @@ -309,7 +309,7 @@ fn create_server(_flags: args::Flags) -> Result { Ok(server) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] fn setup_signals(_config: &Config) -> Option { #[cfg(unix)] { @@ -345,7 +345,7 @@ unsafe extern "system" fn signal_handler(_: u32) -> i32 { 1 } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(skip_all, level = "info"))] fn handle_signals(mut _signals: Option) { #[cfg(unix)] { @@ -407,7 +407,7 @@ fn handle_signals(mut _signals: Option) { } } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result<()> { for _i in 0..count { match Client::connect(address) { @@ -422,12 +422,12 @@ fn wait_socket_working(address: &str, interval_in_ms: u64, count: u32) -> Result Err(other!("time out waiting for socket {}", address)) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn remove_socket_silently(address: &str) { remove_socket(address).unwrap_or_else(|e| warn!("failed to remove file {} {:?}", address, e)) } -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] fn remove_socket(address: &str) -> Result<()> { #[cfg(unix)] { @@ -456,7 +456,7 @@ fn remove_socket(address: &str) -> Result<()> { /// Spawn is a helper func to launch shim process. /// Typically this expected to be called from `StartShim`. -#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), level = "info"))] +#[cfg_attr(feature = "tracing", tracing::instrument(level = "info"))] pub fn spawn(opts: StartOpts, grouping: &str, vars: Vec<(&str, &str)>) -> Result<(u32, String)> { let cmd = env::current_exe().map_err(io_error!(e, ""))?; let cwd = env::current_dir().map_err(io_error!(e, ""))?;