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.
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add Serilog integration #118
Add Serilog integration #118
Changes from all commits
e5e6895
6ce7f19
fb53d8b
f5d8f57
6289801
ed128f1
d564ba9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Looks like
Emit
is simply raising an event to Sentry with any log message created.It would be nicer to enable a support like the one done for
Microsoft.Extensions.Logging
which have 2 log levels switches. One controls what is sent to sentry (creating an event) and one controls what should be added as breadcrumb.This is great because we can store logs messages like
Information
as breadcrumbs and in the event of aLogError
all the Information from that same scope are sent out with the the error event.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.
Not entierly accurate. If you take a look here:
sentry-dotnet/src/Sentry.Serilog/SentrySinkExtensions.cs
Lines 12 to 19 in 6289801
The
restrictedToMinimumLevel
option sets the minimum level for whichEmit
is called by Serilog. I'm not really sure what the best way would be to have a minimum breadcrumb level, since Serilog will not callEmit
if the aforementioned option is, for example, set to Error and the miniumum breadcrumb level is set to Info.We could override
restrictedToMinimumLevel
and set it to the lowest level and then add our own switch that checks it withinEmit
, but that seems to be a bit counterintuitive I think.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.
restrictedToMinimumLevel
is the minimal level to either event or breadcrumb then.For example:
restrictedToMinimumLevel
=Info
, we would capture breadcrumbs (for Info or higher) and send events (error or higher)If
restrictedToMinimumLevel
is Warning, we'd only capture breadcrumbs for Warnings or higher, and send events Error or higher.Finally if
restrictedToMinimumLevel
is Error, then breadcrumbs would be tracked only for Error or higher as well as sending events.A logging integration with Sentry that sends all
Log..
to Sentry doesn't make much sense. Sentry is not a logging tool and flooding it with events that are not grouped together create a storm of noise (i.e: Notifications).Keeping the Info/Warn logs around (breadcrumbs) for a while to be sent in case there's an Error is, in my opinion, the best approach as it gives you the context you need to troubleshoot an issue.