Skip to content

Commit f1cf1f1

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 e8636c5 commit f1cf1f1

File tree

5 files changed

+287
-164
lines changed

5 files changed

+287
-164
lines changed

Diff for: tracing-core/src/field.rs

+2-2
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 {

Diff for: tracing/benches/global_subscriber.rs

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

Diff for: tracing/src/lib.rs

+62
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)