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

add container ID and pod ID to RuntimeContext and logs #879

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
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-wamr/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Engine for WamrEngine {
.as_bytes()
.context("Failed to get bytes from source")?;

log::info!("Create a WAMR module");
containerd_shim_wasm::info!(ctx, "Create a WAMR module");

// TODO: error handling isn't ideal

Expand All @@ -56,7 +56,7 @@ impl Engine for WamrEngine {
let mut module = Module::from_buf(&self.runtime, &wasm_bytes, &mod_name)
.context("Failed to create module from bytes")?;

log::info!("Create a WASI context");
containerd_shim_wasm::info!(ctx, "Create a WASI context");

let wasi_ctx = WasiCtxBuilder::new()
.set_pre_open_path(vec!["/"], vec![])
Expand All @@ -68,19 +68,19 @@ impl Engine for WamrEngine {

// TODO: no way to register a named module with bytes?

log::info!("Create a WAMR instance");
containerd_shim_wasm::info!(ctx, "Create a WAMR instance");

let instance = WamrInst::new(&self.runtime, &module, 1024 * 64)
.context("Failed to create instance")?;

log::info!("Running {func:?}");
containerd_shim_wasm::info!(ctx, "Running {func:?}");
let function =
Function::find_export_func(&instance, &func).context("Failed to find function")?;
let status = function
.call(&instance, &vec![])
.map(|_| 0)
.map_err(|err| {
log::error!("Error: {:?}", err);
containerd_shim_wasm::error!(ctx, "Error: {:?}", err);
err
})
.context("Failed to call function")?;
Expand Down
83 changes: 82 additions & 1 deletion crates/containerd-shim-wasm/src/container/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ pub trait RuntimeContext {
// the platform for the container using the struct defined on the OCI spec definition
// https://github.com/opencontainers/image-spec/blob/v1.1.0-rc5/image-index.md
fn platform(&self) -> &Platform;

// the container id for the running container
fn container_id(&self) -> &str;

// the pod id for the running container (if available)
// In Kubernetes environments, containers run within pods, and the pod ID is usually
// stored in the OCI spec annotations under "io.kubernetes.cri.sandbox-id"
fn pod_id(&self) -> Option<&str> {
None
}
Comment on lines +39 to +48
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. We might consider putting this behind a struct/function that hides this behind something like info() if we have more things we might want to log. But as of now I can't think of anything.

}

/// The source for a WASI module / components.
Expand Down Expand Up @@ -84,6 +94,7 @@ pub(crate) struct WasiContext<'a> {
pub spec: &'a Spec,
pub wasm_layers: &'a [WasmLayer],
pub platform: &'a Platform,
pub id: String,
}

impl RuntimeContext for WasiContext<'_> {
Expand All @@ -101,7 +112,7 @@ impl RuntimeContext for WasiContext<'_> {
.process()
.as_ref()
.and_then(|p| p.env().as_ref())
.map(|e| e.as_slice())
.map(|a| a.as_slice())
.unwrap_or_default()
}

Expand Down Expand Up @@ -134,6 +145,18 @@ impl RuntimeContext for WasiContext<'_> {
fn platform(&self) -> &Platform {
self.platform
}

fn container_id(&self) -> &str {
&self.id
}

fn pod_id(&self) -> Option<&str> {
self.spec
.annotations()
.as_ref()
.and_then(|a| a.get("io.kubernetes.cri.sandbox-id"))
.map(|s| s.as_str())
}
}

#[cfg(test)]
Expand All @@ -160,6 +183,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let args = ctx.args();
Expand All @@ -180,6 +204,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let args = ctx.args();
Expand Down Expand Up @@ -208,6 +233,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let args = ctx.args();
Expand All @@ -230,6 +256,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let path = ctx.entrypoint().source;
Expand Down Expand Up @@ -261,6 +288,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let expected_path = PathBuf::from("hello.wat");
Expand Down Expand Up @@ -301,6 +329,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let expected_path = PathBuf::from("/root/hello.wat");
Expand Down Expand Up @@ -341,6 +370,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let expected_path = PathBuf::from("/root/hello.wat");
Expand Down Expand Up @@ -379,6 +409,7 @@ mod tests {
),
}],
platform: &Platform::default(),
id: "test".to_string(),
};

