From 679983fed70c3104aa8d3dbbda387c81984c5478 Mon Sep 17 00:00:00 2001 From: liuhaozhan Date: Mon, 25 Apr 2022 17:54:17 -0400 Subject: [PATCH] =?UTF-8?q?attributes:=20permit=20setting=20parent=20span?= =?UTF-8?q?=20via=20`#[instrument(parent=20=3D=20=E2=80=A6)]`=20(#2091)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR extends the `#[instrument]` attribute to accept an optionally `parent = …` argument that provides an explicit parent to the generated `Span`. ## Motivation This PR resolves #2021 and partially resolves #879. (It only partly resolves #879 in that it only provides a mechanism for specifying an explicit parent, but *not* for invoking `follows_from`.) ## Solution This PR follows the implementation strategy articulated by @hawkw: https://github.com/tokio-rs/tracing/issues/879#issuecomment-668168468. The user-provided value to the `parent` argument may be any expression, and this expression is provided directly to the invocation of [`span!`](https://tracing.rs/tracing/macro.span.html). --- tracing-attributes/src/attr.rs | 25 +++++++ tracing-attributes/src/expand.rs | 3 + tracing-attributes/src/lib.rs | 23 +++++++ tracing-attributes/tests/parents.rs | 102 ++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+) create mode 100644 tracing-attributes/tests/parents.rs diff --git a/tracing-attributes/src/attr.rs b/tracing-attributes/src/attr.rs index bde089f..34ca3d6 100644 --- a/tracing-attributes/src/attr.rs +++ b/tracing-attributes/src/attr.rs @@ -11,6 +11,7 @@ pub(crate) struct InstrumentArgs { level: Option, pub(crate) name: Option, target: Option, + pub(crate) parent: Option, pub(crate) skips: HashSet, pub(crate) fields: Option, pub(crate) err_mode: Option, @@ -122,6 +123,12 @@ impl Parse for InstrumentArgs { } let target = input.parse::>()?.value; args.target = Some(target); + } else if lookahead.peek(kw::parent) { + if args.target.is_some() { + return Err(input.error("expected only a single `parent` argument")); + } + let parent = input.parse::>()?; + args.parent = Some(parent.value); } else if lookahead.peek(kw::level) { if args.level.is_some() { return Err(input.error("expected only a single `level` argument")); @@ -180,6 +187,23 @@ impl Parse for StrArg { } } +struct ExprArg { + value: Expr, + _p: std::marker::PhantomData, +} + +impl Parse for ExprArg { + fn parse(input: ParseStream<'_>) -> syn::Result { + let _ = input.parse::()?; + let _ = input.parse::()?; + let value = input.parse()?; + Ok(Self { + value, + _p: std::marker::PhantomData, + }) + } +} + struct Skips(HashSet); impl Parse for Skips { @@ -360,6 +384,7 @@ mod kw { syn::custom_keyword!(skip); syn::custom_keyword!(level); syn::custom_keyword!(target); + syn::custom_keyword!(parent); syn::custom_keyword!(name); syn::custom_keyword!(err); syn::custom_keyword!(ret); diff --git a/tracing-attributes/src/expand.rs b/tracing-attributes/src/expand.rs index 5fe71fe..e2004e8 100644 --- a/tracing-attributes/src/expand.rs +++ b/tracing-attributes/src/expand.rs @@ -135,6 +135,8 @@ fn gen_block( let target = args.target(); + let parent = args.parent.iter(); + // filter out skipped fields let quoted_fields: Vec<_> = param_names .iter() @@ -182,6 +184,7 @@ fn gen_block( quote!(tracing::span!( target: #target, + #(parent: #parent,)* #level, #span_name, #(#quoted_fields,)* diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs index 335245f..6ac07ce 100644 --- a/tracing-attributes/src/lib.rs +++ b/tracing-attributes/src/lib.rs @@ -153,6 +153,29 @@ mod expand; /// // ... /// } /// ``` +/// Overriding the generated span's parent: +/// ``` +/// # use tracing_attributes::instrument; +/// #[instrument(parent = None)] +/// pub fn my_function() { +/// // ... +/// } +/// ``` +/// ``` +/// # use tracing_attributes::instrument; +/// // A struct which owns a span handle. +/// struct MyStruct +/// { +/// span: tracing::Span +/// } +/// +/// impl MyStruct +/// { +/// // Use the struct's `span` field as the parent span +/// #[instrument(parent = &self.span, skip(self))] +/// fn my_method(&self) {} +/// } +/// ``` /// /// To skip recording an argument, pass the argument's name to the `skip`: /// diff --git a/tracing-attributes/tests/parents.rs b/tracing-attributes/tests/parents.rs new file mode 100644 index 0000000..c4d375f --- /dev/null +++ b/tracing-attributes/tests/parents.rs @@ -0,0 +1,102 @@ +use tracing::{collect::with_default, Id, Level}; +use tracing_attributes::instrument; +use tracing_mock::*; + +#[instrument] +fn with_default_parent() {} + +#[instrument(parent = parent_span, skip(parent_span))] +fn with_explicit_parent

(parent_span: P) +where + P: Into>, +{ +} + +#[test] +fn default_parent_test() { + let contextual_parent = span::mock().named("contextual_parent"); + let child = span::mock().named("with_default_parent"); + + let (collector, handle) = collector::mock() + .new_span( + contextual_parent + .clone() + .with_contextual_parent(None) + .with_explicit_parent(None), + ) + .new_span( + child + .clone() + .with_contextual_parent(Some("contextual_parent")) + .with_explicit_parent(None), + ) + .enter(child.clone()) + .exit(child.clone()) + .enter(contextual_parent.clone()) + .new_span( + child + .clone() + .with_contextual_parent(Some("contextual_parent")) + .with_explicit_parent(None), + ) + .enter(child.clone()) + .exit(child) + .exit(contextual_parent) + .done() + .run_with_handle(); + + with_default(collector, || { + let contextual_parent = tracing::span!(Level::TRACE, "contextual_parent"); + + with_default_parent(); + + contextual_parent.in_scope(|| { + with_default_parent(); + }); + }); + + handle.assert_finished(); +} + +#[test] +fn explicit_parent_test() { + let contextual_parent = span::mock().named("contextual_parent"); + let explicit_parent = span::mock().named("explicit_parent"); + let child = span::mock().named("with_explicit_parent"); + + let (collector, handle) = collector::mock() + .new_span( + contextual_parent + .clone() + .with_contextual_parent(None) + .with_explicit_parent(None), + ) + .new_span( + explicit_parent + .with_contextual_parent(None) + .with_explicit_parent(None), + ) + .enter(contextual_parent.clone()) + .new_span( + child + .clone() + .with_contextual_parent(Some("contextual_parent")) + .with_explicit_parent(Some("explicit_parent")), + ) + .enter(child.clone()) + .exit(child) + .exit(contextual_parent) + .done() + .run_with_handle(); + + with_default(collector, || { + let contextual_parent = tracing::span!(Level::INFO, "contextual_parent"); + let explicit_parent = tracing::span!(Level::INFO, "explicit_parent"); + + contextual_parent.in_scope(|| { + with_explicit_parent(&explicit_parent); + }); + }); + + handle.assert_finished(); +}