Skip to content

Commit

Permalink
attributes: ensure res and err events inherit target (#2184)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
tbraun96 authored and hawkw committed Jul 1, 2022
1 parent e0b3f79 commit 4a3f299
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 6 deletions.
1 change: 1 addition & 0 deletions tracing-attributes/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ quote = "1"
[dev-dependencies]
tracing = { path = "../tracing", version = "0.1" }
tracing-mock = { path = "../tracing-mock", features = ["tokio-test"] }
tracing-subscriber = { path = "../tracing-subscriber", version = "0.3", features = ["env-filter"] }
tokio-test = { version = "0.3.0" }
tracing-core = { path = "../tracing-core", version = "0.1"}
async-trait = "0.1.44"
Expand Down
16 changes: 10 additions & 6 deletions tracing-attributes/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,19 +200,23 @@ fn gen_block<B: ToTokens>(
))
})();

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,
};

Expand Down
3 changes: 3 additions & 0 deletions tracing-attributes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,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`]:
///
Expand Down
33 changes: 33 additions & 0 deletions tracing-attributes/tests/err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use tracing::subscriber::with_default;
use tracing::Level;
use tracing_attributes::instrument;
use tracing_mock::*;
use tracing_subscriber::filter::EnvFilter;
use tracing_subscriber::layer::SubscriberExt;

use std::convert::TryFrom;
use std::num::TryFromIntError;
Expand Down Expand Up @@ -198,3 +200,34 @@ fn test_err_display_default() {
with_default(subscriber, || 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) = subscriber::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();
}
34 changes: 34 additions & 0 deletions tracing-attributes/tests/ret.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@ use tracing_mock::*;

use tracing::{subscriber::with_default, Level};
use tracing_attributes::instrument;
use tracing_subscriber::layer::SubscriberExt;
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");
Expand All @@ -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) = subscriber::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
Expand Down

0 comments on commit 4a3f299

Please sign in to comment.