From 270261942739db14ff84814dc73b844b145d5db3 Mon Sep 17 00:00:00 2001 From: Pat Hickey Date: Thu, 27 Oct 2022 11:26:54 -0700 Subject: [PATCH] wiggle: allow disable tracing in Wiggle-generated code (#5146) Wiggle generates code that instruments APIs with tracing code. This is handy for diagnosing issues at runtime, but when inspecting the output of Wiggle, it can make the generated code difficult for a human to decipher. This change makes tracing a default but optional feature, allowing users to avoid tracing code with commands like `cargo expand --no-default-features`. This should be no change for current crates depending on `wiggle`, `wiggle-macro`, and `wiggle-generate`. review: add 'tracing' feature to wasi-common review: switch to using macro configuration parsing Co-authored-by: Andrew Brown --- crates/wasi-common/src/snapshots/preview_1.rs | 2 +- .../wiggle/generate/src/codegen_settings.rs | 6 +++ crates/wiggle/generate/src/config.rs | 15 ++++++ crates/wiggle/generate/src/funcs.rs | 48 ++++++++++++------- crates/wiggle/macro/src/lib.rs | 12 +++-- 5 files changed, 63 insertions(+), 20 deletions(-) diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 3b8eae51f5..664a0508e1 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -25,7 +25,7 @@ wiggle::from_witx!({ // keeping that set the same in this macro and the wasmtime_wiggle / lucet_wiggle macros is // tedious, and there is no cost to having a sync function be async in this case. async: *, - wasmtime: false + wasmtime: false, }); impl wiggle::GuestErrorType for types::Errno { diff --git a/crates/wiggle/generate/src/codegen_settings.rs b/crates/wiggle/generate/src/codegen_settings.rs index 144d2be282..ced7b0cbe4 100644 --- a/crates/wiggle/generate/src/codegen_settings.rs +++ b/crates/wiggle/generate/src/codegen_settings.rs @@ -12,6 +12,10 @@ pub struct CodegenSettings { pub errors: ErrorTransform, pub async_: AsyncConf, pub wasmtime: bool, + // Disabling this feature makes it possible to remove all of the tracing + // code emitted in the Wiggle-generated code; this can be helpful while + // inspecting the code (e.g., with `cargo expand`). + pub tracing: bool, } impl CodegenSettings { pub fn new( @@ -19,12 +23,14 @@ impl CodegenSettings { async_: &AsyncConf, doc: &Document, wasmtime: bool, + tracing: bool, ) -> Result { let errors = ErrorTransform::new(error_conf, doc)?; Ok(Self { errors, async_: async_.clone(), wasmtime, + tracing, }) } pub fn get_async(&self, module: &Module, func: &InterfaceFunc) -> Asyncness { diff --git a/crates/wiggle/generate/src/config.rs b/crates/wiggle/generate/src/config.rs index 3d164d70f1..4c71ea7f3e 100644 --- a/crates/wiggle/generate/src/config.rs +++ b/crates/wiggle/generate/src/config.rs @@ -15,6 +15,7 @@ pub struct Config { pub errors: ErrorConf, pub async_: AsyncConf, pub wasmtime: bool, + pub tracing: bool, } mod kw { @@ -24,6 +25,7 @@ mod kw { syn::custom_keyword!(errors); syn::custom_keyword!(target); syn::custom_keyword!(wasmtime); + syn::custom_keyword!(tracing); } #[derive(Debug, Clone)] @@ -32,6 +34,7 @@ pub enum ConfigField { Error(ErrorConf), Async(AsyncConf), Wasmtime(bool), + Tracing(bool), } impl Parse for ConfigField { @@ -67,6 +70,10 @@ impl Parse for ConfigField { input.parse::()?; input.parse::()?; Ok(ConfigField::Wasmtime(input.parse::()?.value)) + } else if lookahead.peek(kw::tracing) { + input.parse::()?; + input.parse::()?; + Ok(ConfigField::Tracing(input.parse::()?.value)) } else { Err(lookahead.error()) } @@ -79,6 +86,7 @@ impl Config { let mut errors = None; let mut async_ = None; let mut wasmtime = None; + let mut tracing = None; for f in fields { match f { ConfigField::Witx(c) => { @@ -105,6 +113,12 @@ impl Config { } wasmtime = Some(c); } + ConfigField::Tracing(c) => { + if tracing.is_some() { + return Err(Error::new(err_loc, "duplicate `tracing` field")); + } + tracing = Some(c); + } } } Ok(Config { @@ -114,6 +128,7 @@ impl Config { errors: errors.take().unwrap_or_default(), async_: async_.take().unwrap_or_default(), wasmtime: wasmtime.unwrap_or(true), + tracing: tracing.unwrap_or(true), }) } diff --git a/crates/wiggle/generate/src/funcs.rs b/crates/wiggle/generate/src/funcs.rs index b30d05eef4..bdcc7a0ea4 100644 --- a/crates/wiggle/generate/src/funcs.rs +++ b/crates/wiggle/generate/src/funcs.rs @@ -84,6 +84,16 @@ fn _define_func( ); ); if settings.get_async(&module, &func).is_sync() { + let traced_body = if settings.tracing { + quote!( + #mk_span + _span.in_scope(|| { + #body + }) + ) + } else { + quote!(#body) + }; ( quote!( #[allow(unreachable_code)] // deals with warnings in noreturn functions @@ -93,15 +103,23 @@ fn _define_func( #(#abi_params),* ) -> Result<#abi_ret, #rt::wasmtime_crate::Trap> { use std::convert::TryFrom as _; - #mk_span - _span.in_scope(|| { - #body - }) + #traced_body } ), bounds, ) } else { + let traced_body = if settings.tracing { + quote!( + use #rt::tracing::Instrument as _; + #mk_span + async move { + #body + }.instrument(_span) + ) + } else { + quote!(#body) + }; ( quote!( #[allow(unreachable_code)] // deals with warnings in noreturn functions @@ -111,11 +129,7 @@ fn _define_func( #(#abi_params),* ) -> impl std::future::Future> + 'a { use std::convert::TryFrom as _; - use #rt::tracing::Instrument as _; - #mk_span - async move { - #body - }.instrument(_span) + #traced_body } ), bounds, @@ -243,7 +257,7 @@ impl witx::Bindgen for Rust<'_> { args.push(quote!(#name)); } } - if func.params.len() > 0 { + if self.settings.tracing && func.params.len() > 0 { let args = func .params .iter() @@ -272,12 +286,14 @@ impl witx::Bindgen for Rust<'_> { let ret = #trait_name::#ident(ctx, #(#args),*).await; }) }; - self.src.extend(quote! { - #rt::tracing::event!( - #rt::tracing::Level::TRACE, - result = #rt::tracing::field::debug(&ret), - ); - }); + if self.settings.tracing { + self.src.extend(quote! { + #rt::tracing::event!( + #rt::tracing::Level::TRACE, + result = #rt::tracing::field::debug(&ret), + ); + }); + } if func.results.len() > 0 { results.push(quote!(ret)); diff --git a/crates/wiggle/macro/src/lib.rs b/crates/wiggle/macro/src/lib.rs index 0e37a79289..72370593a2 100644 --- a/crates/wiggle/macro/src/lib.rs +++ b/crates/wiggle/macro/src/lib.rs @@ -153,6 +153,7 @@ pub fn from_witx(args: TokenStream) -> TokenStream { &config.async_, &doc, config.wasmtime, + config.tracing, ) .expect("validating codegen settings"); @@ -189,9 +190,14 @@ pub fn wasmtime_integration(args: TokenStream) -> TokenStream { let doc = config.c.load_document(); let names = wiggle_generate::Names::new(quote!(wiggle)); - let settings = - wiggle_generate::CodegenSettings::new(&config.c.errors, &config.c.async_, &doc, true) - .expect("validating codegen settings"); + let settings = wiggle_generate::CodegenSettings::new( + &config.c.errors, + &config.c.async_, + &doc, + true, + config.c.tracing, + ) + .expect("validating codegen settings"); let modules = doc.modules().map(|module| { wiggle_generate::wasmtime::link_module(&module, &names, Some(&config.target), &settings)