Skip to content

feat(logs): add log4net integration#5172

Open
Flash0ver wants to merge 7 commits into
mainfrom
feat/logs-log4net
Open

feat(logs): add log4net integration#5172
Flash0ver wants to merge 7 commits into
mainfrom
feat/logs-log4net

Conversation

@Flash0ver
Copy link
Copy Markdown
Member

closes #4731

Changes

Add log4net integration to Sentry Structured Logging.

Docs

  • TODO

@Flash0ver Flash0ver self-assigned this Apr 29, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

❌ Patch coverage is 92.68293% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.20%. Comparing base (7deff34) to head (ca31e0b).

Files with missing lines Patch % Lines
src/Sentry.Log4Net/LevelMapping.cs 75.00% 1 Missing and 2 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Flash0ver Flash0ver marked this pull request as ready for review May 18, 2026 11:19
@Flash0ver Flash0ver requested a review from jamescrosswell as a code owner May 18, 2026 11:19
Comment on lines +103 to +112
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);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

suggestion: reference project Sentry.Testing and simplify assertions with extensions added in #4936

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ca31e0b. Configure here.

Comment on lines +82 to +85
{
_fixture.Hub.GetSpan().Returns((ISpan?)null);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

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.

Add Sentry Logs support to Log4Net

2 participants