Skip to content

Commit 9be7010

Browse files
committed
Tracing speedup
Added a callsite filter for the simple event layer. This allows us to avoid the lock + map lookup for non-app-services crates. I think this is about as good as we can do performance-wise: * Non app-services crates are statically disabled, no lock/map lookup needed * App-services crates are dynamically handled. We need to do the lock/map lookup each time, since we don't know when outside crates will call `register_event_sink`.
1 parent 5358a19 commit 9be7010

File tree

3 files changed

+140
-10
lines changed

3 files changed

+140
-10
lines changed

components/support/rust-log-forwarder/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ pub fn set_logger(logger: Option<Box<dyn AppServicesLogger>>) {
9696
GLOBAL_SUBSCRIBER.call_once(|| {
9797
use tracing_subscriber::prelude::*;
9898
tracing_subscriber::registry()
99-
.with(tracing_support::SimpleEventLayer)
99+
.with(tracing_support::simple_event_layer())
100100
.init();
101101
});
102102

@@ -182,8 +182,8 @@ mod test {
182182
let sink = Arc::new(ForwarderEventSink {});
183183
tracing_support::register_event_sink("rust_log_forwarder", Level::Debug.into(), sink);
184184

185-
tracing::event!(tracing::Level::INFO, "Test message");
186-
tracing::event!(tracing::Level::WARN, "Test message2");
185+
tracing_support::info!("Test message");
186+
tracing_support::warn!("Test message2");
187187
logger.check_records(vec![
188188
Record {
189189
level: Level::Info,

components/support/tracing/src/layer.rs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55
use parking_lot::RwLock;
66
use std::collections::{BTreeMap, HashMap};
77
use std::sync::{Arc, LazyLock};
8-
use tracing_subscriber::Layer;
8+
use tracing::{subscriber::Interest, Metadata};
9+
use tracing_subscriber::{
10+
layer::{Context, Filter},
11+
Layer,
12+
};
913

1014
use crate::EventSink;
1115
use tracing::field::{Field, Visit};
@@ -42,6 +46,13 @@ pub fn unregister_event_sink(target: &str) {
4246
SINKS_BY_TARGET.write().remove(target);
4347
}
4448

49+
pub fn simple_event_layer<S>() -> impl Layer<S>
50+
where
51+
S: tracing::Subscriber + for<'a> tracing_subscriber::registry::LookupSpan<'a>,
52+
{
53+
SimpleEventLayer.with_filter(SimpleEventFilter)
54+
}
55+
4556
pub struct SimpleEventLayer;
4657

4758
impl<S> Layer<S> for SimpleEventLayer
@@ -78,6 +89,33 @@ where
7889
}
7990
}
8091

92+
struct SimpleEventFilter;
93+
94+
impl<S> Filter<S> for SimpleEventFilter
95+
where
96+
S: tracing::Subscriber,
97+
{
98+
fn callsite_enabled(&self, meta: &Metadata<'_>) -> Interest {
99+
if meta.fields().field("tracing_support").is_some() {
100+
// Event came from `tracing_support`'s logging macros.
101+
// Enable the layer for this callsite.
102+
// Whether we actually do anything for an event is controlled by `SimpleEventLayer.on_event()`
103+
Interest::always()
104+
} else {
105+
// Event came from a crate not using `tracing_support`, we don't want to handle it.
106+
// By returning `Interest::never`, tracing_subscriber will ensure that this callsite is
107+
// permanently filtered and we avoid the lock + map lookup.
108+
// In fact, `callsite_enabled` won't even be called again.
109+
Interest::never()
110+
}
111+
}
112+
113+
fn enabled(&self, _meta: &Metadata<'_>, _cx: &Context<'_, S>) -> bool {
114+
// callsite_enabled() does the real filtering, we just return `true` here.
115+
true
116+
}
117+
}
118+
81119
// from https://burgers.io/custom-logging-in-rust-using-tracing
82120
struct JsonVisitor<'a>(&'a mut String, &'a mut BTreeMap<String, serde_json::Value>);
83121

components/support/tracing/src/lib.rs

Lines changed: 98 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,91 @@ mod testing;
66
#[cfg(feature = "testing")]
77
pub use testing::{init_for_tests, init_for_tests_with_level};
88

9-
pub use layer::{register_event_sink, unregister_event_sink, SimpleEventLayer};
10-
pub use tracing::{debug, error, info, trace, warn};
9+
pub use layer::{register_event_sink, simple_event_layer, unregister_event_sink};
10+
// Re-export tracing so that our dependencies can use it.
11+
pub use tracing;
12+
13+
// Define standard logging macros.
14+
//
15+
// These all add `tracing_support = true`, which we use as an event filter in our layer.
16+
// This will statically disable the layer for events from outside crates that use tracing.
17+
// This way we don't pay a performance penalty for those events.
18+
// See `SimpleEventFilter` for details.
19+
20+
#[macro_export]
21+
macro_rules! trace {
22+
(target: $target:expr, $($tt:tt)*) => {
23+
$crate::tracing::trace!(
24+
target: $target,
25+
tracing_support = true,
26+
$($tt)*)
27+
};
28+
($($tt:tt)*) => {
29+
$crate::tracing::trace!(
30+
tracing_support = true,
31+
$($tt)*)
32+
};
33+
}
34+
35+
#[macro_export]
36+
macro_rules! debug {
37+
(target: $target:expr, $($tt:tt)*) => {
38+
$crate::tracing::debug!(
39+
target: $target,
40+
tracing_support = true,
41+
$($tt)*)
42+
};
43+
($($tt:tt)*) => {
44+
$crate::tracing::debug!(
45+
tracing_support = true,
46+
$($tt)*)
47+
};
48+
}
49+
50+
#[macro_export]
51+
macro_rules! info {
52+
(target: $target:expr, $($tt:tt)*) => {
53+
$crate::tracing::info!(
54+
target: $target,
55+
tracing_support = true,
56+
$($tt)*)
57+
};
58+
($($tt:tt)*) => {
59+
$crate::tracing::info!(
60+
tracing_support = true,
61+
$($tt)*)
62+
};
63+
}
64+
65+
#[macro_export]
66+
macro_rules! warn {
67+
(target: $target:expr, $($tt:tt)*) => {
68+
$crate::tracing::warn!(
69+
target: $target,
70+
tracing_support = true,
71+
$($tt)*)
72+
};
73+
($($tt:tt)*) => {
74+
$crate::tracing::warn!(
75+
tracing_support = true,
76+
$($tt)*)
77+
};
78+
}
79+
80+
#[macro_export]
81+
macro_rules! error {
82+
(target: $target:expr, $($tt:tt)*) => {
83+
$crate::tracing::error!(
84+
target: $target,
85+
tracing_support = true,
86+
$($tt)*)
87+
};
88+
($($tt:tt)*) => {
89+
$crate::tracing::error!(
90+
tracing_support = true,
91+
$($tt)*)
92+
};
93+
}
1194

1295
// grr - swift has name collision with `Level`? Can uniifi help make this cleaner?
1396
pub type Level = TracingLevel;
@@ -97,7 +180,7 @@ mod tests {
97180
fn test_app() {
98181
use tracing_subscriber::prelude::*;
99182
tracing_subscriber::registry()
100-
.with(layer::SimpleEventLayer)
183+
.with(layer::simple_event_layer())
101184
.init();
102185

103186
struct Sink {
@@ -120,15 +203,24 @@ mod tests {
120203
let sink = Arc::new(Sink::new());
121204

122205
crate::layer::register_event_sink("first_target", Level::Info, sink.clone());
123-
crate::layer::register_event_sink("second_target", Level::Info, sink.clone());
206+
crate::layer::register_event_sink("second_target", Level::Debug, sink.clone());
124207

125-
tracing::event!(target: "first_target", tracing::Level::INFO, extra = -1, "event message");
208+
info!(target: "first_target", extra=-1, "event message");
209+
debug!(target: "first_target", extra=-2, "event message (should be filtered)");
210+
debug!(target: "second_target", extra=-3, "event message2");
211+
info!(target: "third_target", extra=-4, "event message (should be filtered)");
126212

127-
assert_eq!(sink.events.read().len(), 1);
213+
assert_eq!(sink.events.read().len(), 2);
128214
let event = &sink.events.read()[0];
129215
assert_eq!(event.target, "first_target");
130216
assert_eq!(event.level, Level::Info);
131217
assert_eq!(event.message, "event message");
132218
assert_eq!(event.fields.get("extra").unwrap().as_i64(), Some(-1));
219+
220+
let event2 = &sink.events.read()[1];
221+
assert_eq!(event2.target, "second_target");
222+
assert_eq!(event2.level, Level::Debug);
223+
assert_eq!(event2.message, "event message2");
224+
assert_eq!(event2.fields.get("extra").unwrap().as_i64(), Some(-3));
133225
}
134226
}

0 commit comments

Comments
 (0)