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

docs: better docs on async usage; fancy warning formatting, etc #769

Merged
merged 9 commits into from
Jun 29, 2020

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jun 26, 2020

This branch makes the following changes:

  • Add a new section to the Span::enter docs that explains why
    it should be avoided in async code, and suggests alternatives.
  • Add links to the new section elsewhere.
  • Add new formatting for "Notes" and "Warnings" in the documentation,
    by (mis?)using existing styles from the RustDoc stylesheet.
  • Fixed a handful of inaccuracies or outdated statements I noticed
    while making other changes.

Fixes #667

Signed-off-by: Eliza Weisman eliza@buoyant.io

Fixes  #667

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
sorry david i just really hate this (and the terrible line wrapping that
results from it)

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw added kind/docs crate/subscriber Related to the `tracing-subscriber` crate crate/core Related to the `tracing-core` crate crate/tracing Related to the `tracing` crate labels Jun 26, 2020
@hawkw hawkw requested a review from a team as a code owner June 26, 2020 22:08
@hawkw hawkw self-assigned this Jun 26, 2020
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
we can't add it as adev dependecy, since this creates a circular
dependency

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: David Barsky <me@davidbarsky.com>
README.md Outdated Show resolved Hide resolved
Co-authored-by: David Barsky <me@davidbarsky.com>
@hawkw hawkw merged commit ef8efaa into master Jun 29, 2020
@hawkw hawkw deleted the eliza/async-enter-warning branch June 29, 2020 19:01
Copy link
Collaborator

@yaahc yaahc left a comment

Choose a reason for hiding this comment

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

--

//! </div>
//! <div class="example-wrap" style="display:inline-block">
//! <pre class="ignore" style="white-space:normal;font:inherit;">
//! <strong>Note</strong>:the thread-local scoped dispatcher <code>with_default</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

the newline after this is showing up in the preview

Screenshot from 2020-06-29 11-56-29

/// <div class="tooltip compile_fail" style="">&#x26a0; &#xfe0f;<span class="tooltiptext">Warning</span></div>
/// </div><div class="example-wrap" style="display:inline-block"><pre class="compile_fail" style="white-space:normal;font:inherit;">
/// <strong>Warning</strong>: In general, libraries should <em>not</em> call
/// <code>set_global_default()</code>! Doing so will cause conflicts when
Copy link
Collaborator

Choose a reason for hiding this comment

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

the ! here reads a little oddly

Screenshot from 2020-06-29 11-58-38

Copy link
Member Author

@hawkw hawkw Jun 29, 2020

Choose a reason for hiding this comment

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

That's not specific to the new HTML formatting, I'm pretty sure rustdoc would look the same for any instance of

`some text`!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I figured as much, in this one case I was just saying I thought it looked funny rather than it looks broken.

/// <strong>Deprecated</strong>: The <a href="#method.try_close"><code>try_close</code></a>
/// method is functionally identical, but returns <code>true</code> if the span is now closed.
/// It should be used instead of this method.
/// </pre>
Copy link
Collaborator

Choose a reason for hiding this comment

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

CSS after this entry gets messed up somehow

Screenshot from 2020-06-29 11-59-24

/// <div class="example-wrap" style="display:inline-block">
/// <pre class="ignore" style="white-space:normal;font:inherit;">
/// <strong>Note</strong>: The <code>record_error</code> trait method is only
/// available when the Rust standard library is present, as it requires the `
Copy link
Collaborator

Choose a reason for hiding this comment

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

extra backtick

Suggested change
/// available when the Rust standard library is present, as it requires the `
/// available when the Rust standard library is present, as it requires the

Screenshot from 2020-06-29 12-00-31

