Skip to content

Commit

Permalink
Merge pull request #592 from Mossaka/tracing-optional
Browse files Browse the repository at this point in the history
making `tracaing::instrument` macro optional
  • Loading branch information
jsturtevant committed May 22, 2024
2 parents c1c0cac + 912fd6d commit d970d9f
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 91 deletions.
5 changes: 4 additions & 1 deletion crates/containerd-shim-wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ wasmparser = "0.207.0"
tokio-stream = { version = "0.1" }
prost-types = "0.12" # should match version in containerd-shim
sha256 = { workspace = true }
tracing = { workspace = true }

# tracing
tracing = { workspace = true, optional = true }

[target.'cfg(unix)'.dependencies]
caps = "0.5"
Expand All @@ -61,3 +63,4 @@ rand= "0.8"

[features]
testing = ["dep:containerd-shim-wasm-test-modules", "dep:env_logger", "dep:tempfile", "dep:oci-tar-builder"]
tracing = ["dep:tracing"]
3 changes: 1 addition & 2 deletions crates/containerd-shim-wasm/src/sandbox/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::sync::mpsc::channel;
use std::sync::Arc;

use containerd_shim::{parse, run, Config};
use tracing::{instrument, Span};
use ttrpc::Server;

use crate::sandbox::manager::Shim;
Expand Down Expand Up @@ -37,7 +36,7 @@ macro_rules! revision {
};
}

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn shim_main<'a, I>(
name: &str,
version: &str,
Expand Down
27 changes: 13 additions & 14 deletions crates/containerd-shim-wasm/src/sandbox/containerd/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use tokio::runtime::Runtime;
use tokio::sync::mpsc;
use tokio_stream::wrappers::ReceiverStream;
use tonic::{Code, Request};
use tracing::{instrument, Span};

use super::lease::LeaseGuard;
use crate::container::Engine;
Expand Down Expand Up @@ -54,7 +53,7 @@ pub(crate) struct 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
#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn connect(
address: impl AsRef<Path> + ToString,
namespace: impl ToString,
Expand All @@ -76,7 +75,7 @@ impl Client {
}

