Skip to content

Commit

Permalink
Replace legacy span iterator implementations with SpanRef::scope
Browse files Browse the repository at this point in the history
  • Loading branch information
nightkr committed Jun 9, 2021
1 parent 5c379fb commit cd8148f
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 107 deletions.
43 changes: 13 additions & 30 deletions tracing-subscriber/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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::Chain<registry::FromRoot<'a, L>, std::iter::Once<SpanRef<'a, L>>>>,
);
#[deprecated(note = "moved to crate::registry::ScopeFromRoot")]
pub type Scope<'a, L> = std::iter::Flatten<std::option::IntoIter<registry::ScopeFromRoot<'a, L>>>;

// === impl Layered ===

Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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::Item> {
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::*;
Expand Down
122 changes: 45 additions & 77 deletions tracing-subscriber/src/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<Id>,
}

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<T> = smallvec::SmallVec<T>;
#[cfg(not(feature = "smallvec"))]
type Buf<T> = Vec<T>;
SpanScopeFromRoot {
ScopeFromRoot {
spans: self.collect::<Buf<_>>().into_iter().rev(),
}
}
}

impl<'a, R> Iterator for SpanScope<'a, R>
impl<'a, R> Iterator for Scope<'a, R>
where
R: LookupSpan<'a>,
{
Expand All @@ -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>,
{
Expand All @@ -223,40 +227,48 @@ where
spans: std::iter::Rev<std::vec::IntoIter<SpanRef<'a, R>>>,
}

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::Item> {
self.spans.next()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
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
#[derive(Debug)]
pub struct Parents<'a, R> {
registry: &'a R,
next: Option<Id>,
}
#[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.
///
/// 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<smallvec::IntoIter<SpanRefVecArray<'a, R>>>,
#[cfg(not(feature = "smallvec"))]
inner: std::iter::Rev<std::vec::IntoIter<SpanRef<'a, R>>>,
}
#[deprecated(note = "replaced by ScopeFromRoot")]
pub type FromRoot<'a, R> = ScopeFromRoot<'a, R>;

#[cfg(feature = "smallvec")]
type SpanRefVecArray<'span, L> = [SpanRef<'span, L>; 16];
Expand Down Expand Up @@ -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()),
}
Expand All @@ -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
Expand All @@ -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<SpanRefVecArray<'span, L>>;
#[cfg(not(feature = "smallvec"))]
type SpanRefVec<'span, L> = Vec<SpanRef<'span, L>>;

// 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::<SpanRefVec<'a, _>>();
let inner = parents.into_iter().rev();
FromRoot { inner }
self.parents().from_root()
}

/// Returns a reference to this span's `Extensions`.
Expand All @@ -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<Self::Item> {
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::Item> {
self.inner.next()
}

#[inline]
fn size_hint(&self) -> (usize, Option<usize>) {
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 { .. }")
}
}

0 comments on commit cd8148f

Please sign in to comment.