Skip to content

Commit 11e01b2

Browse files
authored
Bug 2013658 - Register for multiple tracing events (#7202)
Added a new `register_event_sink` method that registers event sinks using the new `EventSinkSpecification` struct. This is general enough that we can use it to implement the older registration methods. This is a breaking change, but Desktop is the only platform that's will be affected. We can resolve the breakage when we do the next vendor.
1 parent fec6486 commit 11e01b2

File tree

5 files changed

+154
-98
lines changed

5 files changed

+154
-98
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
We don't believe this should affect any consumers, since they're were already using the
2222
`tracing-logging` feature and were either also using `tracing-reporting` or not handling error
2323
reporting at all.
24+
* Reworked `register_event_sink` signature to allow it to register an event sink for muliple targets at once.
25+
* Reworked `unregister_event_sink`. It now inputs the return value from `register_event_sink`.
26+
* Removed `register_min_level_event_sink` and `unregister_min_level_event_sink`.
27+
Use the new `register_event_sink` instead.
2428

2529
### Logins
2630
* Opened count method on logins for Android. ([#7207](https://github.com/mozilla/application-services/pull/7207/))

components/support/error/android/src/main/java/mozilla/appservices/errorsupport/RustComponentsErrorTelemetry.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import kotlinx.serialization.Serializable
99
import kotlinx.serialization.decodeFromString
1010
import kotlinx.serialization.json.Json
1111
import mozilla.appservices.tracing.EventSink
12+
import mozilla.appservices.tracing.EventSinkSpecification
13+
import mozilla.appservices.tracing.EventTarget
1214
import mozilla.appservices.tracing.TracingEvent
1315
import mozilla.appservices.tracing.TracingLevel
1416
import mozilla.appservices.tracing.registerEventSink
@@ -28,7 +30,10 @@ public object RustComponentsErrorTelemetry {
2830
*/
2931
fun register() {
3032
Glean.registerPings(Pings)
31-
registerEventSink("app-services-error-reporter", TracingLevel.DEBUG, ErrorEventSink())
33+
val spec = EventSinkSpecification(
34+
targets = listOf(EventTarget("app-services-error-reporter", TracingLevel.DEBUG)),
35+
)
36+
registerEventSink(spec, ErrorEventSink())
3237
}
3338

3439
/**

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44

55
use std::sync::{Arc, Once, OnceLock};
66

7+
use tracing_support::{
8+
register_event_sink, unregister_event_sink, EventSinkId, EventSinkSpecification,
9+
};
10+
11+
static EVENT_SINK_ID: OnceLock<EventSinkId> = OnceLock::new();
712
static MAX_LEVEL: OnceLock<Level> = OnceLock::new();
813
static FOREIGN_LOGGER: OnceLock<Box<dyn AppServicesLogger>> = OnceLock::new();
914
static GLOBAL_SUBSCRIBER: Once = Once::new();
@@ -73,11 +78,19 @@ pub fn set_logger(logger: Option<Box<dyn AppServicesLogger>>) {
7378
let sink = Arc::new(ForwarderEventSink {});
7479
if let Some(logger) = logger {
7580
// Set up a tracing subscriber for crates which use tracing and forward to the foreign log forwarder.
76-
tracing_support::register_min_level_event_sink((*level).into(), sink.clone());
77-
// Set the `FOREIGN_LOGGER` global. If this was called before we just ignore the error.
81+
let event_sink_id = register_event_sink(
82+
EventSinkSpecification {
83+
targets: vec![],
84+
min_level: Some((*level).into()),
85+
},
86+
sink.clone(),
87+
);
88+
// Set the `FOREIGN_LOGGER` and `EVENT_SINK_ID` globals.
89+
// If either was called before we just ignore the error.
7890
FOREIGN_LOGGER.set(logger).ok();
79-
} else {
80-
tracing_support::unregister_min_level_event_sink();
91+
EVENT_SINK_ID.set(event_sink_id).ok();
92+
} else if let Some(event_sink_id) = EVENT_SINK_ID.get() {
93+
unregister_event_sink(*event_sink_id);
8194
}
8295
}
8396

components/support/tracing/src/layer.rs

Lines changed: 89 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2,72 +2,79 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5-
use parking_lot::RwLock;
6-
use std::collections::{BTreeMap, HashMap};
7-
use std::sync::{Arc, LazyLock};
5+
use parking_lot::{const_rwlock, RwLock};
6+
use std::collections::BTreeMap;
7+
use std::sync::{
8+
atomic::{AtomicU32, Ordering},
9+
Arc,
10+
};
811
use tracing::{subscriber::Interest, Metadata};
912
use tracing_subscriber::{
1013
layer::{Context, Filter},
1114
Layer,
1215
};
1316

14-
use crate::EventSink;
17+
use crate::{EventSink, Level};
1518
use tracing::field::{Field, Visit};
1619

17-
struct LogEntry {
18-
level: tracing::Level,
19-
sink: Arc<dyn EventSink>,
20-
}
20+
static SINKS: RwLock<Vec<RegisteredEventSink>> = const_rwlock(Vec::new());
21+
static EVENT_SINK_COUNTER: AtomicU32 = AtomicU32::new(0);
2122

22-
static SINKS_BY_TARGET: LazyLock<RwLock<HashMap<String, LogEntry>>> =
23-
LazyLock::new(|| RwLock::new(HashMap::new()));
23+
#[derive(Clone, Copy, PartialEq, Eq)]
24+
pub struct EventSinkId(u32);
2425

25-
static MIN_LEVEL_SINK: RwLock<Option<LogEntry>> = RwLock::new(None);
26+
uniffi::custom_type!(EventSinkId, u32, {
27+
try_lift: |raw_id| Ok(EventSinkId(raw_id)),
28+
lower: |sink_id| sink_id.0,
29+
});
2630

27-
pub fn register_event_sink(target: &str, level: crate::Level, sink: Arc<dyn EventSink>) {
28-
SINKS_BY_TARGET.write().insert(
29-
target.to_string(),
30-
LogEntry {
31-
level: level.into(),
32-
sink,
33-
},
34-
);
31+
/// Register an event sink using an [EventSinkSpecification]
32+
///
33+
/// Returns an [EventSinkId] that can be used to unregister the sink.
34+
pub fn register_event_sink(spec: EventSinkSpecification, sink: Arc<dyn EventSink>) -> EventSinkId {
35+
let id = EventSinkId(EVENT_SINK_COUNTER.fetch_add(1, Ordering::Relaxed));
36+
SINKS.write().push(RegisteredEventSink { id, spec, sink });
37+
id
3538
}
3639

37-
/// Register an event sink that will receive events based on a minimum level
38-
///
39-
/// If an event's level is at least `level`, then the event will be sent to this sink.
40-
/// If so, sinks registered with `register_event_sink` will still be processed.
41-
///
42-
/// There can only be 1 min-level sink registered at once.
43-
pub fn register_min_level_event_sink(level: crate::Level, sink: Arc<dyn EventSink>) {
44-
*MIN_LEVEL_SINK.write() = Some(LogEntry {
45-
level: level.into(),
46-
sink,
47-
});
40+
struct RegisteredEventSink {
41+
// ID that can be used to unregister this sink
42+
id: EventSinkId,
43+
spec: EventSinkSpecification,
44+
sink: Arc<dyn EventSink>,
4845
}
4946

50-
#[uniffi::export]
51-
pub fn unregister_event_sink(target: &str) {
52-
SINKS_BY_TARGET.write().remove(target);
47+
#[derive(uniffi::Record)]
48+
/// Describes which events to an EventSink
49+
pub struct EventSinkSpecification {
50+
// Send events that match these targets/levels
51+
#[uniffi(default)]
52+
pub targets: Vec<EventTarget>,
53+
// Send events have a `min_level` or above.
54+
#[uniffi(default)]
55+
pub min_level: Option<Level>,
56+
}
57+
58+
#[derive(uniffi::Record, Debug)]
59+
pub struct EventTarget {
60+
pub target: String,
61+
pub level: Level,
5362
}
5463

55-
/// Remove the sink registered with [register_min_level_event_sink], if any.
5664
#[uniffi::export]
57-
pub fn unregister_min_level_event_sink() {
58-
*MIN_LEVEL_SINK.write() = None;
65+
pub fn unregister_event_sink(id: EventSinkId) {
66+
SINKS.write().retain(|info| info.id != id);
5967
}
6068

6169
// UniFFI versions of the registration functions. This input a Box to be compatible with callback
6270
// interfaces
63-
#[uniffi::export(name = "register_event_sink")]
64-
pub fn register_event_sink_box(target: &str, level: crate::Level, sink: Box<dyn EventSink>) {
65-
register_event_sink(target, level, sink.into())
66-
}
6771

68-
#[uniffi::export(name = "register_min_level_event_sink")]
69-
pub fn register_min_level_event_sink_box(level: crate::Level, sink: Box<dyn EventSink>) {
70-
register_min_level_event_sink(level, sink.into())
72+
#[uniffi::export(name = "register_event_sink")]
73+
pub fn register_event_sink_box(
74+
targets: EventSinkSpecification,
75+
sink: Box<dyn EventSink>,
76+
) -> EventSinkId {
77+
register_event_sink(targets, sink.into())
7178
}
7279

7380
pub fn simple_event_layer<S>() -> impl Layer<S>
@@ -88,43 +95,59 @@ where
8895
event: &tracing::Event<'_>,
8996
_ctx: tracing_subscriber::layer::Context<'_, S>,
9097
) {
91-
let target = event.metadata().target();
92-
let prefix = match target.find(':') {
93-
Some(index) => &target[..index],
94-
None => target,
95-
};
96-
if let Some(entry) = &*MIN_LEVEL_SINK.read() {
97-
if entry.level >= *event.metadata().level() {
98-
entry.send_event(event);
99-
}
98+
let sinks = find_sinks_for_event(event);
99+
if sinks.is_empty() {
100+
// return early to skip the conversion below
101+
return;
100102
}
101-
102-
if let Some(entry) = SINKS_BY_TARGET.read().get(prefix) {
103-
let level = *event.metadata().level();
104-
if level <= entry.level {
105-
entry.send_event(event);
106-
}
107-
}
108-
}
109-
}
110-
111-
impl LogEntry {
112-
fn send_event(&self, event: &tracing::Event<'_>) {
113103
let mut fields = BTreeMap::new();
114104
let mut message = String::default();
115105
let mut visitor = JsonVisitor(&mut message, &mut fields);
116106
event.record(&mut visitor);
117-
let event = crate::Event {
107+
let tracing_event = crate::Event {
118108
level: (*event.metadata().level()).into(),
119109
target: event.metadata().target().to_string(),
120110
name: event.metadata().name().to_string(),
121111
message,
122112
fields: serde_json::to_value(&fields).unwrap_or_default(),
123113
};
124-
self.sink.on_event(event);
114+
for sink in sinks {
115+
sink.on_event(tracing_event.clone());
116+
}
125117
}
126118
}
127119

120+
/// Find event sinks that match an event.
121+
fn find_sinks_for_event(event: &tracing::Event<'_>) -> Vec<Arc<dyn EventSink>> {
122+
let target = event.metadata().target();
123+
let prefix = match target.find(':') {
124+
Some(index) => &target[..index],
125+
None => target,
126+
};
127+
let level = Level::from(*event.metadata().level());
128+
129+
// This requires a iterating through the entire SINKS vec, which could have performance impacts
130+
// if we have many sinks registered. In practice, there should only be a handful of these so
131+
// this should be fine.
132+
SINKS
133+
.read()
134+
.iter()
135+
.filter_map(|info| {
136+
if let Some(min_level) = &info.spec.min_level {
137+
if *min_level >= level {
138+
return Some(info.sink.clone());
139+
}
140+
}
141+
for target in info.spec.targets.iter() {
142+
if target.target == prefix && target.level >= level {
143+
return Some(info.sink.clone());
144+
}
145+
}
146+
None
147+
})
148+
.collect()
149+
}
150+
128151
struct SimpleEventFilter;
129152

130153
impl SimpleEventFilter {

components/support/tracing/src/lib.rs

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ pub use filters::{
1212
};
1313

1414
pub use layer::{
15-
register_event_sink, register_min_level_event_sink, simple_event_layer, unregister_event_sink,
16-
unregister_min_level_event_sink,
15+
register_event_sink, simple_event_layer, unregister_event_sink, EventSinkId,
16+
EventSinkSpecification, EventTarget,
1717
};
1818
// Re-export tracing so that our dependencies can use it.
1919
pub use tracing;
@@ -104,7 +104,7 @@ macro_rules! error {
104104
pub type Level = TracingLevel;
105105

106106
// `tracing::Level` is a struct, we want an enum for both uniffi and `log::Level`` compat.
107-
#[derive(uniffi::Enum, Copy, Clone, Debug, PartialEq, Eq)]
107+
#[derive(uniffi::Enum, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
108108
pub enum TracingLevel {
109109
Error,
110110
Warn,
@@ -146,7 +146,7 @@ impl From<Level> for tracing::Level {
146146
// ditto grr re swift name collisions.
147147
pub type Event = TracingEvent;
148148

149-
#[derive(uniffi::Record, Debug)]
149+
#[derive(uniffi::Record, Clone, Debug, PartialEq, Eq)]
150150
pub struct TracingEvent {
151151
pub level: Level,
152152
pub target: String,
@@ -205,26 +205,37 @@ mod tests {
205205
self.events.write().push(event);
206206
}
207207
}
208-
let sink = Arc::new(Sink::new());
209-
let level_sink = Arc::new(Sink::new());
210208

211-
crate::layer::register_event_sink("first_target", Level::Info, sink.clone());
212-
crate::layer::register_event_sink("second_target", Level::Debug, sink.clone());
209+
let sink = Arc::new(Sink::new());
213210

214-
// Only 1 sink can be registered with `register_min_level_event_sink`. The first call
215-
// should be ignored and only the second call should take effect.
216-
crate::layer::register_min_level_event_sink(Level::Warn, sink.clone());
217-
crate::layer::register_min_level_event_sink(Level::Error, level_sink.clone());
211+
// Register an event sink
212+
crate::layer::register_event_sink(
213+
EventSinkSpecification {
214+
targets: vec![
215+
EventTarget {
216+
target: "first_target".into(),
217+
level: Level::Info,
218+
},
219+
EventTarget {
220+
target: "second_target".into(),
221+
level: Level::Debug,
222+
},
223+
],
224+
min_level: Some(Level::Warn),
225+
},
226+
sink.clone(),
227+
);
218228

219229
info!(target: "first_target", extra=-1, "event message");
220230
debug!(target: "first_target", extra=-2, "event message (should be filtered)");
221231
debug!(target: "second_target::submodule", extra=-3, "event message2");
222-
info!(target: "third_target", extra=-4, "event message (should be filtered)");
223-
// This should go to both the level sink and target sink, since it's an error
224-
error!(target: "first_target", extra=-5, "event message");
232+
warn!(target: "third_target", extra=-4, "event message3");
233+
info!(target: "third_target", extra=-5, "event message (should be filtered)");
234+
// Matches both for first_target and for the min-level, it should only be received once by
235+
// the sink
236+
error!(target: "first_target", extra=-6, "event message4");
225237

226-
assert_eq!(sink.events.read().len(), 3);
227-
assert_eq!(level_sink.events.read().len(), 1);
238+
assert_eq!(sink.events.read().len(), 4);
228239

229240
let event = &sink.events.read()[0];
230241
assert_eq!(event.target, "first_target");
@@ -239,15 +250,15 @@ mod tests {
239250
assert_eq!(event2.fields.get("extra").unwrap().as_i64(), Some(-3));
240251

241252
let event3 = &sink.events.read()[2];
242-
assert_eq!(event3.target, "first_target");
243-
assert_eq!(event3.level, Level::Error);
244-
assert_eq!(event3.message, "event message");
245-
assert_eq!(event3.fields.get("extra").unwrap().as_i64(), Some(-5));
246-
247-
let level_event = &level_sink.events.read()[0];
248-
assert_eq!(level_event.target, "first_target");
249-
assert_eq!(level_event.level, Level::Error);
250-
assert_eq!(level_event.message, "event message");
251-
assert_eq!(level_event.fields.get("extra").unwrap().as_i64(), Some(-5));
253+
assert_eq!(event3.target, "third_target");
254+
assert_eq!(event3.level, Level::Warn);
255+
assert_eq!(event3.message, "event message3");
256+
assert_eq!(event3.fields.get("extra").unwrap().as_i64(), Some(-4));
257+
258+
let event4 = &sink.events.read()[3];
259+
assert_eq!(event4.target, "first_target");
260+
assert_eq!(event4.level, Level::Error);
261+
assert_eq!(event4.message, "event message4");
262+
assert_eq!(event4.fields.get("extra").unwrap().as_i64(), Some(-6));
252263
}
253264
}

0 commit comments

Comments
 (0)