Skip to content

Commit 5bdcba5

Browse files
Skepfyrhawkw
authored andcommitted
tracing: fix macros "use of moved value" with log (#1823)
## Motivation Fixes: #196, #1739 Previously, the `field` token trees would be substituted into the log macro invocation and the `ValueSet`, potentially meaning they are executed more than once. ## Solution This changes the `event!` macro (and all the upstream macros like `info!`), so that it only uses each field once when the log feature is enabled. It does this by generating the log using the `ValueSet` meaning the token trees are only substituted once.
1 parent 014c6eb commit 5bdcba5

File tree

5 files changed

+288
-164
lines changed

5 files changed

+288
-164
lines changed

tracing-core/src/field.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -881,8 +881,8 @@ impl<'a> ValueSet<'a> {
881881

882882
/// Visits all the fields in this `ValueSet` with the provided [visitor].
883883
///
884-
/// [visitor]: ../trait.Visit.html
885-
pub(crate) fn record(&self, visitor: &mut dyn Visit) {
884+
/// [visitor]: Visit
885+
pub fn record(&self, visitor: &mut dyn Visit) {
886886
let my_callsite = self.callsite();
887887
for (field, value) in self.values {
888888
if field.callsite() != my_callsite {
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
use std::fmt::Write;
2+
3+
use criterion::{black_box, criterion_group, criterion_main, Criterion};
4+
use tracing::Level;
5+
6+
use tracing::{span, Event, Id, Metadata};
7+
use tracing_core::span::Current;
8+
9+
/// A subscriber that is enabled but otherwise does nothing.
10+
struct EnabledSubscriber;
11+
12+
impl tracing::Subscriber for EnabledSubscriber {
13+
fn new_span(&self, span: &span::Attributes<'_>) -> Id {
14+
let _ = span;
15+
Id::from_u64(0xDEAD_FACE)
16+
}
17+
18+
fn event(&self, event: &Event<'_>) {
19+
let _ = event;
20+
}
21+
22+
fn record(&self, span: &Id, values: &span::Record<'_>) {
23+
let _ = (span, values);
24+
}
25+
26+
fn record_follows_from(&self, span: &Id, follows: &Id) {
27+
let _ = (span, follows);
28+
}
29+
30+
fn enabled(&self, metadata: &Metadata<'_>) -> bool {
31+
let _ = metadata;
32+
true
33+
}
34+
35+
fn enter(&self, span: &Id) {
36+
let _ = span;
37+
}
38+
39+
fn exit(&self, span: &Id) {
40+
let _ = span;
41+
}
42+
}
43+
44+
const NOP_LOGGER: NopLogger = NopLogger;
45+
46+
struct NopLogger;
47+
48+
impl log::Log for NopLogger {
49+
fn enabled(&self, _metadata: &log::Metadata) -> bool {
50+
true
51+
}
52+
53+
fn log(&self, record: &log::Record) {
54+
if self.enabled(record.metadata()) {
55+
let mut this = self;
56+
let _ = write!(this, "{}", record.args());
57+
}
58+
}
59+
60+
fn flush(&self) {}
61+
}
62+
63+
impl Write for &NopLogger {
64+
fn write_str(&mut self, s: &str) -> std::fmt::Result {
65+
black_box(s);
66+
Ok(())
67+
}
68+
}
69+
70+
const N_SPANS: usize = 100;
71+
72+
fn criterion_benchmark(c: &mut Criterion) {
73+
let mut c = c.benchmark_group("global/subscriber");
74+
let _ = tracing::subscriber::set_global_default(EnabledSubscriber);
75+
let _ = log::set_logger(&NOP_LOGGER);
76+
log::set_max_level(log::LevelFilter::Trace);
77+
c.bench_function("span_no_fields", |b| b.iter(|| span!(Level::TRACE, "span")));
78+
79+
c.bench_function("event", |b| {
80+
b.iter(|| {
81+
tracing::event!(Level::TRACE, "hello");
82+
})
83+
});
84+
85+
c.bench_function("enter_span", |b| {
86+
let span = span!(Level::TRACE, "span");
87+
#[allow(clippy::unit_arg)]
88+
b.iter(|| black_box(span.in_scope(|| {})))
89+
});
90+
91+
c.bench_function("span_repeatedly", |b| {
92+
#[inline]
93+
fn mk_span(i: u64) -> tracing::Span {
94+
span!(Level::TRACE, "span", i = i)
95+
}
96+
97+
let n = black_box(N_SPANS);
98+
b.iter(|| (0..n).fold(mk_span(0), |_, i| mk_span(i as u64)))
99+
});
100+
101+
c.bench_function("span_with_fields", |b| {
102+
b.iter(|| {
103+
span!(
104+
Level::TRACE,
105+
"span",
106+
foo = "foo",
107+
bar = "bar",
108+
baz = 3,
109+
quuux = tracing::field::debug(0.99)
110+
)
111+
});
112+
});
113+
}
114+
115+
fn bench_dispatch(c: &mut Criterion) {
116+
let mut group = c.benchmark_group("global/dispatch");
117+
let _ = tracing::subscriber::set_global_default(EnabledSubscriber);
118+
let _ = log::set_logger(&NOP_LOGGER);
119+
log::set_max_level(log::LevelFilter::Trace);
120+
group.bench_function("get_ref", |b| {
121+
b.iter(|| {
122+
tracing::dispatcher::get_default(|current| {
123+
black_box(&current);
124+
})
125+
})
126+
});
127+
group.bench_function("get_clone", |b| {
128+
b.iter(|| {
129+
let current = tracing::dispatcher::get_default(|current| current.clone());
130+
black_box(current);
131+
})
132+
});
133+
group.finish();
134+
}
135+
136+
criterion_group!(benches, criterion_benchmark, bench_dispatch);
137+
criterion_main!(benches);

tracing/src/lib.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,68 @@ pub mod __macro_support {
10891089
.finish()
10901090
}
10911091
}
1092+
1093+
#[cfg(feature = "log")]
1094+
use tracing_core::field::{Field, ValueSet, Visit};
1095+
1096+
/// Utility to format [`ValueSet`] for logging, used by macro-generated code.
1097+
///
1098+
/// /!\ WARNING: This is *not* a stable API! /!\
1099+
/// This type, and all code contained in the `__macro_support` module, is
1100+
/// a *private* API of `tracing`. It is exposed publicly because it is used
1101+
/// by the `tracing` macros, but it is not part of the stable versioned API.
1102+
/// Breaking changes to this module may occur in small-numbered versions
1103+
/// without warning.
1104+
#[cfg(feature = "log")]
1105+
#[allow(missing_debug_implementations)]
1106+
pub struct LogValueSet<'a>(pub &'a ValueSet<'a>);
1107+
1108+
#[cfg(feature = "log")]
1109+
impl<'a> fmt::Display for LogValueSet<'a> {
1110+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1111+
let mut visit = LogVisitor {
1112+
f,
1113+
is_first: true,
1114+
result: Ok(()),
1115+
};
1116+
self.0.record(&mut visit);
1117+
visit.result
1118+
}
1119+
}
1120+
1121+
#[cfg(feature = "log")]
1122+
struct LogVisitor<'a, 'b> {
1123+
f: &'a mut fmt::Formatter<'b>,
1124+
is_first: bool,
1125+
result: fmt::Result,
1126+
}
1127+
1128+
#[cfg(feature = "log")]
1129+
impl Visit for LogVisitor<'_, '_> {
1130+
fn record_debug(&mut self, field: &Field, value: &dyn fmt::Debug) {
1131+
let res = if self.is_first {
1132+
self.is_first = false;
1133+
if field.name() == "message" {
1134+
write!(self.f, "{:?}", value)
1135+
} else {
1136+
write!(self.f, "{}={:?}", field.name(), value)
1137+
}
1138+
} else {
1139+
write!(self.f, " {}={:?}", field.name(), value)
1140+
};
1141+
if let Err(err) = res {
1142+
self.result = self.result.and(Err(err));
1143+
}
1144+
}
1145+
1146+
fn record_str(&mut self, field: &Field, value: &str) {
1147+
if field.name() == "message" {
1148+
self.record_debug(field, &format_args!("{}", value))
1149+
} else {
1150+
self.record_debug(field, &value)
1151+
}
1152+
}
1153+
}
10921154
}
10931155

10941156
mod sealed {

0 commit comments

Comments
 (0)