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
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tracing-subscriber/src/filter/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ use tracing_core::{Interest, Metadata, Subscriber};
/// };
/// use tracing_core::Level;
/// use std::{sync::Arc, fs::File};
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
/// # fn docs() -> Result<(), Box<dyn std::error::Error>> {
hawkw marked this conversation as resolved.
Show resolved Hide resolved
///
/// // A layer that logs events to stdout using the human-readable "pretty"
/// // format.
Expand Down
32 changes: 32 additions & 0 deletions tracing-subscriber/src/layer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,38 @@
//! # Ok(()) }
//! ```
//!
//! ## Runtime-configurable Layers
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//!
//! 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.

//!
//! ```
//! // wrap this in a function so we don't actually create `debug.log` when
//! // running the doctests..
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//! # fn docs() -> Result<(), Box<dyn std::error::Error + 'static>> {
//! use tracing_subscriber::{Registry, prelude::*};
//! use std::{fs::File, sync::Arc};
hawkw marked this conversation as resolved.
Show resolved Hide resolved
//! 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()

//! .pretty();
//!
//! let debug_log = match std::env::var("LOG_PATH") {
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//! Ok(path) => {
//! let file = File::create("debug.log")?;
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//! let layer = tracing_subscriber::fmt::layer()
//! .with_writer(Arc::new(file));
//! Some(layer)
//! },
//! Err(_) => None,
//! };
//!
//! let subscriber = Registry::default()
//! .with(stdout_log)
//! .with(debug_log);
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//! tracing::subscriber::set_global_default(subscriber).expect("Unable to set global subscriber");
davidbarsky marked this conversation as resolved.
Show resolved Hide resolved
//! # Ok(()) }
//! ```
//!
//! [`Subscriber`]: https://docs.rs/tracing-core/latest/tracing_core/subscriber/trait.Subscriber.html
//! [span IDs]: https://docs.rs/tracing-core/latest/tracing_core/span/struct.Id.html
//! [the current span]: Context::current_span
Expand Down