mock: backport tracing-mock to v0.1.x
#3132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hds
force-pushed
the
hds/backport-tracing-mock
branch
2 times, most recently
from
4039ce1 to
9f930da
Compare
There is an incompatibility with the version of Node available on our test runners and wasm32 in Rust 1.82 (#3123). To unblock the CI, this change pins Rust to 1.81 for the tests using the `wasm32-unknown-unknown` target. This is the same strategy used in Tokio to mitigate tokio-rs/tokio#6910 until a more permanent fix can be put in place. This change also bumps the MSRV on the `tracing-examples` crate from 1.63.0 to 1.64.0 to avoid triggering a lint about the MSRV after a change in Tokio 1.41.0 which bumps the required Rust version for the `try_join!` macro. The Tokio MSRV is 1.70 now, so needing this bump for the examples seems reasonable.
hds
force-pushed
the
hds/backport-tracing-mock
branch
3 times, most recently
from
03291b6 to
e7d0678
Compare
hds
force-pushed
the
hds/backport-tracing-mock
branch
from
a770352 to
cee9e04
Compare
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>
hds
force-pushed
the
hds/backport-tracing-mock
branch
from
cee9e04 to
a87faa3
Compare
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>
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>
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.
hds
force-pushed
the
hds/backport-tracing-mock
branch
from
2a2f6ad to
8aecce8
Compare
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
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.
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.
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
davidbarsky
approved these changes
Nov 20, 2024
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
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
tracing-mockcrate has been fully documented and updatedon the
masterbranch, and some of it has been backported to thev0.1.xbranch, but the remaining changes also need backporting.Solution
Backporting remaining
tracing-mockcommits to thev0.1.xbranch.This has been done step by step so that the full CI was run on each commit,
with the idea on merging all commits without squashing.
tracing-testcrate for non-publishable test utils #2466fieldmodule #2443fieldmodule #2443ExpectedIdto link span expectations #3007ExpectedSpanis needed #3097ExpectedSpan#3098Also backported a single CI fix that was needed:
wasm32-unknown-unknowntests #3125