Most of these changes are places where lifetimes were named, but can be
elided. Then a few cases where a lifetime was elided, but actually
resolves to a named lifetime. So lots of lifetimes.
This is the `v0.1.x` branch sister PR to #3164 (for the `master`
branch), since `clippy --fix` on another branch is a much better way to
apply these changes than backporting.
There has been interest around publishing tracing-mock to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
The `expect` module, which contains constructor functions for many of
the other `tracing-mock` modules needs documentation and examples.
This change adds documentation to the `expect` module and all the public
APIs within it. This includes doctests on all the methods which serve as
examples.
The lint for `missing_docs` has been enabled for the entire
`tracing-mock` crate! This has been done together with all the
other lints that are enabled on the other crates in this project.
The `event::msg("message")` constructor was removed, in favor of
requiring an explicit construction via
`expect::event().with_fields(expect::msg("message"))`. This is
appropriate to reduce the API surface that would need to be supported in
the future and also because the `event::msg` constructor could be
overridden by a subsequent usage of `with_fields`. The shorthand
`expect::message()` was renamed to `expect::msg` to make this
change less burdensome.
The `span::named("name")` constructor was removed, in favor of requiring
an explicit construction via `expect::span.with_name("name")`. The
latter isn't much longer and since #3097, a string with the name can
be passed directly everywhere that an `ExpectedSpan` is required.
This change also sets the `missing_docs` lint to warn for the entire
`tracing-mock` crate, making it ready to publish (once backported).
Refs: #539
The `with_ancestry` methods on `NewSpan` and `ExpectedEvent` provide a
way to match whether the span or event is a contextual or explicit root
or if it has a contextual or explicit parent span.
However, in the case of matching on a contextual or explicit parent
span, only the span name could be used for matching. This is
sufficiently precise when testing tracing instrumentation in other
libraries or applications as opposed to testing tracing itself.
It is likely that a user would like to test that some span or event has
a specific span as a parent, and not just any span with a specific name,
in many cases, all the possible parent spans may have the same name.
This is the case when testing tracing instrumentation in Tokio.
To solve this problem, the `Ancestry` struct was renamed to
`ExpectedAncestry` and in the case of expecting an explicit or
conextual parent, an `ExpectedSpan` object can be passed in. This
provides the maximum possible flexibility.
The convenience functions in the `expect` module now take
`Into<ExpectedSpan>` so that existing tests that pass a string type
object for the parent will see the same behaviour as previously and
shorthand use for expected Ids is also available.
Additionally, the span checking code has been unified between the
`MockCollector` and `MockSubscriber` cases and the assertion
descriptions have been improved to make them more readable.
Many of the methods on `MockCollector` take an `ExpectedSpan`. This
often requires significant boilerplate. For example, to expect that a
span with a specific name enters and then exits, the following code is
needed:
```rust
let span = expect::span().named("span name");
let (collector, handle) = collector::mock()
.enter(span.clone())
.exit(span)
.run_with_handle();
```
In order to make using `tracing-mock` more ergonomic and also more
compact, the `MockCollector` and `MockSubscriber` methods that previous
took an `ExpectedSpan`, are now generic over `Into<ExpectedSpan>`.
There are currently 3 implementations of `From` for `ExpectedSpan` which
allow the following shorthand uses:
`T: Into<String>` - an `ExpectedSpan` will be created that expects to
have a name specified by `T`.
```rust
let (collector, handle) = collector::mock()
.enter("span name")
.exit("span name")
.run_with_handle();
```
`&ExpectedId` - an `ExpectedSpan` will be created that expects to have
the expected Id. A reference is taken and cloned internally because the
caller always needs to use an `ExpectedId` in at least 2 calls to the
mock collector/subscriber.
```rust
let id = expect::id();
let (collector, handle) = collector::mock()
.new_span(&id)
.enter(&id)
.run_with_handle();
```
`&ExpectedSpan` - The expected span is taken by reference and cloned.
```rust
let span = expect::span().named("span name");
let (collector, handle) = collector::mock()
.enter(&span)
.exit(&span)
.run_with_handle();
```
In Rust, taking a reference to an object and immediately cloning it is
an anti-pattern. It is considered better to force the user to clone
outside the API to make the cloning explict.
However, in the case of a testing framework, it seems reasonable to
prefer a more concise API, rather than having it more explicit.
To reduce the size of this PR and to avoid unnecessary churn in other
crates, the tests within the tracing repo which use `tracing-mock` will
not be updated to use the new `Into<ExpectedSpan>` capabilities. The new
API is backwards compatible and those tests can remain as they are.
When recording the parent of an event or span, the `MockCollector`
treats an explicit parent of `None` (i.e. an event or span that is an
explicit root) in the same way as if there is no explicit root. This
leads to it picking up the contextual parent or treating the event or
span as a contextual root.
This change refactors the recording of the parent to use `is_contextual`
to distinguish whether or not an explicit parent has been specified. The
actual parent is also written into an `Ancestry` enum so that the
expected and actual values can be compared in a more explicit way.
Additionally, the `Ancestry` struct has been moved into its own module and
the check behavior has been fixed. The error message has also been
unified across all cases.
Another problem with the previous API is that the two methods
`with_contextual_parent` and `with_explicit_parent` are actually
mutually exclusive, a span or event cannot be both of them. It is also a
(small) mental leap for the user to go from `with_*_parent(None)` to
understanding that this means that a span or event is a root (either
contextual or explicit).
As such, the API has been reworked into a single method `with_ancestry`,
which takes an enum with the following four variants:
* `HasExplicitParent(String)` (parent span name)
* `IsExplicitRoot`
* `HasContextualParent(String)` (parent span name)
* `IsContextualRoot`
To make the interface as useable as possible, helper functions have been
defined in the `expect` module which can be used to create the enum
variants. Specifically, these take `Into<String>` parameter for the span
name.
Given the number of different cases involved in checking ancestry,
separate integration tests have been added to `tracing-mock`
specifically for testing all the positive and negative cases when
asserting on the ancestry of events and spans.
There were two tests in `tracing-attributes` which specified both an
explicit and a contextual parent. This behavior was never intended to
work as all events and spans are either contextual or not. The tests
have been corrected to only expect one of the two.
Fixes: #2440
It currently isn't possible to differentiate spans with the same name,
target, and level when setting expectations on `enter`, `exit`, and
`drop_span`. This is not an issue for `tracing-mock`'s original (and
still primary) use case, which is to test `tracing` itself. However,
when testing the tracing instrumentation in library or application code,
this can be a limitation.
For example, when testing the instrumentation in tokio
(tokio-rs/tokio#6112), it isn't possible to set an expectation on which
task span is entered first, because the name, target, and level of those
spans are always identical - in fact, the spans have the same metadata
and only the field values are different.
To make differentiating different spans possible, `ExpectId` has been
introduced. It is an opaque struct which represents a `span::Id` and can
be used to match spans from a `new_span` expectation (where a `NewSpan`
is accepted and all fields and values can be expected) through to
subsequent `enter`, `exit`, and `drop_span` expectations.
An `ExpectedId` is passed to an `ExpectedSpan` which then needs to be
expected with `MockCollector::new_span`. A clone of the `ExpectedId` (or
a clone of the `ExpectedSpan` with the `ExpectedId` already on it) will
then match the ID assigned to the span to the other span lifecycle
expectations.
The `ExpectedId` uses an `Arc<AtomicU64>` which has the ID for the new
span assigned to it, and then its clones will be matched against that
same ID.
In future changes it will also be possible to use this `ExpectedId` to
match parent spans, currently a parent is only matched by name.
This change adds documentation to the tracing-mock `field` module and
all the public APIs within it. This includes doctests on all the methods
which serve as examples.
Additionally, the `field::msg` function (which constructs a field with
name "message" and the provided value) was moved to `expect::message`.
This is part of a unification of all expectation constructors inside the
`expect` module.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
This change adds documentation to the tracing-mock span module and all
the public APIs within it. This includes doctests on all the methods
which serve as examples.
Additionally, the validation on `ExpectedSpan` was improved so that it
validates the level and target during `enter` and `exit` as well as on
`new_span`.
The method `ExpectedSpan::with_field` was renamed to `with_fields`
(plural) to match the same method on `ExpectedEvent` (and because
multiple fields can be passed to it).
A copy-paste typo was also fixed in the documentation for
`ExpectedEvent::with_contextual_parent`.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
There has been interest around publishing tracing-mock to crates.io
for some time. In order to make this possible, it needs to be cleaned up.
There are some test utils in the `tracing-mock` crate which wouldn't
make sense to publish. They provide test futures that are needed in
multiple `tracing-*` crates, but would likely not be needed outside that
context.
This change moves that functionality into a separate `tracing-test`
crate, which should never be published to crates.io.
Refs: #539
Co-authored-by: David Barsky <me@davidbarsky.com>
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
This change adds documentation to the collector module itself and to all the
public APIs in the module. This includes doctests on all the methods
that serve as examples.
Additionally the implementation for the `Expect` struct has been moved
into the module with the definition, this was missed in #2369.
Refs: #539
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.
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
The `subscriber` module needs documentation and examples.
This change adds documentation to the `subscriber` module and all the public
APIs within it. This includes doctests on all the methods which serve as
examples.
The `MockSubscriberBuilder::record` method was removed as its
functionality is not implemented.
Previously, the `MockSubscriber` would verify the scope of an
`ExpectedEvent`, even if `in_scope` hadn't been called. In this case,
that would check that an event was not in a span if `in_scope` had not
been called. `tracing-subscriber` all adhere to this pattern. However it
is different to the behavior of all other expectation methods, where an
explicit call is needed to expect something, otherwise nothing is
checked. As such, the behavior has been modified to align with the rest
of the crate. The previous behavior can be achieved by calling
`in_scope(None)` to verify that an event has no scope. The documentation
for `in_scope` has been updated with an example for this case.
The tests in `tracing-subscriber` which previously verified *implicitly*
that an event had no scope (by not calling `in_scope` at all) have *not*
been modified. It is my opinion that this implicit behavior was never
required.
Refs: #539
There has been interest around publishing tracing-mock to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
The `event` module needs documentation and examples.
This change adds documentation to the event module and all the public
APIs within it. This includes doctests on all the methods which serve as
examples.
The following pattern was applied to the description of most methods:
- Short description of expectation
- Additional clarification (where needed)
- Description of cases that cause the expectation to fail
- Examples
- Successful validation
- Unsuccesful validation
Two changes were also made in the text provided to the user when an
assertion fails for `with_explicit_parent` or `with_contextual_parent`.
One small API changes is also included:
The method `in_scope` has been placed behind the `tracing-subscriber`
feature flag as it currently only works with the `MockSubscriber`, not
with the `MockCollector`. If the feature flag is active and it is used
to set a non-empty scope, the `MockCollector` will panic with
`unimplemented` during validation.
Refs: #539
There has been interest around publishing `tracing-mock` to crates.io
for some time. In order to make this possible, documentation and some
code clean up is needed.
Specifically **I** want to have access to `tracing-mock` within parts of
`tokio` to write tests that ensure that the correct calling location is
picked up for all `spawn*` functions when the `tracing` feature is
enabled.
This change starts that process by adding a README for `tracing-mock`.
The README follows the standard format for all `tracing` crates and
includes 2 examples. The README is included in the `lib.rs` docs using
`#[doc = include_str!(...)]`, so that the same documentation is included
in the crate, and the examples are tested when running doctests.
The README describes steps when using the `tracing` 1.0 from crates.io
and `tracing-mock` from the `v0.1.x` branch. The tests target the
current branch.
Co-authored-by: David Barsky <me@davidbarsky.com>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
* mock: change helper functions to `expect::<thing>`
The current format of test expectations in `tracing-mock` isn't ideal.
The format `span::expect` requires importing `tracing_mock::<thing>` which
may conflict with imports from other tracing crates, especially
`tracing-core`.
So we change the order and move the functions into a module called
`expect` so that:
* `event::expect` becomes `expect::event`
* `span::expect` becomes `expect::span`
* `field::expect` becomes `expect::field`
This format has two advantages.
1. It reads as natural English, e.g "expect span"
2. It is no longer common to import the modules directly.
Regarding point (2), the following format was previously common:
```rust
use tracing_mock::field;
field::expect();
```
This import of the `field` module may then conflict with importing the
same from `tracing_core`, making it necessary to rename one of the
imports.
The same code would now be written:
```rust
use tracing_mock::expect;
expect::field();
```
Which is less likely to conflict.
This change also fixes an unused warning on `MockHandle::new` when the
`tracing-subscriber` feature is not enabled.
Refs: #539
The `tracing-mock` crate provides a mock collector (and a subscriber for
use by the tests in the `tracing-subscriber` crate) which is able to
make assertions about what diagnostics are emitted.
These assertions are defined by structs that match on events, span, and
their fields and metadata. The structs that matched these objects have
been called, up until now, mocks, however this terminology may be
misleading, as the created objects don't mock anything.
There were two different names for similar functionality with `only()`
and `done()` on fields and collectors/subscribers respectively. Using a
single name for these may make it easier to onboard onto `tracing-mock`.
To reduce confusion, these structs have been split into two categories:
mocks and expectations.
Additionally, the `done()` function on the `Collector` and `Subscriber`
mocks has been replaced with `only()`. This matches the similar function
for `ExpectedField`, and may be more intuitive.
The mocks replace some component in the tracing ecosystem when a library
is under test. The expectations define the assertions we wish to make
about traces received by the mocks.
Mocks (per module):
* collector - `MockCollector`, no change
* subscriber - `MockSubscriber`, renamed from `ExpectSubscriber`
Expectations (per module):
* event - `ExpectedEvent`, renamed from `MockEvent`
* span - `ExpectedSpan`, renamed from `MockSpan`
* field - `ExpectedField` and `ExpectedFields`, renamed from `MockField`
and `Expected`. Also `ExpectedValue` renamed from `MockValue`.
* metadata - `ExpectedMetadata`, renamed from `Expected`
Refs: #539
The `tracing-subscriber` module `tests::support` included functionality
to mock a layer (via the `Layer` trait). This code depends on
some items from `tracing_mock::collector` which should otherwise not be
public.
This change moves the mocking functionality inside `tracing-mock` behind
a feature flag. Allowing the `Expect` enum and `MockHandle::new` from
`tracing_mock::collector` to be made `pub(crate)` instead of `pub`.
Since it's now used from two different modules, the `Expect` enum has
been moved to its own module.
This requires a lot of modifications to imports so that we're not doing
wildcard imports from another crate (i.e. in `tracing-subscriber`
importing wildcards from `tracing-mock`).
This PR is based on @hds' PR #2369, but modified to track renamings. I
also deleted all the doc comments temporarily because updating them was
a lot of work and I need to get a release of `tracing-subscriber` out
first.
Closes: #2359
## Motivation
Fix minimal-versions failure.
## Solution
Upgrade all the dependencies to their most recent semver-compatible
version, adjusting back down as necessary for MSRV.
Essentially a cherry-pick of #2231, but redone by hand.
## Tests
- `cargo minimal-versions msrv verify -- cargo check --all-features`
- `cargo minimal-versions msrv verify -- cargo check --no-default-features`
## Methodology
- `cargo update && cargo upgrade --to-lockfile`
- Identify [a bug](https://github.com/killercup/cargo-edit/issues/750) and manually resolve it
- loop; upgrade transitive deps
- `cargo minimal-versions check --all-features`
- Identify failing dep
- `cargo minimal-versions tree -i dep --all-features`
- Find the closest dependency leading to pulling in `dep`
- `cargo add fixdep --optional` to force a more recent more-minimal-versions-correct version
- loop; downgrade to msrv
- `cargo minimal-versions msrv verify -- cargo check --all-features`
- Identify failing dep
- `cargo minimal-versions tree -i dep --all-features`
- Find the version that supports MSRV from lib.rs
- `cargo upgrade dep@msrv`
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>
This PR extends the `#[instrument]` attribute to accept an optional
`follows_from = …` argument that supplies any number of
`Span::follows_from` relationships to the generated `Span`.
## Motivation
This PR resolves#879.
## Solution
This PR largely follows the implementation strategy articulated by
@hawkw:
https://github.com/tokio-rs/tracing/issues/879#issuecomment-668168468
In that comment, @hawkw suggests taking one of two approaches:
1. each `follows_from` relationship is supplied with a distinct
`follows_from` argument
2. the `follows_from` argument is provided once, and its value is a
**list** of indirect causes
I take the second approach, since it is slightly more flexible: it
allows for the number of indirect causes to vary at runtime.
This addition is complemented by changes to `tracing-mock` to permit
making `follows_from` assertions for testing purposes.
## Motivation
Currently on `v0.1.x`, the global callsite registry is implemented as a
`Mutex<Vec<&'static dyn Callsite>>`. This has a few downsides:
* Every time a callsite is registered, the `Mutex` must be locked. This
can cause a deadlock when a `register_callsite` implementation calls
into code that _also_ registers a callsite. See #2020 for details.
* Registering callsites is relatively slow and forces the program to
synchronize on the callsite registry (and not on *individual*
callsites). This means that if two threads are both registering
totally different callsites, both threads must wait for the lock.
Although this overhead is amortized over the entire rest of the
program, it does have an impact in short-running applications where
any given callsite may only be hit once or twice.
* Memory allocation may occur while the registry is locked. This makes
the use of `tracing` inside of memory allocators difficult or
impossible.
On the `master` branch (v0.2.x), PR #988 rewrote the callsite registry
to use an intrusive atomic singly-linked list of `Callsite`s. This
removed the need for locking the callsite registry, and made it possible
to register callsites without ever allocating memory. Because the
callsite registry on v0.2 will *never* allocate, this also makes it
possible to compile `tracing-core` for `no_std` platforms without
requiring `liballoc`. Unfortunately, however, we cannot use an identical
implementation on v0.1.x, because using the intrusive linked-list
registry for *all* callsites requires a breaking change to the
`Callsite` trait (in order to add a way to get the callsite's
linked-list node), which we cannot make on v0.1.x.
## Solution
This branch adds a linked-list callsite registry for v0.1.x in a way
that allows us to benefit from *some* of the advantages of this approach
in a majority of cases. The trick is introducing a new `DefaultCallsite`
type in `tracing-core` that implements the `Callsite` trait. This type
can contain an intrusive list node, and *when a callsite is a
`DefaultCallsite`*, we can register it without having to push it to the
`Mutex<Vec<...>>`. The locked vec still _exists_, for `Callsite`s that
are *not* `DefaultCallsite`s, so we can't remove the `liballoc`
dependency, but in most cases, we can avoid the mutex and allocation.
Naturally, `tracing` has been updated to use the `DefaultCallsite` type
from `tracing-core`, so the `Vec` will only be used in the following
cases:
* User code has a custom implementation of the `Callsite` trait, which
is [not terribly common][1].
* An older version of the `tracing` crate is in use.
This fixes the deadlock described in #2020 when `DefaultCallsite`s are
used. Additionally, it should reduce the performance impact and memory
usage of the callsite registry.
Furthermore, I've changed the subscriber registry so that a
`RwLock<Vec<dyn Dispatch>>` is only used when there actually are
multiple subscribers in use. When there's only a global default
subscriber, we can once again avoid locking for the registry of
subscribers. When the `std` feature is disabled, thread-local scoped
dispatchers are unavailable, so the single global subscriber will always
be used on `no_std`.
We can make additional changes, such as the ones from #2020, to _also_
resolve potential deadlocks when non-default callsites are in use, but
since this branch rewrites a lot of the callsite registry code, that
should probably be done in a follow-up.
[1]: https://cs.github.com/?scopeName=All+repos&scope=&q=%28%2Fimpl+.*Callsite%2F+AND+language%3Arust%29+NOT+%28path%3A%2Ftracing%2F**+OR+path%3A%2Ftracing-*%2F**+OR+path%3A%2Ftokio-trace*%2F**%29%29
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
... instead of `#[path = ""]` importing it everywhere.
Make sure to use a diff review tool that understands file renaming 😅
(GitHub's diff view does.)
## Motivation
Transparency: I want to use the mocking functionality in the development
of a tracing component out-of-tree.
Additionally, this reduces the use of `#[path] mod` and file
multiple-inclusion, which aren't that great of a practice.
## Solution
The tracing test support module was already well self-contained, due to
being `#[path] mod` used in multiple places. As such, extracting it to
its own crate is rather mechanical, with no surprising blockers.
We additionally move the tracing-futures support module contents into
tracing_mock, for convenience. The one function which relies on
tokio-test is made optional.
It's a reasonable result for this functionality to stay unpublished, and
only used inside the repo, but pulling it out into a directly reusable
chunk instead of abusing the module system to reuse it via
multiple-inclusion is an improvement to code structure and
modularization.
