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

subscriber: add example of Option<Layer> #1596

Merged
merged 17 commits into from
Sep 29, 2021

Conversation

davidbarsky
Copy link
Member

This PR contains two changes:

  1. An example as how to use Option<Layer> to toggle layers at runtime, and...
  2. A fix to an example writing to disk when doc tests are run.

@davidbarsky davidbarsky requested review from hawkw and a team as code owners September 24, 2021 18:34
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, this should definitely be featured more prominently in the docs. I had some suggestions, let me know what you think?

tracing-subscriber/src/filter/targets.rs Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
Comment on lines 407 to 408
//! use std::{fs::File, sync::Arc};
//! let stdout_log = tracing_subscriber::fmt::layer()
Copy link
Member

Choose a reason for hiding this comment

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

nit: let's put whitespace here

Suggested change
//! use std::{fs::File, sync::Arc};
//! let stdout_log = tracing_subscriber::fmt::layer()
//! use std::{fs::File, sync::Arc};
//!
//! let stdout_log = tracing_subscriber::fmt::layer()

Copy link
Member

Choose a reason for hiding this comment

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

also, it might be worth adding comments to these so the reader knows what they're doing

Suggested change
//! use std::{fs::File, sync::Arc};
//! let stdout_log = tracing_subscriber::fmt::layer()
//! use std::{fs::File, sync::Arc};
//!
//! // Always log to stdout in a human-readable format.
//! let stdout_log = tracing_subscriber::fmt::layer()

tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
Comment on lines 398 to 400
//! Conditionally enabling/disabling layers at runtime can be challenging due to the generics
//! used Layers implementations. However, A Layer wrapped in an `Option` is also Layers, which
//! allows some runtime configuration:
Copy link
Member

@hawkw hawkw Sep 24, 2021

Choose a reason for hiding this comment

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

some copyedits --- how about something like

Suggested change
//! Conditionally enabling/disabling layers at runtime can be challenging due to the generics
//! used Layers implementations. However, A Layer wrapped in an `Option` is also Layers, which
//! allows some runtime configuration:
//! In some cases, a particular [`Layer`] may be enabled or disabled based on
//! runtime configuration. This can introduce challenges, because the type of a
//! layered [`Subscriber`] depends on which layers are added to it: if an `if`
//! or `match` expression adds some [`Layer`]s in one branch and other layers
//! in another, the [`Subscriber`] values returned by those branches will have
//! different types.
//!
//! However, a [`Layer`] wrapped in an [`Option`] [also implements the `Layer
//! trait][option-impl]. This allows individual layers to be enabled or disabled at
//! runtime while always producing a [`Subscriber`] of the same type. For
//! example:
//!
//! [option-impl]: crate::layer::Layer#impl-Layer<S>-for-Option<L>

Copy link
Member

Choose a reason for hiding this comment

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

also, i think it might be nice to introduce these problems with a compile_fail example that shows code that doesn't work because of creating subscribers of two different types...but it's up to you if you want to spend time on that...

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, those copy edits are way better. sorry, when i wrote this, i felt myself fading terribly, and this much better.

I'll also add a compile_fail example.

@hawkw
Copy link
Member

hawkw commented Sep 25, 2021

@davidbarsky mind if i just go ahead and apply my docs suggestions, or are you still working on this?

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.

example change looks great, thanks! i commented on a few minor nits

tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
Comment on lines 442 to 448
//! # use std::path::PathBuf;
//! # fn docs() -> Result<(), Box<dyn std::error::Error + 'static>> {
//! # struct Config {
//! # is_prod: bool,
//! # path: PathBuf,
//! # }
//! # let cfg = Config { is_prod: false, path: PathBuf::from("debug.log") };
Copy link
Member

Choose a reason for hiding this comment

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

nit/tioli: this could maybe be simplified to avoid having to import PathBuf

Suggested change
//! # use std::path::PathBuf;
//! # fn docs() -> Result<(), Box<dyn std::error::Error + 'static>> {
//! # struct Config {
//! # is_prod: bool,
//! # path: PathBuf,
//! # }
//! # let cfg = Config { is_prod: false, path: PathBuf::from("debug.log") };
//! # fn docs() -> Result<(), Box<dyn std::error::Error + 'static>> {
//! # struct Config {
//! # is_prod: bool,
//! # path: &'static str,
//! # }
//! # let cfg = Config { is_prod: false, path: "debug.log" };

Comment on lines 456 to 457
//! // note: in reality, https://crates.io/crates/tracing-appender should
//! // be used instead of the example below.
Copy link
Member

Choose a reason for hiding this comment

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

