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

shim: add tracing macros #269

Merged
merged 5 commits into from
May 20, 2024
Merged

shim: add tracing macros #269

merged 5 commits into from
May 20, 2024

Conversation

Mossaka
Copy link
Member

@Mossaka Mossaka commented May 17, 2024

This PR introduces tracing instrumentation to the shim crate. It will allow us to collect detailed function entry and exit, args and returns to understand the behaviour of the shim process.

An use case for runwasi shim is that it will collect the traces and transform them to OTLP traces and output them to an jeager endpoint (see containerd/runwasi#582).

From my observation the perf overhead with tracing macros is minimal. If this is a concern, we can try disabling tracing staticlly. Statically, we may use rust conditional compilation feature to add either compile or ignore tracing macros that are added to each function.

Update: I've created a new shim_instrument proc-macro that will statically disable/enable tracing for each function. if the feature tracing is not enabled, the instrument macro is acts as a no-opt. Otherwise, it will apply tracing::instrument to that function.

Signed-off-by: jiaxiao zhou <[email protected]>
@Mossaka Mossaka marked this pull request as ready for review May 17, 2024 00:10
@github-actions github-actions bot added the C-shim Containerd shim label May 17, 2024
@github-actions github-actions bot added T-docs Improvements or additions to documentation T-CI Changes in project's CI labels May 17, 2024
Signed-off-by: jiaxiao zhou <[email protected]>
@mxpv
Copy link
Member

mxpv commented May 20, 2024

Do we need to maintain one more crate for this?
This seems to be achievable with:

#[cfg_attr(feature = "tracing", tracing::instrument)]

@Mossaka
Copy link
Member Author

Mossaka commented May 20, 2024

Do we need to maintain one more crate for this? This seems to be achievable with:

#[cfg_attr(feature = "tracing", tracing::instrument)]

Ah yeah this looks like a better idea 👍.

@github-actions github-actions bot removed T-docs Improvements or additions to documentation T-CI Changes in project's CI labels May 20, 2024
crates/shim/src/args.rs Outdated Show resolved Hide resolved
@mxpv
Copy link
Member

mxpv commented May 20, 2024

Same applies to level.

Unless overridden, a span with the [INFO] will be generated.

@mxpv mxpv added this pull request to the merge queue May 20, 2024
Merged via the queue into containerd:main with commit 200f788 May 20, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-shim Containerd shim
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants