## Motivation
Currently, the `tracing-subscriber` crate has the `MakeWriter` trait for
customizing the io writer used by `fmt`. This trait is necessary (rather
than simply using a `Write` instance) because the default implementation
performs the IO on the thread where an event was recorded, meaning that
a separate writer needs to be acquired by each thread (either by calling
a function like `io::stdout`, by locking a shared `Write` instance,
etc).
Right now there is a blanket impl for `Fn() -> T where T: Write`. This
works fine with functions like `io::stdout`. However, the _other_ common
case for this trait is locking a shared writer.
Therefore, it makes sense to see an implementation like this:
``` rust
impl<'a, W: io::Write> MakeWriter for Mutex<W>
where
W: io::Write,
{
type Writer = MutexWriter<'a, W>;
fn make_writer(&self) -> Self::Writer {
MutexWriter(self.lock().unwrap())
}
}
pub struct MutexWriter<'a, W>(MutexGuard<'a, W>);
impl<W: io::Write> io::Write for MutexWriter<'_, W> {
// write to the shared writer in the `MutexGuard`...
}
```
Unfortunately, it's impossible to write this. Since `MakeWriter` always
takes an `&self` parameter and returns `Self::Writer`, the generic
parameter is unbounded:
```
Checking tracing-subscriber v0.2.4 (/home/eliza/code/tracing/tracing-subscriber)
error[E0207]: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates
--> tracing-subscriber/src/fmt/writer.rs:61:6
|
61 | impl<'a, W: io::Write> MakeWriter for Mutex<W>
| ^^ unconstrained lifetime parameter
error: aborting due to previous error
```
This essentially precludes any `MakeWriter` impl where the writer is
borrowed from the type implementing `MakeWriter`. This is a significant
blow to the usefulness of the trait. For example, it prevented the use
of `MakeWriter` in `tracing-flame` as suggested in
https://github.com/tokio-rs/tracing/pull/631#discussion_r391138233.
## Proposal
This PR changes `MakeWriter` to be generic over a lifetime `'a`:
```rust
pub trait MakeWriter<'a> {
type Writer: io::Write;
fn make_writer(&'a self) -> Self::Writer;
}
```
The `self` parameter is now borrowed for the `&'a` lifetime, so it is
okay to return a writer borrowed from `self`, such as in the `Mutex`
case.
I've also added an impl of `MakeWriter` for `Mutex<T> where T: Writer`.
Unfortunately, this is a breaking change and will need to wait until we
release `tracing-subscriber` 0.3.
Fixes#675.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This PR:
* Adds an example demonstrating how `fmt::Subscriber` can be used to write to stdout and a file simultaneously.
* Removes a few mentions of "Layers" in the examples directory; opting for "Subscribers" instead.
Resolves#1091
The trait being augmented is the 'Collect' trait, so name the trait
that extends should be called 'CollectExt'.
This is in keeping with the extension trait naming conventions
documented in RFC 445 and the spirit of the discussion in
tokio-rs/tracing PR #1015.
## Motivation
Support the latest OpenTelemetry specification
## Solution
In order to support the latest spec, this patch makes the following
breaking API changes:
* Update `opentelemetry` to 0.10.x
* Update `CompatSpan` to reflect changes to `Span` trait
* Record `u64` values as strings as they are no longer supported in
OpenTelemetry.
Additionally the following non-public api, doc, and example changes:
* Update examples and docs to use new simplified pipeline builder.
* As `opentelemetry::api` no longer exports trace types, internally use
`opentelemetry::trace as otel` to disambiguate from tracing types.
* Remove `rand` dependency as it is no longer needed
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Right now TracedError is pretty much incompatible with enum heavy error
handling implementations. The way its currently designed you'd have to make
sure that each leaf error independently is wrapped in a `TracedError` so that
all sources end up capturing a SpanTrace.
## Solution
To resolve this I've added a `map` method for transforming the inner type of a
TracedError while preserving the existing SpanTrace. This is still a bit
cumbersome, where before you'd be able to write `result.map_err(|source|
MyError { source, some_context })` now you'll have to write
`result.map_err(|source| source.map(|source| MyError { source, some_context
}))`. To fix this we should probably eventually add an extension trait on
`Result` but I want to experiment with that half of the change in `zebra`
before committing to an API, since it can easily be done out of tree.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, the `FmtContext` type implements `FormatFields` using the
subscriber's field formatter. However, this is difficult to use. The
`FmtContext` may _not_ be passed to functions expecting a `for<'writer>
FormatFields<'writer>`, because it only implements `FormatFields` for
its _own_ lifetime. This means the writer must have the same lifetime as
the context, which is not correct.
## Solution
This branch fixes this by changing the impl of `FormatFields` for
`FmtContext` to be generic over both the context's lifetime _and_ the
field formatter's lifetime. Additionally, I've added a method for
borrowing the actual field formatter as its concrete type, in case the
`FormatEvent` impl puts additional constraints on its type or is only
implemented for a specific named type, and wants to actually _use_ that
type.
## Motivation
#940
## Solution
This changes the docs.rs links, many of which were broken (or would have
been broken on the next release). It also converts some links I missed
the first time (maybe they were added after I opened the PR?).
Depends on #1067
## Motivation
Currently, the `Compact` event formatter in `tracing-subscriber`
is...kind of bad, and nobody seems to use it. It outputs the names of
all the spans in the current context, and (inexplicably) the _names_ of
all the fields on the current span, but not their values. This isn't
very useful.
## Solution
This branch rewrites the `Compact` formatter to be a little less bad.
Now, we output all the fields from the current span context, but skip
span names, and we shorten the level to a single character when it's
enabled. Additionally, using the `compact` formatter disables targets by
default. Now, the lines are nicely short, but maybe still useful.
Example output before this change:
...and after:
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, the `tracing_subscriber::fmt` module contains only
single-line event formatters. Some users have requested a
human-readable, pretty-printing event formatter optimized for
aesthetics.
## Solution
This branch adds a new `Pretty` formatter which uses an _excessively_
pretty output format. It's neither compact, single-line oriented, nor
easily machine-readable, but it looks _quite_ nice, in my opinion. This
is well suited for local development or potentially for user-facing logs
in a CLI application.
Additionally, I tried to improve the docs for the different formatters
currently provided, including example output. Check out [the Netlify
preview][1]!
[1]: https://deploy-preview-1067--tracing-rs.netlify.app/tracing_subscriber/fmt/index.html#formatters
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
hawkw/sharded-slab#45 changes `sharded-slab` so that the per-shard
metadata is allocated only when a new shard is created, rather than all
up front when the slab is created. This fixes the very large amount of
memory allocated by simply creating a new `Registry` without actually
collecting any traces.
## Solution
This branch updates `tracing-subscriber` to depend on `sharded-slab`
0.1.0, which includes the upstream fix.
In addition, this branch the registry from using `sharded_slab::Slab` to
`sharded_slab::Pool`. This allows us to clear hashmap allocations for
extensions in-place, retaining the already allocated maps. This should
improve `new_span` performance a bit.
Fixes#1005
This PR removes the deprecated items in `tracing-subscriber`. It _does
not_ remove `Collect::drop_span` as that will happen in a separate PR.
Note that branch builds atop of #1015, which means:
- #1015 needs to be merged first
- This PR will be rebased against the master branch containing #1015.
- _Then_ this can safely be merged.
This PR renames the following:
- `tracing_core::Subscriber` to `tracing_core::Collect`
- `tracing_subscriber::layer::Layer` to `tracing_subscriber::layer::Subscribe`
Authored-by: David Barsksy <dbarsky@amazon.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
We've tried very hard to make sure that entering and exiting spans is
lightweight...in the `tracing-core` core machinery. Unfortunately, we
haven't done any benchmarking of how subscriber implementations actually
handle enter/exit events. It turns out that in `tracing-subscriber`'s
`Registry`, there's actually significant overhead for entering a span:
calling `span.enter()` may take as long as 69 ns (on my machine).
## Solution
I've written some microbenchmarks for entering and exiting enabled spans
using `tracing-subscriber::fmt`, comparing them with the overhead of
calling `enter` on an enabled span. Based on this, I've made some
performance improvements. These optimizations include:
- Removing the `HashSet` that's used for tracking previously entered
span IDs, in favor of linear search. Span stacks probably never deep
enough for a hashmap to be faster than iterating over a couple of
vec indices.
- Preallocating the vec used for the span stack to hold at least 64
elements. This means we'll never have a lag spike from reallocating,
as I think it'll almost never be deeper than 64 IDs.
- Only cloning/dropping an ID's ref count for the _first_ ID in the stack.
This makes entering and exiting enabled spans significantly faster:
It would be nice to continue optimizing this, but this might be about
the best it gets given the other requirements that we're now making
assertions about.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, the `reload` layer is generic over both the type of the layer
being reloaded, *and* the type of the subscriber that the layer is
layered onto. This means that the `reload::Handle` type that's used to
reload the value of the layer *also* is parameterized over the
subscriber's type.
The subscriber type parameter makes the `reload` API significantly
harder to use. Any time a `reload::Handle` is returned by a function,
taken as an argument, or stored in a struct, the full type of the
subscriber under the layer must be written out --- and often, it is
quite long. What makes this worse is that sometimes the type of the
subscriber may vary at runtime based on some configuration, while the
type of the layer that's reloaded remains the same. For example, in
Linkerd, we've had to do [this][1], which is really not ideal.
## Solution
This branch removes the `Subscriber` type parameter from `reload::Layer`
and `reload::Handle`. Now, the `Handle` type is only generic over the
type of the inner layer that's being reloaded. It turns out that the
`Subscriber` type parameter was only necessary to add a `L: Layer<S>`
bound to `reload::Layer`'s constructor, which isn't really necessary ---
if the layer does not implement `Layer<S>`, the type error will occur
when `Subscriber::with` is actually used to layer it, which is fine.
I also changed the `with_filter_reloading` option on the `FmtSubscriber`
builder to also work with `LevelFilter`s, since there's no reason for it
not to, and added an example.
Since this breaks existing code, this change has to be made as part of
0.3.
[1]: 6c484f6dcd/linkerd/app/core/src/trace.rs (L19-L36)
## Motivation
It is currently possible to create a span graph which includes a span
that has both an invalid parent otel context _and_ a missing trace id by
assigning an invalid extracted parent context to a non-root span.
Constructing this particular graph will currently cause a panic.
## Solution
Explicitly assign invalid trace / span ids when sampling using the otel
SDK if the span builder does not contain these values.
## Motivation
Use of `echo "::add-path::<PATH>"` has been deprecated on GitHub CI.
## Solution
Update CI to use new path setter `echo "<PATH>" >> $GITHUB_PATH`.
Co-authored-by: Darin Morrison <darinmorrison@users.noreply.github.com>
## Motivation
For the non-async case, early returns could escape the block so that the
error handling was skipped.
I was wondering for quite some time now what was going on here 😆
## Solution
Just put the block into the all-mighty "try-catch-closure" construct.
Annotating the return type of the closure improves the error message in
case of a mismatch between the function's return type and body.
Example code:
```
fn with_err() -> Result<(), String> {
Ok(5)
}
```
Without return type annotation:
```
error[E0308]: mismatched types
--> src/main.rs:2:37
|
2 | fn with_err() -> Result<(), String> {
| _____________________________________^
3 | | Ok(5)
4 | | }
| |_^ expected `()`, found integer
```
With return type annotation:
```
error[E0308]: mismatched types
--> src/main.rs:3:8
|
3 | Ok(5)
| ^ expected `()`, found integer
```
## Motivation
Presently, `tracing` and `tracing-core` allow disabling the Rust
standard library. However, even when used with the standard library
disabled, they still require the use of `alloc`, which means some form
of global allocator must exist. This can be a significant limitation on
some very constrained embedded devices. Additionally, the use of any
heap allocations on these devices ought to be avoided.
Now that the callsite registry is no longer a `Vec`, we don't need heap
allocations for storing registered callsites. However, `Dispatch`s are
currently _always_ stored in `Arc`s, and the global registry always
contains a `Vec` of `Weak` refs to them. This is required in order to
support thread-local scoped dispatcher API, where multiple `Dispatch`es
can coexist. However, on `no-std` platforms without threads, we already
don't support the thread-local scoped dispatch API. In this case, we can
reduce the complexity and the number of allocations required by the
dispatcher.
Additionally, even when the standard library is in use, if the
dispatcher is a global default rather than a scoped dispatcher, it
remains set forever. This means it can never be dropped. When this is
the case, it's possible to clone the dispatcher by cloning a `'static`
reference, rather than a (comparatively) costly `Arc` bump.
## Solution
This branch changes the global default dispatcher to be represented as a
`&'static dyn Subscriber` reference on all platforms. In addition, the
dependency on `liballoc` is now feature flagged, allowing it to be
disabled along with the dependency on `libstd`. This means that
`tracing` can now be used with _no_ heap allocations required.
In order to use `tracing` without `liballoc`, `Dispatch`es can only be
created from `&'static dyn Subscriber` references, since this is the
only way to erase their types that exists without boxed trait objects.
Therefore, I've added a new `Dispatch::from_static` constructor that is
always available. This is more limiting than `Dispatch::new` --- some
ways of modeling runtime composition of the subscriber are much harder
(although it could still contain state). However, users without `alloc`
are probably fairly used to this --- the `log` crate [has the same
limitation](https://docs.rs/log/0.4.11/log/#use-with-std) when used
without `std`.
Also, I've changed the global subscriber registry when `std` _is_
enabled to use a `RwLock` rather than a mutex. Now, multiple callsites
can be registered simultaneously, and a write lock is only necessary to
add a new subscriber or deregister old ones. It's now fine to use
`RwLock` here, because we no longer use any locking at all when `std` is
not enabled --- we always use the single global default subscriber, so
we don't require a list of currently live subscribers.
I've modified the `tracing` and `tracing-core` crates to add new feature
flags for enabling `std` and `alloc` separately.
Closes#861
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Fixes https://github.com/tokio-rs/tracing/issues/975.
## Solution
This implements the warning in `tracing-subscriber` only, as mentioned
in
https://github.com/tokio-rs/tracing/issues/975#issuecomment-692903469.
It warns whenever directives are parsed, including programmatically and
through environment variables. It does not include the suggested new API
which returns the filters that wouldn't be parsed.
- List filters that would be ignored
- Mention 'static max level'
- Describe how to enable the logging
Example output:
```
$ RUST_LOG=off,debug,silenced[x]=trace cargo run -q
warning: some trace filter directives would enable traces that are disabled statically
| `debug` would enable the DEBUG level for all targets
| `silenced[x]=trace` would enable the TRACE level for the `silenced` target
= note: the static max level is info
= note: to enable DEBUG logging, remove the `max_level_info` feature
```
## Motivation
This is code refactoring. Currently, there is a lot of code that declares
modules as follows:
```rust
#[cfg(feature = "registry")]
#[cfg_attr(docsrs, doc(cfg(feature = "registry")))]
pub use registry::Registry;
#[cfg(feature = "registry")]
#[cfg_attr(docsrs, doc(cfg(feature = "registry")))]
pub fn registry() -> Registry {
Registry::default()
}
```
It is verbose to write a module declaration multiple times using the feature
attribute. Also, if the definition for the same feature spans multiple places,
it will be a little harder to read the code.
## Solution
You can combine features attributes in one place by writing as follows. It also
eliminates the need to write the same code over and over again.
```rust
cfg_feature!("registry", {
pub use registry::Registry;
pub fn registry() -> Registry {
Registry::default()
}
});
```
If this code is accepted, we will extend the declaration of the module using
macros to other files as well as `tracing-subscriber/lib.rs`.
## Motivation
Since `core` and `alloc` are unconditionally available, there is no
need to use a facade module for most of std.
Follow-up to https://github.com/tokio-rs/tracing/pull/960#discussion_r497655633:
these modules aren't necessary.
## Solution
Most of the imports could be switch to `use core` or `use alloc` without
trouble. The only exceptions were for `Mutex` and `Once`, which are only
available with the full standard library. These had a `sync` module
added to `tracing-core` which acts as a facade, re-exporting `std` if
available and `spin` if not.
## Motivation
Currently, the tests for the linked list implementation in
`tracing_core::callsite` declare two static `Registration`s in the test
module, which are used in multiple tests. Since these tests are all run
in one binary, in separate threads, these statics are shared across all
tests. This means that --- depending on the order in which tests are run
--- these `Registration`s may already have values for their `next`
pointers. In particular, there's a potential issue where the `for_each`
in the test `linked_list_push` can loop repeatedly, if one of the
`Registration`s already had a next pointer from a previous test. See:
https://github.com/tokio-rs/tracing/pull/1008#issuecomment-703041964
## Solution
This branch declares separate `Registration` statics for each test.
These are not shared between multiple test threads.
We still reuse the same callsite statics, since their state does not
change during the tests --- only the `Registration`s change. I also
refactored the test code a little bit to use only a single type
implementing `Callsite`, rather than two separate types, since the
callsite impl is not actually used and this makes the code somewhat more
concise.
Finally, I added a new test for pushing more than two callsites.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
