-
-
Notifications
You must be signed in to change notification settings - Fork 166
feat(logs): add ability to capture and send logs #823
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
Merged
Merged
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
33f85f6
feat(logs): add log protocol types
lcian fc0dbed
remove unnecessary type
lcian a1c5800
lints
lcian e57ccab
remove debug prints
lcian 3c903a0
details
lcian f44b001
docstring
lcian 85bfe34
Merge branch 'master' into lcian/feat/logs-types
lcian ad09fab
add full envelope test
lcian 89b0e16
cargo fmt
lcian 046f5e4
cargo fmt
lcian 7a33e36
changelog
lcian f67fa67
wip
lcian 58098ae
refactor
lcian 8ed9e15
refactor
lcian 4d8e557
remove file
lcian d1fa386
Merge branch 'master' into lcian/feat/logs-types
lcian a6ba441
add from
lcian 51b2470
severity number changes
lcian 7ef7157
changelog
lcian bd6e293
improve
lcian 2cff09d
improve
lcian 1a3a088
improve
lcian e915747
improve
lcian 5c4fde5
feat(logs): add basic API
lcian 855bd1c
honor enable_logs
lcian dce7c0a
nit
lcian 81ac086
move feature to UNSTABLE_logs
lcian 48c25ec
changelog
lcian de8a0df
non-exhaustive
lcian d851cb8
generic from value
lcian a36c769
severity_number optional
lcian b4070c1
simplify test
lcian 85704c0
link to docs in code
lcian 9ce8c2f
merge base and updates
lcian 00af37d
changelog
lcian 31037fe
doc string
lcian a2c39a9
doc string
lcian b6f2556
Merge branch 'lcian/feat/logs-types' into lcian/feat/logs-api
lcian bd5ab3f
update test
lcian 914a414
update test
lcian ba64c78
changelog
lcian 5f0e3aa
Update sentry-core/src/client.rs
lcian 49a4e0c
address feedback
lcian 313b278
use variable
lcian ab64707
Merge branch 'master' into lcian/feat/logs-api
lcian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,11 @@ use std::panic::RefUnwindSafe; | |
use std::sync::{Arc, RwLock}; | ||
use std::time::Duration; | ||
|
||
use rand::random; | ||
#[cfg(feature = "UNSTABLE_logs")] | ||
use crate::protocol::EnvelopeItem; | ||
#[cfg(feature = "release-health")] | ||
use sentry_types::protocol::v7::SessionUpdate; | ||
use crate::protocol::SessionUpdate; | ||
use rand::random; | ||
use sentry_types::random_uuid; | ||
|
||
use crate::constants::SDK_INFO; | ||
|
@@ -18,6 +20,8 @@ use crate::types::{Dsn, Uuid}; | |
#[cfg(feature = "release-health")] | ||
use crate::SessionMode; | ||
use crate::{ClientOptions, Envelope, Hub, Integration, Scope, Transport}; | ||
#[cfg(feature = "UNSTABLE_logs")] | ||
use sentry_types::protocol::v7::{Log, LogAttribute}; | ||
|
||
impl<T: Into<ClientOptions>> From<T> for Client { | ||
fn from(o: T) -> Client { | ||
|
@@ -363,6 +367,83 @@ impl Client { | |
random::<f32>() < rate | ||
} | ||
} | ||
|
||
/// Captures a log and sends it to Sentry. | ||
#[cfg(feature = "UNSTABLE_logs")] | ||
pub fn capture_log(&self, log: Log, scope: &Scope) { | ||
if !self.options().enable_logs { | ||
return; | ||
} | ||
if let Some(ref transport) = *self.transport.read().unwrap() { | ||
if let Some(log) = self.prepare_log(log, scope) { | ||
let mut envelope = Envelope::new(); | ||
let logs: EnvelopeItem = vec![log].into(); | ||
envelope.add_item(logs); | ||
transport.send_envelope(envelope); | ||
} | ||
} | ||
} | ||
|
||
/// Prepares a log to be sent, setting the `trace_id` and other default attributes, and | ||
/// processing it through `before_send_log`. | ||
#[cfg(feature = "UNSTABLE_logs")] | ||
fn prepare_log(&self, mut log: Log, scope: &Scope) -> Option<Log> { | ||
scope.apply_to_log(&mut log, self.options.send_default_pii); | ||
|
||
self.set_log_default_attributes(&mut log); | ||
|
||
if let Some(ref func) = self.options.before_send_log { | ||
log = func(log)?; | ||
} | ||
|
||
Some(log) | ||
} | ||
|
||
#[cfg(feature = "UNSTABLE_logs")] | ||
fn set_log_default_attributes(&self, log: &mut Log) { | ||
if !log.attributes.contains_key("sentry.environment") { | ||
if let Some(environment) = self.options.environment.as_ref() { | ||
log.attributes.insert( | ||
"sentry.sdk.version".to_owned(), | ||
LogAttribute(environment.clone().into()), | ||
); | ||
} | ||
} | ||
|
||
if !log.attributes.contains_key("sentry.release") { | ||
if let Some(release) = self.options.release.as_ref() { | ||
log.attributes.insert( | ||
"sentry.release".to_owned(), | ||
LogAttribute(release.clone().into()), | ||
); | ||
} | ||
} | ||
|
||
if !log.attributes.contains_key("sentry.sdk.name") { | ||
log.attributes.insert( | ||
"sentry.sdk.name".to_owned(), | ||
LogAttribute(self.sdk_info.name.to_owned().into()), | ||
); | ||
} | ||
|
||
if !log.attributes.contains_key("sentry.sdk.version") { | ||
log.attributes.insert( | ||
"sentry.sdk.version".to_owned(), | ||
LogAttribute(self.sdk_info.version.to_owned().into()), | ||
); | ||
} | ||
|
||
// TODO: set OS (and Rust?) context | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We cannot do this at the moment because |
||
|
||
if !log.attributes.contains_key("server.address") { | ||
if let Some(server) = &self.options.server_name { | ||
log.attributes.insert( | ||
"server.address".to_owned(), | ||
LogAttribute(server.clone().into()), | ||
); | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Make this unwind safe. It's not out of the box because of the | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
So this is what all other SDKs are doing? IMO it might be a bit wasteful to copy the same fields into the log event for every single log event.
In the likely case that users are not logging any attributes, this will allocate a hashmap, and clone at least
4 * 2
Strings into it. That is quite some overhead.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.
Yeah that's true. Unfortunately this is what the spec mandates, every single log should have these default attributes.
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.
Do you see any way of optimizing it? @Swatinem
Maybe we could pre populate a Map with some of the default attributes and store it somewhere to reuse it, but we would still need to clone it for every log.
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.
I was thinking about using a more optimized string type (like my own https://docs.rs/smol_buf/latest/smol_buf/) that can avoid clones in favor of refcounting or small string optimization.
That would cut down on memory usage and allocations a bit. Though the hashmap might still be heavyweight, and hard to avoid.
Not sure these things are worth the effort though? Considering that those would appear everywhere in public types, so it would likely cause some pain for users as well.
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.
Yeah, I don't know.
I was also thinking of optimizations using
Cow
s, providing special fields to store the default attributes that would take in aCow
. This way we wouldn't clone.But the special fields would need to be
pub
anyway, because the struct lives insentry-types
.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.
I think we can merge it as is for now and we can always think about optimizing in the future.
However I agree it kinda sucks we need to attach this to every single log.