This adds a section to the `Level` docs explaining the comparison rules
for `Level` and `LevelFilter`. It turns out this wasn't really
explicitly documented anywhere.
I may have gone a bit overboard on this, but I think the new docs should
be helpful...
See https://github.com/tokio-rs/tracing/pull/1274#discussion_r658339699
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: David Barsky <me@davidbarsky.com>
## Motivation
Fixes#1429
## Solution
Implemented as described in #1429, and migrated all internal uses of the
deprecated methods. All tests passed both before and after the migration
(9ec8130).
- Add a new method `SpanRef::scope(&self)` that returns a leaf-to-root
`Iterator`, including the specified leaf
- Add a new method `Scope::from_root(self)` (where `Scope` is the type
returned by `SpanRef::scope` defined earlier) that returns a
root-to-leaf `Iterator` that ends at the current leaf (in other
words: it's essentially the same as
`Scope::collect::<Vec<_>>().into_iter().rev()`)
- Deprecate all existing iterators, since they can be replaced by the
new unified mechanism:
- `Span::parents` is equivalent to `Span::scope().skip(1)` (although
the `skip` is typically a bug)
- `Span::from_root` is equivalent to `Span::scope().skip(1).from_root()`
(although the `skip` is typically a bug)
- `Context::scope` is equivalent to
`Context::lookup_current().scope().from_root()` (although the
`lookup_current` is sometimes a bug, see also #1428)
Impl `Value` for `&mut T` where `T: Value`.
#1378 improve the `tracing::instrument` macro's recording behavior: all
primitive types implementing the `Value` trait will be recorded as
fields of that type instead of `fmt::Debug`. This PR is a prerequisite
for those `&mut` function arguments to achieve that.
## Motivation
Get the string representation of the `Level` is quite a common usecase.
Without this method, I normally need to implement it by myself, which
is fairly noisy.
```rust
#[inline]
fn level_to_str(level: &tracing::Level) -> &'static str {
match *level {
tracing::Level::TRACE => "TRACE",
tracing::Level::DEBUG => "DEBUG",
tracing::Level::INFO => "INFO",
tracing::Level::WARN => "WARN",
tracing::Level::ERROR => "ERROR"
}
}
```
## Solution
Add an `as_str()` method for `Level`. Similar to [log::Level::as_str()][1].
[1]: https://docs.rs/log/0.4.14/log/enum.Level.html#method.as_str
This branch fixes some annoying new clippy lints no one cares about.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
# Conflicts:
# tracing-subscriber/src/layer.rs
## Motivation (#1374)
Users may wish to erase the type of a `Subscriber`
implementation, such as when it is dynamically constructed from a
complex parameterized type. PR #1358 added a `Subscriber` implementation
for `Box<dyn Subscriber + Send + Sync + 'static>`, allowing the use of
type-erased trait objects. In some cases, it may also be useful to share
a type-erased subscriber, _without_ using `Dispatch` --- such as when
different sets of `tracing-subscriber` subscribers are layered on one
shared subscriber.
## Solution
This branch builds on #1358 by adding an `impl Subscriber for Arc<dyn
Subscriber + Send + Sync + 'static>`. I also added quick tests for both
`Arc`ed and `Box`ed subscribers.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
In some cases, users may wish to erase the type of a `Subscriber`
implementation, such as when it is dynamically constructed from a
complex parameterized type. When doing so, it's important to ensure that
all trait methods with default implementations are properly forwarded to
the inner erased type. For example, if the type does not implement
`try_close`, but the inner erased subscriber does, then the the
subscriber will not be notified when spans close — which could result in
a memory leak.
To avoid potential footguns resulting from users implementing
type-erased subscribers incorrectly, this branch adds a new `impl
Subscriber for Box<dyn Subscriber + Send + Sync + 'static>` in
`tracing-core`. This is also somewhat more ergonomic than any solution
in another crate, since the implementation is for `Box<dyn Subscriber +
...>` directly, rather than some `BoxedSubscriber` newtype.
## Motivation
Exposing an accessor for the `FieldSet` on `Attributes` can motivate the
subscriber to achieve some performance improvement, such as
`OpenTelemetrySubscriber` #1327.
## Alternative
Make `ValueSet::field_set()` be `pub` instead of `pub(crate)`.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This backports PR #1335 from `master` to `v0.1.x`.
Clippy now warns about implementing `Into` rather than `From`, since
`From` automatically provides `Into` but `Into` does not provide `From`.
This commit fixes the direction of those conversions, placating Clippy.
Additionally, it fixes a redundant use of slice syntax that Clippy also
complained about.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch fixes two known warnings.
The first fixed warning was in `tracing-core/src/callsite.rs`, where
clippy suggested using `std::ptr::eq` to compare addresses instead of
casting `&T` to a `*const T`, which `&T` coerces to _anyways_. Below is
the warning:
```
error: use `std::ptr::eq` when comparing raw pointers
--> tracing-core/src/callsite.rs:238:9
|
238 | self.0 as *const _ as *const () == other.0 as *const _ as *const ()
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::ptr::eq(self.0 as *const _, other.0 as *const _)`
|
= note: `-D clippy::ptr-eq` implied by `-D warnings`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq
```
The second fixed warning is a bit more complex. As of Rust 1.50,
[`AtomicUsize::compare_and_swap`][1] has been deprecated in favor of
[`AtomicUsize::compare_exchange`][2] and
[`AtomicUsize::compare_exchange_weak`][3]. I've moved
`tracing_core::dispatch::set_global_default` to use
`AtomicUsize::compare_exchange` as `AtomicUsize::compare_exchange_weak`
is designed for atomic loads in loops. Here are a few notes on the
differences between `AtomicUsize::compare_and_swap` and
`AtomicUsize::compare_exchange`:
- `AtomicUsize::compare_exchange` returns a result, where the
`Result::Ok(_)` branch contains the prior value. This is equivalent
to the now-deprecated [`AtomicUsize::compare_and_swap`][1] returning
the `current` parameter upon a successful compare-and-swap operation,
but success is now established through a `Result`, not an equality
operation.
- `AtomicUsize::compare_exchange` accepts an `Ordering` for the failure
case of a compare-and-swap. The [migration guide suggests][4] using
`Ordering::SeqCst` for both the success and failure cases and I saw
no reason to depart from its guidance.
After this branch is approved, I'll backport these fixes to the `v0.1.0` branch.
[1]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_and_swap
[2]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange
[3]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#method.compare_exchange_weak
[4]: https://doc.rust-lang.org/nightly/std/sync/atomic/struct.AtomicUsize.html#migrating-to-compare_exchange-and-compare_exchange_weak
This backports PR #991 to v0.1.x. This is primarily necessary for the MSRV
bump, since some dependencies no longer compile on Rust 1.40.0.
This has already been approved on `master`, in PR #991, so it should be
fine to ship.
## Motivation
This will avoid breaking CI on new releases of clippy. It also makes the
code a little easier to read.
## Solution
- Convert `match val { pat => true, _ => false }` to `matches!(val, pat)`
- Remove unnecessary closures
- Convert `self: &mut Self` to `&mut self`
This bumps the MSRV to 1.42.0 for `matches!`.
The latest version of rust is 1.46.0, so as per
https://github.com/tokio-rs/tracing#supported-rust-versions this is not
considered a breaking change.
I didn't fix the following warning because the fix was not trivial/needed
a decision:
```
warning: you are deriving `Ord` but have implemented `PartialOrd` explicitly
--> tracing-subscriber/src/filter/env/field.rs:16:32
|
16 | #[derive(Debug, Eq, PartialEq, Ord)]
| ^^^
|
= note: `#[warn(clippy::derive_ord_xor_partial_ord)]` on by default
note: `PartialOrd` implemented here
--> tracing-subscriber/src/filter/env/field.rs:98:1
|
98 | / impl PartialOrd for Match {
99 | | fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
100 | | // Ordering for `Match` directives is based first on _whether_ a value
101 | | // is matched or not. This is semantically meaningful --- we would
... |
121 | | }
122 | | }
| |_^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
```
As a side note, this found a bug in clippy 😆https://github.com/rust-lang/rust-clippy/issues/6089
### Fixed
- Incorrect inlining of `Event::dispatch` and `Event::child_of`, which
could result in `dispatcher::get_default` being inlined at the
callsite (#994)
### Added
- `Copy` implementations for `Level` and `LevelFilter` (#992)
Thanks to new contributors @jyn514 and @TaKO8Ki for contributing
to this release!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Presently, functions for recording new spans and events that can get the
current dispatcher (`Event::dispatch`, `Event::child_of`, `Span::new`,
and `Span::new_root`) are marked as `#[inline]`. This results in these
methods getting inlined in most cases, and `dispatcher::get_default` is
sometimes inlined into *those* methods. This, then, inlines all of
`dispatcher::get_default` into the callsites of the `span!` and `event!`
macros, generating *way* too much code (including thread local accesses)
inline. Since there's an `expect` in `get_default`, this also means that
unwinding code is inlined into functions that wouldn't otherwise panic,
which is also not great. Inlining too much code this results in code that
optimizes poorly in many cases.
## Solution
This PR removes `#[inline]` attributes from these functions, ensuring
that the TLS hits are always behind a function call.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
This makes both structs easier to use because you no longer have to
worry about borrow errors while working with them. There's no downside
to making them `Copy` since both are wrappers around a `usize`.
Usually, we try to avoid adding `Copy` implementations for public API
types, as it presents a forward-compatibility hazard in the event that
the internal representation of those types must change to one that is no
longer trivially copyable. However, in this case, we rely on `Level` and
`LevelFilter` having a `usize` representation (and there's a test
asserting that `LevelFilter`s can be transmuted to `usize`). Any change
to the internal representation would already be breaking, so it's okay to
commit to `Copy` here.
Ideally, this would make `Metadata::level` return `Level` instead of
`&Level`. However that's a breaking change, so I didn't make it here.
## Solution
Derive `Copy` for both structs and fix various warnings that popped up
as a result.
Since this changes crates that depend on tracing-core to remove clone
calls and rely on Level and LevelFilter being Copy, those crates will no
longer compile with older versions of tracing-core. This bumps the
version number for `tracing-core` and sets the minimum version to the
new version for all affected crates. This avoids compile errors with a
cached `Cargo.lock`.
## Motivation
In order to get a compiler warning (or error) when links are broken.
Closes#940
## Solution
- [x] add `deny(broken_intra_doc_links)`
- [x] add a note to the CONTRIBUTING.md documentation on building docs locally
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
# 0.1.16 (September 8, 2020)
### Fixed
- Added a conversion from `Option<Level>` to `LevelFilter`. This
resolves a previously unreported regression where `Option<Level>`
was no longer a valid LevelFilter. (#966)
This changes the logo to a wordmark. This should be consistent
with the ones for all the other Tokio crates, when we add them
elsewhere.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Fixed
- When combining `Interest` from multiple subscribers, if the interests
differ, the current subscriber is now always asked if a callsite
should be enabled (#927)
### Added
- Internal API changes to support optimizations in the `tracing` crate
(#943)
- **docs**: Multiple fixes and improvements (#913, #941)
## Motivation
Currently, tracing's macros generate a *lot* of code at the expansion
site, most of which is not hidden behind function calls. Additionally,
several functions called in the generated code are inlined when they
probably shouldn't be. This results in a really gross disassembly for
code that calls a `tracing` macro.
Inlining code _can_ be faster, by avoiding function calls. However, so
much code can have some negative impacts. Obviously, the output binary
is much larger. Perhaps more importantly, though, this code may optimize
much less well --- for example, functions that contain tracing macros
are unlikely to be inlined, since they are quite large.
## Solution
This branch "outlines" most of the macro code, with the exception of the
two hottest paths for skipping a disabled callsite (the global max level
and the per-callsite cache). Actually recording the span or event,
performing runtime filtering via `Subscriber::enabled`, and registering
the callsite for the first time, are behind a function call. This means
that the fast paths still don't require a stack frame, but the amount of
code they jump past is _significantly_ shorter.
Also, while working on this, I noticed that the event macro expansions
were hitting the dispatcher thread-local two separate times for enabled
events. That's not super great. I folded these together into one
`LocalKey::with` call, so that we don't access the thread-local twice.
Building `ValueSet`s to actually record a span or event is the path that
generates the most code at the callsite currently. In order to put this
behind a function call, it was necessary to capture the local variables
that comprise the `ValueSet` using a closure. While closures have
impacted build times in the past, my guess is that this may be offset a
bit by generating way less code overall. It was also necessary to add
new `dispatcher`-accessing functions that take a `FnOnce` rather than a
`FnMut`. This is because fields are _sometimes_ moved into the
`ValueSet` by value rather than by ref, when they are inside of a call
to `tracing::debug` or `display` that doesn't itself borrow the value.
Not supporting this would be a breaking change, so we had to make it
work. Capturing a `FnOnce` into the `FnMut` in an `&mut Option` seemed
inefficient, so I did this instead.
## Before & After
Here's the disassembly of the following function:
```rust
#[inline(never)]
pub fn event() {
tracing::info!("hello world");
}
```
<details>
<summary>On master:</summary>
```assembly
event:
; pub fn event() {
push r15
push r14
push r12
push rbx
sub rsp, 184
; Relaxed => intrinsics::atomic_load_relaxed(dst),
lea rax, [rip + 227515]
mov rax, qword ptr [rax]
; Self::ERROR_USIZE => Self::ERROR,
add rax, -3
cmp rax, 3
jb 395 <event+0x1b1>
lea rbx, [rip + 227427]
mov qword ptr [rsp + 96], rbx
; Acquire => intrinsics::atomic_load_acq(dst),
mov rax, qword ptr [rip + 227431]
; self.state_and_queue.load(Ordering::Acquire) == COMPLETE
cmp rax, 3
; if self.is_completed() {
jne 36 <event+0x63>
mov qword ptr [rsp], rbx
; Relaxed => intrinsics::atomic_load_relaxed(dst),
mov rax, qword ptr [rip + 227398]
; if interest.is_always() {
test rax, -3
je 86 <event+0xa8>
mov rdi, rsp
; crate::dispatcher::get_default(|current| current.enabled(self.meta))
call -922 <tracing_core::dispatcher::get_default::h8c0486e541dd0400>
; tracing::info!("hello world");
test al, al
jne 84 <event+0xb2>
jmp 334 <event+0x1b1>
lea rax, [rsp + 96]
; let mut f = Some(f);
mov qword ptr [rsp + 136], rax
lea rax, [rsp + 136]
; self.call_inner(false, &mut |_| f.take().unwrap()());
mov qword ptr [rsp], rax
lea rdi, [rip + 227357]
lea rcx, [rip + 219174]
mov rdx, rsp
xor esi, esi
call qword ptr [rip + 226083]
mov qword ptr [rsp], rbx
; Relaxed => intrinsics::atomic_load_relaxed(dst),
mov rax, qword ptr [rip + 227312]
; if interest.is_always() {
test rax, -3
jne -86 <event+0x52>
; 0 => Interest::never(),
cmp rax, 2
; tracing::info!("hello world");
jne 255 <event+0x1b1>
; &self.meta
mov rbx, qword ptr [rip + 227295]
; tracing::info!("hello world");
lea r14, [rip + 2528]
mov rdi, rbx
call r14
lea r12, [rsp + 136]
mov rdi, r12
mov rsi, rax
call qword ptr [rip + 225926]
mov rdi, rbx
call r14
mov r14, rax
mov r15, rsp
mov rdi, r15
mov rsi, r12
call qword ptr [rip + 225926]
; Some(val) => val,
cmp qword ptr [rsp + 8], 0
je 194 <event+0x1c0>
mov rax, qword ptr [rsp + 32]
mov qword ptr [rsp + 128], rax
movups xmm0, xmmword ptr [rsp]
movups xmm1, xmmword ptr [rsp + 16]
movaps xmmword ptr [rsp + 112], xmm1
movaps xmmword ptr [rsp + 96], xmm0
; Arguments { pieces, fmt: None, args }
lea rax, [rip + 218619]
mov qword ptr [rsp], rax
mov qword ptr [rsp + 8], 1
mov qword ptr [rsp + 16], 0
lea rax, [rip + 165886]
mov qword ptr [rsp + 32], rax
mov qword ptr [rsp + 40], 0
lea rax, [rsp + 96]
; tracing::info!("hello world");
mov qword ptr [rsp + 72], rax
mov qword ptr [rsp + 80], r15
lea rax, [rip + 218570]
mov qword ptr [rsp + 88], rax
lea rax, [rsp + 72]
; ValueSet {
mov qword ptr [rsp + 48], rax
mov qword ptr [rsp + 56], 1
mov qword ptr [rsp + 64], r14
lea rax, [rsp + 48]
; Event {
mov qword ptr [rsp + 136], rax
mov qword ptr [rsp + 144], rbx
mov qword ptr [rsp + 152], 1
lea rdi, [rsp + 136]
; crate::dispatcher::get_default(|current| {
call -1777 <tracing_core::dispatcher::get_default::h32aa5a3d270b4ae2>
; }
add rsp, 184
pop rbx
pop r12
pop r14
pop r15
ret
; None => expect_failed(msg),
lea rdi, [rip + 165696]
lea rdx, [rip + 218426]
mov esi, 34
call qword ptr [rip + 226383]
ud2
```
</details>
<details>
<summary>And on this branch:</summary>
```assembly
event:
; pub fn event() {
sub rsp, 24
; Relaxed => intrinsics::atomic_load_relaxed(dst),
lea rax, [rip + 219189]
mov rax, qword ptr [rax]
; Self::ERROR_USIZE => Self::ERROR,
add rax, -3
cmp rax, 3
jb 59 <event+0x53>
; Relaxed => intrinsics::atomic_load_relaxed(dst),
mov rcx, qword ptr [rip + 219105]
; 0 => Interest::never(),
test rcx, rcx
je 47 <event+0x53>
mov al, 1
cmp rcx, 1
je 8 <event+0x34>
cmp rcx, 2
jne 38 <event+0x58>
mov al, 2
lea rcx, [rip + 219077]
mov qword ptr [rsp + 16], rcx
mov byte ptr [rsp + 15], al
lea rdi, [rsp + 15]
lea rsi, [rsp + 16]
; tracing_core::dispatcher::get_current(|current| {
call -931 <tracing_core::dispatcher::get_current::he31e3ce09e66e5fa>
; }
add rsp, 24
ret
; _ => self.register(),
lea rdi, [rip + 219041]
call qword ptr [rip + 218187]
; InterestKind::Never => true,
test al, al
; tracing::info!("hello world");
jne -53 <event+0x34>
jmp -24 <event+0x53>
nop dword ptr [rax + rax]
```
</details>
So, uh...that seems better.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
PR #934 fixed a bug in the CI configuration where MSRV checks were not
being run correctly. After this was fixed, it was necessary to bump the
MSRV to 1.40.0, as the tests were no longer actually passing on 1.39,
because some dependencies no longer support it.
While updating the documentation to indicate that the new MSRV is 1.40,
I noticed that the note on the MSRV was located inconsistently in the
READMEs and `lib.rs` documentation of various crates, and missing
entirely in some cases. Additionally, there have been some questions on
what our MSRV _policies_ are, and whether MSRV bumps are considered
breaking changes (see e.g. #936).
## Solution
I've updated all the MSRV notes in the documentation and READMEs to
indicate that the MSRV is 1.40. I've also ensured that the MSRV note is
in the same place for every crate (at the end of the "Overview" section
in the docs), and that it's formatted consistently.
Furthermore, I added a new section to the READMEs and `lib.rs` docs
explaining the current MSRV policy in some detail. Hopefully, this
should answer questions like #936 in the future. The MSRV note in the
overview section includes a link to the section with further details.
Finally, while doing this, I noticed a couple of crates
(`tracing-journald` and `tracing-serde`) were missing top-level `lib.rs`
docs. Rather than just adding an MSRV note and nothing else, I went
ahead and fixed this using documentation from those crate's READMEs.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, when multiple subscribers are in use, the way that `Interest`
values are combined results in surprising behavior.
If _any_ subscriber returns `Interest::always` for a callsite, that
interest currently takes priority over any other `Interest`s. This means
that if two threads have separate subscribers, one of which enables a
callsite `always`, and the other `never`, _both_ subscribers will always
record that callsite, without the option to disable it. This is not
quite right. Instead, we should check whether the current subscriber
will enable that callsite in this case.
This issue is described in #902.
## Solution
This branch changes the rules for combining `Interest`s so that `always`
is only returned if _both_ `Interest`s are `always`. If only _one_ is
`always`, the combined interest is now `sometimes`, instead. This means
that when multiple subscribers exist, `enabled` will always be called on
the _current_ subscriber, except when _all_ subscribers have indicated
that they are `always` or `never` interested in a callsite.
I've added tests that reproduce the issues with leaky filters.
Fixing this revealed an additional issue where `tracing-subscriber`'s
`EnvFilter` assumes that `enabled` is only called for events, and never
for spans, because it always returns either `always` or `never` from
`register_callsite` for spans. Therefore, the dynamic span matching
directives that might enable a span were never checked in `enabled`.
However, under the new interest-combining rules, `enabled` *may* still
be called even if a subscriber returns an `always` or `never` interest,
if *another* subscriber exists that returned an incompatible interest.
This PR fixes that, as well.
Depends on #919
This was previously approved by @yaahc as PR #920, but I
accidentally merged it into #919 _after_ that branch merged, rather
than into master (my bad!).
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This puts the splash SVG in every crate's README, so
that it will be rendered on crates.io.
I also changed the link to an absolute URL, to ensure that
it works even outside of the repo.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, when rebuilding the interest cache, the max level is
calculated by tracking a max level, and comparing it with each active
`Subscriber`'s max level hint, and setting the max value to the hint if
the hint is higher than the current value.
This is correct. However, the max level in this calculation starts at
`TRACE`. This means that regardless of what any subscriber returns for
its hint, after rebuilding the interest cache, the max level will
*always* be `TRACE`. This is wrong — it means that the fast path will
not be taken in may cases where it *should* be.
The max level calculation was started at `TRACE` rather than `OFF`,
because when this code was written with it starting at `OFF`, all the
tests broke, because the test subscribers don't provide hints. This was
the incorrect solution to that problem, however. This caused the tests
to break because we were *ignoring* all `None` hints, and not changing
the current max value when a subscriber returns `None`. This means that
if all subscribers returned `None` for their max level hint, the max
would remain `OFF`. However, what we *should* have done is assume that
if a subscriber doesn't provide a hint, it won't be filtering based on
levels, and assume that the max level is `TRACE` for that subscriber.
Whoopsie!
## Solution
Now, the max level should be calculated correctly for subscribers that
*do* provide hints, and should still be `TRACE` if a subscriber does not
have a hint.
I've also added new tests to ensure that the global max level filter is
used when provided, by asserting that `enabled` is not called with spans
that would be excluded by the max level. These tests fail on master, but
pass after this change.
Additionally, note that fixing this bug revealed a separate issue in
`tracing-subscriber`'s implementation of `max_level_hint` for
`EnvFilter`. When the `EnvFilter` includes directives that filter on
span field values, it will enable all spans with fields with that name,
so that events *within* the span can be enabled up to the filtering
directive's level. This may not be the ideal behavior, but it's what
we're doing currently, and we have tests for it. Therefore, it was
necessary to change the `max_level_hint` implementation to take this
into account by always returning `TRACE` when there are field value
filters.
That means that this branch requires #907 order for the
`tracing-subscriber` tests to pass.
Fixes: #906
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Fixed
- Missing `fmt::Display` impl for `field::DisplayValue` causing a
compilation failure when the "log" feature is enabled (#887)
Thanks to @d-e-s-o for contributing to this release!
### Added
- `LevelFilter` type and `LevelFilter::current()` for returning the
highest level that any subscriber will enable (#853)
- `Subscriber::max_level_hint` optional trait method, for setting the
value returned by `LevelFilter::current()` (#853)
### Fixed
- **docs**: Removed outdated reference to a Tokio API that no longer exists
(#857)
Thanks to new contributor @dignati for contributing to this release!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Tracing currently stores cached filter evaluations in a per-callsite
cache. Once filters have been evaluated, this allows us to skip disabled
callsites with a single load and compare, which is a nice fast path for
skipping. However, determining the value to store in the callsite cache
is expensive: it requires acquiring a lock to register the callsite, and
asking the subscriber for an `Interest` value to cache.
In long-running applications, such as servers, daemons, and interactive
applications that are used for a long period of time, this cost is not
very visible; it's amortized over the rest of the application's
lifetime. However, in batch-oriented programs and CLI tools that run to
completion, the cost of building the initial cached value has a much
bigger impact. If a given callsite is hit twice over the thirty seconds
that a CLI tool runs for (for example), the cost of the initial
evaluation is _much_ more meaningful than if the same callsite is hit
hundreds of thousands of times by a server that runs for several weeks
before its restarted. Even in a long running application, though, it
could lead to a surprising latency hit if a given callsite is only hit
after a long period of operation.
Per-callsite caching allows high performance for more granular filters,
since it tracks the state of _every_ callsite; we still get a fast path
for skipping a `trace` event in the `foo::bar` module if the `trace`
level is only enabled for the `baz` module. Because of this, we didn't
initially believe that a global maximum level fast path was necessary.
However, in `rustc`, it turns out that the cost of just building the
callsite cache for the first time has a noticeable impact on
performance. See here for details:
https://github.com/rust-lang/rust/pull/74726#issuecomment-664181180
## Solution
This branch adds a faster fast path for skipping callsites whose level
is higher than the highest level enabled for _any_ target/metadata
combination, similar to the `log` crate's `max_level`.
Unlike the `log` crate, which only ever has a single global logger,
`tracing` allows multiple `Subscriber`s to coexist in the same process.
Therefore, our implementation is somewhat different than `log`'s. We
can't simply provide a `set_max_level` function call, because _another_
subscriber may have already set a lower max level, and just overwriting
the max level with ours may result in diagnostics being skipped that
another subscriber wants to collect.
Instead, we've added a `max_level_hint` method to the `Subscriber`
trait, returning an `Option<LevelFilter>`. By default, this returns
`None`, for backwards compatibility. However, if it returns `Some`, the
resulting `LevelFilter` is stored in a global `MAX_LEVEL` atomic. This
method is called by the `register_dispatch` function, which handles the
creation of a new `Dispatch`. This allows us to compare the max level
from a new subscriber with those of all other currently active
subscribers. Similarly, when cached interests are invalidated (such as
by changing a filter, or when a `Subscriber` is dropped), we can
reevaluate the max level.
The path for _checking_ the max level (e.g. in the macros) should be
very efficient. We load the atomic, convert the `usize` value into a
`LevelFilter`, and compare it to the callsite's level. With help from
@eddyb, I was able to implement the conversion such that it should
hopefully be an identity conversion in release mode, rather than
requiring rustc to generate a LUT. The macros have been updated to check
the max level before anything requiring hitting the callsite cache _or_
the subscriber TLS var.
Also, I've updated `tracing` and `tracing-subscriber` to reexport the
`LevelFilter` type from `tracing-core` rather than defining their own.
The API should be a superset of those crates' versions.
Thanks to @oli-obk for uncovering this issue when adding `tracing` to
`rustc`, and for doing a lot of the preliminary analysis of the cause!
So far, there doesn't appear to be a significant change in any of `tracing`'s
internal benchmarks on this branch. However, my guess is that this is
because of `criterion`'s outlier elimination: since the cache is constructed
only on the first iteration, it's pretty likely that `criterion` considers this an
outlier and throws it out entirely, so the benchmark results on master don't
include the cost of evaluating the filter for the initial time.
Incidentally, this PR also fixes a bug in the `tracing` crate's version
of `LevelFilter`, where the `OFF` level actually enabled *everything*,
rather than *nothing*. Oopsie!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Now that the new https://tokio.rs is live, we can add the new Tracing
logo to the RustDoc!
I also added a couple missing `html_root_url` attributes in published
crates.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Changed
- Replaced use of `inner_local_macros` with `$crate::` (#729)
### Added
- `must_use` warning to guards returned by `dispatcher::set_default`
(#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)
### Fixed
- Compiler error when `tracing-core/std` feature is enabled but
`tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
(#749)
- Documentation formatting issues (#715, #771)
Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nvzqz for
contributing to this release!
## Motivation
PR #769 was merged with some incorrect docs formatting due to
incorrect HTML that resulted from overzealous find-and-replace
misuse. See https://github.com/tokio-rs/tracing/pull/769#pullrequestreview-439416330
## Solution
This PR un-splodes it.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This branch makes the following changes:
- Add a new section to the `Span::enter` docs that explains why
it should be avoided in async code, and suggests alternatives.
- Add links to the new section elsewhere.
- Add new formatting for "Notes" and "Warnings" in the documentation,
by (mis?)using existing styles from the RustDoc stylesheet.
- Fixed a handful of inaccuracies or outdated statements I noticed
while making other changes.
Fixes #667
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Consumers of the crate should be able to handle the 0 case separately
and avoiding panicking within `Id::from_u64`.
## Solution
Expose direct conversion between the inner type.
This allows handling the 0 case separately and avoiding panicking within
`Id::from_u64`.
When the following dependencies and their features were specified:
```toml
tracing-core = { version = "0.1", default-features = false, features = ["std"] }
tracing = { version = "0.1", default-features = false }
```
The build would fail with the following error:
```
error[E0412]: cannot find type `Once` in crate `tracing_core`
--> tracing/src/lib.rs:840:35
|
840 | pub type Once = tracing_core::Once<()>;
| ^^^^ not found in `tracing_core`
|
```
This happened because `tracing-core` exports `Once` only if its `std`
feature is disabled. And the depending `tracing` crate assumed that if
its `std` feature was disabled, so would be the `std` feature in
`tracing-core`.
This is a violation of the [undocumented "features must be additive"
guideline][ag]. In this commit tracing-core is adjusted to export `Once`
regardless of whether `std` is disabled or not.
[ag]: https://github.com/rust-lang/cargo/issues/4328
## Motivation
Clippy [now warns us][1] that comparing trait object pointers with
`ptr::eq` is bad, because vtable addresses are not guaranteed to be
unique due to the compiler merging vtables, and vtables for the same
trait impl may have different addresses in different compilation units.
In practice, I don't believe this actually effects `tracing-core`'s use
case for this comparison. Because callsites must be static, the data
component of the trait object wide pointer will always be unique, even
if the compiler merges the vtables for all the identical generated
`Callsite` impls (which it may very well do!).
## Solution
Although this is probably not an issue in practice, I still thought it
was good to fix the clippy warning, and to be more explicit that it's
the data pointer comparison that's load-bearing here. I've updated the
`PartialEq` impl for `callsite::Identifier` to cast to `*const ()` to
extract the data address from the wide pointer.
[1]: https://rust-lang.github.io/rust-clippy/master/index.html#vtable_address_comparisons
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
There are unnecessary spaces. Also, there is a point that requires space.
* chore: Remove duplicated spaces
* chore: Add space to make code consistent
## Motivation
Currently, the only way to interact with `Value`s is to record them with
a visitor. In the most common case, where typed data is not needed,
`Value`s are recorded with their `fmt::Debug` implementations — a
visitor which does not implement the `record_${TYPE}` traits will
automatically fall back to recording all primitive value types with
`Debug`. However, implementing a visitor requires a bit of boilerplate:
an entire trait implementation has to be written, and a visitor object
passed to `record`.
## Solution
This branch hopes to simplify this common case by adding a `Debug`
implementation to _all_ `dyn Value` trait objects. This is equivalent
to a visitor that only implements `record_debug`, but requiring less
boilerplate.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, `tracing` uses GitHub Actions for CI. However, we have
previously used both Travis and Azure Pipelines CI. Some artifacts of
previous CI configurations, such as badges and config files, still exist
in the repo. This can be confusing, since some of these CI
configurations imply things that aren't currently true (such as old MSRV
versions).
## Solution
This branch removes the following:
- Azure Pipelines badges from cargo metadata. These currently show up
as "never built" on crates.io, since the Azure build is turned off. So, we
should just remove them.
- `.travis.yml`. We don't use Travis and there's no sense keeping around
an old config file.
- `azure-pipelines.yml`. Similarly, we no longer need this.
Fixes: #669
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
### Added
- `field::Empty` type for declaring empty fields whose values will be
recorded later (#548)
- `field::Value` implementations for `Wrapping` and `NonZero*` numbers
(#538)
### Fixed
- Broken and unresolvable links in RustDoc (#595)
Thanks to @oli-cosmian for contributing to this release!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This commit fixes several RustDoc links which were either broken
(without references) or had unresolvable references.
It looks like nightly RustDoc has recently gotten much smarter about
finding links that were unresolvable. These had always been broken, but
they used to silently 404. Now, the nightly build of RustDoc will emit a
warning, which we deny, causing the build to fail. This should fix CI
(as well as actually fixing the wrong links).
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Right now, there is no way to indicate that a span field _currently_
has no value but that a value will be recorded later. Tracing used to
support this concept, but when we added local-variable shorthand, it was
removed from the macros, as we needed to use the previous syntax for
indicating empty fields for local variable shorthand. See #77 for
details.
This commit side-steps the problem of determining an appropriate
_syntax_ for empty fields by adding a special value _type_, called
`Empty`. Now, empty values can be indicated like
```rust
span!("my_span", foo = 5, bar = tracing::field::Empty);
```
The `Empty` type implements `Value`, but its' `record` method is a no-op
that doesn't' call any functions on the visitor. Therefore, when a field
is empty, the visitor will not see it. An empty field can then be
recorded later using `Subscriber::record` (or the corresponding `Span`
method in `tracing`).
Closes#77
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
For tracing there's no difference between a `NonZero*` or `Wrapping`
type and their underlying type. This allows such types to directly be
referenced by the tracing macros without requiring additional method
calls or boilerplate.
## Solution
Implemented `Value` for `Wrapping<T: Value>` and for all the `NonZero*`
types from `std::num`.
This required a slight refactoring of the macros generating the regular
integer impls as to reduce the amount of generated code.
