Motivation
To release `tracing 0.2` from `master`, we need to stabilize `tracing-core`. That includes introducing `valuable`.
Solution
Integrate `valuable` types into the `ValueSet` and `FieldSet` structs. Follow-up PRs will clean up and remove more code.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Follow up to #2239.
Discovered that
```rust
tracing_subscriber::fmt().with_ansi(false).finish();
tracing::info!("hello");
```
doesn't actually do anything, because `finish` returns a `Collector` and
`init` is the thing that actually installs the collector.
## Solution
Added `#[must_use]` which suggests using `try_init` instead.
## Motivation
Test msrv and minimal-versions compliance in the currently best-known
fashion.
## Solution
- Use cargo-hack and cargo-minimal-versions
- Also I replaced uses of actions-rs (abandoned) while I was passing by
## Motivation
When a global default dispatcher has already been set, the
`dispatch::set_global_default` function fails with a
`SetGlobalDefaultError`. The `fmt::Display` impl for this type includes
a message explaining that the error indicates that a global default has
already been set, but the `fmt::Debug` impl is derived, and just looks
like this:
```
SetGlobalDefaultError { _no_construct: () }
```
which isn't particularly helpful.
Unfortunately, when `unwrap()`ping or `expect()`ing a `Result`, the
`fmt::Debug` implementation for the error type is used, rather than
`fmt::Display`. This means that the error message will not explain to
the user why setting the global default dispatcher failed, which is a
bummer.
## Solution
This branch replaces the derived `Debug` impl with a manually
implemented one that prints the same message as the `Display` impl, but
formatted like a tuple struct with a single string field. This avoids
emitting `Debug` output that's *just* a textual human-readable message,
rather than looking like Rust code, but ensures the message is visible
to the user when writing code like
```rust
tracing::dispatch::set_global_default(foo).unwrap();
```
The mesasge now looks like:
```
SetGlobalDefaultError("a global default trace dispatcher has already been set")
```
which should be a bit easier to debug.
## Motivation
Like for `Collect` and `Subscribe`, allow per-layer `Filter`s to filter
based on event fields.
## Solution
Add `Filter::event_enabled`, plumb it through the combinator
implementations, and call it from `Filtered`.
The bit I'm the least confident about is the check in `Registry`'s
implementation, but I *think* it matches what `event` is doing and
everything seems to function correctly.
## Motivation
I've wanted to serialize fieldset of current span.
## Solution
Expose already existing `SerializeFieldSet` for users by implementing `AsSerde` for `FieldSet`.
## Motivation
This makes it possible to fully "override" some base `Targets` filter
with another (e.g. user-supplied) filter. Without some way to obtain the
default, only explicit targets can be overridden (via `IntoIter` and
friends).
See also https://github.com/tokio-rs/tracing/issues/1790#issuecomment-999739222
## Solution
We can add a method to `Targets` that filters the underlying
`DirectiveSet` for the default directive. This works because
`DirectiveSet::add` will replace directives with the same
`target`/`field_names`, which is always `None`/`vec![]` for the
directive added by `with_default` (and in fact we are only concerned
with `target`, since no other `Targets` API allows adding directives
with a `None` target).
Ideally the method would be named `default`, corresponding to
`with_default`, however this conflicts with `Default::default` and so
would be a breaking change (and harm ergonomics). `default_level` seemed
a better name than `get_default`, since "getters" of this style are
generally considered unidiomatic<sup>[citation needed]</sup>.
Example usage:
```rust
let mut filter = Targets::new().with_target("some_module", LevelFilter::INFO);
// imagine this came from `RUST_LOG` or similar
let override: Targets = "trace".parse().unwrap();
// merge the override
if let Some(default) = override.default_level() {
filter = filter.with_default(default);
}
for (target, level) in override.iter() {
filter = filter.with_target(target, level);
}
```
Closes#1790
In the upstream `opentelemetry` crate, the `trace` and `metrics`
features are gated by separate feature flags. This allows users who are
only using OpenTelemetry for tracing, or who are only using it for
metrics, to pick and choose what they depend on.
Currently, the release version of `tracing-opentelemetry` only provides
tracing functionality, and therefore, it only depends on `opentelemetry`
with the `trace` feature enabled. However, the metrics support added in
#2185 adds a dependency on the `opentelemetry/metrics` feature. This is
currently always enabled. We should probably follow the same approach as
upstream `opentelemetry`, and allow enabling/disabling metrics and
tracing separately.
This branch adds a `metrics` feature to `tracing-opentelemetry`, and
makes the `MetricsSubscriber` from #2185 gated on the `metrics` feature.
This feature flag is on by default, like the upstream
`opentelemetry/metrics` feature, but it can be disabled using
`default-features = false`.
We should probably do something similar for the tracing components of
the crate, and make them gated on a `trace` feature flag, but adding a
feature flag to released APIs is not semver-compatible, so we should
save that until the next breaking release.
## Motivation
The `RollingFileAppender` currently only supports a filename suffix. A
lot of editors have support for log files using the `.log` extension. It
would be nice to be able to configure what gets added after the date.
## Solution
- Add a `Builder::filename_suffix` method, taking a string.
- If the string is non-empty, this gets appended to the filename after
the date.
- This isn't an `AsRef<Path>` because it's not supposed to be a `Path`
- Update the date appending logic to handle cases when the suffix or
prefix is empty
- Split each part with a `.` so the final output would be
`prefix.date.suffix`
- Make sure to remove unnecessary `.` when a prefix or suffix is empty
- Add tests related to those changes
## Notes
It would probably be nicer to simply have a completely configurable file
name format, but I went with the easiest approach that achieved what I
needed.
Closes#1477
## 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 😇
A `FieldSet` is equal to another `FieldSet` if they share the same
callsite and fields (provided in the same order). This ensures
that a `Field` applicable to one `FieldSet` is applicable to any
equal `FieldSet`. A `Metadata` is equal to another `Metadata` if
all of their contained metadata is exactly equal.
This change manually re-implements `PartialEq` and `Eq` for
`Metadata` and `FieldSet` to define their equality strictly in
terms of callsite equality. In debug builds, the equality of
these types' other fields is also checked.
Documentation is added to both `Metadata` and `FieldSet`
explaining this behavior.
The expectation that, in a well-behaving application, `Metadata`
and `FieldSet`s with equal callsites will be otherwise equal is
documented on `Callsite::metadata`. This is not a breaking change,
as previous releases did not define equality for `Metadata` or
`FieldSet`. The `Callsite` trait remains safe, as this expectation
is not (yet) a safety-critical property.
## Motivation
Several currently open PRs, such as #2225 and #2221, add new
configuration parameters to the rolling file appender in
`tracing-appender`. The best way to add new optional configuration
settings without breaking existing APIs or creating a very large number
of new constructors is to add a builder interface.
## Solution
Since a number of PRs would all need to add the builder API, introducing
potential conflicts, this branch _just_ adds the builder interface
without adding any new configuration options. Once this merges, the
existing in-flight PRs can be rebased onto this branch to use the
builder interface without conflicting with each other.
Also, the `Builder::build` method is fallible and returns a `Result`,
rather than panicking. This is a relatively common pattern in Rust ---
for example, `std::thread::Builder::spawn` returns a `Result` if a new
thread cannot be spawned, while `std::thread::spawn` simply panics. This
allows users to handle appender initialization errors gracefully without
breaking the API of the existing `new` constructor.
Fixes#1953
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Previously, using `record("x", "y")` would give an error:
```
error[E0277]: the size for values of type `str` cannot be known at compilation time
--> src/main.rs:3:22
|
243 | span.record("x", "y");
| ^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `Sized` is not implemented for `str`
note: required by a bound in `Span::record`
--> /home/jnelson/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/tracing-0.1.32/src/span.rs:1184:30
|
1184 | pub fn record<Q: ?Sized, V>(&self, field: &Q, value: &V) -> &Self
| ^ required by this bound in `Span::record`
```
Now it works fine, as tested by the doc-example.
This doesn't break any existing code, because there's a generic `impl<T: Value> Value for &T`: https://docs.rs/tracing/0.1.35/tracing/trait.Value.html#impl-Value-for-%26%27a%20T
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
I find it useful when debugging applications with lots of threads to
easily identity them by their names.
## Solution
Just name the thread as other crates such as `sentry-rust` are doing.
Co-authored-by: Guillaume Desmottes <guillaume@desmottes.be>
Motivation:
When a user clicks they they may accidentally get an old version of the crate. Its also out of sync with the current main readme.
Solution:
removed the hard coded links
Older versions use node 12 which is no longer supported (end-of-life on April 30, 2022). Also, @main/@master is not very good as they may run into unreleased breaking changes.
Motivation:
Currently, there is no way to publish metrics via tracing.
Solution:
Update the tracing-opentelemetry crate to publish metrics for event
fields that contain specific prefixes in the name.
Right now, we lazily instantiate and store one metrics object
per-callsite, but a future improvement that we should add to tracing
itself is the ability to store data per-callsite, so that we don't have
to do a HashMap lookup each time we want to publish a metric.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: David Barsky <me@davidbarsky.com>
## Motivation
When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`
## Solution
Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
The second invocation would always fail, since that version had already
been published. This command failing would cause the script to exit with
an error, and thus fail to push the release tag.
This commit fixes this by removing the second `cargo publish`.
## 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
These are incorrect: currently, when you have a `None` layer, the `None`
hint it returns causes the default `TRACE` to win, which is inaccurate.
Similarly, `Vec` should default to `OFF` if it has no `Subscribe`s in it
## Solution
Change the default hints to `Some(OFF)`
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
This is wrong.
## Solution
Make it unwrong.
As described by the documentation on `Subscribe::event_enabled`, the
return value sets the global filtering of an event. This code used to
say that that `Option::<impl Subscribe>::None` existing in the
subscriber stack should disable any subscriber in the stack seeing
events
The gitter room is not used anymore.
This commit updates CONTRIBUTING.md to guide the reader to the
discord channel.
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This fixes a Clippy lint for explicitly calling `drop` on a value
without a `Drop` impl, and a lint for `let` bindings whose
value is `()`.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Although it shares the name, `event_enabled!` *does not* call
`Collect::event_enabled`. `Collect::event_enabled` is only called once
the event fields are available.
Currently, `tracing`'s benchmark suite benchmarks the same behaviors
(e.g. creating a span, recording an event, etc) across a handful of
cases: with no default dispatcher, with a global default, and with a
scoped (thread-local) default. We use criterion's `benchmark_group` to
represent each kind of dispatcher, and `bench_function` for each
behavior being measured.
This is actually kind of backwards relative to how Criterion is
_supposed_ to be used. `bench_function` is intended for comparing
different implementations of the *same* thing, with the
`benchmark_group` representing what's being compared. If we inverted the
structure of these benchmarks, Criterion would give us nicer plots that
would allow comparing the performance of each dispatch type on the same
task.
This PR reorganizes the benchmarks so that each behavior being tested
(such as entering a span or recording an event) is a `benchmark_group`,
and each dispatch type (none, global, or scoped) is a `bench_function`
within that group. Now, Criterion will generate plots for each group
comparing the performance of each dispatch type in that benchmark.
For example, we now get nice comparisons like this:
Unfortunately, this required splitting each benchmark type out into its
own file. This is because, once we set the global default dispatcher
within one benchmark group, it would remain set for the entire lifetime
of the process --- there would be no way to test something else with no
global default. But, I think this is fine, even though it means we now
have a bunch of tiny files: it also allows us to run an individual
benchmark against every combination of dispatch types, without having to
run unrelated benches. This is potentially useful if (for example)
someone is changing only the code for recording events, and not spans.
This branch makes the following changes to the CI test workflows:
- Consolidate all tests into a single workflow again. We had previously
broken things out to allow restarting only some failed checks, but now
GitHub Actions allows restarting individual jobs, which is much nicer,
and we can combine everything into one workflow.
- Gate starting any tests/checks on an initial `cargo check` run. This
should mean that if code doesn't compile, we don't spin up a huge
number of test jobs that all end up failing, and delaying other PRs'
CI runs.
- Use `cargo nextest` for running tests. This should make test runs a
bit quicker, and also get us other nice features like retries for
flaky tests.
- Switch to `taiki-e/install-action` for installing stuff like
`cargo-hack`, `nextest`, and `wasm-pack`. This is a bit nicer than
just `curl`ing stuff.
- Use a matrix for testing across toolchains/OSes, instead of having
separate jobs. This reduces the complexity of the CI workflow a bit.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
In many cases, new releases of a dependency can break compatibility with
`tracing`'s minimum supported Rust version (MSRV). It shouldn't be
necessary for a `tracing` crate to bump its MSRV when a dependency does,
as users on older Rust versions should be able to depend on older
versions of that crate explicitly and avoid bumping. Instead, we should
probably just run our MSRV checks with minimal dependency versions. This
way, we don't need to bump our MSRV when the latest version of a
dependency does, unless we actually *need* to pick up that new version.
This branch changes the `check_msrv` CI jobs to do that. I also did some
minor dependency editing to actually make tracing build with
`-Zminimal-versions`.
Note that `tracing-futures` is currently excluded from the MSRV build
because it depends on a really ancient version of Tokio that pulls in
broken deps. We should probably drop support for Tokio 0.1 and release a
new version of that crate, but for now, we have to skip it from the CI
job temporarily.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Allow filter layers to filter on the contents of events (see #2007).
## Solution
This branch adds a new `Collect::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the `Collect::event`
method, and if it returns `false`, `Collect::event` is not called.
For simple collectors (e.g. not using `Subscribe` layers), the
`event_enabled` method is not particulary necessary, as the collector
can just skip the `event` call. However, this branch also adds a new
`Subscribe::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, C>) -> bool;
```
This is called before `Subscribe::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Collect::event`).
This allows filter subscribers to filter out an `Event` based on its
fields.
Closes#2007
## Motivation
I've received a request at work to record 128-bit integers and realized
that we should probably support recording them.
## Solution
Added two methods to the `Visit` trait, `record_i128` and `record_u128`.
However, I didn't add the size conversions present for 64-bit integers,
as 128-bit integers are, at this time, more specialized.
These broader impls supersede the previous impls where the inner type was a
`dyn Collect`. With these generic impls, you no longer must (but still can,
if you wish) cast the inner type of a boxed or arc'd collector to
`dyn Collect` to use it as a `Collect`.
## 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.
## Motivation
The `reload` subscriber doesn't (and can't) implement downcasting correctly,
which breaks certain subscribers like the opentelemetry one.
## Solution
Most uses of the `reload` module (including mine) are just to change the
filter. Therefore, this PR implements `Filter` for `reload::Subscriber`
to allow users to not need to wrap the whole layer trait. Another
advantage of this is that the common-case critical sections are
shortened.
Adds inherent `downcast_ref` and `is` inherent methods to:
- `dyn Subscriber + Send`
- `dyn Subscriber + Sync`
- `dyn Subscriber + Send + Sync`
- `Layered`
These additional implementations reduce the circumstances in which one
must cast to `dyn Subscriber` (which, previously, was the only type for
which `downcast_ref` and `is` were available).
* fix use of `cargo --list` in bin/publish
* fix shellcheck lints in bin/publish
* set -euo pipefail
* fix cargo hack exit status check
* fix wrong emptystring args
* prompt before installing cargo-hack
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This patch adds a bit more context around why we are creating a smaller
scope for the spans, and also what happens when we call
`global::shutdown_tracer_provider()` (that comment was copied from
the`rust-opentelemetry` repo).
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Replace `lazy_static` with `once_cell`. Fixes#2146.
## Motivation
`lazy_static!`, while a declarative macro, is a macro nonetheless. It
can add quite a bit of additional compilation time cost.
`once_cell::sync::Lazy` does the same thing with generics, and can be
used more flexibly (i.e. non-static lazily initialized values), and has
been proposed to be added to `std` (see linked issue).
I'm trying to reduce the compile time and dependency tree complexity of
a dependent project: [bevy](https://bevyengine.org), which is using
tracing. `lazy_static` and `once_cell` are both in our dependency tree
and both end up doing the same thing.
## Solution
Migrate to `once_cell`.
* appender: add initial set of benches
This patch adds blocking and nonblocking benchmarks. This code is from
an old PR (#703) that was never merged, and now ported to TOT so that it
compiles.
Co-authored-by: Zeki Sherif <9832640+zekisherif@users.noreply.github.com>
* switch to no-op writers in benchmarks
* fix macro resolution issue
Co-authored-by: Zeki Sherif <9832640+zekisherif@users.noreply.github.com>
Co-authored-by: David Barsky <me@davidbarsky.com>
