mock: improve ergonomics when an `ExpectedSpan` is needed #3097

hds · 2024-10-03T21:24:03Z

Motivation

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:

let span = expect::span().named("span name");

let (collector, handle) = collector::mock()
    .enter(span.clone())
    .exit(span)
    .run_with_handle();

Solution

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.

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.

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.

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.

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.

## 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 `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. ## Solution 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

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.

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

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.

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

hds requested review from hawkw, davidbarsky and a team as code owners October 3, 2024 21:24

hds force-pushed the hds/mock-into-expected-span branch 2 times, most recently from 4bac100 to 5daa282 Compare October 4, 2024 16:14

hds changed the base branch from hds/mock-expected-span-check to master October 4, 2024 16:14

hds force-pushed the hds/mock-into-expected-span branch from 5daa282 to f3570da Compare October 16, 2024 10:24

hds force-pushed the hds/mock-into-expected-span branch from f3570da to 8c80981 Compare October 17, 2024 12:57

davidbarsky approved these changes Oct 29, 2024

View reviewed changes

hds merged commit cb9ec62 into master Oct 29, 2024
56 checks passed

hds deleted the hds/mock-into-expected-span branch October 29, 2024 14:38

hds mentioned this pull request Oct 30, 2024

mock: complete API documentation including expect module #2494

Merged

This was referenced Nov 18, 2024

mock: backport tracing-mock to v0.1.x #3132

Merged

Backport everything up to November 2024 #3144

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mock: improve ergonomics when an `ExpectedSpan` is needed #3097

mock: improve ergonomics when an `ExpectedSpan` is needed #3097

hds commented Oct 3, 2024

mock: improve ergonomics when an ExpectedSpan is needed #3097

mock: improve ergonomics when an ExpectedSpan is needed #3097

Conversation

hds commented Oct 3, 2024

Motivation

Solution

mock: improve ergonomics when an `ExpectedSpan` is needed #3097

mock: improve ergonomics when an `ExpectedSpan` is needed #3097