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.
This PR removes tracing-opentelemetry to a dedicated repo located at
https://github.com/tokio-rs/tracing-opentelemetry. (Note that at time of
writing this PR, the new repo has not be made public). We're moving
tracing-opentelemetry to a dedicated repository for the following
reasons:
1. opentelemetry's MSRV is higher than that of `tracing`'s.
2. more importantly, the main `tracing` repo is getting a bit unweildy
and it feels unreasonable to maintain backports for crates that
integrate with the larger tracing ecosystem.
(https://github.com/tokio-rs/tracing-opentelemetry does not have the
examples present in this repo; this will occur in a PR that will be
linked from _this_ PR.)
## Motivation
Support the latest OpenTelemetry specification.
## Solution
Update `opentelemetry` to the latest `0.18.x` release. Breaking changes
in the metrics spec have removed value recorders and added histograms so
the metrics layer's `value.` prefix has been changed to `histogram.` and
behaves accordingly. Additionally the `PushController` configuration for
the metrics layer has been simplified to accept a `BasicController` that
can act in either push or pull modes. Finally trace sampling in the
sdk's `PreSampledTracer` impl has been updated to match the sampling
logic in https://github.com/open-telemetry/opentelemetry-rust/pull/839.
* Update MSRV to 1.56
* Update examples
* Fix async-trait dep
## 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
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.
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>
This adds a minimal-versions check to the tracing project. Adapted from
`tokio-rs/tokio`. Adding this avoids breaking downstream dependencies
from accidentally under-constraining minimal versions of dependencies
when they depend on tracing.
I've currently just introduced the check. I will try to and do encourage
others to add patches to fix this where possible since it can be a fair
bit of work to chase down a version of all dependencies that passes
minimal-versions and is msrv. I've also seen some really odd
windows-specific issues (which are not being tested for here).
This is currently only testing `tracing`, `tracing-core`, and
`tracing-subscriber`. Packages such as `tracing-futures` are proving to
be a bit harder to deal with due to having features which enable very
old dependencies.
Steps to test the build minimal versions locally:
```sh
cargo install cargo-hack
rustup default nightly
cargo hack --remove-dev-deps --workspace
cargo update -Z minimal-versions
cargo hack check --all-features --ignore-private
```
CC: tokio-rs/tokio#4513
## Motivation
The `tracing`' crate's feature powerset is kind of unmanageably huge due
to the large number of `max_level_XXX` and `release_max_level_XXX`
feature flags. This is why we currently _don't_ run a `cargo-hack`
feature powerset check for it on CI. However, I forgot about that when I
added feature powerset checks to the `bin/publish` script, so publishing
a `tracing` release results in a combinatorial explosion that takes a
*very* long time to complete.
## Solution
It turns out that `cargo-hack` actually has flags for controlling what
features are included in the powerset (`--include-features` and
`--exclude-features`). This branch modifies `bin/publish` to use those
flags when checking the `tracing` and `tracing-subscriber` crate.
Additionally, I've modified the CI `cargo-hack` job to use the same
flags, so that it can now check `tracing` and `tracing-subscriber`.
This allows us to remove the manual feature check jobs for those crates
from CI.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
This updates all crates' MSRVs to 1.49 if they were not already greater
than that (`tracing-appender` is at 1.53). Rust 1.49+ is required to
update `parking_lot` to v0.12 (see #1878). Also, `futures-task` (which I
believe is only needed as a transitive dep) now needs 1.45+, so this
also fixes our CI build.
Because `tracing-opentelemetry` previously required 1.46.0, it had a
separate CI MSRV job. Since 1.49.0 is greater than 1.46.0, the separate
check for `tracing-opentelemetry` is no longer needed.
In the process, I removed deprecated uses of
`core::atomic::spin_loop_hint`, which is replaced with
`core::hint::spin_loop` in 1.49.
The current tests for the compile-time maximum level were not being run
in `--release` mode, and did not previously assert anything about
`Span`s.
This also adds some test cases for various `#[instrument]`-ed functions.
## Motivation
Support the latest OpenTelemetry specification.
## Solution
Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
The `time` crate bumped its MSRV to 1.53.0 in v0.3.6:
2d37c01aff
Since `tracing-appender` has a non-optional dependency on `time`, it's
necessary to increase its MSRV to track this.
Hopefully this should automatically publish a GitHub release with
each individual crate's changelog when an appropriate tag is pushed,
provided I haven't made a YAML mistake.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
* chore(CI): split CI into separate workflows
Tracing runs a very large number of jobs on CI. Right now, those jobs
are all defined in a single workflow. Unfortunately, GitHub Actions only
allows restarting failed actions at the workflow level --- if a single
job fails spuriously and we want to rerun it, we currently have to
re-run *all* the jobs we run on CI. This makes rerunning a single failed
job take much longer than is generally necessary.
This branch changes the CI configuration to split each logical set of
jobs into a separate workflow. This way, if (for example) the MSRV `cargo
check` job fails spuriously, we can restart only the MSRV checks
workflow, and not have to re-run (for example) all the WASM test jobs
too.
This doesn't actually change what any of these jobs do, just moves them
into separate yaml files with separate names.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
This PR continues the work started in
https://github.com/tokio-rs/tracing/pull/1646 to replace `chrono` with
`time`. I'll refer to @hawkw's motivation:
> Currently, `tracing-subscriber` supports the `chrono` crate for
> timestamp formatting, via a default-on feature flag. When this code
> was initially added to `tracing-subscriber`, the `time` crate did not
> have support for the timestamp formatting options we needed.
>
> Unfortunately, the `chrono` crate's maintenance status is now in
> question (see #1598). Furthermore, `chrono` depends on version 0.1 of
> the `time` crate, which contains a security vulnerability
> (https://rustsec.org/advisories/RUSTSEC-2020-0071.html). This
> vulnerability is fixed in more recent releases of `time`, but `chrono`
> still uses v0.1.
## Solution
I've replaced chrono with time 0.3. Unfortunately, some of chrono's
builders for DateTimes are not present in `time`, which required the
usage of `macro_rules!` macros to construct some of the hard-coded
times. I also took the opportunity to tidy some of the tests and change
the internal representation of `Rotation::NEVER` from year 9,999 to an
`Option::None`.
This branch changes `tracing-appender`'s MSRV from Rust 1.42 to Rust
1.51, the MSRV for the `time` crate when certain required feature flags
are enabled. This does *not* effect the MSRV for other crates in this
repository.
This branch adds some environment variables to configure *all* CI jobs.
In particular, we:
- disable incremental compilation
- increase the number of retries for network requests in `cargo` and
`rustup`
- emit shortened backtraces from panics
This config was blatantly stolen from linkerd/linkerd2-proxy#7137. :)
Incremental compilation is useful as part of an edit-build-test-edit
cycle, as it lets the compiler avoid recompiling code that hasn't
changed. However, on CI, we're not making small edits; we're almost
always building the entire project from scratch. Thus, incremental
compilation on CI actually introduces *additional* overhead to support
making future builds faster...but no future builds will ever occur in
any given CI environment.
See https://matklad.github.io/2021/09/04/fast-rust-builds.html#ci-workflow
for details.
Increasing retries for network requests should help reduce flakiness a
bit.
## Motivation
Currently, `cargo audit` checks are run on every push that modifies
`Cargo.toml` or lockfiles. The intention behind this was to fail changes
that introduce dependencies that have security advisories. However, it
turns out that this is not actually the primary use-case for `cargo audit`
runs. Typically, when a dependency of a `tracing` crate has a
security advisory, this isn't newly introduced by a PR, but a new
*advisory* that was just announced for a library we *already* depended
on. In this case, this isn't a failure that should block any particular
branch from merging; instead, it's a *new issue* that should block
effected crates from being *released*.
## Solution
This branch changes the audit workflow from running on pushes to running
on a schedule (nightly). When using `actions-rs/audit-check` in a
scheduled mode, it will automatically open new issues if any
dependencies have security advisories (see
https://github.com/actions-rs/audit-check#scheduled-audit for details).
This means those advisories can be fixed separately while still allowing
unrelated branches to pass CI. This is, IMO, a better workflow for
handling security advisories.
If we introduce release automation in the future, we should ensure that
the release automation process checks that the crate being released has
no open security advisory issues, and fails the *release* if any such
issues are still open.
So...it turns out that passing `--bins` actually makes `cargo check` do
*nothing* on library-only projects without binaries. This
was...surprising.
It turns out that because of this, our MSRV check CI run is basically a
nop, and we accidentally merged code that doesn't compile on 1.42.0. I'm
very dumb lol.
## Motivation
When reviewing https://github.com/tokio-rs/tracing/pull/1523, I
suggested that the examples use `fmt::Layer` directly to allow readers
to copy-and-paste examples into their own code. However, @hawkw pointed
out that rustdoc doesn't receive feature flags from Cargo, which means
that running tracing-subscriber's tests under `--no-default-features`
would fail.
## Solution
It turned out that that we're only running `cargo check
--no-default-features` for tracing-subscriber. Running `cargo test
--no-default-features` resulted in compilation failures, but it seems
like nobody noticed. Building on
https://github.com/tokio-rs/tracing/pull/1532, this branch argues that
running tracing-subscriber's tests under `cargo test --lib --tests
--no-default-features` is _probably_ fine if we're able to skip the
doctests (which are _not_ aware of Cargo's feature flags).
The default behavior for GitHub Actions `matrix` strategies is to cancel
all remaining jobs in a `matrix` job if *any* of them fail. However,
this sometimes obscures important information. For example, we have a
step that runs tests on stable, beta, and nightly Rust. In this case,
it's useful to know whether a failure occurred only on nightly, or if it
effects *every* Rust version.
Furthermore, our branch protection settings don't require the nightly
build to pass in order to merge a PR, because there are sometimes
spurious failures due to nightly-specific bugs. We *do* require the
stable build to pass. However, because the `matrix` strategy will cancel
all jobs in the matrix if any of them fails, this means that we
implicitly *do* require the tests to pass on nightly, as if the nightly
job fails, it will also cancel the required stable job.
This commit updates the github actions config to disable failfast for
some build matrices.
This commit removes the `--tests` from the MSRV build's `cargo check`
invocation. This way, CI will not try to compile dev dependencies on the
MSRV. This is fine, because the tests not building on 1.42.0 won't stop
1.42.0 users from depending on `tracing` crates.
Unfortunately, this means we have to remove the CI job that runs tests
on 1.42.0. But, a significant change in the behavior we're testing for
on our MSRV vs on stable seems unlikely.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Currently, CI runs `cargo check --all --bins --tests --benches` on our
MSRV. This is failing, because the latest `criterion` version now
depends on a revision of `bitflags` that doesn't build on Rust 1.42.0
(due to `const fn`s not being const on 1.42.0).
We *could* solve this by bumping our MSRV to a version where the
appropriate functions are `const fn`, or we could use a `=` dependency
to pin to a specific older version of `bitflags` that compiles on
1.42.0. However, both of these solutions seem kind of unfortunate.
This commit just removes the `--benches` from the MSRV `cargo check`
run. The benchmarks are an internal development tool, not an officially
supported part of `tracing`. Users on 1.42.0 can still depend on
`tracing` and use it in their code even if the benchmarks don't compile
on their (fairly old) Rust version. Therefore, it should be fine to just
remove the benchmarks from the MSRV check.
I split the MSRV and stable `cargo check` jobs into separate jobs, so we
still check that commits don't break the benchmarks on stable Rust.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Tracing's examples depend on a number of external crates that the core
`tracing` crates don't depend on. Sometimes, crates that we only depend
on for examples will break compatibility with our MSRV. It's not ideal
to bump our supported MSRV just for an example, since the actual
`tracing` crates will still build fine.
## Solution
Instead, this PR updates the CI config to exclude examples from the
MSRV check. This way, we don't have to bump MSRV for examples-only
dependencies.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## 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
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
## Motivation
refs #642
Start Testing the core crate in a wasm environment. This requires a
special runner, and an extra dev-dependency.
All tests have been ported over except for
`span::spans_always_go_to_the_subscriber_that_tagged_them_even_across_threads`
because that spins up a thread, and wasm has no concept of threads being
spun up.
## Solution
Add a wasm-pack tester, and properly tag all tests.
refs #642
## Motivation
More, and more code is targeting wasm architectures. This is useful for
not only browsers, but also useful for environments that want
extensibility where the host environment is protected, but still allows
extensibility in a variety of languages. Tracing can help these
architectures just as it can any other architecture.
## Solution
start building all crates, except `tracing-futures` in
wasm32-unknown-unknown architecture. this will validate going forward we
don't break wasm32 build support.
in the future we can start running tests crate by crate, and actually
run those tests. the reason we're doing this is every single test has to
be marked with another special attribute, and add a dev-only dependency.
we don't wanna create a huge pr that has to have conflicts fixed all the
time. in the future we can also look at adding in support for
tracing-futures.
## Motivation
The current MSRV check is lazybones, it's meaningless unless we override
toolchain. Actually, we cannot compile tracing crates with 1.39.0, it
should work with 1.44.0 or later because of the `protobuf` crate and
others.
## Solution
This enforces to override toolchain and updates MSRV to 1.40.0, this
should compile all the crates fine. And replaces
`actions/checkout@master` with `main` as they've renamed the default
branch name to `main`.
It turns out that the test run with `--all-features` should never be
able to pass, due to the `static_max_level` features. Setting
`--all-features` will enable `static_max_level_off`, which disables all
tracing. Therefore, all the integration tests will fail.
However, due to a bug in `tracing`'s `LevelFilter::OFF` implementation,
`static_max_level_off` actually _enables_ everything. This makes the
tests pass accidentally. If we fix this bug, though, everything will be
disabled, as it should be, causing the tests to fail. Therefore, we
can't merge a PR that fixes this, like #853, without first resolving the
issues with the tests.
Unfortunately, these are integration tests, so we cannot just put
```rust
#[cfg(not(any(feature = "static_max_level_off", feature = ...))]
```
on the tests to skip them when the feature is set --- features set on
the _crate_ being tested are not visible to integration tests.
Similarly, we cannot use
```rust
#[cfg(test)]
```
to not set the max level when running the tests, because the test cfg is
not set for the crate when running integration tests, only unit tests.
We could change all these tests to be integration tests, I suppose.
However, we are already running a `cargo hack` job that tests every
feature combination for `tracing` _except_ for the `static_max_level`
ones, so I think it's fine to just remove this.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Adding myself to contributors list so I get tagged on tracing-appender PRs
## Solution
Update CODEOWNERS file
Co-authored-by: Zeki Sherif <zekshi@amazon.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
This branch adds a CODEOWNERS file to configure Github to automatically
assign reviewers for pull requests. In general, when a crate or large
subsystem of a crate was contributed by an individual person, we should
use this file to ensure that that person is assigned to review future
changes to that code.
The @tokio-rs/tracing team should still be tagged to approve all pull
requests, in addition to the person who originally contributed the code,
so the CODEOWNERS file ensures this as well.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Our CI builds currently have an issue where `cargo hack` doesn't work
after loading a cached build of the tool. This causes builds to fail.
See #648.
## Solution
This is an admittedly hacky workaround: I've added a step to check if
`cargo hack` exists by running `cargo hack --help`. If this doesn't
succeed, we assume it doesn't exist and forcibly install it from
crates.io.
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
## Motivation
Currently, `tracing-futures` and `tracing-attributes` both have separate
crates for tests that require `std::future` and async/await syntax.
These date back to before these features were stable, and the tests were
separated because they could only be run on nighly Rust. This has not
been the case for a long while, and `tracing` now supports `std::future`
as the default, requiring opt-in support for `futures` 0.1. I think we
had forgotten to remove the special-cased tests when we made that
transition.
## Solution
This branch removes the special-cased test crates, and moves all
`std::future` and async/await tests into the main test dirs for these
crates. I've also removed the separate CI steps that ran these.
This should simplify things significantly!
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
I've made some improvements to the CI workflow script.
I've consistently used the `actions-rs` actions where possible and have
enabled caching for several of the jobs, bringing their completion time
from minutes down to seconds. I've also added a job that runs `cargo
audit`.
I added the `.gitattributes` since I was developing on windows. The
rust-lang repo [also uses
this](caa231d998/.gitattributes (L3)).
The `Cargo.lock` needs to be committed (or regenerated on CI if you
prefer; although this will need to be done for each job which is why
I've opted not to do that) in order to generate the hashes for caching
keys. Note that when `Cargo.lock` is updated, the first subsequent CI
run will be slower since the caches need to be repopulated.
Some further improvements would be to cache `~/.rustup` but I'm not sure
the best way to do that here. Options would be to either manually
maintain a lock file used to drive the caching functionality (this is
what I've opted for on my own projects) or alternatively to try and
scrape the currently available versions from some command that fetches
info from upstream.
This adds a fair bit of complexity to the CI script and some of the
choices (committing the lock file for a library) are maybe a bit
controversial so if you'd rather not merge it's okay. Just thought I'd
try to make things a little more efficient!
* Use actions-rs/cargo for problem matchers
* Use actions-rs/toolchain consistently
* Run checkout step before toolchain
* Cache cargo-hack
* Remove unnecessary build steps
* Run cargo-audit