i'm not sure if it's really important to add the note about tracing-appender...i wouldn't say it's a hard and fast rule that that crate should always be used when logging to a file, if the user doesn't intend to do things like rotating the log file. not a blocker, though....

tracing-subscriber/src/layer/mod.rs Outdated Show resolved Hide resolved
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, looks great to me! thanks for adding this (and for putting up with all my nitpicks!)

@hawkw hawkw merged commit 66fd693 into v0.1.x Sep 29, 2021
@hawkw hawkw deleted the davidbarsky/add-option-example-to-layer branch September 29, 2021 16:58
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.2.25 (October 5, 2021)

This release fixes an issue where a `Layer` implementation's custom
`downcast_raw` implementation was lost when wrapping that layer with a
per-layer filter.

### Fixed

- **registry**: Forward `Filtered::downcast_raw` to wrapped `Layer`
  ([#1619])

### Added

- Documentation improvements ([#1596], [#1601])

Thanks to @bryanburgers for contributing to this release!

[#1619]: #1619
[#1601]: #1601
[#1596]: #1596
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.2.25 (October 5, 2021)

This release fixes an issue where a `Layer` implementation's custom
`downcast_raw` implementation was lost when wrapping that layer with a
per-layer filter.

### Fixed

- **registry**: Forward `Filtered::downcast_raw` to wrapped `Layer`
  ([#1619])

### Added

- Documentation improvements ([#1596], [#1601])

Thanks to @bryanburgers for contributing to this release!

[#1619]: #1619
[#1601]: #1601
[#1596]: #1596
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Oct 5, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Oct 6, 2021
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([#1600], [#1605],
  [#1614], [#1616], [#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([#1602])
- Documentation fixes and improvements ([#1595], [#1601], [#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[#1585]: #1585
[#1595]: #1596
[#1597]: #1597
[#1600]: #1600
[#1601]: #1601
[#1602]: #1602
[#1605]: #1605
[#1614]: #1614
[#1616]: #1616
[#1617]: #1617
hawkw added a commit that referenced this pull request Mar 23, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 23, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Mar 24, 2022
This PR contains two changes:
1. An example as how to use `Option<Subscribe>` to toggle subscribers at
   runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
This PR contains two changes:
1. An example as how to use `Option<Layer>` to toggle layers at runtime, and...
2. A fix to an example writing to disk when doc tests are run.

Co-authored-by: Eliza Weisman <[email protected]>
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.2.25 (October 5, 2021)

This release fixes an issue where a `Layer` implementation's custom
`downcast_raw` implementation was lost when wrapping that layer with a
per-layer filter.

### Fixed

- **registry**: Forward `Filtered::downcast_raw` to wrapped `Layer`
  ([tokio-rs#1619])

### Added

- Documentation improvements ([tokio-rs#1596], [tokio-rs#1601])

Thanks to @bryanburgers for contributing to this release!

[tokio-rs#1619]: tokio-rs#1619
[tokio-rs#1601]: tokio-rs#1601
[tokio-rs#1596]: tokio-rs#1596
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.1.29 (October 5th, 2021

This release adds support for recording `Option<T> where T: Value` as
typed `tracing` field values. It also includes significant performance
improvements for functions annotated with the `#[instrument]` attribute
when the generated span is disabled.

### Changed

- `tracing-core`: updated to v0.1.21
- `tracing-attributes`: updated to v0.1.19

### Added

- **field**: `Value` impl for `Option<T> where T: Value` ([tokio-rs#1585])
- **attributes**: - improved performance when skipping
  `#[instrument]`-generated spans below the max level ([tokio-rs#1600], [tokio-rs#1605],
  [tokio-rs#1614], [tokio-rs#1616], [tokio-rs#1617])

### Fixed

- **instrument**: added missing `Future` implementation for
  `WithSubscriber`, making the `WithDispatch` extension trait actually
  useable ([tokio-rs#1602])
- Documentation fixes and improvements ([tokio-rs#1595], [tokio-rs#1601], [tokio-rs#1597])

Thanks to @BrianBurgers, @mattiast, @DCjanus, @oli-obk, and @matklad for
contributing to this release!

[tokio-rs#1585]: tokio-rs#1585
[tokio-rs#1595]: tokio-rs#1596
[tokio-rs#1597]: tokio-rs#1597
[tokio-rs#1600]: tokio-rs#1600
[tokio-rs#1601]: tokio-rs#1601
[tokio-rs#1602]: tokio-rs#1602
[tokio-rs#1605]: tokio-rs#1605
[tokio-rs#1614]: tokio-rs#1614
[tokio-rs#1616]: tokio-rs#1616
[tokio-rs#1617]: tokio-rs#1617
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.

2 participants