// wrapper around read that will read the entire content file
#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn read_content(&self, digest: impl ToString) -> Result<Vec<u8>> {
self.rt.block_on(async {
let req = ReadContentRequest {
Expand All @@ -98,7 +97,7 @@ impl Client {

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

// wrapper around lease that will create a lease and return a guard that will delete the lease when dropped
#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn lease(&self, reference: String) -> Result<LeaseGuard> {
self.rt.block_on(async {
let mut lease_labels = HashMap::new();
Expand Down Expand Up @@ -146,7 +145,7 @@ impl Client {
})
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn save_content(
&self,
data: Vec<u8>,
Expand Down Expand Up @@ -272,7 +271,7 @@ impl Client {
})
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn get_info(&self, content_digest: &str) -> Result<Info> {
self.rt.block_on(async {
let req = InfoRequest {
Expand All @@ -295,7 +294,7 @@ impl Client {
})
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn update_info(&self, info: Info) -> Result<Info> {
self.rt.block_on(async {
let req = UpdateRequest {
Expand All @@ -321,7 +320,7 @@ impl Client {
})
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn get_image(&self, image_name: impl ToString) -> Result<Image> {
self.rt.block_on(async {
let name = image_name.to_string();
Expand All @@ -343,7 +342,7 @@ impl Client {
})
}

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

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn get_container(&self, container_name: impl ToString) -> Result<Container> {
self.rt.block_on(async {
let id = container_name.to_string();
Expand All @@ -381,7 +380,7 @@ impl Client {
})
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn get_image_manifest_and_digest(&self, image_name: &str) -> Result<(ImageManifest, String)> {
let image = self.get_image(image_name)?;
let image_digest = self.extract_image_content_sha(&image)?;
Expand All @@ -392,7 +391,7 @@ 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.
#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn load_modules<T: Engine>(
&self,
containerd_id: impl ToString,
Expand Down Expand Up @@ -523,7 +522,7 @@ impl Client {
Ok((layers, platform))
}

#[instrument(skip_all, parent = Span::current(), level = "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn read_wasm_layer(
&self,
original_config: &oci_spec::image::Descriptor,
Expand Down
3 changes: 1 addition & 2 deletions crates/containerd-shim-wasm/src/sandbox/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::path::{Path, PathBuf};
use std::time::Duration;

use chrono::{DateTime, Utc};
use tracing::{instrument, Span};

use super::error::Error;
use super::sync::WaitableCell;
Expand Down Expand Up @@ -137,7 +136,7 @@ pub trait Instance: 'static {

/// Waits for the instance to finish and retunrs its exit code
/// This is a blocking call.
#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn wait(&self) -> (u32, DateTime<Utc>) {
self.wait_timeout(None).unwrap()
}
Expand Down
9 changes: 4 additions & 5 deletions crates/containerd-shim-wasm/src/sandbox/instance_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ use std::path::{Path, PathBuf};

use anyhow::{bail, Context, Result};
use serde::{Deserialize, Serialize};
use tracing::{instrument, Span};

use super::Error;

/// Return the root path for the instance.
///
/// The root path is the path to the directory containing the container's state.
#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn get_instance_root<P: AsRef<Path>>(
root_path: P,
instance_id: &str,
Expand All @@ -27,7 +26,7 @@ pub fn get_instance_root<P: AsRef<Path>>(
/// Checks if the container exists.
///
/// The root path is the path to the directory containing the container's state.
#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn instance_exists<P: AsRef<Path>>(root_path: P, container_id: &str) -> Result<bool> {
let instance_root = construct_instance_root(root_path, container_id)?;
Ok(instance_root.exists())
Expand All @@ -38,7 +37,7 @@ struct Options {
root: Option<PathBuf>,
}

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn determine_rootdir(
bundle: impl AsRef<Path>,
namespace: &str,
Expand All @@ -57,7 +56,7 @@ pub fn determine_rootdir(
Ok(path)
}

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn construct_instance_root<P: AsRef<Path>>(root_path: P, container_id: &str) -> Result<PathBuf> {
let root_path = root_path.as_ref().canonicalize().with_context(|| {
format!(
Expand Down
11 changes: 5 additions & 6 deletions crates/containerd-shim-wasm/src/sandbox/shim/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use containerd_shim::util::write_address;
use containerd_shim::{self as shim, api, ExitSignal};
use oci_spec::runtime::Spec;
use shim::Flags;
use tracing::{instrument, Span};

use crate::sandbox::instance::Instance;
use crate::sandbox::shim::events::{RemoteEventSender, ToTimestamp};
Expand Down Expand Up @@ -46,7 +45,7 @@ where
{
type T = Local<I>;

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

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, 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 @@ -81,12 +80,12 @@ where
Ok(address)
}

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

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, 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 @@ -100,7 +99,7 @@ where
)
}

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
fn delete_shim(&mut self) -> shim::Result<api::DeleteResponse> {
Ok(api::DeleteResponse {
exit_status: 137,
Expand Down
19 changes: 9 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 @@ -2,7 +2,6 @@ use std::sync::{Arc, OnceLock, RwLock};
use std::time::Duration;

use chrono::{DateTime, Utc};
use tracing::{instrument, Span};

use crate::sandbox::instance::Nop;
use crate::sandbox::shim::instance_option::InstanceOption;
Expand All @@ -17,7 +16,7 @@ pub(super) struct InstanceData<T: Instance> {
}

impl<T: Instance> InstanceData<T> {
#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn new_instance(id: impl AsRef<str>, cfg: InstanceConfig<T::Engine>) -> Result<Self> {
let id = id.as_ref().to_string();
let instance = InstanceOption::Instance(T::new(id, Some(&cfg))?);
Expand All @@ -29,7 +28,7 @@ impl<T: Instance> InstanceData<T> {
})
}

#[instrument(skip_all, parent = Span::current(), level= "Info")]
#[cfg_attr(feature = "tracing", tracing::instrument(parent = tracing::Span::current(), skip_all, level = "Info"))]
pub fn new_base(id: impl AsRef<str>, cfg: InstanceConfig<T::Engine>) -> Result<Self> {
let id = id.as_ref().to_string();
let instance = InstanceOption::Nop(Nop::new(id, None)?);
Expand All @@ -41,17 +40,17 @@ impl<T: Instance> InstanceData<T> {
})
}

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

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

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

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

self.instance.kill(signal)
}

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

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

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

0 comments on commit d970d9f

Please sign in to comment.