-
-
Notifications
You must be signed in to change notification settings - Fork 167
ref(log): send logs by default when logs feature flag is enabled #915
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
dfb3a39
65f7c97
fb6dc78
79261c2
c5668e7
b79bfd7
e7a8769
0567156
cfbb994
7d96f91
a8a5c4c
979411c
eae136a
0da7c5e
71749b9
53c2165
af75932
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,28 @@ | ||
use log::Record; | ||
use sentry_core::protocol::{Breadcrumb, Event}; | ||
|
||
use bitflags::bitflags; | ||
|
||
#[cfg(feature = "logs")] | ||
use crate::converters::log_from_record; | ||
use crate::converters::{breadcrumb_from_record, event_from_record, exception_from_record}; | ||
|
||
/// The action that Sentry should perform for a [`log::Metadata`]. | ||
#[derive(Debug)] | ||
pub enum LogFilter { | ||
/// Ignore the [`Record`]. | ||
Ignore, | ||
/// Create a [`Breadcrumb`] from this [`Record`]. | ||
Breadcrumb, | ||
/// Create a message [`Event`] from this [`Record`]. | ||
Event, | ||
/// Create an exception [`Event`] from this [`Record`]. | ||
Exception, | ||
/// Create a [`sentry_core::protocol::Log`] from this [`Record`]. | ||
#[cfg(feature = "logs")] | ||
Log, | ||
bitflags! { | ||
/// The action that Sentry should perform for a [`log::Metadata`]. | ||
#[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
pub struct LogFilter: u32 { | ||
/// Ignore the [`Record`]. | ||
const Ignore = 0b0000; | ||
/// Create a [`Breadcrumb`] from this [`Record`]. | ||
const Breadcrumb = 0b0001; | ||
/// Create a message [`Event`] from this [`Record`]. | ||
const Event = 0b0010; | ||
/// Create an exception [`Event`] from this [`Record`]. | ||
const Exception = 0b0100; | ||
/// Create a [`sentry_core::protocol::Log`] from this [`Record`]. | ||
#[cfg(feature = "logs")] | ||
const Log = 0b1000; | ||
} | ||
} | ||
|
||
/// The type of Data Sentry should ingest for a [`log::Record`]. | ||
|
@@ -34,6 +38,29 @@ pub enum RecordMapping { | |
/// Captures the [`sentry_core::protocol::Log`] to Sentry. | ||
#[cfg(feature = "logs")] | ||
Log(sentry_core::protocol::Log), | ||
/// Captures multiple items to Sentry. | ||
/// Nesting multiple `RecordMapping::Combined` is not supported and will cause the mappings to | ||
/// be ignored. | ||
Combined(CombinedRecordMapping), | ||
} | ||
|
||
/// A list of record mappings. | ||
#[derive(Debug)] | ||
pub struct CombinedRecordMapping(Vec<RecordMapping>); | ||
|
||
impl From<RecordMapping> for CombinedRecordMapping { | ||
fn from(value: RecordMapping) -> Self { | ||
match value { | ||
RecordMapping::Combined(combined) => combined, | ||
_ => CombinedRecordMapping(vec![value]), | ||
} | ||
} | ||
} | ||
|
||
|
||
impl From<Vec<RecordMapping>> for CombinedRecordMapping { | ||
fn from(value: Vec<RecordMapping>) -> Self { | ||
Self(value) | ||
} | ||
} | ||
|
||
/// The default log filter. | ||
|
@@ -42,7 +69,13 @@ pub enum RecordMapping { | |
/// `warning` and `info`, and `debug` and `trace` logs are ignored. | ||
pub fn default_filter(metadata: &log::Metadata) -> LogFilter { | ||
match metadata.level() { | ||
#[cfg(feature = "logs")] | ||
log::Level::Error => LogFilter::Exception | LogFilter::Log, | ||
#[cfg(not(feature = "logs"))] | ||
log::Level::Error => LogFilter::Exception, | ||
#[cfg(feature = "logs")] | ||
log::Level::Warn | log::Level::Info => LogFilter::Breadcrumb | LogFilter::Log, | ||
#[cfg(not(feature = "logs"))] | ||
log::Level::Warn | log::Level::Info => LogFilter::Breadcrumb, | ||
log::Level::Debug | log::Level::Trace => LogFilter::Ignore, | ||
} | ||
|
@@ -132,30 +165,50 @@ impl<L: log::Log> SentryLogger<L> { | |
|
||
impl<L: log::Log> log::Log for SentryLogger<L> { | ||
fn enabled(&self, metadata: &log::Metadata<'_>) -> bool { | ||
self.dest.enabled(metadata) || !matches!((self.filter)(metadata), LogFilter::Ignore) | ||
self.dest.enabled(metadata) || !((self.filter)(metadata) == LogFilter::Ignore) | ||
} | ||
|
||
fn log(&self, record: &log::Record<'_>) { | ||
let item: RecordMapping = match &self.mapper { | ||
let items: RecordMapping = match &self.mapper { | ||
Some(mapper) => mapper(record), | ||
None => match (self.filter)(record.metadata()) { | ||
LogFilter::Ignore => RecordMapping::Ignore, | ||
LogFilter::Breadcrumb => RecordMapping::Breadcrumb(breadcrumb_from_record(record)), | ||
LogFilter::Event => RecordMapping::Event(event_from_record(record)), | ||
LogFilter::Exception => RecordMapping::Event(exception_from_record(record)), | ||
None => { | ||
let filter = (self.filter)(record.metadata()); | ||
let mut items = vec![]; | ||
if filter.contains(LogFilter::Breadcrumb) { | ||
items.push(RecordMapping::Breadcrumb(breadcrumb_from_record(record))); | ||
} | ||
if filter.contains(LogFilter::Event) { | ||
items.push(RecordMapping::Event(event_from_record(record))); | ||
} | ||
if filter.contains(LogFilter::Exception) { | ||
items.push(RecordMapping::Event(exception_from_record(record))); | ||
} | ||
#[cfg(feature = "logs")] | ||
LogFilter::Log => RecordMapping::Log(log_from_record(record)), | ||
}, | ||
if filter.contains(LogFilter::Log) { | ||
items.push(RecordMapping::Log(log_from_record(record))); | ||
} | ||
RecordMapping::Combined(CombinedRecordMapping(items)) | ||
} | ||
}; | ||
|
||
match item { | ||
RecordMapping::Ignore => {} | ||
RecordMapping::Breadcrumb(b) => sentry_core::add_breadcrumb(b), | ||
RecordMapping::Event(e) => { | ||
sentry_core::capture_event(e); | ||
let items = CombinedRecordMapping::from(items); | ||
|
||
for item in items.0 { | ||
match item { | ||
RecordMapping::Ignore => {} | ||
RecordMapping::Breadcrumb(breadcrumb) => sentry_core::add_breadcrumb(breadcrumb), | ||
RecordMapping::Event(event) => { | ||
sentry_core::capture_event(event); | ||
} | ||
#[cfg(feature = "logs")] | ||
RecordMapping::Log(log) => { | ||
sentry_core::Hub::with_active(|hub| hub.capture_log(log)) | ||
} | ||
RecordMapping::Combined(_) => { | ||
sentry_core::sentry_debug!( | ||
"[SentryLogger] found nested CombinedEventMapping, ignoring" | ||
) | ||
} | ||
} | ||
#[cfg(feature = "logs")] | ||
RecordMapping::Log(log) => sentry_core::Hub::with_active(|hub| hub.capture_log(log)), | ||
} | ||
|
||
self.dest.log(record) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
#![cfg(feature = "test")] | ||
|
||
// Test `log` integration with combined `LogFilter`s. | ||
// This must be in a separate file because `log::set_boxed_logger` can only be called once. | ||
|
||
#[test] | ||
fn test_log_combined_filters() { | ||
let logger = sentry_log::SentryLogger::new().filter(|md| match md.level() { | ||
log::Level::Error => sentry_log::LogFilter::Breadcrumb | sentry_log::LogFilter::Event, | ||
log::Level::Warn => sentry_log::LogFilter::Event, | ||
_ => sentry_log::LogFilter::Ignore, | ||
}); | ||
|
||
log::set_boxed_logger(Box::new(logger)) | ||
.map(|()| log::set_max_level(log::LevelFilter::Trace)) | ||
.unwrap(); | ||
|
||
let events = sentry::test::with_captured_events(|| { | ||
log::error!("Both a breadcrumb and an event"); | ||
log::warn!("An event"); | ||
log::trace!("Ignored"); | ||
}); | ||
|
||
assert_eq!(events.len(), 2); | ||
|
||
assert_eq!( | ||
events[0].message, | ||
Some("Both a breadcrumb and an event".to_owned()) | ||
); | ||
|
||
assert_eq!(events[1].message, Some("An event".to_owned())); | ||
assert_eq!(events[1].breadcrumbs.len(), 1); | ||
assert_eq!( | ||
events[1].breadcrumbs[0].message, | ||
Some("Both a breadcrumb and an event".into()) | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: SentryLogger Fails to Handle Combined RecordMappings
The
SentryLogger::log
method doesn't handle the newRecordMapping::Combined
variant. This means any mappings wrapped within aCombined
variant are silently ignored, preventing logs from being processed and sent to Sentry.