Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions changelog.d/config_reload_panic_fix.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed a panic in the tracing rate limiter when config reload failed. While the panic didn't kill Vector (it was caught by tokio's task
runtime), it could cause unexpected behavior. The rate limiter now gracefully handles events without standard message fields.

authors: pront
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
large-error-threshold = 256 # in bytes
allow-unwrap-in-tests = true

# for `disallowed_method`:
# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
Expand Down
3 changes: 3 additions & 0 deletions lib/tracing-limit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ edition = "2024"
publish = false
license = "MPL-2.0"

[lints.clippy]
unwrap-used = "forbid"

[dependencies]
tracing-core = { version = "0.1", default-features = false }
tracing-subscriber = { workspace = true, features = ["registry", "std"] }
Expand Down
6 changes: 3 additions & 3 deletions lib/tracing-limit/benches/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,17 @@ where
}

fn on_new_span(&self, span: &span::Attributes<'_>, _id: &span::Id, _ctx: Context<'_, S>) {
let mut visitor = Visitor(self.mutex.lock().unwrap());
let mut visitor = Visitor(self.mutex.lock().expect("mutex should not be poisoned"));
span.record(&mut visitor);
}

fn on_record(&self, _id: &span::Id, values: &span::Record<'_>, _ctx: Context<'_, S>) {
let mut visitor = Visitor(self.mutex.lock().unwrap());
let mut visitor = Visitor(self.mutex.lock().expect("mutex should not be poisoned"));
values.record(&mut visitor);
}

fn on_event(&self, event: &Event<'_>, _ctx: Context<'_, S>) {
let mut visitor = Visitor(self.mutex.lock().unwrap());
let mut visitor = Visitor(self.mutex.lock().expect("mutex should not be poisoned"));
event.record(&mut visitor);
}

Expand Down
50 changes: 45 additions & 5 deletions lib/tracing-limit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,15 +377,17 @@ where
let valueset = fields.value_set(&values);
let event = Event::new(metadata, &valueset);
self.inner.on_event(&event, ctx.clone());
} else {
let values = [(
&fields.field(RATE_LIMIT_FIELD).unwrap(),
Some(&rate_limit as &dyn Value),
)];
} else if let Some(rate_limit_field) = fields.field(RATE_LIMIT_FIELD) {
let values = [(&rate_limit_field, Some(&rate_limit as &dyn Value))];

let valueset = fields.value_set(&values);
let event = Event::new(metadata, &valueset);
self.inner.on_event(&event, ctx.clone());
} else {
// If the event metadata has neither a "message" nor "internal_log_rate_limit" field,
// we cannot create a proper synthetic event. This can happen with custom debug events
// that have their own field structure. In this case, we simply skip emitting the
// rate limit notification rather than panicking.
}
}
}
Expand Down Expand Up @@ -1012,4 +1014,42 @@ mod test {
]
);
}

#[test]
#[serial]
fn events_with_custom_fields_no_message_dont_panic() {
// Verify events without "message" or "internal_log_rate_limit" fields don't panic
// when rate limiting skips suppression notifications.
let (events, sub) = setup_test(1);
tracing::subscriber::with_default(sub, || {
// Use closure to ensure all events share the same callsite
let emit_event = || {
debug!(component_id = "test_component", utilization = 0.85);
};

// First window: emit 5 events, only the first one should be logged
for _ in 0..5 {
emit_event();
MockClock::advance(Duration::from_millis(100));
}

// Advance to the next window
MockClock::advance(Duration::from_millis(1000));

// Second window: this event should be logged
emit_event();
});

let events = events.lock().unwrap();

// First event from window 1, first event from window 2
// Suppression notifications are skipped (no message field)
assert_eq!(
*events,
vec![
event!("", component_id: "test_component", utilization: "0.85"),
event!("", component_id: "test_component", utilization: "0.85"),
]
);
}
}
Loading