Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tracing: add parameters info to the trace instead of skipping them #853

Merged
merged 1 commit into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/containerd-shim-wasm/src/sandbox/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ fn log_mem() {
pub fn shim_main<'a, I>(
name: &str,
version: &str,
revision: impl Into<Option<&'a str>>,
shim_version: impl Into<Option<&'a str>>,
revision: impl Into<Option<&'a str>> + std::fmt::Debug,
shim_version: impl Into<Option<&'a str>> + std::fmt::Debug,
config: Option<Config>,
) where
I: 'static + Instance + Sync + Send,
Expand Down Expand Up @@ -194,12 +194,12 @@ pub fn shim_main<'a, I>(
log_mem();
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))]
fn shim_main_inner<'a, I>(
name: &str,
version: &str,
revision: impl Into<Option<&'a str>>,
shim_version: impl Into<Option<&'a str>>,
revision: impl Into<Option<&'a str>> + std::fmt::Debug,
shim_version: impl Into<Option<&'a str>> + std::fmt::Debug,
config: Option<Config>,
) where
I: 'static + Instance + Sync + Send,
Expand Down
49 changes: 28 additions & 21 deletions crates/containerd-shim-wasm/src/sandbox/containerd/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ static PRECOMPILE_PREFIX: &str = "runwasi.io/precompiled";
// Conservatively set the max to 15MB to leave room for message overhead
static MAX_WRITE_CHUNK_SIZE_BYTES: i64 = 1024 * 1024 * 15;

