Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mock: document public APIs in subscriber module #2446

Merged
merged 3 commits into from
Jan 26, 2023
Merged

Conversation

hds
Copy link
Contributor

@hds hds commented Jan 17, 2023

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 subscriber module needs documentation and examples.

Solution

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 `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
@hds hds requested review from hawkw, davidbarsky and a team as code owners January 17, 2023 21:50
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding the docs! i left some small wording suggestions

tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Show resolved Hide resolved
tracing-mock/src/subscriber.rs Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
tracing-mock/src/subscriber.rs Outdated Show resolved Hide resolved
hds and others added 2 commits January 18, 2023 15:48
These are the last few where I actually had to type something. (-:
hds added a commit that referenced this pull request Jan 18, 2023
The PR documenting the subscriber module.
@hds hds requested a review from hawkw January 18, 2023 17:28
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this looks good to me.

@davidbarsky, do you also want to take a look at the docs? If not, I'm happy to merge this PR.

@davidbarsky
Copy link
Member

@davidbarsky, do you also want to take a look at the docs? If not, I'm happy to merge this PR.

@hawkw no, let's land these.

@hawkw hawkw merged commit 4cda338 into master Jan 26, 2023
@hawkw hawkw deleted the hds/mock-docs-subscriber branch January 26, 2023 23:50
davidbarsky pushed a commit that referenced this pull request Sep 27, 2023
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
hawkw pushed a commit that referenced this pull request Oct 1, 2023
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
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
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: tokio-rs#539
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants