mock: match parent span on ExpectedSpan
#3098
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/mock-parent-by-expected-span
branch
from
0ea1cfe
to
1f5c9a7
Compare
hds
force-pushed
the
hds/mock-into-expected-span
branch
from
5daa282
to
f3570da
Compare
hds
force-pushed
the
hds/mock-parent-by-expected-span
branch
from
1f5c9a7
to
f1068c5
Compare
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.
hds
force-pushed
the
hds/mock-into-expected-span
branch
from
f3570da
to
8c80981
Compare
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.
hds
force-pushed
the
hds/mock-parent-by-expected-span
branch
from
f1068c5
to
39438f4
Compare
davidbarsky
approved these changes
Oct 29, 2024
hds
added a commit
that referenced
this pull request
Nov 9, 2024
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.
This was referenced Nov 18, 2024
hds
added a commit
that referenced
this pull request
Nov 20, 2024
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.
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
The
with_ancestrymethods onNewSpanandExpectedEventprovide away 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.
Solution
To solve this problem, the
Ancestrystruct was renamed toExpectedAncestryand in the case of expecting an explicit orconextual parent, an
ExpectedSpanobject can be passed in. Thisprovides the maximum possible flexibility.
The convenience functions in the
expectmodule now takeInto<ExpectedSpan>so that existing tests that pass a string typeobject 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
MockCollectorandMockSubscribercases and the assertiondescriptions have been improved to make them more readable.