There was only a single case of the new `needless_as_bytes` lint which
was triggered and needed to be fixed.
There was also a "UI" test in `tracing-attributes` that needed to be
updated because the error text has changed (it gives more details of
course).
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>
## Motivation
When recording the parent of an event or span, the `MockCollector`
treats an explicit parent of `None` (i.e. an event or span that is an
explicit root) in the same way as if there is no explicit root. This
leads to it picking up the contextual parent or treating the event or
span as a contextual root.
## Solution
This change refactors the recording of the parent to use `is_contextual`
to distinguish whether or not an explicit parent has been specified. The
actual parent is also written into an `Ancestry` enum so that the
expected and actual values can be compared in a more explicit way.
Additionally, the `Ancestry` struct has been moved into its own module and
the check behavior has been fixed. The error message has also been
unified across all cases.
Another problem with the previous API is that the two methods
`with_contextual_parent` and `with_explicit_parent` are actually
mutually exclusive, a span or event cannot be both of them. It is also a
(small) mental leap for the user to go from `with_*_parent(None)` to
understanding that this means that a span or event is a root (either
contextual or explicit).
As such, the API has been reworked into a single method `with_ancestry`,
which takes an enum with the following four variants:
* `HasExplicitParent(String)` (parent span name)
* `IsExplicitRoot`
* `HasContextualParent(String)` (parent span name)
* `IsContextualRoot`
To make the interface as useable as possible, helper functions have been
defined in the `expect` module which can be used to create the enum
variants. Specifically, these take `Into<String>` parameter for the span
name.
Given the number of different cases involved in checking ancestry,
separate integration tests have been added to `tracing-mock`
specifically for testing all the positive and negative cases when
asserting on the ancestry of events and spans.
There were two tests in `tracing-attributes` which specified both an
explicit and a contextual parent. This behavior was never intended to
work as all events and spans are either contextual or not. The tests
have been corrected to only expect one of the two.
Fixes: #2440
## Motivation
Currently, a keyword like `type` fails compilation as (a path segment of) a field name, for no clear reason. Trying to use `r#type` instead leads to the `r#` being part of the field name, which is unhelpful¹.
## Solution
Don't require the field path to match a `macro_rules!` `expr`, use repeated `tt` instead. I can't tell why this was ever required: The internal stringify macro was introduced in 55091c92ed (diff-315c02cd05738da173861537577d159833f70f79cfda8cd7cf1a0d7a28ace31b) with an `expr` matcher without any explanation, and no tests are failing from making it match upstream's `stringify!` input format.
Special thanks to whoever implemented the unstable `macro-backtrace` feature in rustc, otherwise this would have been nigh impossible to track down!
¹ this can likely be fixed too by some sort of "unraw" macro that turns `r#foo` into `foo`, but that's a separate change not made in this PR
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.
This change adds documentation to the tracing-mock span module and all
the public APIs within it. This includes doctests on all the methods
which serve as examples.
Additionally, the validation on `ExpectedSpan` was improved so that it
validates the level and target during `enter` and `exit` as well as on
`new_span`.
The method `ExpectedSpan::with_field` was renamed to `with_fields`
(plural) to match the same method on `ExpectedEvent` (and because
multiple fields can be passed to it).
A copy-paste typo was also fixed in the documentation for
`ExpectedEvent::with_contextual_parent`.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
## Motivation
There has been interest around publishing tracing-mock to crates.io
for some time. In order to make this possible, it needs to be cleaned up.
## Solution
There are some test utils in the `tracing-mock` crate which wouldn't
make sense to publish. They provide test futures that are needed in
multiple `tracing-*` crates, but would likely not be needed outside that
context.
This change moves that functionality into a separate `tracing-test`
crate, which should never be published to crates.io.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
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
Clippy doesn't like the redefinition of a binding with itself. I don't
think this was necessary for the bug we were reproducing here (as the
error test doesn't do this), so I removed it.
As part of landing #2728, I noticed that the `-Zminimal-versions` check fails
due to proc-macro2 1.0.40 referencing a since-removed, nightly-only
feature (`proc_macro_span_shrink`). Since this change doesn't change the MSRV
of `proc-macro2` (or `tracing`, for that matter), this feels like a safe change
to make.
## 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.
updated UI tests using TRYBUILD=overwrite with the latest stable version of Rust
## Motivation
UI tests are failing on the latest stable version of Rust
## Solution
Run `TRYBUILD=overwrite cargo test` to update the effected files.
## Motivation
Currently it is not possible to disable ANSI in `fmt::Subscriber`
without enabling the "ansi" crate feature. This makes it difficult for
users to implement interoperable settings that are controllable with
crate features without having to pull in the dependencies "ansi" does.
I hit this while writing an application with multiple logging options
set during compile-time and I wanted to cut down on dependencies if
possible.
## Solution
This changes `fmt::Subscriber::with_ansi()` to not require the "ansi"
feature flag. This way, `with_ansi(false)` can be called even when the
"ansi" feature is disabled. Calling `with_ansi(true)` when the "ansi"
feature is not enabled will panic in debug mode, or print a warning if
debug assertions are disabled.
Co-authored-by: daxpedda <daxpedda@gmail.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## 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
The error message suggesting that you remove a semicolon to return a
value (instead of unit) was updated slightly in rust-lang/rust#102650,
which was included in Rust 1.66.0.
This causes one of the tests in tracing-attributes to fail. We fix it by
using the updated error message.
* mock: change helper functions to `expect::<thing>`
The current format of test expectations in `tracing-mock` isn't ideal.
The format `span::expect` requires importing `tracing_mock::<thing>` which
may conflict with imports from other tracing crates, especially
`tracing-core`.
So we change the order and move the functions into a module called
`expect` so that:
* `event::expect` becomes `expect::event`
* `span::expect` becomes `expect::span`
* `field::expect` becomes `expect::field`
This format has two advantages.
1. It reads as natural English, e.g "expect span"
2. It is no longer common to import the modules directly.
Regarding point (2), the following format was previously common:
```rust
use tracing_mock::field;
field::expect();
```
This import of the `field` module may then conflict with importing the
same from `tracing_core`, making it necessary to rename one of the
imports.
The same code would now be written:
```rust
use tracing_mock::expect;
expect::field();
```
Which is less likely to conflict.
This change also fixes an unused warning on `MockHandle::new` when the
`tracing-subscriber` feature is not enabled.
Refs: #539
The `tracing-mock` crate provides a mock collector (and a subscriber for
use by the tests in the `tracing-subscriber` crate) which is able to
make assertions about what diagnostics are emitted.
These assertions are defined by structs that match on events, span, and
their fields and metadata. The structs that matched these objects have
been called, up until now, mocks, however this terminology may be
misleading, as the created objects don't mock anything.
There were two different names for similar functionality with `only()`
and `done()` on fields and collectors/subscribers respectively. Using a
single name for these may make it easier to onboard onto `tracing-mock`.
To reduce confusion, these structs have been split into two categories:
mocks and expectations.
Additionally, the `done()` function on the `Collector` and `Subscriber`
mocks has been replaced with `only()`. This matches the similar function
for `ExpectedField`, and may be more intuitive.
The mocks replace some component in the tracing ecosystem when a library
is under test. The expectations define the assertions we wish to make
about traces received by the mocks.
Mocks (per module):
* collector - `MockCollector`, no change
* subscriber - `MockSubscriber`, renamed from `ExpectSubscriber`
Expectations (per module):
* event - `ExpectedEvent`, renamed from `MockEvent`
* span - `ExpectedSpan`, renamed from `MockSpan`
* field - `ExpectedField` and `ExpectedFields`, renamed from `MockField`
and `Expected`. Also `ExpectedValue` renamed from `MockValue`.
* metadata - `ExpectedMetadata`, renamed from `Expected`
Refs: #539
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>
The `async_fn` test file in `tracing-attributes` contains several
functions that exist just to test whether they _compile_, rather than
make assertions about their behavior. Because these functions are never
called, they (naturally) emit dead code warnings.
This commit adds `#[allow(dead_code)]` to the compilation tests, plus a
comment explaining why we do this.
The error messages for these tests appear to have changed a bit with
Rust 1.64. This commit includes changes to the `.stderr` file after
running the tests with `TRYBUILD=overwrite`, in order to update the
expected output for the latest Rust.
## 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
Fix minimal-versions failure.
## Solution
Upgrade all the dependencies to their most recent semver-compatible
version, adjusting back down as necessary for MSRV.
## Context
[cargo-minimal-versions](https://lib.rs/crates/cargo-minimal-versions)
is wonderful. With this PR, the full repo passes under all of
- `cargo hack --workspace minimal-versions check --all-features`
- `cargo +1.49 hack --workspace --exclude tracing-appender
minimal-versions check --all-features`
- `cargo +1.53 hack -p tracing-appender minimal-versions check
--all-features`
- All of CI 😇
## 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
## Motivation
Currently, it is rather difficult to record `String`s as field values,
even though `&str` is a primitive `Value` type. For example, this code
does not currently compile:
```rust
let my_string = String::from("hello world!");
tracing::debug!(my_string);
```
Instead, it is necessary to explicitly call `String::as_str` or a
similar conversion method:
```rust
let my_string = String::from("hello world!");
tracing::debug!(my_string = my_string.as_str());
```
This is unfortunate, as it makes a fairly straightforward, commomplace
task (recording a `String` as a field) unnecessarily verbose.
## Solution
This branch adds an `impl Value for String` in `tracing-core` when the
"alloc" feature flag is enabled. The impl simply calls `String::as_str`
and then calls `record_str`. Because `Value` takes an `&self`, and there
are preexisting `impl<T: Value> Value` for `&T` and `&mut T`, the string
is not consumed, and `&String` or `&mut String`s can also be used as
`Value`s.
I've also added tests validating that this actually works.