assert!(matches!(ctx.entrypoint().source, Source::Oci(_)));
Expand All @@ -402,6 +433,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let envs = ctx.envs();
Expand All @@ -423,6 +455,7 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let envs = ctx.envs();
Expand All @@ -442,11 +475,59 @@ mod tests {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test".to_string(),
};

let envs = ctx.envs();
assert_eq!(envs.len(), 2);

Ok(())
}

#[test]
fn test_get_pod_id() -> Result<()> {
use std::collections::HashMap;

let mut annotations = HashMap::new();
annotations.insert(
"io.kubernetes.cri.sandbox-id".to_string(),
"test-pod-id".to_string(),
);

let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(ProcessBuilder::default().cwd("/").build()?)
.annotations(annotations)
.build()?;

let ctx = WasiContext {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test-container".to_string(),
};

assert_eq!(ctx.pod_id(), Some("test-pod-id"));

Ok(())
}

#[test]
fn test_get_pod_id_no_annotation() -> Result<()> {
let spec = SpecBuilder::default()
.root(RootBuilder::default().path("rootfs").build()?)
.process(ProcessBuilder::default().cwd("/").build()?)
.build()?;

let ctx = WasiContext {
spec: &spec,
wasm_layers: &[],
platform: &Platform::default(),
id: "test-container".to_string(),
};

assert_eq!(ctx.pod_id(), None);

Ok(())
}
}
98 changes: 98 additions & 0 deletions crates/containerd-shim-wasm/src/container/log.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/// Macros for logging with context information (container ID) automatically included.
#[macro_export]
macro_rules! log_with_ctx {
// Info level log
(info, $ctx:expr, $($arg:tt)+) => {
{
let ctx = $ctx;
match ctx.pod_id() {
Some(pod_id) => log::info!(instance = ctx.container_id(), pod = pod_id; $($arg)+),
None => log::info!(instance = ctx.container_id(); $($arg)+)
}
}
};

// Debug level log
(debug, $ctx:expr, $($arg:tt)+) => {
{
let ctx = $ctx;
match ctx.pod_id() {
Some(pod_id) => log::debug!(instance = ctx.container_id(), pod = pod_id; $($arg)+),
None => log::debug!(instance = ctx.container_id(); $($arg)+)
}
}
};

// Warn level log
(warn, $ctx:expr, $($arg:tt)+) => {
{
let ctx = $ctx;
match ctx.pod_id() {
Some(pod_id) => log::warn!(instance = ctx.container_id(), pod = pod_id; $($arg)+),
None => log::warn!(instance = ctx.container_id(); $($arg)+)
}
}
};

// Error level log
(error, $ctx:expr, $($arg:tt)+) => {
{
let ctx = $ctx;
match ctx.pod_id() {
Some(pod_id) => log::error!(instance = ctx.container_id(), pod = pod_id; $($arg)+),
None => log::error!(instance = ctx.container_id(); $($arg)+)
}
}
};

// Trace level log
(trace, $ctx:expr, $($arg:tt)+) => {
{
let ctx = $ctx;
match ctx.pod_id() {
Some(pod_id) => log::trace!(instance = ctx.container_id(), pod = pod_id; $($arg)+),
None => log::trace!(instance = ctx.container_id(); $($arg)+)
}
}
};
}

/// Convenience macro for info level logs
#[macro_export]
macro_rules! info {
($ctx:expr, $($arg:tt)+) => {
$crate::log_with_ctx!(info, $ctx, $($arg)+)
};
}

/// Convenience macro for debug level logs
#[macro_export]
macro_rules! debug {
($ctx:expr, $($arg:tt)+) => {
$crate::log_with_ctx!(debug, $ctx, $($arg)+)
};
}

/// Convenience macro for warn level logs
#[macro_export]
macro_rules! warn {
($ctx:expr, $($arg:tt)+) => {
$crate::log_with_ctx!(warn, $ctx, $($arg)+)
};
}

/// Convenience macro for error level logs
#[macro_export]
macro_rules! error {
($ctx:expr, $($arg:tt)+) => {
$crate::log_with_ctx!(error, $ctx, $($arg)+)
};
}

/// Convenience macro for trace level logs
#[macro_export]
macro_rules! trace {
($ctx:expr, $($arg:tt)+) => {
$crate::log_with_ctx!(trace, $ctx, $($arg)+)
};
}
1 change: 1 addition & 0 deletions crates/containerd-shim-wasm/src/container/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

mod context;
mod engine;
pub mod log;
mod path;
mod wasm;

Expand Down
Loading
Loading