Skip to content

Commit

Permalink
chore: docs and tests cleanups(tokio-rs#1556)
Browse files Browse the repository at this point in the history
* small doc cleanups

* subscriber: Use NoSubscriber in Layer tests

No need for another no-op subscriber.

* subscriber: add some compile-time tests for dyn Layer

Make sure that `Box<dyn Layer<S>>` and `Arc<dyn Layer<S>>` meet all the
constraints to be actually usable.
  • Loading branch information
jsgf authored and kaffarell committed May 22, 2024
1 parent ea501e5 commit d18eaf2
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 35 deletions.
4 changes: 2 additions & 2 deletions tracing-core/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,10 @@ impl Interest {
}
}

/// A no-op [`Subscriber`]
/// A no-op [`Subscriber`].
///
/// [`NoSubscriber`] implements the [`Subscriber`] trait by never being enabled,
/// never being interested in any callsite, and drops all spans and events.
/// never being interested in any callsite, and dropping all spans and events.
#[derive(Debug, Copy, Clone)]
pub struct NoSubscriber(());

Expand Down
22 changes: 15 additions & 7 deletions tracing-subscriber/src/fmt/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ pub trait MakeWriterExt<'a>: MakeWriter<'a> {
/// determine if a writer should be produced for a given span or event.
///
/// If the predicate returns `false`, the wrapped [`MakeWriter`]'s
/// [`make_writer_for`][mwf] will return [`OptionalWriter::none`].
/// [`make_writer_for`][mwf] will return [`OptionalWriter::none`][own].
/// Otherwise, it calls the wrapped [`MakeWriter`]'s
/// [`make_writer_for`][mwf] method, and returns the produced writer.
///
Expand Down Expand Up @@ -404,6 +404,7 @@ pub trait MakeWriterExt<'a>: MakeWriter<'a> {
///
/// [`Metadata`]: tracing_core::Metadata
/// [mwf]: MakeWriter::make_writer_for
/// [own]: EitherWriter::none
fn with_filter<F>(self, filter: F) -> WithFilter<Self, F>
where
Self: Sized,
Expand Down Expand Up @@ -446,7 +447,7 @@ pub trait MakeWriterExt<'a>: MakeWriter<'a> {

/// Combines `self` with another type implementing [`MakeWriter`], returning
/// a new [`MakeWriter`] that calls `other`'s [`make_writer`] if `self`'s
/// `make_writer` returns [`OptionalWriter::none`].
/// `make_writer` returns [`OptionalWriter::none`][own].
///
/// # Examples
///
Expand All @@ -466,6 +467,7 @@ pub trait MakeWriterExt<'a>: MakeWriter<'a> {
/// ```
///
/// [`make_writer`]: MakeWriter::make_writer
/// [own]: EitherWriter::none
fn or_else<W, B>(self, other: B) -> OrElse<Self, B>
where
Self: MakeWriter<'a, Writer = OptionalWriter<W>> + Sized,
Expand Down Expand Up @@ -596,13 +598,15 @@ pub struct WithMinLevel<M> {

/// A [`MakeWriter`] combinator that wraps a [`MakeWriter`] with a predicate for
/// span and event [`Metadata`], so that the [`MakeWriter::make_writer_for`]
/// method returns [`OptionalWriter::some`] when the predicate returns `true`,
/// and [`OptionalWriter::none`] when the predicate returns `false`.
/// method returns [`OptionalWriter::some`][ows] when the predicate returns `true`,
/// and [`OptionalWriter::none`][own] when the predicate returns `false`.
///
/// This is returned by the [`MakeWriterExt::with_filter`] method. See the
/// method documentation for details.
///
/// [`Metadata`]: tracing_core::Metadata
/// [ows]: EitherWriter::some
/// [own]: EitherWriter::none
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub struct WithFilter<M, F> {
make: M,
Expand All @@ -611,10 +615,12 @@ pub struct WithFilter<M, F> {

/// Combines a [`MakeWriter`] that returns an [`OptionalWriter`] with another
/// [`MakeWriter`], so that the second [`MakeWriter`] is used when the first
/// [`MakeWriter`] returns [`OptionalWriter::none`].
/// [`MakeWriter`] returns [`OptionalWriter::none`][own].
///
/// This is returned by the [`MakeWriterExt::or_else] method. See the
/// method documentation for details.
///
/// [own]: EitherWriter::none
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub struct OrElse<A, B> {
inner: A,
Expand Down Expand Up @@ -874,12 +880,13 @@ impl<T> From<Option<T>> for OptionalWriter<T> {

impl<M> WithMaxLevel<M> {
/// Wraps the provided [`MakeWriter`] with a maximum [`Level`], so that it
/// returns [`OptionalWriter::none`] for spans and events whose level is
/// returns [`OptionalWriter::none`][own] for spans and events whose level is
/// more verbose than the maximum level.
///
/// See [`MakeWriterExt::with_max_level`] for details.
///
/// [`Level`]: tracing_core::Level
/// [own]: EitherWriter::none
pub fn new(make: M, level: tracing_core::Level) -> Self {
Self { make, level }
}
Expand Down Expand Up @@ -907,12 +914,13 @@ impl<'a, M: MakeWriter<'a>> MakeWriter<'a> for WithMaxLevel<M> {

impl<M> WithMinLevel<M> {
/// Wraps the provided [`MakeWriter`] with a minimum [`Level`], so that it
/// returns [`OptionalWriter::none`] for spans and events whose level is
/// returns [`OptionalWriter::none`][own] for spans and events whose level is
/// less verbose than the maximum level.
///
/// See [`MakeWriterExt::with_min_level`] for details.
///
/// [`Level`]: tracing_core::Level
/// [own]: EitherWriter::none
pub fn new(make: M, level: tracing_core::Level) -> Self {
Self { make, level }
}
Expand Down
48 changes: 22 additions & 26 deletions tracing-subscriber/src/layer/tests.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,5 @@
use super::*;

pub(crate) struct NopSubscriber;

impl Subscriber for NopSubscriber {
fn register_callsite(&self, _: &'static Metadata<'static>) -> Interest {
Interest::never()
}

fn enabled(&self, _: &Metadata<'_>) -> bool {
false
}

fn new_span(&self, _: &span::Attributes<'_>) -> span::Id {
span::Id::from_u64(1)
}

fn record(&self, _: &span::Id, _: &span::Record<'_>) {}
fn record_follows_from(&self, _: &span::Id, _: &span::Id) {}
fn event(&self, _: &Event<'_>) {}
fn enter(&self, _: &span::Id) {}
fn exit(&self, _: &span::Id) {}
}
use tracing_core::subscriber::NoSubscriber;

#[derive(Debug)]
pub(crate) struct NopLayer;
Expand Down Expand Up @@ -64,16 +43,19 @@ impl Subscriber for StringSubscriber {
}

fn assert_subscriber(_s: impl Subscriber) {}
fn assert_layer<S: Subscriber>(_l: &impl Layer<S>) {}

#[test]
fn layer_is_subscriber() {
let s = NopLayer.with_subscriber(NopSubscriber);
let s = NopLayer.with_subscriber(NoSubscriber::default());
assert_subscriber(s)
}

#[test]
fn two_layers_are_subscriber() {
let s = NopLayer.and_then(NopLayer).with_subscriber(NopSubscriber);
let s = NopLayer
.and_then(NopLayer)
.with_subscriber(NoSubscriber::default());
assert_subscriber(s)
}

Expand All @@ -82,10 +64,24 @@ fn three_layers_are_subscriber() {
let s = NopLayer
.and_then(NopLayer)
.and_then(NopLayer)
.with_subscriber(NopSubscriber);
.with_subscriber(NoSubscriber::default());
assert_subscriber(s)
}

#[test]
fn box_layer_is_layer() {
let l: Box<dyn Layer<NoSubscriber> + Send + Sync> = Box::new(NopLayer);
assert_layer(&l);
l.with_subscriber(NoSubscriber::default());
}

#[test]
fn arc_layer_is_layer() {
let l: Arc<dyn Layer<NoSubscriber> + Send + Sync> = Arc::new(NopLayer);
assert_layer(&l);
l.with_subscriber(NoSubscriber::default());
}

#[test]
fn downcasts_to_subscriber() {
let s = NopLayer
Expand All @@ -102,7 +98,7 @@ fn downcasts_to_layer() {
let s = StringLayer("layer_1".into())
.and_then(StringLayer2("layer_2".into()))
.and_then(StringLayer3("layer_3".into()))
.with_subscriber(NopSubscriber);
.with_subscriber(NoSubscriber::default());
let layer = <dyn Subscriber>::downcast_ref::<StringLayer>(&s).expect("layer 1 should downcast");
assert_eq!(&layer.0, "layer_1");
let layer =
Expand Down

0 comments on commit d18eaf2

Please sign in to comment.