Most of these changes are places where lifetimes were named, but can be
elided. Then a few cases where a lifetime was elided, but actually
resolves to a named lifetime. So lots of lifetimes.
When using `#[tracing::instrument]` and the `async unsafe` modifiers
the generated function read `unsafe async fn`, which is wrong. Corrected
the order and added a test.
Fixes: #2576
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
With the release of Rust 1.74, there are some new or modified clippy
lints that need adaption in the code.
The main change was the removal of the `private_in_public`.
https://rust-lang.github.io/rfcs/2145-type-privacy.html
Then two more changes were required, in one case to adhere a lint and
the other to allow it. When talking about what an "application" needs to
do when setting up `tracing-error`, it makes sense to include `fn
main()` in the doctest, even though the lint recommends against it.
When using a function annotated with `#[instrument]` it parses the
parameters of the function and records them either using `Value` or
using `std::fmt::Debug`. There were a few types that implement `Value`
but were missing the RecordTypes array. Added them + a unit test for a
single one.
Fixed: #2775
## Motivation
`#[tracing::instrument]` uses `unreachable!()` macro which needlessly
expands to panicking and formatting code. It only needs any `!` type.
## Solution
`loop {}` works just as well for a `!` type, and it crates less work for
the compiler. The code is truly unreachable, so the message would never
be useful. Rust used to be concerned about semantics of empty loops in
LLVM, but this [has been solved](https://reviews.llvm.org/D85393).
## Motivation
The instrument macro currently doesn't work with the "log" crate
feature: #2585
## Solution
Change the generated code to create a span if either
`tracing::if_log_enabled!` or `tracing::level_enabled!`. I'm not sure
how to add a test for this or if this is the best solution.
Fixes#2585
## Motivation
#2609 added an allow to generated code to allow a lint that was added in
Clippy 1.70.0. This was released with a patch bump so anyone who uses an
older version and latest tracing gets a compilation warning about an
unkonwn lint.
## Solution
Allowing unkonwn lints should fix this now and prevent similar issues in
the future. If the lints are unknown it will most likely be because the
lints are introduced only in newer compiler. There is just a higher risk
that a future contributor tries to add another allow and if they make a
typo, the issue will not be caught.
## Motivation
syn 2.0 is out!
## Solution
Update to syn 2.0 🚀
Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
As part of upgrading syn to 2.0 (e.g.,
https://github.com/tokio-rs/tracing/pull/2516), we need to bump the MSRV
to 1.56. As part of this PR, I've:
- Updated the text descriptions of what would be an in-policy MSRV bump
to use more recent versions of rustc. The _niceness_ of said version
numbers are purely coincidental.
- I've removed some of the exceptions made in CI.yml in order to support
some crates with a higher MSRV.
## Motivation
The current description for the default level of the `err` return value
event _strongly implies_ it's the same as the span. However, the
implementation actually defaults to `ERROR`.
## Solution
This PR documents that, so future generations don't have to chase down
the truth, like I did. 😉
## Motivation
The `#[instrument]` macro cannot be used on `const fn`s, because the
generated code will perform runtime tracing behavior. However, when
adding the attribute to a `const fn`, the compiler errors generated
currently are somewhat unclear (see #2414). It would be better if we
generated a less verbose error that simply states that `#[instrument]`
is not supported on `const fn`s.
## Solution
This branch changes the `#[instrument]` macro to detect when the
annotated function is a `const fn`, and emit a simpler, more descritpive
error message. The new error simply states that the `#[instrument]`
attribute cannot be used on `const fn`s, and should be much less
confusing to the user.
Fixes#2414
This branch adds documentation and tests noting that the `#[instrument]`
macro accepts `tracing::Level` directly. Using `tracing::Level` directly
allows for IDE autocomplete and earlier detection of typos.
The documentation for tracing-attributes was also rewritten to remove
the usage of the second-person perspective, making it more consistent
with the rest of tracing's documentation.
Co-authored-by: David Barsky <me@davidbarsky.com>
This branch adds the ability to override the level of the events
generated by the `ret` and `err` arguments to `#[instrument]`. An
overridden level can be specified with:
```rust
#[instrument(ret(level = "info"))]
```
```rust
#[instrument(err(level = Level::Debug))]
```
and so on.
This syntax is fully backwards compatible with existing uses of the
attribute.
In addition, some refactoring was done to how levels are parsed and how
the tokens for a specified level is generated.
Fixes#2330
## Motivation
When the `instrument` attribute is used on a function with inner
attributes, the proc macro generates code above the attributes within
the function block that causes compilation errors. These should be
parsed out separately and handled.
Fixes#2294
## Solution
I updated `MaybeItemFn` and `MaybeItemFnRef` to so they hold both the
outer and inner attributes for the instrumented function and updated the
codegen to inlcude them in the appropriate locations.
I couldn't preserve the existing implementation of
`From<&'_ ItemFn> for MaybeItemFnRef<'_, Box<Block>>`, because it is
now necessary to separate the inner and outer attributes of the
`ItemFn` into two separate `Vec`s. That implementation was replaced
with a `From<ItemFn> for MaybeItemFn`, which uses `Iterator::partition`
to separate out the inner and outer attributes.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Return type errors on instrumented async functions are a bit vague,
since the type error originates within the macro itself due to the
indirection of additional `async {}` blocks generated in the proc-macro
(and due to the way that inference propagates around in Rust).
This leads to a pretty difficult to understand error. For example:
```rust
#[instrument]
async fn foo() -> String {
""
}
```
results in...
```
error[E0308]: mismatched types
--> src/main.rs:1:1
|
1 | #[tracing::instrument]
| ^^^^^^^^^^^^^^^^^^^^^^- help: try using a conversion method: `.to_string()`
| |
| expected struct `String`, found `&str`
```
## Solution
Installs a fake `return` statement as the first thing that happens in
the auto-generated block of an instrumented async function.
This causes the coercion machinery within rustc to infer the right
return type (matching the the outer function) eagerly, as opposed to
after the `async {}` block has been type-checked.
This will cause us to both be able to point out the return type span
correctly, and properly suggest fixes on the expressions that cause the
type mismatch.
After this change, the example code above compiles to:
```
error[E0308]: mismatched types
--> src/main.rs:3:5
|
3 | ""
| ^^- help: try using a conversion method: `.to_string()`
| |
| expected struct `String`, found `&str`
|
note: return type inferred to be `String` here
--> src/main.rs:2:20
|
2 | async fn foo() -> String {
| ^^^^^^
```
## 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
This PR extends the `#[instrument]` attribute to accept an optional
`follows_from = …` argument that supplies any number of
`Span::follows_from` relationships to the generated `Span`.
## Motivation
This PR resolves#879.
## Solution
This PR largely follows the implementation strategy articulated by
@hawkw:
https://github.com/tokio-rs/tracing/issues/879#issuecomment-668168468
In that comment, @hawkw suggests taking one of two approaches:
1. each `follows_from` relationship is supplied with a distinct
`follows_from` argument
2. the `follows_from` argument is provided once, and its value is a
**list** of indirect causes
I take the second approach, since it is slightly more flexible: it
allows for the number of indirect causes to vary at runtime.
This addition is complemented by changes to `tracing-mock` to permit
making `follows_from` assertions for testing purposes.
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).
## Motivation
When using an `async` block (as an alternative to `async fn`, e.g. when
implementing a trait), `#[instrument]` adds extra braces around the
wrapped `async` block. This causes `rustc` to emit an `unused_braces`
lint in some cases (usually for single-line `async` blocks, as far as I
can tell). See #1831 for an example.
## Solution
Since the `async` block extracted by `AsyncInfo::from_fn` already has
braces around its statements, there's no need to wrap it with additional
braces. This updates `gen_block` to remove those extra braces when
generating the code providing the value of
`__tracing_instrument_future`.
- [x] add repros for `unused_braces` issue
- [x] remove extra braces from async blocks
Fixes#1831
This updates all crates' MSRVs to 1.49 if they were not already greater
than that (`tracing-appender` is at 1.53). Rust 1.49+ is required to
update `parking_lot` to v0.12 (see #1878). Also, `futures-task` (which I
believe is only needed as a transitive dep) now needs 1.45+, so this
also fixes our CI build.
Because `tracing-opentelemetry` previously required 1.46.0, it had a
separate CI MSRV job. Since 1.49.0 is greater than 1.46.0, the separate
check for `tracing-opentelemetry` is no longer needed.
In the process, I removed deprecated uses of
`core::atomic::spin_loop_hint`, which is replaced with
`core::hint::spin_loop` in 1.49.
This should take care of half of #1863, meaning it should instrument
functions returning an unboxed `async move` block.
This unfortunately does not cover generic `impl Future` returns, such as
`std::future::ready()` and similar :-(
* attributes: implement `#[instrument(ret)]`
## Motivation
Currently, users have to explicitly call logging functions at the end of
functions if they wanted to record values being returned. For example,
```rust
fn increment(a: i32) -> i32 {
let succ = a + 1;
tracing::debug!(?succ);
succ
}
```
The code above will be significantly simpler if we provide the
functionality to record returned values.
## Solution
This PR adds the exact feature we're talking here, which enables users
to write the code below instead.
```rust
#[instrument(ret)]
fn increment(a: i32) -> i32 {
a + 1
}
```
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, `tracing-attributes` consists of one very large `lib.rs`
module that's 1358 lines long. In my opinion, this makes the code
somewhat hard to navigate.
## Solution
This branch refactors `tracing-attributes` so that most of the code is
split into two separate modules: `attrs.rs`, which contains the types
representing the attribute arguments and the code for parsing them, and
`expand.rs`, which contains the code for actually expanding an
`#[instrument]` macro to generate the instrumented code. For the most
part, this was a pretty clean split.
I also did a small change to the way `async-trait` support is
implemented; moving the two steps for generating instrumented code for
`async-trait` to two methods on the `AsyncTraitInfo` struct; one for
determining if the instrumented function is also an `async-trait` method
and finding the necessary information to instrument it, and one for
generating the instrumented code. This feels a bit neater than doing all
of that in the `gen_function` function.
There shouldn't be any functional changes or significant
implementatation changes (besides the `async-trait` change) in this
branch; just moving code around.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Recent `rust-analyzer` versions enabled automatic expansion of proc
macro attributes by default. This is a problem with `#[instrument]`,
because it currently produces a `compile_error!` when parsing the code
inside the `#[instrument]`ed function fails, and *discards* those
tokens. This means that if the `#[instrument]` attribute is placed on a
function whose implementation fails to parse, recent versions of
`rust-analyzer` will no longer be able to display diagnostics for those
errors. In some cases, this can also break autocompletion.
## Solution
This branch changes `#[instrument]` to always expand to the tokens
contained in the `#[instrument]`ed function body, regardless of whether
or not they could be parsed successfully. Now, an error is only emitted
when the `#[instrument]` attribute *itself* could not be parsed. Since
the instrumented function is always expanded, any errors within that
function can be displayed properly by `rust-analyzer`.
Fixes#1633.
## Motivation
This PR attempts to solve #1630 by introducing `err(Debug)` meta to
`intrument` attribute macro. As `err` meta causes the error (`e`)
returned by instrumented function to be passed to `tracing::error!(error
= %e)` i.e. makes it use the `Display` implementation of `e`, the newly
added `err(Debug)` makes expands to `tracing::error!(error = ?e)` which
makes the `error!` macro to use `Debug` implementation for `e`. `err`
and `err(Debug)` are mutually exclusive, adding both will create a
compilation error.
`err(Display)` is also supported to specify `Display` explicitly.
As tried to describe, for some types implementing `Error` it might be
more suitable to use `Debug` implementation as in the case of
`eyre::Result`. This frees us to manually go over the error chain and
print them all, so that `instrument` attribute macro would do it for us.
## Solution
- Added a custom keyword `err(Debug)` similar to `err`,
- Add `err(Debug)` field to `InstrumentArgs`,
- Add parsing for `err(Debug)` arg and check for conflicts with `err`,
- Generate `tracing::error!(error = ?e)` when `err(Debug)` is `true` and
`tracing::error!(error = %e)` when `err(Display)` or `err` is `true`,
- Interpolate generated `err_block` into `Err` branches in both async
and sync return positions, if `err` or `err(Debug)` is `true`.
## Motivation
Apparently, using `quote_spanned!` can trigger a Clippy bug where the
text `else`, even inside a comment, _may_ cause the
`suspicious_else_formatting` lint to be triggered incorrectly (see
rust-lang/rust-clippy#7760 and rust-lang/rust-clippy#6249). This causes
the lint to fire in some cases when the `#[instrument]` attribute is
used on `async fn`s. See issue #1613 for details.
## Solution
It turns out that some of the uses of `quote_spanned!` in the
`tracing-attributes` code generation are not needed. We really only need
`quote_spanned!` when actually interpolating the user provided code into
a block, not in the `tracing-attributes` code that inserts the generated
code for producing the span etc. Replacing some of these
`quote_spanned!` uses with the normal `quote!` macro still generates
correct location diagnostics for errors in the user code, but fixes the
incorrect clippy lint.
I've added a few test cases that should reproduce the bug.
Fixes#1613
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
The changes in #1607 introduced a potential compilation error when using
the `#[instrument]` attribute on `async fn`s that return a type that
includes a closure or is otherwise unnameable. This is because the
future's body code was quoted in two separate places in order to have a
separate branch when the span is statically disabled. This means that
when a closure is returned, it will technically have two distinct types
based on whether or not the span is enabled, since it originates from
two separate source code locations (although `quote_spanned!` obscures
this, so the compiler diagnostic will appear to have two closures
originating from the same location).
## Solution
This branch fixes this issue by changing the code generated for
`#[instrument]`ed async functions. Unfortunately, for async functions,
we can't have the optimization of not creating the span at all when the
level is disabled, because we need to create the span _before_ creating
the future, as it may borrow arguments.
I've also added tests reproducing issue #1615Fixes#1615
Currently, `tracing-attributes` generates a `let _ = ();` in between the
`if tracing::level_enabled!(...) {}` and the function body. This is
intended to suppress the `clippy::suspicious_else_formatting` lint,
which is generated when an `if` is followed immediately by a block with
no whitespace in between. Since we can't add whitespace in the generated
code (as `quote` produces a stream of _rust tokens_, not text), we
can't suppress the lint without adding a no-op statement.
However, unfortunately, the no-op let triggers a _different_ lint
(`clippy::let_unit_value`), when `clippy::pedantic` is enabled. This is
kind of annoying for some users.
This branch changes the code to suppress the
`suspicious_else_formatting` lint using `#[allow(...)]` attributes,
instead. The warning is turned back on inside of user code, since users
probably want the warning.
## Motivation
In #1600, the `instrument` code generation was changed to avoid ever
constructing a `Span` struct if the level is explicitly disabled.
However, for `async` functions, `#[instrument]` will currently still
create the span, but simply skips constructing an `Instrument` future if
the level is disabled.
## Solution
This branch changes the `#[instrument]` code generation for async blocks
to totally skip constructing the span if the level is disabled. I also
simplfied the code generation a bit by combining the shared code between
the `err` and non-`err` cases, reducing code duplication a bit.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Adding `#[instrument(level = "debug")]` attributes to functions in rustc
caused a performance regression (in release, where `debug!` is fully
optimized out) across all crates:
https://github.com/rust-lang/rust/pull/89048#issuecomment-929566530
While trying to debug this, I noticed that spans don't have the same
advantage that events have wrt to how LLVM sees them. Spans (or more
precisely, the enter-guard), will get dropped at the end of the scope,
which throws a spanner into the LLVM optimization pipeline. I am not
entirely sure where the problem is, but I am moderately certain that the
issue is that even entering a dummy span is too much code for LLVM to
reliably (or at all) optimize out.
## Solution
My hope is that in trusting the Rust compiler to generate cool code when using
drop flags, we can essentially generate a drop flag that depends on something we
know (due to events working as expected) to be optimizable.
So instead of doing
```rust
let _x = span!();
let _y = _x.enter();
// lotsa code
drop(_y)
```
we do
```rust
let _x;
let _y;
let must_drop = false;
if level_enabled!(DEBUG) {
must_drop = true;
_x = span!();
_y = _x.enter();
}
// lotsa code
if must_drop {
drop(_y)
}
```
I believe this will allow LLVM to properly optimize this again. Testing that
right now, but I wanted to open this PR immediately for review.
## Motivation
The default behavior of `tracing::instrument` attribution will record
all of the function arguments as `fmt::Debug`, which is overwhelmed and
unnecessary for those primitive types, such as `bool`, `u8`, `i8`,
`u16`, `i16`, `u32`, `i32`, `u64`, `i64`, `usize`, and `isize`. Another
concerning reason is that we‘ll lose the type info of those primitive
types when record by a `Visitor`, while those type infos is essential to
some people. For example, I need to show my spans in Jaeger UI.
## Solution
Make the `tracing::instrument` records other function arguments as
`fmt::Debug ` while not for those primitive types.
However, I'm not good at naming. Maybe the `RecordType` enum and its
variant aren't a good name? I'd love to seek suggestions. Thanks.
Fixes#1296.
I had forgotten to use all the input statements in #1228, so we would
delete nearly all the statement of sync functions that return a boxed
future :/
It works with both the old and new version of async-trait (except for
one doc test that failed previously, and that works with the new
version). One nice thing is that the code is simpler (e.g.g no self
renaming to _self, which will enable some simplifications in the
future).
A minor nitpick is that I disliked the deeply nested pattern matching in
get_async_trait_kind (previously: get_async_trait_function), so I
"flattened" that a bit.
Fixes #1219.
