From 63cff144c8913380690118bc592a7c0599f5735c Mon Sep 17 00:00:00 2001 From: Thomas Braun <38082993+tbraun96@users.noreply.github.com> Date: Fri, 1 Jul 2022 12:47:52 -0400 Subject: [PATCH] attributes: ensure `res` and `err` events inherit `target` (#2184) ## Motivation Currently, when an `#[instrument]` attribute has an overridden target, the events generated by `ret` and `err` arguments do not inherit that target. For example, if I write ```rust #[tracing::instrument(target = "some_target", ret) fn do_stuff() -> Something { // ... } ``` the `do_stuff` span will have the target "some_target", but the return value event generated by `ret` will have the current module path as its target, and there is no way to change the return value event's target. ## Solution This branch changes the macro expansion for `#[instrument]` with the `ret` and/or `err` arguments so that an overridden target is propagated to the events generated by `ret` and `err`. Fixes #2183 --- tracing-attributes/Cargo.toml | 1 + tracing-attributes/src/expand.rs | 16 +++++++++------ tracing-attributes/src/lib.rs | 3 +++ tracing-attributes/tests/err.rs | 33 +++++++++++++++++++++++++++++++ tracing-attributes/tests/ret.rs | 34 ++++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/tracing-attributes/Cargo.toml b/tracing-attributes/Cargo.toml index 8b17aa37ab..3cd6fda842 100644 --- a/tracing-attributes/Cargo.toml +++ b/tracing-attributes/Cargo.toml @@ -43,6 +43,7 @@ tracing = { path = "../tracing", version = "0.2" } tracing-mock = { path = "../tracing-mock", features = ["tokio-test"] } tokio-test = { version = "0.3.0" } tracing-core = { path = "../tracing-core", version = "0.2"} +tracing-subscriber = { path = "../tracing-subscriber", version = "0.3", features = ["env-filter"] } async-trait = "0.1.44" [badges] diff --git a/tracing-attributes/src/expand.rs b/tracing-attributes/src/expand.rs index 0206beb89f..d4ef29fc7a 100644 --- a/tracing-attributes/src/expand.rs +++ b/tracing-attributes/src/expand.rs @@ -200,19 +200,23 @@ fn gen_block( )) })(); + let target = args.target(); + let err_event = match args.err_mode { Some(FormatMode::Default) | Some(FormatMode::Display) => { - Some(quote!(tracing::error!(error = %e))) + Some(quote!(tracing::error!(target: #target, error = %e))) } - Some(FormatMode::Debug) => Some(quote!(tracing::error!(error = ?e))), + Some(FormatMode::Debug) => Some(quote!(tracing::error!(target: #target, error = ?e))), _ => None, }; let ret_event = match args.ret_mode { - Some(FormatMode::Display) => Some(quote!(tracing::event!(#level, return = %x))), - Some(FormatMode::Default) | Some(FormatMode::Debug) => { - Some(quote!(tracing::event!(#level, return = ?x))) - } + Some(FormatMode::Display) => Some(quote!( + tracing::event!(target: #target, #level, return = %x) + )), + Some(FormatMode::Default) | Some(FormatMode::Debug) => Some(quote!( + tracing::event!(target: #target, #level, return = ?x) + )), _ => None, }; diff --git a/tracing-attributes/src/lib.rs b/tracing-attributes/src/lib.rs index f93b37a7f9..c7e81b32da 100644 --- a/tracing-attributes/src/lib.rs +++ b/tracing-attributes/src/lib.rs @@ -269,6 +269,9 @@ mod expand; /// } /// ``` /// +/// If a `target` is specified, both the `ret` and `err` arguments will emit outputs to +/// the declared target (or the default channel if `target` is not specified). +/// /// The `ret` and `err` arguments can be combined in order to record an event if a /// function returns [`Result::Ok`] or [`Result::Err`]: /// diff --git a/tracing-attributes/tests/err.rs b/tracing-attributes/tests/err.rs index 1629ac102f..d062a2b7c1 100644 --- a/tracing-attributes/tests/err.rs +++ b/tracing-attributes/tests/err.rs @@ -2,6 +2,8 @@ use tracing::collect::with_default; use tracing::Level; use tracing_attributes::instrument; use tracing_mock::*; +use tracing_subscriber::filter::EnvFilter; +use tracing_subscriber::subscribe::CollectExt; use std::convert::TryFrom; use std::num::TryFromIntError; @@ -219,3 +221,34 @@ fn test_err_display_default() { with_default(collector, || err().ok()); handle.assert_finished(); } + +#[test] +fn test_err_custom_target() { + let filter: EnvFilter = "my_target=error".parse().expect("filter should parse"); + let span = span::mock().named("error_span").with_target("my_target"); + + let (subscriber, handle) = collector::mock() + .new_span(span.clone()) + .enter(span.clone()) + .event( + event::mock() + .at_level(Level::ERROR) + .with_target("my_target"), + ) + .exit(span.clone()) + .drop_span(span) + .done() + .run_with_handle(); + + let subscriber = subscriber.with(filter); + + with_default(subscriber, || { + let error_span = tracing::error_span!(target: "my_target", "error_span"); + + { + let _enter = error_span.enter(); + tracing::error!(target: "my_target", "This should display") + } + }); + handle.assert_finished(); +} diff --git a/tracing-attributes/tests/ret.rs b/tracing-attributes/tests/ret.rs index 878a901d02..fce206e0f6 100644 --- a/tracing-attributes/tests/ret.rs +++ b/tracing-attributes/tests/ret.rs @@ -4,12 +4,19 @@ use tracing_mock::*; use tracing::{collect::with_default, Level}; use tracing_attributes::instrument; +use tracing_subscriber::subscribe::CollectExt; +use tracing_subscriber::EnvFilter; #[instrument(ret)] fn ret() -> i32 { 42 } +#[instrument(target = "my_target", ret)] +fn ret_with_target() -> i32 { + 42 +} + #[test] fn test() { let span = span::mock().named("ret"); @@ -30,6 +37,33 @@ fn test() { handle.assert_finished(); } +#[test] +fn test_custom_target() { + let filter: EnvFilter = "my_target=info".parse().expect("filter should parse"); + let span = span::mock() + .named("ret_with_target") + .with_target("my_target"); + + let (subscriber, handle) = collector::mock() + .new_span(span.clone()) + .enter(span.clone()) + .event( + event::mock() + .with_fields(field::mock("return").with_value(&tracing::field::debug(42))) + .at_level(Level::INFO) + .with_target("my_target"), + ) + .exit(span.clone()) + .drop_span(span) + .done() + .run_with_handle(); + + let subscriber = subscriber.with(filter); + + with_default(subscriber, ret_with_target); + handle.assert_finished(); +} + #[instrument(level = "warn", ret)] fn ret_warn() -> i32 { 42