From 3a5d175685368c3ede9c497f12ce55bfe21b3855 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 22 Jun 2021 19:33:42 +0200 Subject: [PATCH] subscriber: unify span traversal (#1431) Fixes #1429 (forwardport from v0.1.x branch) Implemented as described in #1429, and migrated all internal uses of the deprecated methods. All tests passed both before and after the migration (9ec8130). - Add a new method `SpanRef::scope(&self)` that returns a leaf-to-root `Iterator`, including the specified leaf - Add a new method `Scope::from_root(self)` (where `Scope` is the type returned by `SpanRef::scope` defined earlier) that returns a root-to-leaf `Iterator` that ends at the current leaf (in other words: it's essentially the same as `Scope::collect::>().into_iter().rev()`) - Deprecate all existing iterators, since they can be replaced by the new unified mechanism: - `Span::parents` is equivalent to `Span::scope().skip(1)` (although the `skip` is typically a bug) - `Span::from_root` is equivalent to `Span::scope().skip(1).from_root()` (although the `skip` is typically a bug) - `Context::scope` is equivalent to `Context::lookup_current().scope().from_root()` (although the `lookup_current` is sometimes a bug, see also #1428) --- .../examples/sloggish/sloggish_collector.rs | 3 +- tracing-core/src/metadata.rs | 4 +- tracing-error/src/subscriber.rs | 3 +- tracing-flame/src/lib.rs | 23 +- tracing-journald/src/lib.rs | 10 +- tracing-subscriber/src/filter/env/mod.rs | 7 +- tracing-subscriber/src/filter/level.rs | 2 +- tracing-subscriber/src/fmt/fmt_subscriber.rs | 15 +- tracing-subscriber/src/fmt/format/json.rs | 6 +- tracing-subscriber/src/fmt/format/mod.rs | 42 +-- tracing-subscriber/src/fmt/format/pretty.rs | 10 +- tracing-subscriber/src/registry/mod.rs | 344 +++++++++++++++--- tracing-subscriber/src/subscribe.rs | 96 +++-- 13 files changed, 412 insertions(+), 153 deletions(-) diff --git a/examples/examples/sloggish/sloggish_collector.rs b/examples/examples/sloggish/sloggish_collector.rs index c22afa6ddf..79eb56355a 100644 --- a/examples/examples/sloggish/sloggish_collector.rs +++ b/examples/examples/sloggish/sloggish_collector.rs @@ -121,7 +121,6 @@ impl<'a> Visit for Event<'a> { Style::new().bold().paint(format!("{:?}", value)) ) .unwrap(); - self.comma = true; } else { write!( &mut self.stderr, @@ -130,8 +129,8 @@ impl<'a> Visit for Event<'a> { value ) .unwrap(); - self.comma = true; } + self.comma = true; } } diff --git a/tracing-core/src/metadata.rs b/tracing-core/src/metadata.rs index 5de3b1a2e4..af212e9063 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -832,10 +832,10 @@ mod tests { (LevelFilter::TRACE, Some(Level::TRACE)), ]; for (filter, level) in mapping.iter() { - assert_eq!(filter.clone().into_level(), *level); + assert_eq!(filter.into_level(), *level); match level { Some(level) => { - let actual: LevelFilter = level.clone().into(); + let actual: LevelFilter = (*level).into(); assert_eq!(actual, *filter); } None => { diff --git a/tracing-error/src/subscriber.rs b/tracing-error/src/subscriber.rs index 631dc04c63..75065af954 100644 --- a/tracing-error/src/subscriber.rs +++ b/tracing-error/src/subscriber.rs @@ -98,8 +98,7 @@ where let span = subscriber .span(id) .expect("registry should have a span for the current ID"); - let parents = span.parents(); - for span in std::iter::once(span).chain(parents) { + for span in span.scope() { let cont = if let Some(fields) = span.extensions().get::>() { f(span.metadata(), fields.fields.as_str()) } else { diff --git a/tracing-flame/src/lib.rs b/tracing-flame/src/lib.rs index 33d86b8d9e..654c0fbdba 100644 --- a/tracing-flame/src/lib.rs +++ b/tracing-flame/src/lib.rs @@ -394,12 +394,10 @@ where let first = ctx.span(id).expect("expected: span id exists in registry"); - if !self.config.empty_samples && first.from_root().count() == 0 { + if !self.config.empty_samples && first.parent().is_none() { return; } - let parents = first.from_root(); - let mut stack = String::new(); if !self.config.threads_collapsed { @@ -408,9 +406,12 @@ where stack += "all-threads"; } - for parent in parents { - stack += "; "; - write(&mut stack, parent, &self.config).expect("expected: write to String never fails"); + if let Some(second) = first.parent() { + for parent in second.scope().from_root() { + stack += "; "; + write(&mut stack, parent, &self.config) + .expect("expected: write to String never fails"); + } } write!(&mut stack, " {}", samples.as_nanos()) @@ -440,7 +441,6 @@ where let samples = self.time_since_last_event(); let first = expect!(ctx.span(&id), "expected: span id exists in registry"); - let parents = first.from_root(); let mut stack = String::new(); if !self.config.threads_collapsed { @@ -448,20 +448,15 @@ where } else { stack += "all-threads"; } - stack += "; "; - for parent in parents { + for parent in first.scope().from_root() { + stack += "; "; expect!( write(&mut stack, parent, &self.config), "expected: write to String never fails" ); - stack += "; "; } - expect!( - write(&mut stack, first, &self.config), - "expected: write to String never fails" - ); expect!( write!(&mut stack, " {}", samples.as_nanos()), "expected: write to String never fails" diff --git a/tracing-journald/src/lib.rs b/tracing-journald/src/lib.rs index 5278974f57..b40d946a3c 100644 --- a/tracing-journald/src/lib.rs +++ b/tracing-journald/src/lib.rs @@ -126,7 +126,7 @@ where let span = ctx.span(id).expect("unknown span"); let mut buf = Vec::with_capacity(256); - let depth = span.parents().count(); + let depth = span.scope().skip(1).count(); writeln!(buf, "S{}_NAME", depth).unwrap(); put_value(&mut buf, span.name().as_bytes()); @@ -143,7 +143,7 @@ where fn on_record(&self, id: &Id, values: &Record, ctx: Context) { let span = ctx.span(id).expect("unknown span"); - let depth = span.parents().count(); + let depth = span.scope().skip(1).count(); let mut exts = span.extensions_mut(); let buf = &mut exts.get_mut::().expect("missing fields").0; values.record(&mut SpanVisitor { @@ -157,7 +157,11 @@ where let mut buf = Vec::with_capacity(256); // Record span fields - for span in ctx.scope() { + for span in ctx + .lookup_current() + .into_iter() + .flat_map(|span| span.scope().from_root()) + { let exts = span.extensions(); let fields = exts.get::().expect("missing fields"); buf.extend_from_slice(&fields.0); diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs index 58a8356c8b..8ea6c5c85e 100644 --- a/tracing-subscriber/src/filter/env/mod.rs +++ b/tracing-subscriber/src/filter/env/mod.rs @@ -267,7 +267,7 @@ impl EnvFilter { let bold = Style::new().bold(); let mut warning = Color::Yellow.paint("warning"); warning.style_ref_mut().is_bold = true; - format!("{}{} {}", warning, bold.clone().paint(":"), bold.paint(msg)) + format!("{}{} {}", warning, bold.paint(":"), bold.paint(msg)) }; eprintln!("{}", msg); }; @@ -304,7 +304,6 @@ impl EnvFilter { }; let level = directive .level - .clone() .into_level() .expect("=off would not have enabled any filters"); ctx(&format!( @@ -392,8 +391,8 @@ impl Subscribe for EnvFilter { return Some(LevelFilter::TRACE); } std::cmp::max( - self.statics.max_level.clone().into(), - self.dynamics.max_level.clone().into(), + self.statics.max_level.into(), + self.dynamics.max_level.into(), ) } diff --git a/tracing-subscriber/src/filter/level.rs b/tracing-subscriber/src/filter/level.rs index 4cd679d1ae..426faa7f8e 100644 --- a/tracing-subscriber/src/filter/level.rs +++ b/tracing-subscriber/src/filter/level.rs @@ -22,6 +22,6 @@ impl crate::Subscribe for LevelFilter { } fn max_level_hint(&self) -> Option { - self.clone().into() + Some(*self) } } diff --git a/tracing-subscriber/src/fmt/fmt_subscriber.rs b/tracing-subscriber/src/fmt/fmt_subscriber.rs index ba501bc9f5..f6e71f8077 100644 --- a/tracing-subscriber/src/fmt/fmt_subscriber.rs +++ b/tracing-subscriber/src/fmt/fmt_subscriber.rs @@ -2,7 +2,7 @@ use crate::{ field::RecordFields, fmt::{format, FormatEvent, FormatFields, MakeWriter, TestWriter}, registry::{LookupSpan, SpanRef}, - subscribe::{self, Context, Scope}, + subscribe::{self, Context}, }; use format::{FmtSpan, TimingDisplay}; use std::{ @@ -756,8 +756,10 @@ where F: FnMut(&SpanRef<'_, S>) -> Result<(), E>, { // visit all the current spans - for span in self.ctx.scope() { - f(&span)?; + if let Some(leaf) = self.ctx.lookup_current() { + for span in leaf.scope().from_root() { + f(&span)?; + } } Ok(()) } @@ -816,7 +818,12 @@ where /// the current span. /// /// [stored data]: SpanRef - pub fn scope(&self) -> Scope<'_, S> + #[deprecated( + note = "wraps layer::Context::scope, which is deprecated", + since = "0.2.19" + )] + #[allow(deprecated)] + pub fn scope(&self) -> crate::subscribe::Scope<'_, S> where S: for<'lookup> LookupSpan<'lookup>, { diff --git a/tracing-subscriber/src/fmt/format/json.rs b/tracing-subscriber/src/fmt/format/json.rs index 5e39a85830..d00a0b0204 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/tracing-subscriber/src/fmt/format/json.rs @@ -95,8 +95,10 @@ where use serde::ser::SerializeSeq; let mut serializer = serializer_o.serialize_seq(None)?; - for span in self.0.scope() { - serializer.serialize_element(&SerializableSpan(&span, self.1))?; + if let Some(leaf_span) = self.0.lookup_current() { + for span in leaf_span.scope().from_root() { + serializer.serialize_element(&SerializableSpan(&span, self.1))?; + } } serializer.end() diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index bb0c15d5fd..bcd827e316 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -9,7 +9,6 @@ use crate::{ use std::{ fmt::{self, Write}, - iter, marker::PhantomData, }; use tracing_core::{ @@ -650,10 +649,7 @@ where .and_then(|id| ctx.ctx.span(&id)) .or_else(|| ctx.ctx.lookup_current()); - let scope = span.into_iter().flat_map(|span| { - let parents = span.parents(); - iter::once(span).chain(parents) - }); + let scope = span.into_iter().flat_map(|span| span.scope()); #[cfg(feature = "ansi")] let dimmed = if self.ansi { Style::new().dimmed() @@ -876,9 +872,7 @@ where .and_then(|id| self.ctx.ctx.span(&id)) .or_else(|| self.ctx.ctx.lookup_current()); - let scope = span - .into_iter() - .flat_map(|span| span.from_root().chain(iter::once(span))); + let scope = span.into_iter().flat_map(|span| span.scope().from_root()); for span in scope { write!(f, "{}", bold.paint(span.metadata().name()))?; @@ -1414,27 +1408,27 @@ pub(super) mod test { #[test] fn fmt_span_combinations() { let f = FmtSpan::NONE; - assert_eq!(f.contains(FmtSpan::NEW), false); - assert_eq!(f.contains(FmtSpan::ENTER), false); - assert_eq!(f.contains(FmtSpan::EXIT), false); - assert_eq!(f.contains(FmtSpan::CLOSE), false); + assert!(!f.contains(FmtSpan::NEW)); + assert!(!f.contains(FmtSpan::ENTER)); + assert!(!f.contains(FmtSpan::EXIT)); + assert!(!f.contains(FmtSpan::CLOSE)); let f = FmtSpan::ACTIVE; - assert_eq!(f.contains(FmtSpan::NEW), false); - assert_eq!(f.contains(FmtSpan::ENTER), true); - assert_eq!(f.contains(FmtSpan::EXIT), true); - assert_eq!(f.contains(FmtSpan::CLOSE), false); + assert!(!f.contains(FmtSpan::NEW)); + assert!(f.contains(FmtSpan::ENTER)); + assert!(f.contains(FmtSpan::EXIT)); + assert!(!f.contains(FmtSpan::CLOSE)); let f = FmtSpan::FULL; - assert_eq!(f.contains(FmtSpan::NEW), true); - assert_eq!(f.contains(FmtSpan::ENTER), true); - assert_eq!(f.contains(FmtSpan::EXIT), true); - assert_eq!(f.contains(FmtSpan::CLOSE), true); + assert!(f.contains(FmtSpan::NEW)); + assert!(f.contains(FmtSpan::ENTER)); + assert!(f.contains(FmtSpan::EXIT)); + assert!(f.contains(FmtSpan::CLOSE)); let f = FmtSpan::NEW | FmtSpan::CLOSE; - assert_eq!(f.contains(FmtSpan::NEW), true); - assert_eq!(f.contains(FmtSpan::ENTER), false); - assert_eq!(f.contains(FmtSpan::EXIT), false); - assert_eq!(f.contains(FmtSpan::CLOSE), true); + assert!(f.contains(FmtSpan::NEW)); + assert!(!f.contains(FmtSpan::ENTER)); + assert!(!f.contains(FmtSpan::EXIT)); + assert!(f.contains(FmtSpan::CLOSE)); } } diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 724116b2b5..586f0f3933 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -5,10 +5,7 @@ use crate::{ registry::LookupSpan, }; -use std::{ - fmt::{self, Write}, - iter, -}; +use std::fmt::{self, Write}; use tracing_core::{ field::{self, Field}, Collect, Event, Level, @@ -186,10 +183,7 @@ where .and_then(|id| ctx.span(&id)) .or_else(|| ctx.lookup_current()); - let scope = span.into_iter().flat_map(|span| { - let parents = span.parents(); - iter::once(span).chain(parents) - }); + let scope = span.into_iter().flat_map(|span| span.scope()); for span in scope { let meta = span.metadata(); diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index eefdd3d98d..d572807649 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -58,6 +58,8 @@ //! [`Collect`]: tracing_core::collect::Collect //! [ctx]: crate::subscribe::Context //! [lookup]: crate::subscribe::Context::span() +use std::fmt::Debug; + use tracing_core::{field::FieldSet, span::Id, Metadata}; /// A module containing a type map of span extensions. @@ -164,14 +166,112 @@ pub struct SpanRef<'a, R: LookupSpan<'a>> { data: R::Data, } +/// An iterator over the parents of a span, ordered from leaf to root. +/// +/// This is returned by the [`SpanRef::scope`] method. +#[derive(Debug)] +pub struct Scope<'a, R> { + registry: &'a R, + next: Option, +} + +impl<'a, R> Scope<'a, R> +where + R: LookupSpan<'a>, +{ + /// Flips the order of the iterator, so that it is ordered from root to leaf. + /// + /// The iterator will first return the root span, then that span's immediate child, + /// and so on until it finally returns the span that [`SpanRef::scope`] was called on. + /// + /// If any items were consumed from the [`Scope`] before calling this method then they + /// will *not* be returned from the [`ScopeFromRoot`]. + /// + /// **Note**: this will allocate if there are many spans remaining, or if the + /// "smallvec" feature flag is not enabled. + #[allow(clippy::wrong_self_convention)] + pub fn from_root(self) -> ScopeFromRoot<'a, R> { + #[cfg(feature = "smallvec")] + type Buf = smallvec::SmallVec; + #[cfg(not(feature = "smallvec"))] + type Buf = Vec; + ScopeFromRoot { + spans: self.collect::>().into_iter().rev(), + } + } +} + +impl<'a, R> Iterator for Scope<'a, R> +where + R: LookupSpan<'a>, +{ + type Item = SpanRef<'a, R>; + + fn next(&mut self) -> Option { + let curr = self.registry.span(self.next.as_ref()?)?; + self.next = curr.parent_id().cloned(); + Some(curr) + } +} + +/// An iterator over the parents of a span, ordered from root to leaf. +/// +/// This is returned by the [`Scope::from_root`] method. +pub struct ScopeFromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + #[cfg(feature = "smallvec")] + spans: std::iter::Rev>>, + #[cfg(not(feature = "smallvec"))] + spans: std::iter::Rev>>, +} + +impl<'a, R> Iterator for ScopeFromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + type Item = SpanRef<'a, R>; + + #[inline] + fn next(&mut self) -> Option { + self.spans.next() + } + + #[inline] + fn size_hint(&self) -> (usize, Option) { + self.spans.size_hint() + } +} + +impl<'a, R> Debug for ScopeFromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.pad("ScopeFromRoot { .. }") + } +} + /// An iterator over the parents of a span. /// /// This is returned by the [`SpanRef::parents`] method. /// +/// [`SpanRef::parents`]: struct.SpanRef.html#method.parents +#[deprecated(note = "replaced by `Scope`")] #[derive(Debug)] -pub struct Parents<'a, R> { - registry: &'a R, - next: Option, +pub struct Parents<'a, R>(Scope<'a, R>); + +#[allow(deprecated)] +impl<'a, R> Iterator for Parents<'a, R> +where + R: LookupSpan<'a>, +{ + type Item = SpanRef<'a, R>; + + fn next(&mut self) -> Option { + self.0.next() + } } /// An iterator over a span's parents, starting with the root of the trace @@ -179,12 +279,23 @@ pub struct Parents<'a, R> { /// /// For additional details, see [`SpanRef::from_root`]. /// -/// [`Span::from_root`]: SpanRef::from_root() -pub struct FromRoot<'a, R: LookupSpan<'a>> { - #[cfg(feature = "smallvec")] - inner: std::iter::Rev>>, - #[cfg(not(feature = "smallvec"))] - inner: std::iter::Rev>>, +/// [`Span::from_root`]: struct.SpanRef.html#method.from_root +#[deprecated(note = "replaced by `ScopeFromRoot`", since = "0.2.19")] +#[derive(Debug)] +pub struct FromRoot<'a, R>(ScopeFromRoot<'a, R>) +where + R: LookupSpan<'a>; + +#[allow(deprecated)] +impl<'a, R> Iterator for FromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + type Item = SpanRef<'a, R>; + + fn next(&mut self) -> Option { + self.0.next() + } } #[cfg(feature = "smallvec")] @@ -233,17 +344,95 @@ where }) } + /// Returns an iterator over all parents of this span, starting with this span, + /// ordered from leaf to root. + /// + /// The iterator will first return the span, then the span's immediate parent, + /// followed by that span's parent, and so on, until it reaches a root span. + /// + /// ```rust + /// use tracing::{span, Collect}; + /// use tracing_subscriber::{ + /// subscribe::{Context, Subscribe}, + /// prelude::*, + /// registry::LookupSpan, + /// }; + /// + /// struct PrintingSubscriber; + /// impl Subscribe for PrintingSubscriber + /// where + /// C: Collect + for<'lookup> LookupSpan<'lookup>, + /// { + /// fn on_enter(&self, id: &span::Id, ctx: Context) { + /// let span = ctx.span(id).unwrap(); + /// let scope = span.scope().map(|span| span.name()).collect::>(); + /// println!("Entering span: {:?}", scope); + /// } + /// } + /// + /// tracing::collect::with_default(tracing_subscriber::registry().with(PrintingSubscriber), || { + /// let _root = tracing::info_span!("root").entered(); + /// // Prints: Entering span: ["root"] + /// let _child = tracing::info_span!("child").entered(); + /// // Prints: Entering span: ["child", "root"] + /// let _leaf = tracing::info_span!("leaf").entered(); + /// // Prints: Entering span: ["leaf", "child", "root"] + /// }); + /// ``` + /// + /// If the opposite order (from the root to this span) is desired, calling [`Scope::from_root`] on + /// the returned iterator reverses the order. + /// + /// ```rust + /// # use tracing::{span, Collect}; + /// # use tracing_subscriber::{ + /// # subscribe::{Context, Subscribe}, + /// # prelude::*, + /// # registry::LookupSpan, + /// # }; + /// # struct PrintingSubscriber; + /// impl Subscribe for PrintingSubscriber + /// where + /// C: Collect + for<'lookup> LookupSpan<'lookup>, + /// { + /// fn on_enter(&self, id: &span::Id, ctx: Context) { + /// let span = ctx.span(id).unwrap(); + /// let scope = span.scope().from_root().map(|span| span.name()).collect::>(); + /// println!("Entering span: {:?}", scope); + /// } + /// } + /// + /// tracing::collect::with_default(tracing_subscriber::registry().with(PrintingSubscriber), || { + /// let _root = tracing::info_span!("root").entered(); + /// // Prints: Entering span: ["root"] + /// let _child = tracing::info_span!("child").entered(); + /// // Prints: Entering span: ["root", "child"] + /// let _leaf = tracing::info_span!("leaf").entered(); + /// // Prints: Entering span: ["root", "child", "leaf"] + /// }); + /// ``` + pub fn scope(&self) -> Scope<'a, R> { + Scope { + registry: self.registry, + next: Some(self.id()), + } + } + /// Returns an iterator over all parents of this span, starting with the /// immediate parent. /// /// The iterator will first return the span's immediate parent, followed by /// that span's parent, followed by _that_ span's parent, and so on, until a /// it reaches a root span. + #[deprecated( + note = "equivalent to `self.parent().into_iter().flat_map(SpanRef::scope)`, but consider whether excluding `self` is actually intended" + )] + #[allow(deprecated)] pub fn parents(&self) -> Parents<'a, R> { - Parents { + Parents(Scope { registry: self.registry, - next: self.parent().map(|parent| parent.id()), - } + next: self.parent_id().cloned(), + }) } /// Returns an iterator over all parents of this span, starting with the @@ -253,20 +442,15 @@ where /// next span, and then the next, until this span's immediate parent is /// returned. /// - /// **Note**: if the "smallvec" feature flag is not enabled, this may - /// allocate. + /// **Note**: this will allocate if there are many spans remaining, or if the + /// "smallvec" feature flag is not enabled. + #[deprecated( + note = "equivalent to `self.parent().into_iter().flat_map(|span| span.scope().from_root())`, but consider whether excluding `self` is actually intended", + since = "0.2.19" + )] + #[allow(deprecated)] pub fn from_root(&self) -> FromRoot<'a, R> { - #[cfg(feature = "smallvec")] - type SpanRefVec<'span, L> = smallvec::SmallVec>; - #[cfg(not(feature = "smallvec"))] - type SpanRefVec<'span, L> = Vec>; - - // an alternative way to handle this would be to the recursive approach that - // `fmt` uses that _does not_ entail any allocation in this fmt'ing - // spans path. - let parents = self.parents().collect::>(); - let inner = parents.into_iter().rev(); - FromRoot { inner } + FromRoot(self.parents().0.from_root()) } /// Returns a reference to this span's `Extensions`. @@ -286,43 +470,87 @@ where } } -impl<'a, R> Iterator for Parents<'a, R> -where - R: LookupSpan<'a>, -{ - type Item = SpanRef<'a, R>; - fn next(&mut self) -> Option { - let id = self.next.take()?; - let span = self.registry.span(&id)?; - self.next = span.parent().map(|parent| parent.id()); - Some(span) - } -} - -// === impl FromRoot === +#[cfg(test)] +mod tests { + use crate::{ + prelude::*, + registry::LookupSpan, + subscribe::{Context, Subscribe}, + }; + use std::sync::{Arc, Mutex}; + use tracing::{span, Collect}; + + #[test] + fn spanref_scope_iteration_order() { + let last_entered_scope = Arc::new(Mutex::new(Vec::new())); + + #[derive(Default)] + struct RecordingSubscriber { + last_entered_scope: Arc>>, + } -impl<'span, R> Iterator for FromRoot<'span, R> -where - R: LookupSpan<'span>, -{ - type Item = SpanRef<'span, R>; + impl Subscribe for RecordingSubscriber + where + S: Collect + for<'lookup> LookupSpan<'lookup>, + { + fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { + let span = ctx.span(id).unwrap(); + let scope = span.scope().map(|span| span.name()).collect::>(); + *self.last_entered_scope.lock().unwrap() = scope; + } + } - #[inline] - fn next(&mut self) -> Option { - self.inner.next() + let _guard = tracing::collect::set_default(crate::registry().with(RecordingSubscriber { + last_entered_scope: last_entered_scope.clone(), + })); + + let _root = tracing::info_span!("root").entered(); + assert_eq!(&*last_entered_scope.lock().unwrap(), &["root"]); + let _child = tracing::info_span!("child").entered(); + assert_eq!(&*last_entered_scope.lock().unwrap(), &["child", "root"]); + let _leaf = tracing::info_span!("leaf").entered(); + assert_eq!( + &*last_entered_scope.lock().unwrap(), + &["leaf", "child", "root"] + ); } - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } -} + #[test] + fn spanref_scope_fromroot_iteration_order() { + let last_entered_scope = Arc::new(Mutex::new(Vec::new())); -impl<'span, R> std::fmt::Debug for FromRoot<'span, R> -where - R: LookupSpan<'span>, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.pad("FromRoot { .. }") + #[derive(Default)] + struct RecordingSubscriber { + last_entered_scope: Arc>>, + } + + impl Subscribe for RecordingSubscriber + where + S: Collect + for<'lookup> LookupSpan<'lookup>, + { + fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { + let span = ctx.span(id).unwrap(); + let scope = span + .scope() + .from_root() + .map(|span| span.name()) + .collect::>(); + *self.last_entered_scope.lock().unwrap() = scope; + } + } + + let _guard = tracing::collect::set_default(crate::registry().with(RecordingSubscriber { + last_entered_scope: last_entered_scope.clone(), + })); + + let _root = tracing::info_span!("root").entered(); + assert_eq!(&*last_entered_scope.lock().unwrap(), &["root"]); + let _child = tracing::info_span!("child").entered(); + assert_eq!(&*last_entered_scope.lock().unwrap(), &["root", "child",]); + let _leaf = tracing::info_span!("leaf").entered(); + assert_eq!( + &*last_entered_scope.lock().unwrap(), + &["root", "child", "leaf"] + ); } } diff --git a/tracing-subscriber/src/subscribe.rs b/tracing-subscriber/src/subscribe.rs index 48fa2f83c2..4ec1f7eda6 100644 --- a/tracing-subscriber/src/subscribe.rs +++ b/tracing-subscriber/src/subscribe.rs @@ -567,9 +567,24 @@ pub struct Identity { /// [stored data]: super::registry::SpanRef #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -pub struct Scope<'a, L: LookupSpan<'a>>( - Option, std::iter::Once>>>, -); +#[deprecated(note = "renamed to crate::registry::ScopeFromRoot", since = "0.2.19")] +#[derive(Debug)] +pub struct Scope<'a, L>(std::iter::Flatten>>) +where + L: LookupSpan<'a>; + +#[cfg(feature = "registry")] +#[allow(deprecated)] +impl<'a, L> Iterator for Scope<'a, L> +where + L: LookupSpan<'a>, +{ + type Item = SpanRef<'a, L>; + + fn next(&mut self) -> Option { + self.0.next() + } +} // === impl Layered === @@ -1098,15 +1113,59 @@ where /// [stored data]: super::registry::SpanRef #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] + #[deprecated( + note = "equivalent to `self.current_span().id().and_then(|id| self.span_scope(id).from_root())` but consider passing an explicit ID instead of relying on the contextual span", + since = "0.2.19" + )] + #[allow(deprecated)] pub fn scope(&self) -> Scope<'_, C> where C: for<'lookup> registry::LookupSpan<'lookup>, { - let scope = self.lookup_current().map(|span| { - let parents = span.from_root(); - parents.chain(std::iter::once(span)) - }); - Scope(scope) + Scope( + self.lookup_current() + .as_ref() + .map(registry::SpanRef::scope) + .map(registry::Scope::from_root) + .into_iter() + .flatten(), + ) + } + + /// Returns an iterator over the [stored data] for all the spans in the + /// current context, starting with the specified span and ending with the + /// root of the trace tree and ending with the current span. + /// + ///
+ ///
ⓘNote
+ ///
+ ///
+ /// 
+    /// Note: Compared to scope this
+    /// returns the spans in reverse order (from leaf to root). Use
+    /// Scope::from_root
+    /// in case root-to-leaf ordering is desired.
+    ///
+ /// + ///
+ ///
ⓘNote
+ ///
+ ///
+ /// 
+    /// Note: This requires the wrapped subscriber to implement the
+    /// LookupSpan trait.
+    /// See the documentation on Context's
+    /// declaration for details.
+    ///
+ /// + /// [stored data]: ../registry/struct.SpanRef.html + #[cfg(feature = "registry")] + #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] + pub fn span_scope(&self, id: &span::Id) -> Option> + where + C: for<'lookup> registry::LookupSpan<'lookup>, + { + Some(self.span(id)?.scope()) } } @@ -1135,27 +1194,6 @@ impl Identity { } } -// === impl Scope === - -#[cfg(feature = "registry")] -#[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -impl<'a, L: LookupSpan<'a>> Iterator for Scope<'a, L> { - type Item = SpanRef<'a, L>; - - #[inline] - fn next(&mut self) -> Option { - self.0.as_mut()?.next() - } -} - -#[cfg(feature = "registry")] -#[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -impl<'a, L: LookupSpan<'a>> std::fmt::Debug for Scope<'a, L> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.pad("Scope { .. }") - } -} - #[cfg(test)] pub(crate) mod tests { use super::*;