Skip to content

Add logger and category from Serilog SourceContext. #316

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

Conversation

krisztiankocsis
Copy link
Contributor

Logger and category (breadcrumbs) are lost when using Serilog extension. This PR adds support for them to store into the event and breadcrumbs.

@codecov-io
Copy link

codecov-io commented Dec 1, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@a169fcd). Click here to learn what that means.
The diff coverage is 91.22%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #316   +/-   ##
========================================
  Coverage          ?   92.1%           
========================================
  Files             ?      82           
  Lines             ?    2115           
  Branches          ?       0           
========================================
  Hits              ?    1948           
  Misses            ?     167           
  Partials          ?       0
Impacted Files Coverage Δ
.../Sentry.AspNetCore/SentryAspNetCoreOptionsSetup.cs 100% <ø> (ø)
...grations/AppDomainUnhandledExceptionIntegration.cs 100% <ø> (ø)
src/Sentry.AspNetCore/SentryMiddleware.cs 98.55% <ø> (ø)
...ry/Extensibility/DefaultRequestPayloadExtractor.cs 100% <100%> (ø)
src/Sentry.NLog/SentryNLogOptions.cs 100% <100%> (ø)
src/Sentry.Serilog/SentrySink.cs 83.95% <100%> (ø)
src/Sentry.NLog/SentryNLogUser.cs 100% <100%> (ø)
.../Sentry.AspNetCore/ApplicationBuilderExtensions.cs 93.33% <100%> (ø)
src/Sentry/Internal/AppDomainAdapter.cs 85.71% <100%> (ø)
src/Sentry/Internal/JsonSerializer.cs 91.66% <100%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a169fcd...b727137. Read the comment docs.

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your addition.
Could you please add some tests to verify the behavior?

@krisztiankocsis
Copy link
Contributor Author

Sure, I'll in the weekend.

@bruno-garcia
Copy link
Member

Thanks for you PR and sorry for the delay.
Left a single comment before we can get this merged.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-Authored-By: Bruno Garcia <bruno@brunogarcia.com>
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix the build and add the test later.
Tks

@bruno-garcia
Copy link
Member

Actually I just noticed you commited the suggestion but it involved getting rid of the logger variable altogether and replacing all occurrences with context.

Could you please fix that so it compiles?

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete logger and use context so it compiles

@bruno-garcia
Copy link
Member

Thanks!

@bruno-garcia bruno-garcia merged commit ead4119 into getsentry:master Feb 7, 2020
@krisztiankocsis krisztiankocsis deleted the feat/serilog-category branch February 7, 2020 16:25
@bruno-garcia
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants