feat(logs): add log4net integration#5172
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5172 +/- ##
==========================================
+ Coverage 74.13% 74.20% +0.06%
==========================================
Files 508 510 +2
Lines 18282 18323 +41
Branches 3574 3587 +13
==========================================
+ Hits 13553 13596 +43
- Misses 3860 3861 +1
+ Partials 869 866 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| log.TryGetAttribute("sentry.environment", out object? environment).Should().BeTrue(); | ||
| environment.Should().Be("test-environment"); | ||
| log.TryGetAttribute("sentry.release", out object? release).Should().BeTrue(); | ||
| release.Should().Be("test-release"); | ||
| log.TryGetAttribute("sentry.origin", out object? origin).Should().BeTrue(); | ||
| origin.Should().Be("auto.log.log4net"); | ||
| log.TryGetAttribute("sentry.sdk.name", out object? sdkName).Should().BeTrue(); | ||
| sdkName.Should().Be(SentryAppender.SdkName); | ||
| log.TryGetAttribute("sentry.sdk.version", out object? sdkVersion).Should().BeTrue(); | ||
| sdkVersion.Should().Be(SentryAppender.NameAndVersion.Version); |
There was a problem hiding this comment.
suggestion: reference project Sentry.Testing and simplify assertions with extensions added in #4936
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ca31e0b. Configure here.
| const string? template = null; // cannot get format-string from `log4net.Util.SystemStringFormat` via `LoggingEvent.MessageObject` | ||
| var parameters = ImmutableArray<KeyValuePair<string, object>>.Empty; // cannot get arguments from `log4net.Util.SystemStringFormat` via `LoggingEvent.MessageObject` | ||
|
|
||
| var log = SentryLog.Create(hub, timestamp, level.Value, loggingEvent.RenderedMessage, template, parameters); |
There was a problem hiding this comment.
Missing null check for RenderedMessage in structured logging
Medium Severity
loggingEvent.RenderedMessage is passed directly to SentryLog.Create as a non-nullable string message parameter, but it can return null in log4net (e.g., when no message object is set). The existing code in the same class defensively handles this — line 103 uses !string.IsNullOrWhiteSpace(loggingEvent.RenderedMessage) and falls back to string.Empty. The new structured logging path lacks this guard, which could result in a null value flowing into SentryLog.Message and causing issues during serialization.
Reviewed by Cursor Bugbot for commit ca31e0b. Configure here.
| { | ||
| _fixture.Hub.GetSpan().Returns((ISpan?)null); | ||
| } | ||
|
|
There was a problem hiding this comment.
ThreadContext.Properties not cleaned up after test, polluting subsequent tests
The four ThreadContext.Properties entries set here are never removed, which can cause DoAppend_StructuredLogging_Properties (asserts ContainSingle) and DoAppend_StructuredLogging_DefaultProperties (asserts NotContain) to fail when they run on the same thread afterward.
Verification
Checked SentryAppender.Structured.cs: the implementation calls loggingEvent.GetProperties() which in log4net merges the event's fixed properties with the active ThreadContext. DoAppend_StructuredLogging_DefaultProperties (line 155) constructs a LoggingEvent without setting Properties, so GetProperties() will return any live ThreadContext entries; the test then asserts NotContain(attribute => attribute.Key.StartsWith("property.")). DoAppend_StructuredLogging_Properties (line 120) sets explicit event-level properties and asserts ContainSingle(attribute => attribute.Key.StartsWith("property.")). If the four keys ('Text-Property', 'Number-Property', etc.) are still in ThreadContext those assertions fail. Neither the test body nor Dispose() in SentryAppenderTests clears these entries.
Identified by Warden code-review · 4QD-UFE


closes #4731
Changes
Add
log4netintegration to Sentry Structured Logging.Docs