-
-
Notifications
You must be signed in to change notification settings - Fork 223
feat: add Serilog
integration
#4462
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
Conversation
|
@sentry review |
|
||
private void CaptureStructuredLog(IHub hub, LogEvent logEvent, string formatted, string? template) | ||
{ | ||
var traceHeader = hub.GetTraceHeader() ?? SentryTraceHeader.Empty; |
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.
Is the ParentSpanId
required in the protocol for StructuredLogging? An empty SpanId probably isn't that useful otherwise eh?
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.
No ... it's an optional "Default Attribute" ... the trace_id
is required.
Oops ... I made this mistake in the Sentry-SDK and Microsoft.Extensions.Logging integrations, too.
I'll create a follow-up PR to fix this issue in all integrations.
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.
follow-up for both the SDK-Logger and the Microsoft.Extensions.Logging
integration: #4565
Looking forward to when this gets out, would really benefit my team |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4462 +/- ##
==========================================
+ Coverage 73.34% 73.45% +0.10%
==========================================
Files 479 480 +1
Lines 17509 17589 +80
Branches 3445 3469 +24
==========================================
+ Hits 12842 12920 +78
+ Misses 3788 3785 -3
- Partials 879 884 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 left a couple of comments, nothing blocking though. LGTM!
Co-authored-by: James Crosswell <[email protected]>
Woohoo, thank you again for your amazing efforts out there, @Flash0ver, @jamescrosswell, and team! This will really help out a lot with my projects. 🙏🙏🙏 |
release the kraken, I mean nuget package, please? Also idiot proof (I'm the idiot) docs please ❤️ |
We got 4 Structured-Logs-related fixes in the pipeline, small yet impactful changes, that I'd like to ship in one release. With 3 developers working on these, I'm confident with releasing early next week. Also, docs are ready to be published too (getsentry/sentry-docs#14693). |
Changes
Add
Serilog
integration to Sentry Structured Logging.Docs
Dogfooding
getsentry/symbol-collector
->SymbolCollector.Server