From 0e45e8375273113bd89657929991c3220c868c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Jun 2021 23:12:50 +0200 Subject: [PATCH 01/30] Implement SpanRef::scope and SpanRef::scope.from_root iterators See #1429 --- tracing-subscriber/src/registry/mod.rs | 68 ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index cc84e5fb5c..fcd0744a53 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -171,6 +171,62 @@ 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 SpanScope<'a, R> { + registry: &'a R, + next: Option, +} + +impl<'a, R> SpanScope<'a, R> +where + R: LookupSpan<'a>, +{ + /// Flips the order of the iterator, so that it is ordered from root to leaf. + #[allow(clippy::wrong_self_convention)] + pub fn from_root(self) -> SpanScopeFromRoot<'a, R> { + SpanScopeFromRoot { + spans: self.collect::>().into_iter().rev(), + } + } +} + +impl<'a, R> Iterator for SpanScope<'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 [`SpanScope::from_root`] method. +pub struct SpanScopeFromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + spans: std::iter::Rev>>, +} + +impl<'a, R> Iterator for SpanScopeFromRoot<'a, R> +where + R: LookupSpan<'a>, +{ + type Item = SpanRef<'a, R>; + + fn next(&mut self) -> Option { + self.spans.next() + } +} + /// An iterator over the parents of a span. /// /// This is returned by the [`SpanRef::parents`] method. @@ -241,6 +297,18 @@ 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. + pub fn scope(&self) -> SpanScope<'a, R> { + SpanScope { + registry: self.registry, + next: Some(self.id()), + } + } + /// Returns an iterator over all parents of this span, starting with the /// immediate parent. /// From 5c379fb31c45793200eb5f62b34ef934266a347a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Wed, 9 Jun 2021 23:29:24 +0200 Subject: [PATCH 02/30] Implement smallvec optimization for SpanScopeFromRoot --- tracing-subscriber/src/registry/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index fcd0744a53..6cac0918ec 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -187,8 +187,12 @@ where /// Flips the order of the iterator, so that it is ordered from root to leaf. #[allow(clippy::wrong_self_convention)] pub fn from_root(self) -> SpanScopeFromRoot<'a, R> { + #[cfg(feature = "smallvec")] + type Buf = smallvec::SmallVec; + #[cfg(not(feature = "smallvec"))] + type Buf = Vec; SpanScopeFromRoot { - spans: self.collect::>().into_iter().rev(), + spans: self.collect::>().into_iter().rev(), } } } @@ -213,6 +217,9 @@ pub struct SpanScopeFromRoot<'a, R> where R: LookupSpan<'a>, { + #[cfg(feature = "smallvec")] + spans: std::iter::Rev>>, + #[cfg(not(feature = "smallvec"))] spans: std::iter::Rev>>, } From cd8148fe035f0731bbda1324c80c47d1cc0cb770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 00:12:46 +0200 Subject: [PATCH 03/30] Replace legacy span iterator implementations with SpanRef::scope --- tracing-subscriber/src/layer.rs | 43 +++------ tracing-subscriber/src/registry/mod.rs | 122 +++++++++---------------- 2 files changed, 58 insertions(+), 107 deletions(-) diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index ffdd89c10c..7fe7bc1adb 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -7,7 +7,7 @@ use tracing_core::{ }; #[cfg(feature = "registry")] -use crate::registry::{self, LookupSpan, Registry, SpanRef}; +use crate::registry::{self, LookupSpan, Registry}; use std::{any::TypeId, marker::PhantomData}; /// A composable handler for `tracing` events. @@ -586,9 +586,8 @@ pub struct Identity { /// [`Context::scope`]: struct.Context.html#method.scope #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -pub struct Scope<'a, L: LookupSpan<'a>>( - Option, std::iter::Once>>>, -); +#[deprecated(note = "moved to crate::registry::ScopeFromRoot")] +pub type Scope<'a, L> = std::iter::Flatten>>; // === impl Layered === @@ -1114,15 +1113,20 @@ where /// [stored data]: ../registry/struct.SpanRef.html #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] + #[deprecated( + note = "equivalent to self.lookup_current().into_iter().flat_map(|span| span.scope().from_root()), but consider whether lookup_current is a bug" + )] + #[allow(deprecated)] pub fn scope(&self) -> Scope<'_, S> where S: 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) + self.lookup_current() + .as_ref() + .map(registry::SpanRef::scope) + .map(registry::Scope::from_root) + .into_iter() + .flatten() } } @@ -1151,27 +1155,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::*; diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 6cac0918ec..25cfc0999a 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -62,6 +62,8 @@ //! [lookup]: ../layer/struct.Context.html#method.span //! [`LookupSpan`]: trait.LookupSpan.html //! [`SpanData`]: trait.SpanData.html +use std::fmt::Debug; + use tracing_core::{field::FieldSet, span::Id, Metadata}; /// A module containing a type map of span extensions. @@ -175,29 +177,31 @@ pub struct SpanRef<'a, R: LookupSpan<'a>> { /// /// This is returned by the [`SpanRef::scope`] method. #[derive(Debug)] -pub struct SpanScope<'a, R> { +pub struct Scope<'a, R> { registry: &'a R, next: Option, } -impl<'a, R> SpanScope<'a, R> +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. + + /// **Note**: if the "smallvec" feature flag is not enabled, this may allocate. #[allow(clippy::wrong_self_convention)] - pub fn from_root(self) -> SpanScopeFromRoot<'a, R> { + pub fn from_root(self) -> ScopeFromRoot<'a, R> { #[cfg(feature = "smallvec")] type Buf = smallvec::SmallVec; #[cfg(not(feature = "smallvec"))] type Buf = Vec; - SpanScopeFromRoot { + ScopeFromRoot { spans: self.collect::>().into_iter().rev(), } } } -impl<'a, R> Iterator for SpanScope<'a, R> +impl<'a, R> Iterator for Scope<'a, R> where R: LookupSpan<'a>, { @@ -212,8 +216,8 @@ where /// An iterator over the parents of a span, ordered from root to leaf. /// -/// This is returned by the [`SpanScope::from_root`] method. -pub struct SpanScopeFromRoot<'a, R> +/// This is returned by the [`Scope::from_root`] method. +pub struct ScopeFromRoot<'a, R> where R: LookupSpan<'a>, { @@ -223,15 +227,30 @@ where spans: std::iter::Rev>>, } -impl<'a, R> Iterator for SpanScopeFromRoot<'a, R> +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. @@ -239,11 +258,8 @@ where /// This is returned by the [`SpanRef::parents`] method. /// /// [`SpanRef::parents`]: struct.SpanRef.html#method.parents -#[derive(Debug)] -pub struct Parents<'a, R> { - registry: &'a R, - next: Option, -} +#[deprecated(note = "replaced by Scope")] +pub type Parents<'a, R> = Scope<'a, R>; /// An iterator over a span's parents, starting with the root of the trace /// tree. @@ -251,12 +267,8 @@ pub struct Parents<'a, R> { /// For additonal details, see [`SpanRef::from_root`]. /// /// [`Span::from_root`]: struct.SpanRef.html#method.from_root -pub struct FromRoot<'a, R: LookupSpan<'a>> { - #[cfg(feature = "smallvec")] - inner: std::iter::Rev>>, - #[cfg(not(feature = "smallvec"))] - inner: std::iter::Rev>>, -} +#[deprecated(note = "replaced by ScopeFromRoot")] +pub type FromRoot<'a, R> = ScopeFromRoot<'a, R>; #[cfg(feature = "smallvec")] type SpanRefVecArray<'span, L> = [SpanRef<'span, L>; 16]; @@ -309,8 +321,8 @@ where /// /// 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. - pub fn scope(&self) -> SpanScope<'a, R> { - SpanScope { + pub fn scope(&self) -> Scope<'a, R> { + Scope { registry: self.registry, next: Some(self.id()), } @@ -322,11 +334,14 @@ where /// 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.scope().skip(1), but consider whether the skip is actually intended" + )] + #[allow(deprecated)] pub fn parents(&self) -> Parents<'a, R> { - Parents { - registry: self.registry, - next: self.parent().map(|parent| parent.id()), - } + let mut scope = self.scope(); + scope.next(); + scope } /// Returns an iterator over all parents of this span, starting with the @@ -338,18 +353,12 @@ where /// /// **Note**: if the "smallvec" feature flag is not enabled, this may /// allocate. + #[deprecated( + note = "equivalent to self.scope().skip(1).from_root(), but consider whether the skip is actually intended" + )] + #[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 } + self.parents().from_root() } /// Returns a reference to this span's `Extensions`. @@ -368,44 +377,3 @@ where self.data.extensions_mut() } } - -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 === - -impl<'span, R> Iterator for FromRoot<'span, R> -where - R: LookupSpan<'span>, -{ - type Item = SpanRef<'span, R>; - - #[inline] - fn next(&mut self) -> Option { - self.inner.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() - } -} - -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 { .. }") - } -} From 84afb38406e769b69ba2b798abfa517e078ccb93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 00:24:20 +0200 Subject: [PATCH 04/30] Migrate tracing-subscriber to SpanRef::scope --- tracing-subscriber/src/fmt/fmt_layer.rs | 12 ++++++++---- tracing-subscriber/src/fmt/format/json.rs | 6 ++++-- tracing-subscriber/src/fmt/format/mod.rs | 13 +++---------- tracing-subscriber/src/fmt/format/pretty.rs | 10 ++-------- 4 files changed, 17 insertions(+), 24 deletions(-) diff --git a/tracing-subscriber/src/fmt/fmt_layer.rs b/tracing-subscriber/src/fmt/fmt_layer.rs index eeddabd471..451d6e6685 100644 --- a/tracing-subscriber/src/fmt/fmt_layer.rs +++ b/tracing-subscriber/src/fmt/fmt_layer.rs @@ -1,7 +1,7 @@ use crate::{ field::RecordFields, fmt::{format, FormatEvent, FormatFields, MakeWriter, TestWriter}, - layer::{self, Context, Scope}, + layer::{self, Context}, registry::{LookupSpan, SpanRef}, }; use format::{FmtSpan, TimingDisplay}; @@ -804,8 +804,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(()) } @@ -864,7 +866,9 @@ where /// the current span. /// /// [stored data]: ../registry/struct.SpanRef.html - pub fn scope(&self) -> Scope<'_, S> + #[deprecated(note = "wraps crate::layer::Context::scope")] + #[allow(deprecated)] + pub fn scope(&self) -> crate::layer::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 5cc522f6a4..c2a0ced8f4 100644 --- a/tracing-subscriber/src/fmt/format/json.rs +++ b/tracing-subscriber/src/fmt/format/json.rs @@ -98,8 +98,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 3950847cb8..b07bba2d15 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -7,10 +7,7 @@ use crate::{ registry::LookupSpan, }; -use std::{ - fmt::{self, Write}, - iter, -}; +use std::fmt::{self, Write}; use tracing_core::{ field::{self, Field, Visit}, span, Event, Level, Subscriber, @@ -861,9 +858,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 { seen = true; @@ -933,9 +928,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()))?; diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index d98cef813d..ab1180823f 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}, Event, Level, Subscriber, @@ -187,10 +184,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(); From 9ec8130eb7a67504ce531d124bff28b464d92fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 00:39:42 +0200 Subject: [PATCH 05/30] Migrate layer crates to use SpanRef::scope --- tracing-error/src/layer.rs | 3 +-- tracing-flame/src/lib.rs | 20 ++++++++------------ tracing-journald/src/lib.rs | 10 +++++++--- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/tracing-error/src/layer.rs b/tracing-error/src/layer.rs index 05e50c7d86..911d567414 100644 --- a/tracing-error/src/layer.rs +++ b/tracing-error/src/layer.rs @@ -90,8 +90,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().from_root() { 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 e307d09e9b..6f7523715a 100644 --- a/tracing-flame/src/lib.rs +++ b/tracing-flame/src/lib.rs @@ -398,12 +398,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 { @@ -412,7 +410,11 @@ where stack += "all-threads"; } - for parent in parents { + let mut parents = first.scope(); + parents + .next() + .expect("expected: scope begins with leaf scope"); + for parent in parents.from_root() { stack += "; "; write(&mut stack, parent, &self.config).expect("expected: write to String never fails"); } @@ -444,7 +446,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 { @@ -452,20 +453,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 8b3cfaed56..db3b78ccf3 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()) + { let exts = span.extensions(); let fields = exts.get::().expect("missing fields"); buf.extend_from_slice(&fields.0); From d12cd5142f8e2e4cbfa458dee267b29fe6337626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 00:46:28 +0200 Subject: [PATCH 06/30] Reverted tracing-error span iteration order Oops --- tracing-error/src/layer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-error/src/layer.rs b/tracing-error/src/layer.rs index 911d567414..d2ddf65b2f 100644 --- a/tracing-error/src/layer.rs +++ b/tracing-error/src/layer.rs @@ -90,7 +90,7 @@ where let span = subscriber .span(id) .expect("registry should have a span for the current ID"); - for span in span.scope().from_root() { + for span in span.scope() { let cont = if let Some(fields) = span.extensions().get::>() { f(span.metadata(), fields.fields.as_str()) } else { From 4d0fd19144275f6b81d378e73853496964910be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 00:49:46 +0200 Subject: [PATCH 07/30] Fixed flipped scope order in tracing-journald --- tracing-journald/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-journald/src/lib.rs b/tracing-journald/src/lib.rs index db3b78ccf3..395b067dce 100644 --- a/tracing-journald/src/lib.rs +++ b/tracing-journald/src/lib.rs @@ -160,7 +160,7 @@ where for span in ctx .lookup_current() .into_iter() - .flat_map(|span| span.scope()) + .flat_map(|span| span.scope().from_root()) { let exts = span.extensions(); let fields = exts.get::().expect("missing fields"); From 8d162d4d0d496291c19a2e12777296a46cd6d1e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 22:30:59 +0200 Subject: [PATCH 08/30] Bump tracing-subscriber version --- tracing-error/Cargo.toml | 2 +- tracing-flame/Cargo.toml | 2 +- tracing-journald/Cargo.toml | 2 +- tracing-subscriber/Cargo.toml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tracing-error/Cargo.toml b/tracing-error/Cargo.toml index 6a3abb2827..eb0962e02a 100644 --- a/tracing-error/Cargo.toml +++ b/tracing-error/Cargo.toml @@ -38,7 +38,7 @@ default = ["traced-error"] traced-error = [] [dependencies] -tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.7", default-features = false, features = ["registry", "fmt"] } +tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.19", default-features = false, features = ["registry", "fmt"] } tracing = { path = "../tracing", version = "0.1.12", default-features = false, features = ["std"] } [badges] diff --git a/tracing-flame/Cargo.toml b/tracing-flame/Cargo.toml index dd2a994581..3ec28fcc62 100644 --- a/tracing-flame/Cargo.toml +++ b/tracing-flame/Cargo.toml @@ -25,7 +25,7 @@ default = ["smallvec"] smallvec = ["tracing-subscriber/smallvec"] [dependencies] -tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.3", default-features = false, features = ["registry", "fmt"] } +tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.19", default-features = false, features = ["registry", "fmt"] } tracing = { path = "../tracing", version = "0.1.12", default-features = false, features = ["std"] } lazy_static = "1.3.0" diff --git a/tracing-journald/Cargo.toml b/tracing-journald/Cargo.toml index 3acce8ece4..905a288266 100644 --- a/tracing-journald/Cargo.toml +++ b/tracing-journald/Cargo.toml @@ -16,4 +16,4 @@ keywords = ["tracing", "journald"] [dependencies] tracing-core = { path = "../tracing-core", version = "0.1.10" } -tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.5" } +tracing-subscriber = { path = "../tracing-subscriber", version = "0.2.19" } diff --git a/tracing-subscriber/Cargo.toml b/tracing-subscriber/Cargo.toml index 277c843412..a518c41642 100644 --- a/tracing-subscriber/Cargo.toml +++ b/tracing-subscriber/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "tracing-subscriber" -version = "0.2.18" +version = "0.2.19" authors = [ "Eliza Weisman ", "David Barsky ", From cafb3a67d6a14f0fa738f93a1a046de65a8e271e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 22:32:44 +0200 Subject: [PATCH 09/30] Update tracing-subscriber/src/fmt/fmt_layer.rs Co-authored-by: Eliza Weisman --- tracing-subscriber/src/fmt/fmt_layer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-subscriber/src/fmt/fmt_layer.rs b/tracing-subscriber/src/fmt/fmt_layer.rs index 451d6e6685..4879492a91 100644 --- a/tracing-subscriber/src/fmt/fmt_layer.rs +++ b/tracing-subscriber/src/fmt/fmt_layer.rs @@ -866,7 +866,7 @@ where /// the current span. /// /// [stored data]: ../registry/struct.SpanRef.html - #[deprecated(note = "wraps crate::layer::Context::scope")] + #[deprecated(note = "wraps layer::Context::scope, which is deprecated")] #[allow(deprecated)] pub fn scope(&self) -> crate::layer::Scope<'_, S> where From b31e9d758e8f3d52a732a99ac9f69065fa201402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 22:47:21 +0200 Subject: [PATCH 10/30] Correct note about Scope::from_root allocating --- tracing-subscriber/src/registry/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 25cfc0999a..8331ec245c 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -187,8 +187,9 @@ where R: LookupSpan<'a>, { /// Flips the order of the iterator, so that it is ordered from root to leaf. - - /// **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. #[allow(clippy::wrong_self_convention)] pub fn from_root(self) -> ScopeFromRoot<'a, R> { #[cfg(feature = "smallvec")] @@ -351,8 +352,8 @@ 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.scope().skip(1).from_root(), but consider whether the skip is actually intended" )] From 5bad9d4fd3761ebd8a88e15ec4a1f62ae37dd717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 22:53:22 +0200 Subject: [PATCH 11/30] Add example for Scope::from_root --- tracing-subscriber/src/registry/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 8331ec245c..b2a9074642 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -188,6 +188,12 @@ where { /// 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)] From 72d6a9ef30eb890f46c0cd6751dd3c28cc93c750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 10 Jun 2021 22:54:54 +0200 Subject: [PATCH 12/30] Mention Scope::from_root in SpanRef::scope --- tracing-subscriber/src/registry/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index b2a9074642..5da007688e 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -328,6 +328,8 @@ where /// /// 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. + /// + /// If the opposite order is desired then you can chain on a call to [`Scope::from_root`]. pub fn scope(&self) -> Scope<'a, R> { Scope { registry: self.registry, From 681f7cb72b9abc7768af2f758a2e4d322e244d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 11:33:20 +0200 Subject: [PATCH 13/30] Turn layer::Scope back into a newtype --- tracing-subscriber/src/layer.rs | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index 7fe7bc1adb..6c129ab4e4 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -6,6 +6,7 @@ use tracing_core::{ Event, LevelFilter, }; +use crate::registry::SpanRef; #[cfg(feature = "registry")] use crate::registry::{self, LookupSpan, Registry}; use std::{any::TypeId, marker::PhantomData}; @@ -587,7 +588,22 @@ pub struct Identity { #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] #[deprecated(note = "moved to crate::registry::ScopeFromRoot")] -pub type Scope<'a, L> = std::iter::Flatten>>; +#[derive(Debug)] +pub struct Scope<'a, L>(std::iter::Flatten>>) +where + L: LookupSpan<'a>; + +#[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 === @@ -1121,12 +1137,14 @@ where where S: for<'lookup> registry::LookupSpan<'lookup>, { - self.lookup_current() - .as_ref() - .map(registry::SpanRef::scope) - .map(registry::Scope::from_root) - .into_iter() - .flatten() + Scope( + self.lookup_current() + .as_ref() + .map(registry::SpanRef::scope) + .map(registry::Scope::from_root) + .into_iter() + .flatten(), + ) } } From 82004f9a618600e788d5ec22f5ff4fab7e462333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 11:56:12 +0200 Subject: [PATCH 14/30] Add usage examples for SpanRef::scope --- tracing-subscriber/src/registry/mod.rs | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 5da007688e..3e1c37f568 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -329,7 +329,41 @@ where /// 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_subscriber::registry::{Registry, LookupSpan}; + /// tracing::subscriber::with_default(tracing_subscriber::registry(), || { + /// let _root = tracing::info_span!("root").entered(); + /// let _child = tracing::info_span!("child").entered(); + /// let leaf = tracing::info_span!("leaf"); + /// tracing::dispatcher::get_default(|dispatch| { + /// let registry = dispatch.downcast_ref::().unwrap(); + /// let leaf_scope = registry.span(&leaf.id().unwrap()).unwrap().scope(); + /// assert_eq!( + /// leaf_scope.map(|span| span.name()).collect::>(), + /// vec!["leaf", "child", "root"] + /// ); + /// }); + /// }); + /// ``` + /// /// If the opposite order is desired then you can chain on a call to [`Scope::from_root`]. + /// + /// ```rust + /// use tracing_subscriber::registry::{Registry, LookupSpan}; + /// tracing::subscriber::with_default(tracing_subscriber::registry(), || { + /// let _root = tracing::info_span!("root").entered(); + /// let _child = tracing::info_span!("child").entered(); + /// let leaf = tracing::info_span!("leaf"); + /// tracing::dispatcher::get_default(|dispatch| { + /// let registry = dispatch.downcast_ref::().unwrap(); + /// let leaf_scope = registry.span(&leaf.id().unwrap()).unwrap().scope(); + /// assert_eq!( + /// leaf_scope.from_root().map(|span| span.name()).collect::>(), + /// vec!["root", "child", "leaf"] + /// ); + /// }); + /// }); + /// ``` pub fn scope(&self) -> Scope<'a, R> { Scope { registry: self.registry, From 3e94ef6d80ef84331bbf651b1355aa4874402768 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 12:00:24 +0200 Subject: [PATCH 15/30] Update tracing-subscriber/src/registry/mod.rs Co-authored-by: Eliza Weisman --- tracing-subscriber/src/registry/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 3e1c37f568..d88e9bebda 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -346,7 +346,8 @@ where /// }); /// ``` /// - /// If the opposite order is desired then you can chain on a call to [`Scope::from_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_subscriber::registry::{Registry, LookupSpan}; From 1fb6ccd5139668d20bcbadcaf7b707941a0beed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 12:00:35 +0200 Subject: [PATCH 16/30] Update tracing-subscriber/src/registry/mod.rs Co-authored-by: Eliza Weisman --- tracing-subscriber/src/registry/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index d88e9bebda..2559da2108 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -327,7 +327,7 @@ where /// 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. + /// followed by that span's parent, and so on, until it reaches a root span. /// /// ```rust /// use tracing_subscriber::registry::{Registry, LookupSpan}; From 1a474845fda771d9ea1aa6b5d211caf76ad349a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 12:03:21 +0200 Subject: [PATCH 17/30] Simplify tracing-flame span iteration --- tracing-flame/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tracing-flame/src/lib.rs b/tracing-flame/src/lib.rs index 6f7523715a..e0392e9e68 100644 --- a/tracing-flame/src/lib.rs +++ b/tracing-flame/src/lib.rs @@ -410,13 +410,12 @@ where stack += "all-threads"; } - let mut parents = first.scope(); - parents - .next() - .expect("expected: scope begins with leaf scope"); - for parent in parents.from_root() { - 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()) From 5319df09b230114d607555255d6882d6346cd5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 11 Jun 2021 12:23:05 +0200 Subject: [PATCH 18/30] Only build layer::Scope Iterator when registry is enabled --- tracing-subscriber/src/layer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index 6c129ab4e4..05f7894c48 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -6,9 +6,8 @@ use tracing_core::{ Event, LevelFilter, }; -use crate::registry::SpanRef; #[cfg(feature = "registry")] -use crate::registry::{self, LookupSpan, Registry}; +use crate::registry::{self, LookupSpan, Registry, SpanRef}; use std::{any::TypeId, marker::PhantomData}; /// A composable handler for `tracing` events. @@ -593,6 +592,7 @@ pub struct Scope<'a, L>(std::iter::Flatten; +#[cfg(feature = "registry")] #[allow(deprecated)] impl<'a, L> Iterator for Scope<'a, L> where From 92fedf9d3a1fd0928afe7231e549304861118909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 15 Jun 2021 15:09:57 +0200 Subject: [PATCH 19/30] Clarify and correct deprecation messages a bit --- tracing-subscriber/src/registry/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 2559da2108..3230e8fceb 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -379,7 +379,7 @@ where /// 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.scope().skip(1), but consider whether the skip is actually intended" + 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> { @@ -398,7 +398,7 @@ where /// **Note**: this will allocate if there are many spans remaining, or if the /// "smallvec" feature flag is not enabled. #[deprecated( - note = "equivalent to self.scope().skip(1).from_root(), but consider whether the skip is actually intended" + note = "equivalent to `self.parent().into_iter().flat_map(|span| span.scope().from_root())`, but consider whether excluding self is actually intended" )] #[allow(deprecated)] pub fn from_root(&self) -> FromRoot<'a, R> { From 6b077f494ba04a580bf1248dc3748e2142cc14d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 15 Jun 2021 15:14:48 +0200 Subject: [PATCH 20/30] Convert registry::{Parents, FromRoot} to newtypes as well --- tracing-subscriber/src/registry/mod.rs | 45 +++++++++++++++++++++----- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 3230e8fceb..f4501d01cf 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -265,8 +265,21 @@ where /// This is returned by the [`SpanRef::parents`] method. /// /// [`SpanRef::parents`]: struct.SpanRef.html#method.parents -#[deprecated(note = "replaced by Scope")] -pub type Parents<'a, R> = Scope<'a, R>; +#[deprecated(note = "replaced by `Scope`")] +#[derive(Debug)] +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 /// tree. @@ -274,8 +287,23 @@ pub type Parents<'a, R> = Scope<'a, R>; /// For additonal details, see [`SpanRef::from_root`]. /// /// [`Span::from_root`]: struct.SpanRef.html#method.from_root -#[deprecated(note = "replaced by ScopeFromRoot")] -pub type FromRoot<'a, R> = ScopeFromRoot<'a, R>; +#[deprecated(note = "replaced by `ScopeFromRoot`")] +#[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")] type SpanRefVecArray<'span, L> = [SpanRef<'span, L>; 16]; @@ -383,9 +411,10 @@ where )] #[allow(deprecated)] pub fn parents(&self) -> Parents<'a, R> { - let mut scope = self.scope(); - scope.next(); - scope + Parents(Scope { + registry: self.registry, + next: self.parent_id().cloned(), + }) } /// Returns an iterator over all parents of this span, starting with the @@ -402,7 +431,7 @@ where )] #[allow(deprecated)] pub fn from_root(&self) -> FromRoot<'a, R> { - self.parents().from_root() + FromRoot(self.parents().0.from_root()) } /// Returns a reference to this span's `Extensions`. From 6b4d4b307e92ff96231a09830446cddd85c7fad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 15 Jun 2021 17:06:59 +0200 Subject: [PATCH 21/30] Add `Context::span_scope` helper, improve documentation on why `Context::scope` is harmful --- tracing-subscriber/src/layer.rs | 38 ++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index 05f7894c48..044a88284a 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -1130,7 +1130,7 @@ where #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] #[deprecated( - note = "equivalent to self.lookup_current().into_iter().flat_map(|span| span.scope().from_root()), but consider whether lookup_current is a bug" + 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" )] #[allow(deprecated)] pub fn scope(&self) -> Scope<'_, S> @@ -1146,6 +1146,42 @@ where .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 + S: for<'lookup> registry::LookupSpan<'lookup>, + { + Some(self.span(id)?.scope()) + } } impl<'a, S> Context<'a, S> { From 00fb1dc012afc5b58df58787a1f0fe3e8ad440fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Thu, 17 Jun 2021 23:46:32 +0200 Subject: [PATCH 22/30] Apply suggestions from code review Co-authored-by: Eliza Weisman --- tracing-subscriber/src/fmt/fmt_layer.rs | 5 ++++- tracing-subscriber/src/layer.rs | 8 ++++++-- tracing-subscriber/src/registry/mod.rs | 10 +++++++--- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/fmt/fmt_layer.rs b/tracing-subscriber/src/fmt/fmt_layer.rs index 4879492a91..6658a041d7 100644 --- a/tracing-subscriber/src/fmt/fmt_layer.rs +++ b/tracing-subscriber/src/fmt/fmt_layer.rs @@ -866,7 +866,10 @@ where /// the current span. /// /// [stored data]: ../registry/struct.SpanRef.html - #[deprecated(note = "wraps layer::Context::scope, which is deprecated")] + #[deprecated( + note = "wraps layer::Context::scope, which is deprecated", + since = "0.2.19", + )] #[allow(deprecated)] pub fn scope(&self) -> crate::layer::Scope<'_, S> where diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index 044a88284a..b66facd662 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -586,7 +586,10 @@ pub struct Identity { /// [`Context::scope`]: struct.Context.html#method.scope #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -#[deprecated(note = "moved to crate::registry::ScopeFromRoot")] +#[deprecated( + note = "renamed to crate::registry::ScopeFromRoot", + since = "0.2.19", +)] #[derive(Debug)] pub struct Scope<'a, L>(std::iter::Flatten>>) where @@ -1130,7 +1133,8 @@ where #[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" + 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<'_, S> diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index f4501d01cf..41d5e6f8c4 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -287,7 +287,10 @@ where /// For additonal details, see [`SpanRef::from_root`]. /// /// [`Span::from_root`]: struct.SpanRef.html#method.from_root -#[deprecated(note = "replaced by `ScopeFromRoot`")] +#[deprecated( + note = "replaced by `ScopeFromRoot`", + since = "0.2.19", +)] #[derive(Debug)] pub struct FromRoot<'a, R>(ScopeFromRoot<'a, R>) where @@ -407,7 +410,7 @@ where /// 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" + 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> { @@ -427,7 +430,8 @@ where /// **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" + 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> { From 235ab8980279038b5dce613c1ba02e254929b2bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 18 Jun 2021 00:26:34 +0200 Subject: [PATCH 23/30] Show SpanRef::scope examples in an actual Layer --- tracing-subscriber/src/registry/mod.rs | 87 ++++++++++++++++++-------- 1 file changed, 60 insertions(+), 27 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 41d5e6f8c4..68b4901100 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -287,10 +287,7 @@ where /// For additonal details, see [`SpanRef::from_root`]. /// /// [`Span::from_root`]: struct.SpanRef.html#method.from_root -#[deprecated( - note = "replaced by `ScopeFromRoot`", - since = "0.2.19", -)] +#[deprecated(note = "replaced by `ScopeFromRoot`", since = "0.2.19")] #[derive(Debug)] pub struct FromRoot<'a, R>(ScopeFromRoot<'a, R>) where @@ -361,19 +358,37 @@ where /// followed by that span's parent, and so on, until it reaches a root span. /// /// ```rust - /// use tracing_subscriber::registry::{Registry, LookupSpan}; - /// tracing::subscriber::with_default(tracing_subscriber::registry(), || { + /// use tracing::{span, Subscriber}; + /// use tracing_subscriber::{ + /// layer::{Context, Layer}, + /// prelude::*, + /// registry::{LookupSpan, Registry}, + /// }; + /// # // Use static mut to avoid tainting the visible part of the example with state + /// # static mut last_entered_scope: Vec<&'static str> = vec![]; + /// struct PrintingLayer; + /// impl Layer for PrintingLayer + /// where + /// S: Subscriber + 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); + /// # unsafe { last_entered_scope = scope; } + /// } + /// } + /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { /// let _root = tracing::info_span!("root").entered(); + /// // Prints: Entering span: ["root"] /// let _child = tracing::info_span!("child").entered(); - /// let leaf = tracing::info_span!("leaf"); - /// tracing::dispatcher::get_default(|dispatch| { - /// let registry = dispatch.downcast_ref::().unwrap(); - /// let leaf_scope = registry.span(&leaf.id().unwrap()).unwrap().scope(); - /// assert_eq!( - /// leaf_scope.map(|span| span.name()).collect::>(), - /// vec!["leaf", "child", "root"] - /// ); - /// }); + /// // Prints: Entering span: ["child", "root"] + /// let _leaf = tracing::info_span!("leaf").entered(); + /// // Prints: Entering span: ["leaf", "child", "root"] + /// # assert_eq!( + /// # unsafe { &last_entered_scope }, + /// # &["leaf", "child", "root"] + /// # ); /// }); /// ``` /// @@ -381,19 +396,37 @@ where /// the returned iterator reverses the order. /// /// ```rust - /// use tracing_subscriber::registry::{Registry, LookupSpan}; - /// tracing::subscriber::with_default(tracing_subscriber::registry(), || { + /// # use tracing::{span, Subscriber}; + /// # use tracing_subscriber::{ + /// # layer::{Context, Layer}, + /// # prelude::*, + /// # registry::{LookupSpan, Registry}, + /// # }; + /// # // Use static mut to avoid tainting the visible part of the example with state + /// # static mut last_entered_scope: Vec<&'static str> = vec![]; + /// # struct PrintingLayer; + /// impl Layer for PrintingLayer + /// where + /// S: Subscriber + 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); + /// # unsafe { last_entered_scope = scope; } + /// } + /// } + /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { /// let _root = tracing::info_span!("root").entered(); + /// // Prints: Entering span: ["root"] /// let _child = tracing::info_span!("child").entered(); - /// let leaf = tracing::info_span!("leaf"); - /// tracing::dispatcher::get_default(|dispatch| { - /// let registry = dispatch.downcast_ref::().unwrap(); - /// let leaf_scope = registry.span(&leaf.id().unwrap()).unwrap().scope(); - /// assert_eq!( - /// leaf_scope.from_root().map(|span| span.name()).collect::>(), - /// vec!["root", "child", "leaf"] - /// ); - /// }); + /// // Prints: Entering span: ["root", "child"] + /// let _leaf = tracing::info_span!("leaf").entered(); + /// // Prints: Entering span: ["root", "child", "leaf"] + /// # assert_eq!( + /// # unsafe { &last_entered_scope }, + /// # &["root", "child", "leaf"] + /// # ); /// }); /// ``` pub fn scope(&self) -> Scope<'a, R> { @@ -431,7 +464,7 @@ where /// "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", + since = "0.2.19" )] #[allow(deprecated)] pub fn from_root(&self) -> FromRoot<'a, R> { From cb41242be6888aa163ef9d6d22a1a7cbadaaeee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 18 Jun 2021 00:42:21 +0200 Subject: [PATCH 24/30] Fix Clippy warnings None of these are really related to this PR, but Clippy was unhappy... --- tracing-subscriber/src/filter/env/mod.rs | 7 +++---- tracing-subscriber/src/filter/level.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/tracing-subscriber/src/filter/env/mod.rs b/tracing-subscriber/src/filter/env/mod.rs index f1c71416e0..4ae87697dc 100644 --- a/tracing-subscriber/src/filter/env/mod.rs +++ b/tracing-subscriber/src/filter/env/mod.rs @@ -271,7 +271,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); }; @@ -308,7 +308,6 @@ impl EnvFilter { }; let level = directive .level - .clone() .into_level() .expect("=off would not have enabled any filters"); ctx(&format!( @@ -396,8 +395,8 @@ impl Layer 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 0c981f3475..0fa601260a 100644 --- a/tracing-subscriber/src/filter/level.rs +++ b/tracing-subscriber/src/filter/level.rs @@ -22,6 +22,6 @@ impl crate::Layer for LevelFilter { } fn max_level_hint(&self) -> Option { - self.clone().into() + Some(*self) } } From 2b093df244ea7f9da386100f93b2ee29871d80b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 18 Jun 2021 00:50:00 +0200 Subject: [PATCH 25/30] Fix the rest of the Clippy warnings Because GitHub still wasn't happy. --- .../examples/sloggish/sloggish_subscriber.rs | 3 +- tracing-core/src/metadata.rs | 4 +-- tracing-subscriber/src/fmt/format/mod.rs | 32 +++++++++---------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/examples/examples/sloggish/sloggish_subscriber.rs b/examples/examples/sloggish/sloggish_subscriber.rs index 94cf90a378..f438ed6526 100644 --- a/examples/examples/sloggish/sloggish_subscriber.rs +++ b/examples/examples/sloggish/sloggish_subscriber.rs @@ -133,7 +133,6 @@ impl<'a> Visit for Event<'a> { Style::new().bold().paint(format!("{:?}", value)) ) .unwrap(); - self.comma = true; } else { write!( &mut self.stderr, @@ -142,8 +141,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 91c23a41e9..7bf7428a62 100644 --- a/tracing-core/src/metadata.rs +++ b/tracing-core/src/metadata.rs @@ -829,10 +829,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-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index b07bba2d15..7a551bee46 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -1503,27 +1503,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)); } } From 8774877cd996cfed89bfc61c6273999eb279ba00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 18 Jun 2021 00:54:42 +0200 Subject: [PATCH 26/30] Rustfmt --- tracing-subscriber/src/fmt/fmt_layer.rs | 2 +- tracing-subscriber/src/layer.rs | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/fmt/fmt_layer.rs b/tracing-subscriber/src/fmt/fmt_layer.rs index 6658a041d7..ab1806d4dd 100644 --- a/tracing-subscriber/src/fmt/fmt_layer.rs +++ b/tracing-subscriber/src/fmt/fmt_layer.rs @@ -868,7 +868,7 @@ where /// [stored data]: ../registry/struct.SpanRef.html #[deprecated( note = "wraps layer::Context::scope, which is deprecated", - since = "0.2.19", + since = "0.2.19" )] #[allow(deprecated)] pub fn scope(&self) -> crate::layer::Scope<'_, S> diff --git a/tracing-subscriber/src/layer.rs b/tracing-subscriber/src/layer.rs index ad10bbd1af..03391e4fa6 100644 --- a/tracing-subscriber/src/layer.rs +++ b/tracing-subscriber/src/layer.rs @@ -580,10 +580,7 @@ pub struct Identity { /// [`Context::scope`]: struct.Context.html#method.scope #[cfg(feature = "registry")] #[cfg_attr(docsrs, doc(cfg(feature = "registry")))] -#[deprecated( - note = "renamed to crate::registry::ScopeFromRoot", - since = "0.2.19", -)] +#[deprecated(note = "renamed to crate::registry::ScopeFromRoot", since = "0.2.19")] #[derive(Debug)] pub struct Scope<'a, L>(std::iter::Flatten>>) where @@ -1116,7 +1113,7 @@ where #[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", + since = "0.2.19" )] #[allow(deprecated)] pub fn scope(&self) -> Scope<'_, S> From a9eb2d58135dcfa4fd47104f7bf22b539139bba9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Fri, 18 Jun 2021 01:03:36 +0200 Subject: [PATCH 27/30] Fix broken doctests on MSRV (1.42.0) --- tracing-subscriber/src/registry/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 61480ca5b0..9224322c87 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -362,7 +362,7 @@ where /// registry::{LookupSpan, Registry}, /// }; /// # // Use static mut to avoid tainting the visible part of the example with state - /// # static mut last_entered_scope: Vec<&'static str> = vec![]; + /// # static mut last_entered_scope: Vec<&'static str> = Vec::new(); /// struct PrintingLayer; /// impl Layer for PrintingLayer /// where @@ -400,7 +400,7 @@ where /// # registry::{LookupSpan, Registry}, /// # }; /// # // Use static mut to avoid tainting the visible part of the example with state - /// # static mut last_entered_scope: Vec<&'static str> = vec![]; + /// # static mut last_entered_scope: Vec<&'static str> = Vec::new(); /// # struct PrintingLayer; /// impl Layer for PrintingLayer /// where From b651701c0931606399e1243029169591a9e04bf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Sun, 20 Jun 2021 13:32:31 +0200 Subject: [PATCH 28/30] Move SpanRef::scope assertions into real tests Rather than keeping them merged with the doctests --- tracing-subscriber/src/registry/mod.rs | 103 +++++++++++++++++++++---- 1 file changed, 87 insertions(+), 16 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 9224322c87..951d1634c7 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -359,10 +359,8 @@ where /// use tracing_subscriber::{ /// layer::{Context, Layer}, /// prelude::*, - /// registry::{LookupSpan, Registry}, + /// registry::LookupSpan, /// }; - /// # // Use static mut to avoid tainting the visible part of the example with state - /// # static mut last_entered_scope: Vec<&'static str> = Vec::new(); /// struct PrintingLayer; /// impl Layer for PrintingLayer /// where @@ -372,7 +370,6 @@ where /// let span = ctx.span(id).unwrap(); /// let scope = span.scope().map(|span| span.name()).collect::>(); /// println!("Entering span: {:?}", scope); - /// # unsafe { last_entered_scope = scope; } /// } /// } /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { @@ -382,10 +379,6 @@ where /// // Prints: Entering span: ["child", "root"] /// let _leaf = tracing::info_span!("leaf").entered(); /// // Prints: Entering span: ["leaf", "child", "root"] - /// # assert_eq!( - /// # unsafe { &last_entered_scope }, - /// # &["leaf", "child", "root"] - /// # ); /// }); /// ``` /// @@ -397,10 +390,8 @@ where /// # use tracing_subscriber::{ /// # layer::{Context, Layer}, /// # prelude::*, - /// # registry::{LookupSpan, Registry}, + /// # registry::LookupSpan, /// # }; - /// # // Use static mut to avoid tainting the visible part of the example with state - /// # static mut last_entered_scope: Vec<&'static str> = Vec::new(); /// # struct PrintingLayer; /// impl Layer for PrintingLayer /// where @@ -410,7 +401,6 @@ where /// let span = ctx.span(id).unwrap(); /// let scope = span.scope().from_root().map(|span| span.name()).collect::>(); /// println!("Entering span: {:?}", scope); - /// # unsafe { last_entered_scope = scope; } /// } /// } /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { @@ -420,10 +410,6 @@ where /// // Prints: Entering span: ["root", "child"] /// let _leaf = tracing::info_span!("leaf").entered(); /// // Prints: Entering span: ["root", "child", "leaf"] - /// # assert_eq!( - /// # unsafe { &last_entered_scope }, - /// # &["root", "child", "leaf"] - /// # ); /// }); /// ``` pub fn scope(&self) -> Scope<'a, R> { @@ -484,3 +470,88 @@ where self.data.extensions_mut() } } + +#[cfg(test)] +mod tests { + use crate::{ + layer::{Context, Layer}, + prelude::*, + registry::LookupSpan, + }; + use std::sync::{Arc, Mutex}; + use tracing::{span, Subscriber}; + + #[test] + fn spanref_scope_iteration_order() { + let last_entered_scope = Arc::new(Mutex::new(Vec::new())); + #[derive(Default)] + struct PrintingLayer { + last_entered_scope: Arc>>, + } + impl Layer for PrintingLayer + where + S: Subscriber + 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; + } + } + tracing::subscriber::with_default( + crate::registry().with(PrintingLayer { + 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"] + ); + }, + ); + } + + #[test] + fn spanref_scope_fromroot_iteration_order() { + let last_entered_scope = Arc::new(Mutex::new(Vec::new())); + #[derive(Default)] + struct PrintingLayer { + last_entered_scope: Arc>>, + } + impl Layer for PrintingLayer + where + S: Subscriber + 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; + } + } + tracing::subscriber::with_default( + crate::registry().with(PrintingLayer { + 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"] + ); + }, + ); + } +} From 7ef5c0a6ae1369b173f9dee17ce7628608ac0d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 22 Jun 2021 11:22:57 +0200 Subject: [PATCH 29/30] Apply suggestions from code review Co-authored-by: Eliza Weisman --- tracing-subscriber/src/registry/mod.rs | 36 ++++++++++++++++---------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 951d1634c7..4e48c1339c 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -361,6 +361,7 @@ where /// prelude::*, /// registry::LookupSpan, /// }; + /// /// struct PrintingLayer; /// impl Layer for PrintingLayer /// where @@ -372,6 +373,7 @@ where /// println!("Entering span: {:?}", scope); /// } /// } + /// /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { /// let _root = tracing::info_span!("root").entered(); /// // Prints: Entering span: ["root"] @@ -402,7 +404,8 @@ where /// let scope = span.scope().from_root().map(|span| span.name()).collect::>(); /// println!("Entering span: {:?}", scope); /// } - /// } + /// } + /// /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { /// let _root = tracing::info_span!("root").entered(); /// // Prints: Entering span: ["root"] @@ -484,10 +487,12 @@ mod tests { #[test] fn spanref_scope_iteration_order() { let last_entered_scope = Arc::new(Mutex::new(Vec::new())); + #[derive(Default)] struct PrintingLayer { last_entered_scope: Arc>>, } + impl Layer for PrintingLayer where S: Subscriber + for<'lookup> LookupSpan<'lookup>, @@ -498,31 +503,33 @@ mod tests { *self.last_entered_scope.lock().unwrap() = scope; } } - tracing::subscriber::with_default( + + let _guard = tracing::subscriber::set_default( crate::registry().with(PrintingLayer { 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"] - ); - }, + }) + ); + + 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"] ); } #[test] fn spanref_scope_fromroot_iteration_order() { let last_entered_scope = Arc::new(Mutex::new(Vec::new())); + #[derive(Default)] struct PrintingLayer { last_entered_scope: Arc>>, } + impl Layer for PrintingLayer where S: Subscriber + for<'lookup> LookupSpan<'lookup>, @@ -537,6 +544,7 @@ mod tests { *self.last_entered_scope.lock().unwrap() = scope; } } + tracing::subscriber::with_default( crate::registry().with(PrintingLayer { last_entered_scope: last_entered_scope.clone(), From 1f32622423589243c071ac0866aea235622b27bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Teo=20Klestrup=20R=C3=B6ijezon?= Date: Tue, 22 Jun 2021 11:25:59 +0200 Subject: [PATCH 30/30] Use subscriber guard consistently in scope tests --- tracing-subscriber/src/registry/mod.rs | 49 ++++++++++++-------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/tracing-subscriber/src/registry/mod.rs b/tracing-subscriber/src/registry/mod.rs index 4e48c1339c..0761c528a8 100644 --- a/tracing-subscriber/src/registry/mod.rs +++ b/tracing-subscriber/src/registry/mod.rs @@ -404,7 +404,7 @@ where /// let scope = span.scope().from_root().map(|span| span.name()).collect::>(); /// println!("Entering span: {:?}", scope); /// } - /// } + /// } /// /// tracing::subscriber::with_default(tracing_subscriber::registry().with(PrintingLayer), || { /// let _root = tracing::info_span!("root").entered(); @@ -492,7 +492,7 @@ mod tests { struct PrintingLayer { last_entered_scope: Arc>>, } - + impl Layer for PrintingLayer where S: Subscriber + for<'lookup> LookupSpan<'lookup>, @@ -503,13 +503,11 @@ mod tests { *self.last_entered_scope.lock().unwrap() = scope; } } - - let _guard = tracing::subscriber::set_default( - crate::registry().with(PrintingLayer { - last_entered_scope: last_entered_scope.clone(), - }) - ); - + + let _guard = tracing::subscriber::set_default(crate::registry().with(PrintingLayer { + 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(); @@ -524,12 +522,12 @@ mod tests { #[test] fn spanref_scope_fromroot_iteration_order() { let last_entered_scope = Arc::new(Mutex::new(Vec::new())); - + #[derive(Default)] struct PrintingLayer { last_entered_scope: Arc>>, } - + impl Layer for PrintingLayer where S: Subscriber + for<'lookup> LookupSpan<'lookup>, @@ -544,22 +542,19 @@ mod tests { *self.last_entered_scope.lock().unwrap() = scope; } } - - tracing::subscriber::with_default( - crate::registry().with(PrintingLayer { - 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"] - ); - }, + + let _guard = tracing::subscriber::set_default(crate::registry().with(PrintingLayer { + 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"] ); } }