Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify span traversal #1431

Merged
merged 33 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0e45e83
Implement SpanRef::scope and SpanRef::scope.from_root iterators
nightkr Jun 9, 2021
5c379fb
Implement smallvec optimization for SpanScopeFromRoot
nightkr Jun 9, 2021
cd8148f
Replace legacy span iterator implementations with SpanRef::scope
nightkr Jun 9, 2021
84afb38
Migrate tracing-subscriber to SpanRef::scope
nightkr Jun 9, 2021
9ec8130
Migrate layer crates to use SpanRef::scope
nightkr Jun 9, 2021
d12cd51
Reverted tracing-error span iteration order
nightkr Jun 9, 2021
4d0fd19
Fixed flipped scope order in tracing-journald
nightkr Jun 9, 2021
8d162d4
Bump tracing-subscriber version
nightkr Jun 10, 2021
cafb3a6
Update tracing-subscriber/src/fmt/fmt_layer.rs
nightkr Jun 10, 2021
b31e9d7
Correct note about Scope::from_root allocating
nightkr Jun 10, 2021
0d1294d
Merge branch 'feature/unified-span-iteration' of github.com:appva/tra…
nightkr Jun 10, 2021
5bad9d4
Add example for Scope::from_root
nightkr Jun 10, 2021
72d6a9e
Mention Scope::from_root in SpanRef::scope
nightkr Jun 10, 2021
681f7cb
Turn layer::Scope back into a newtype
nightkr Jun 11, 2021
82004f9
Add usage examples for SpanRef::scope
nightkr Jun 11, 2021
3e94ef6
Update tracing-subscriber/src/registry/mod.rs
nightkr Jun 11, 2021
1fb6ccd
Update tracing-subscriber/src/registry/mod.rs
nightkr Jun 11, 2021
1a47484
Simplify tracing-flame span iteration
nightkr Jun 11, 2021
3c9386f
Merge branch 'feature/unified-span-iteration' of github.com:appva/tra…
nightkr Jun 11, 2021
5319df0
Only build layer::Scope Iterator when registry is enabled
nightkr Jun 11, 2021
92fedf9
Clarify and correct deprecation messages a bit
nightkr Jun 15, 2021
6b077f4
Convert registry::{Parents, FromRoot} to newtypes as well
nightkr Jun 15, 2021
6b4d4b3
Add `Context::span_scope` helper, improve documentation on why `Conte…
nightkr Jun 15, 2021
00fb1dc
Apply suggestions from code review
nightkr Jun 17, 2021
235ab89
Show SpanRef::scope examples in an actual Layer
nightkr Jun 17, 2021
3c917b2
Merge remote-tracking branch 'origin/v0.1.x' into feature/unified-spa…
nightkr Jun 17, 2021
cb41242
Fix Clippy warnings
nightkr Jun 17, 2021
2b093df
Fix the rest of the Clippy warnings
nightkr Jun 17, 2021
8774877
Rustfmt
nightkr Jun 17, 2021
a9eb2d5
Fix broken doctests on MSRV (1.42.0)
nightkr Jun 17, 2021
b651701
Move SpanRef::scope assertions into real tests
nightkr Jun 20, 2021
7ef5c0a
Apply suggestions from code review
nightkr Jun 22, 2021
1f32622
Use subscriber guard consistently in scope tests
nightkr Jun 22, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions examples/examples/sloggish/sloggish_subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -142,8 +141,8 @@ impl<'a> Visit for Event<'a> {
value
)
.unwrap();
self.comma = true;
}
self.comma = true;
}
}

Expand Down
4 changes: 2 additions & 2 deletions tracing-core/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion tracing-error/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
3 changes: 1 addition & 2 deletions tracing-error/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
nightkr marked this conversation as resolved.
Show resolved Hide resolved
let cont = if let Some(fields) = span.extensions().get::<FormattedFields<F>>() {
f(span.metadata(), fields.fields.as_str())
} else {
Expand Down
2 changes: 1 addition & 1 deletion tracing-flame/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"

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 @@ -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 {
Expand All @@ -412,9 +410,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 @@ -444,28 +445,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
2 changes: 1 addition & 1 deletion tracing-journald/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
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();
hawkw marked this conversation as resolved.
Show resolved Hide resolved

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<S>) {
let span = ctx.span(id).expect("unknown span");
let depth = span.parents().count();
let depth = span.scope().skip(1).count();
hawkw marked this conversation as resolved.
Show resolved Hide resolved
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()
Comment on lines +160 to +161
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.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
2 changes: 1 addition & 1 deletion tracing-subscriber/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "tracing-subscriber"
version = "0.2.18"
version = "0.2.19"
authors = [
"Eliza Weisman <eliza@buoyant.io>",
"David Barsky <me@davidbarsky.com>",
Expand Down
7 changes: 3 additions & 4 deletions tracing-subscriber/src/filter/env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down Expand Up @@ -308,7 +308,6 @@ impl EnvFilter {
};
let level = directive
.level
.clone()
.into_level()
.expect("=off would not have enabled any filters");
ctx(&format!(
Expand Down Expand Up @@ -396,8 +395,8 @@ impl<S: Subscriber> Layer<S> 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(),
)
}

Expand Down
2 changes: 1 addition & 1 deletion tracing-subscriber/src/filter/level.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ impl<S: Subscriber> crate::Layer<S> for LevelFilter {
}

fn max_level_hint(&self) -> Option<LevelFilter> {
self.clone().into()
Some(*self)
}
}
15 changes: 11 additions & 4 deletions tracing-subscriber/src/fmt/fmt_layer.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -864,7 +866,12 @@ where
/// the current span.
///
/// [stored data]: ../registry/struct.SpanRef.html
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::layer::Scope<'_, S>
where
S: for<'lookup> LookupSpan<'lookup>,
{
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 @@ -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()
Expand Down
45 changes: 19 additions & 26 deletions tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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()))?;
Expand Down Expand Up @@ -1510,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));
}
}
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},
Event, Level, Subscriber,
Expand Down Expand Up @@ -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();
Expand Down
Loading