@@ -202,8 +209,15 @@ pub trait Visit {

/// Records a type implementing `Error`.
///
/// **Note**: this is only enabled when the Rust standard library is
/// <div class="information">
Copy link
Collaborator

Choose a reason for hiding this comment

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

something fucky is going on here...

Screenshot from 2020-06-29 12-01-32

/// of those callsites are not equivalent.
/// </pre>
/// </div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar formatting bleedthrough

Screenshot from 2020-06-29 12-02-22

/// <div class="example-wrap" style="display:inline-block">
/// <div class="example-wrap" style="display:inline-block">
/// <pre class="ignore" style="white-space:normal;font:inherit;">
/// <strong>Note</strong>: Span IDs must be greater than zero.</pre></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

formatting effects need to be scoped here as well

Screenshot from 2020-06-29 12-04-17

/// **Note:** This method (and [`Layer::enabled`]) determine whether a
/// span or event is globally enabled, _not_ whether the individual layer
/// will be notified about that span or event. This is intended to be used
/// <div class="information">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one sploded

Uploading Screenshot from 2020-06-29 12-07-23.png…

/// **Note**: users of the `LookupSpan` trait should typically call the
/// [`span`] method rather than this method. The `span` method is
/// implemented by calling `span_data`, but returns a reference which is
/// <div class="information">
Copy link
Collaborator

Choose a reason for hiding this comment

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

sploded

Screenshot from 2020-06-29 12-08-49

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gonna stop on this one, These all seem to be the same issue, I'm assuming once you fix it the rest will be minor formatting issues and it will be easier to review them without the sploded formatting everywhere distracting me.

hawkw added a commit that referenced this pull request Jun 30, 2020
## Motivation

PR #769 was merged with some incorrect docs formatting due to
incorrect HTML that resulted from overzealous find-and-replace
misuse. See #769 (review)

## Solution

This PR un-splodes it.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 1, 2020
Changed:

- **parking_lot**: Updated the optional `parking_lot` dependency to
  accept the latest `parking_lot` version (#774)

Fixed

- **fmt**: Fixed events with explicitly overridden parent spans being
  formatted  as though they were children of the current span (#767)

Added

- **fmt**: Added the option to print synthesized events when spans are
  created, entered, exited, and closed, including span durations (#761)
- Documentation clarification and improvement (#762, #769)

Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this
release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 1, 2020
Changed:

- **parking_lot**: Updated the optional `parking_lot` dependency to
  accept the latest `parking_lot` version (#774)

Fixed

- **fmt**: Fixed events with explicitly overridden parent spans being
  formatted  as though they were children of the current span (#767)

Added

- **fmt**: Added the option to print synthesized events when spans are
  created, entered, exited, and closed, including span durations (#761)
- Documentation clarification and improvement (#762, #769)

Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this
release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 1, 2020
### Changed

- **parking_lot**: Updated the optional `parking_lot` dependency to
  accept the latest `parking_lot` version (#774)

### Fixed

- **fmt**: Fixed events with explicitly overridden parent spans being
  formatted  as though they were children of the current span (#767)

### Added

- **fmt**: Added the option to print synthesized events when spans are
  created, entered, exited, and closed, including span durations (#761)
- Documentation clarification and improvement (#762, #769)

Thanks to @rkuhn, @greenwoodcm, and @Ralith for contributing to this
release!

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
hawkw added a commit that referenced this pull request Jul 8, 2020
Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nqvz for
contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
### Changed

- Replaced use of `inner_local_macros` with `$crate::` (#729)

### Added

- `must_use` warning to guards returned by `dispatcher::set_default`
  (#686)
- `fmt::Debug` impl to `dyn Value`s (#696)
- Functions to convert between `span::Id` and `NonZeroU64` (#770)
- More obvious warnings in documentation (#769)

### Fixed

- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)
- Clippy warning on vtable address comparison in `callsite::Identifier`
  (#749)
- Documentation formatting issues (#715, #771)

Thanks to @bkchr, @majecty, @taiki-e, @nagisa, and @nvzqz for
contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
Added

- **attributes**: Support for arbitrary expressions as fields in
  `#[instrument]` (#672)
- **attributes**: `#[instrument]` now emits a compiler warning when
  ignoring unrecognized input (#672, #786)
- Improved documentation on using `tracing` in async code (#769)

Changed

- Updated `tracing-core` dependency to 0.1.11

Fixed

- **macros**: Excessive monomorphization in macros, which could lead to
  longer compilation times (#787)
- **log**: Compiler warnings in macros when `log` or `log-always` features
  are enabled (#753)
- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)

Thanks to @nagisa for contributing to this release!
hawkw added a commit that referenced this pull request Jul 8, 2020
### Added

- **attributes**: Support for arbitrary expressions as fields in
  `#[instrument]` (#672)
- **attributes**: `#[instrument]` now emits a compiler warning when
  ignoring unrecognized input (#672, #786)
- Improved documentation on using `tracing` in async code (#769)

### Changed

- Updated `tracing-core` dependency to 0.1.11

### Fixed

- **macros**: Excessive monomorphization in macros, which could lead to
  longer compilation times (#787)
- **log**: Compiler warnings in macros when `log` or `log-always` features
  are enabled (#753)
- Compiler error when `tracing-core/std` feature is enabled but
  `tracing/std` is not (#760)

Thanks to @nagisa, and everyone who contributed to the new `tracing-core`
and `tracing-attributes` versions, for contributing to this release!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/core Related to the `tracing-core` crate crate/subscriber Related to the `tracing-subscriber` crate crate/tracing Related to the `tracing` crate kind/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Readme example says it uses set_global_default, but does not do so
4 participants