Skip to content

Commit

Permalink
subscriber: unify span traversal (#1445)
Browse files Browse the repository at this point in the history
Forward-port of #1431 and #1434

* subscriber: unify span traversal (#1431)

## Motivation

Fixes #1429 (forwardport from v0.1.x branch)

## Solution

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::<Vec<_>>().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)

* subscriber: add Context method for resolving an Event's SpanRef (#1434)

## Motivation

Fixes #1428 (forward-port from v0.1.x)

## Solution

Adds a new `Context::event_span` method as proposed in the issue.

No existing formatters were changed to use it yet, that seems like a
secondary issue (especially if they already work correctly).
  • Loading branch information
nightkr authored Jun 24, 2021
1 parent 60f816b commit f54136c
Show file tree
Hide file tree
Showing 9 changed files with 412 additions and 174 deletions.
3 changes: 1 addition & 2 deletions tracing-error/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ where
let span = collector
.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::<FormattedFields<F>>() {
f(span.metadata(), fields.fields.as_str())
} else {
Expand Down
23 changes: 9 additions & 14 deletions tracing-flame/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
Expand Down Expand Up @@ -440,28 +441,22 @@ 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 {
THREAD_NAME.with(|name| stack += name.as_str());
} 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"
Expand Down
10 changes: 7 additions & 3 deletions tracing-journald/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -143,7 +143,7 @@ where

fn on_record(&self, id: &Id, values: &Record, ctx: Context<C>) {
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::<SpanFields>().expect("missing fields").0;
values.record(&mut SpanVisitor {
Expand All @@ -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::<SpanFields>().expect("missing fields");
buf.extend_from_slice(&fields.0);
Expand Down
20 changes: 5 additions & 15 deletions tracing-subscriber/src/fmt/fmt_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -756,8 +756,10 @@ where
F: FnMut(&SpanRef<'_, C>) -> 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(())
}
Expand Down Expand Up @@ -811,18 +813,6 @@ where
self.ctx.lookup_current()
}

/// Returns an iterator over the [stored data] for all the spans in the
/// current context, starting the root of the trace tree and ending with
/// the current span.
///
/// [stored data]: SpanRef
pub fn scope(&self) -> Scope<'_, C>
where
C: for<'lookup> LookupSpan<'lookup>,
{
self.ctx.scope()
}

/// Returns the current span for this formatter.
pub fn current_span(&self) -> Current {
self.ctx.current_span()
Expand Down
6 changes: 4 additions & 2 deletions tracing-subscriber/src/fmt/format/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
10 changes: 2 additions & 8 deletions tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use crate::{

use std::{
fmt::{self, Write},
iter,
marker::PhantomData,
};
use tracing_core::{
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()))?;
Expand Down
10 changes: 2 additions & 8 deletions tracing-subscriber/src/fmt/format/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit f54136c

Please sign in to comment.