#[derive(Debug)]
pub struct Client {
inner: Channel,
namespace: String,
Expand All @@ -57,10 +58,10 @@ impl WriteContent {
// sync wrapper implementation from https://tokio.rs/tokio/topics/bridging
impl Client {
// wrapper around connection that will establish a connection and create a client
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
pub async fn connect(
address: impl AsRef<Path>,
namespace: impl Into<String>,
address: impl AsRef<Path> + std::fmt::Debug,
namespace: impl Into<String> + std::fmt::Debug,
) -> Result<Client> {
let inner = containerd_client::connect(address.as_ref())
.await
Expand All @@ -73,8 +74,8 @@ impl Client {
}

// wrapper around read that will read the entire content file
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
async fn read_content(&self, digest: impl ToString) -> Result<Vec<u8>> {
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn read_content(&self, digest: impl ToString + std::fmt::Debug) -> Result<Vec<u8>> {
let req = ReadContentRequest {
digest: digest.to_string(),
..Default::default()
Expand All @@ -93,8 +94,8 @@ impl Client {

// used in tests to clean up content
#[allow(dead_code)]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
async fn delete_content(&self, digest: impl ToString) -> Result<()> {
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn delete_content(&self, digest: impl ToString + std::fmt::Debug) -> Result<()> {
let req = DeleteContentRequest {
digest: digest.to_string(),
};
Expand All @@ -107,7 +108,7 @@ impl Client {
}

// wrapper around lease that will create a lease and return a guard that will delete the lease when dropped
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn lease(&self, reference: String) -> Result<LeaseGuard> {
let mut lease_labels = HashMap::new();
// Unwrap is safe here since 24 hours is a valid time
Expand Down Expand Up @@ -136,7 +137,7 @@ impl Client {
))
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn save_content(
&self,
data: Vec<u8>,
Expand Down Expand Up @@ -258,7 +259,7 @@ impl Client {
Ok(WriteContent { lease, digest })
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn get_info(&self, content_digest: &Digest) -> Result<Info> {
let req = InfoRequest {
digest: content_digest.to_string(),
Expand All @@ -276,7 +277,7 @@ impl Client {
Ok(info)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn update_info(&self, info: Info) -> Result<Info> {
let mut req = UpdateRequest {
info: Some(info.clone()),
Expand All @@ -299,8 +300,8 @@ impl Client {
Ok(info)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
async fn get_image(&self, image_name: impl ToString) -> Result<Image> {
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn get_image(&self, image_name: impl ToString + std::fmt::Debug) -> Result<Image> {
let name = image_name.to_string();
let req = GetImageRequest { name };
let req = with_namespace!(req, self.namespace);
Expand All @@ -319,7 +320,7 @@ impl Client {
Ok(image)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
fn extract_image_content_sha(&self, image: &Image) -> Result<String> {
let digest = image
.target
Expand All @@ -335,8 +336,11 @@ impl Client {
Ok(digest)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
async fn get_container(&self, container_name: impl ToString) -> Result<Container> {
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn get_container(
&self,
container_name: impl ToString + std::fmt::Debug,
) -> Result<Container> {
let id = container_name.to_string();
let req = GetContainerRequest { id };
let req = with_namespace!(req, self.namespace);
Expand All @@ -355,7 +359,7 @@ impl Client {
Ok(container)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn get_image_manifest_and_digest(
&self,
image_name: &str,
Expand All @@ -370,10 +374,13 @@ impl Client {
// load module will query the containerd store to find an image that has an OS of type 'wasm'
// If found it continues to parse the manifest and return the layers that contains the WASM modules
// and possibly other configuration layers.
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(
feature = "tracing",
tracing::instrument(skip(engine), level = "Debug")
)]
pub async fn load_modules<T: Engine>(
&self,
containerd_id: impl ToString,
containerd_id: impl ToString + std::fmt::Debug,
engine: &T,
) -> Result<(Vec<oci::WasmLayer>, Platform)> {
let container = self.get_container(containerd_id.to_string()).await?;
Expand Down Expand Up @@ -508,7 +515,7 @@ impl Client {
Ok((layers, platform))
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
async fn read_wasm_layer(
&self,
original_config: &oci_spec::image::Descriptor,
Expand Down Expand Up @@ -949,7 +956,7 @@ mod tests {
x
}

#[derive(Clone)]
#[derive(Clone, Debug)]
struct FakePrecomiplerEngine {
precompile_id: Option<String>,
precompiled_layers: HashMap<String, Vec<u8>>,
Expand Down
4 changes: 2 additions & 2 deletions crates/containerd-shim-wasm/src/sandbox/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use super::error::Error;

/// Generic options builder for creating a wasm instance.
/// This is passed to the `Instance::new` method.
#[derive(Clone, Serialize, Deserialize)]
#[derive(Clone, Serialize, Deserialize, Debug)]
pub struct InstanceConfig {
/// Optional stdin named pipe path.
stdin: PathBuf,
Expand Down Expand Up @@ -122,7 +122,7 @@ pub trait Instance: 'static {

/// Waits for the instance to finish and returns its exit code
/// This is a blocking call.
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), parent = tracing::Span::current(), level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Info"))]
fn wait(&self) -> (u32, DateTime<Utc>) {
self.wait_timeout(None).unwrap()
}
Expand Down
6 changes: 3 additions & 3 deletions crates/containerd-shim-wasm/src/sandbox/instance_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ struct Options {
/// file. Otherwise, the root directory is determined by joining the `rootdir` and `namespace`.
///
/// The default root directory is `/run/containerd/<wasm engine name>/<namespace>`.
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
pub fn determine_rootdir(
bundle: impl AsRef<Path>,
bundle: impl AsRef<Path> + std::fmt::Debug,
namespace: &str,
rootdir: impl AsRef<Path>,
rootdir: impl AsRef<Path> + std::fmt::Debug,
) -> Result<PathBuf, Error> {
let file = match File::open(bundle.as_ref().join("options.json")) {
Ok(f) => f,
Expand Down
13 changes: 8 additions & 5 deletions crates/containerd-shim-wasm/src/sandbox/shim/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ where
{
type T = Local<I>;

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))]
fn new(_runtime_id: &str, args: &Flags, _config: &mut shim::Config) -> Self {
Cli {
engine: Default::default(),
Expand All @@ -55,7 +55,7 @@ where
}
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))]
fn start_shim(&mut self, opts: containerd_shim::StartOpts) -> shim::Result<String> {
let dir = current_dir().map_err(|err| ShimError::Other(err.to_string()))?;
let spec = Spec::load(dir.join("config.json")).map_err(|err| {
Expand All @@ -76,12 +76,15 @@ where
Ok(address)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))]
fn wait(&mut self) {
self.exit.wait();
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(
feature = "tracing",
tracing::instrument(skip(publisher), level = "Info")
)]
fn create_task_service(&self, publisher: RemotePublisher) -> Self::T {
let events = RemoteEventSender::new(&self.namespace, publisher);
let exit = self.exit.clone();
Expand All @@ -95,7 +98,7 @@ where
)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Info"))]
fn delete_shim(&mut self) -> shim::Result<api::DeleteResponse> {
Ok(api::DeleteResponse {
exit_status: 137,
Expand Down
23 changes: 13 additions & 10 deletions crates/containerd-shim-wasm/src/sandbox/shim/instance_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ pub(super) struct InstanceData<T: Instance> {
}

impl<T: Instance> InstanceData<T> {
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
pub fn new(id: impl AsRef<str>, cfg: InstanceConfig) -> Result<Self> {
#[cfg_attr(feature = "tracing", tracing::instrument(level = "Debug"))]
pub fn new(id: impl AsRef<str> + std::fmt::Debug, cfg: InstanceConfig) -> Result<Self> {
let id = id.as_ref().to_string();
let instance = T::new(id, &cfg)?;
Ok(Self {
Expand All @@ -26,17 +26,17 @@ impl<T: Instance> InstanceData<T> {
})
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn pid(&self) -> Option<u32> {
self.pid.get().copied()
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn config(&self) -> &InstanceConfig {
&self.cfg
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn start(&self) -> Result<u32> {
let mut s = self.state.write().unwrap();
s.start()?;
Expand All @@ -56,15 +56,15 @@ impl<T: Instance> InstanceData<T> {
res
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn kill(&self, signal: u32) -> Result<()> {
let mut s = self.state.write().unwrap();
s.kill()?;

self.instance.kill(signal)
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn delete(&self) -> Result<()> {
let mut s = self.state.write().unwrap();
s.delete()?;
Expand All @@ -79,16 +79,19 @@ impl<T: Instance> InstanceData<T> {
res
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn wait(&self) -> (u32, DateTime<Utc>) {
let res = self.instance.wait();
let mut s = self.state.write().unwrap();
*s = TaskState::Exited;
res
}

#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Debug"))]
pub fn wait_timeout(&self, t: impl Into<Option<Duration>>) -> Option<(u32, DateTime<Utc>)> {
#[cfg_attr(feature = "tracing", tracing::instrument(skip(self), level = "Debug"))]
pub fn wait_timeout(
&self,
t: impl Into<Option<Duration>> + std::fmt::Debug,
) -> Option<(u32, DateTime<Utc>)> {
let res = self.instance.wait_timeout(t);
if res.is_some() {
let mut s = self.state.write().unwrap();
Expand Down
Loading
Loading