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

Fixes a typo and adds a note about the supported field syntax #1232

Merged
merged 3 commits into from Feb 17, 2021

Conversation

lfrancke
Copy link

@lfrancke lfrancke commented Feb 9, 2021

The instrument macro does not explain the syntax that can be used and
the main tracing docs only say that this syntax is valid for span and
event macros.

@lfrancke lfrancke requested review from hawkw and a team as code owners February 9, 2021 10:15
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.

Looks great --- I had a couple minor suggestions. Thanks for adding this!

tracing/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines 195 to 196
//! The [`span!`] and [`event!`] macros as well as the `#[tracing::instrument]` attribute
//! use fairly similar syntax, with some exceptions.
Copy link
Member

Choose a reason for hiding this comment

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

if you're up for it, it would be nice to make this part of the change on the master branch as well --- these docs should be about the same, unlike the attribute's docs.

@lfrancke
Copy link
Author

Thanks for the review!
I'll incorporate the changes but it'll take me a few days.

@hawkw
Copy link
Member

hawkw commented Feb 11, 2021

@lfrancke mind if I just go ahead and make the suggested changes here? I'd like to publish a tracing-attributes release today with #1236, and it would be nice to get the docs fixes in as well.

@lfrancke
Copy link
Author

lfrancke commented Feb 11, 2021 via email

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@@ -192,8 +192,8 @@
//!
//! ## Using the Macros
//!
//! The [`span!`] and [`event!`] macros use fairly similar syntax, with some
//! exceptions.
//! The [`span!`] and [`event!`] macros as well as the `#[instrument]` attribute
Copy link
Member

Choose a reason for hiding this comment

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

it might be nice if #[instrument] was a link as well, but not a hard blocker.

@hawkw hawkw merged commit 9d4b7e7 into tokio-rs:v0.1.x Feb 17, 2021
hawkw added a commit that referenced this pull request Feb 17, 2021
Fixed

- **attributes**: Compiler error when using `#[instrument(err)]` on
  functions which return `impl Trait` ([#1236])
- Fixed broken match arms in event macros ([#1239])
- Documentation improvements ([#1232])

Thanks to @bkchr and @lfranke for contributing to this release!

[#1236]: #1236
[#1239]: #1239
[#1232]: #1232

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Feb 17, 2021
### Fixed

- **attributes**: Compiler error when using `#[instrument(err)]` on
  functions which return `impl Trait` ([#1236])
- Fixed broken match arms in event macros ([#1239])
- Documentation improvements ([#1232])

Thanks to @bkchr and @lfranke for contributing to this release!

[#1236]: #1236
[#1239]: #1239
[#1232]: #1232

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@lfrancke lfrancke deleted the instrument_doc branch February 18, 2021 10:12
@lfrancke
Copy link
Author

I'm sorry for the super slow responses. I'm swamped at the moment.
I still have a fix for main/master on my list but if anyone gets to it before me I won't be angry :)

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.

None yet

2 participants