Repository navigation
fix(log4net): send event properties as log attributes on log4net 3.x - #5559
Merged
Merged
Conversation
log4net 3.x added IDictionary<string, object?> to PropertiesDictionary, which changed what its non-generic IEnumerable yields: KeyValuePair instead of DictionaryEntry. The appender is compiled against the 2.0.12 floor and walks that non-generic enumerator, so its `is DictionaryEntry` pattern silently matched nothing on 3.x and no property.* attribute was ever set. Read through GetKeys()/the indexer instead, as the sibling SentryEvent path already does - that API behaves identically on both majors, so the appender keeps working whether the consumer resolves log4net 2.x or 3.x and the package floor stays at 2.0.12. Adds Sentry.Log4Net.V3.Tests, which links the same appender tests and runs them against log4net 3.4.0. Without this fix it reproduces exactly the three reported failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The .slnf files are generated by scripts/generate-solution-filters.ps1, and Sentry-CI-Build-Linux-NoMobile.slnf is based on the committed .generated.NoMobile.slnx rather than Sentry.slnx. Without regenerating it, the Linux legs failed to restore with MSB4025: the filter included a project that was not in the solution. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jamescrosswell
commented
Sep 10, 2026
The project name says what it is, and the rationale for it lives in the PR description. Also tightens the appender comment to state the constraint (the enumerator differs between majors) rather than narrate the bug that exposed it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sentry.Log4Net.V3.Tests is the regression mechanism. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5559 +/- ##
=======================================
Coverage 74.75% 74.76%
=======================================
Files 515 515
Lines 18963 18963
Branches 3694 3694
=======================================
+ Hits 14176 14177 +1
+ Misses 3909 3907 -2
- Partials 878 879 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jamescrosswell
marked this pull request as ready for review
September 14, 2026 00:18
jamescrosswell
commented
Sep 14, 2026
jamescrosswell
requested review from
bruno-garcia,
lucas-zimerman and
ric-oliv
and removed request for
bruno-garcia
September 14, 2026 00:30
ric-oliv
approved these changes
Sep 22, 2026
3 tasks
This was referenced Oct 1, 2026
Closed
Closed
This was referenced Oct 19, 2026
jonty-uk-gov-mirror
pushed a commit
to uk-gov-mirror/DFE-Digital.teaching-record-system
that referenced
this pull request
Oct 23, 2026
Updated [Sentry.AspNetCore](https://github.com/getsentry/sentry-dotnet) from 6.11.1 to 6.12.0. <details> <summary>Release notes</summary> _Sourced from [Sentry.AspNetCore's releases](https://github.com/getsentry/sentry-dotnet/releases)._ ## 6.12.0 ### Features ✨ - feat(otel): mark span processor based OpenTelemetry setup methods as obsolete by @jamescrosswell in [#5586](getsentry/sentry-dotnet#5586) ### Fixes 🐛 - fix(blazor): logging options set in UseSentry now take effect by @ric-oliv in [#5640](getsentry/sentry-dotnet#5640) - fix(log4net): send event properties as log attributes on log4net 3.x by @jamescrosswell in [#5559](getsentry/sentry-dotnet#5559) - fix: TraceIgnoreStatusCodes no longer drops outgoing HTTP requests traced with OpenTelemetry by @jamescrosswell in [#5643](getsentry/sentry-dotnet#5643) - fix: an exception in BeforeSend or BeforeSendTransaction now drops the item instead of sending it by @jamescrosswell in [#5610](getsentry/sentry-dotnet#5610) - fix: telemetry dropped by user callbacks and by validation is now counted in client reports by @jamescrosswell in [#5607](getsentry/sentry-dotnet#5607) - fix: exceptions thrown from BeforeBreadcrumb, log filters and BeforeScreenshotCapture no longer reach the application by @jamescrosswell in [#5606](getsentry/sentry-dotnet#5606) - fix: transaction processors no longer discard each other's results by @jamescrosswell in [#5621](getsentry/sentry-dotnet#5621) - fix: Throw ArgumentException for empty traceId in DynamicSamplingContext by @chiliec in [#5599](getsentry/sentry-dotnet#5599) ### Dependencies ⬆️ #### Deps - chore(deps): update Cocoa SDK to v9.30.0 by @github-actions in [#5656](getsentry/sentry-dotnet#5656) - chore(deps): update Cocoa SDK to v9.29.2 by @github-actions in [#5624](getsentry/sentry-dotnet#5624) - chore(deps): update Java SDK to v8.58.0 by @github-actions in [#5622](getsentry/sentry-dotnet#5622) - chore(deps): update Native SDK to v0.17.1 by @github-actions in [#5625](getsentry/sentry-dotnet#5625) - chore(deps): update Native SDK to v0.17.0 by @github-actions in [#5614](getsentry/sentry-dotnet#5614) ### Other - meta: remove duplicate Blank Issue template by @SatvikMishra08 in [#5601](getsentry/sentry-dotnet#5601) Commits viewable in [compare view](getsentry/sentry-dotnet@6.11.1...6.12.0). </details> Updated [Sentry.Extensions.Logging](https://github.com/getsentry/sentry-dotnet) from 6.11.1 to 6.12.0. <details> <summary>Release notes</summary> _Sourced from [Sentry.Extensions.Logging's releases](https://github.com/getsentry/sentry-dotnet/releases)._ ## 6.12.0 ### Features ✨ - feat(otel): mark span processor based OpenTelemetry setup methods as obsolete by @jamescrosswell in [#5586](getsentry/sentry-dotnet#5586) ### Fixes 🐛 - fix(blazor): logging options set in UseSentry now take effect by @ric-oliv in [#5640](getsentry/sentry-dotnet#5640) - fix(log4net): send event properties as log attributes on log4net 3.x by @jamescrosswell in [#5559](getsentry/sentry-dotnet#5559) - fix: TraceIgnoreStatusCodes no longer drops outgoing HTTP requests traced with OpenTelemetry by @jamescrosswell in [#5643](getsentry/sentry-dotnet#5643) - fix: an exception in BeforeSend or BeforeSendTransaction now drops the item instead of sending it by @jamescrosswell in [#5610](getsentry/sentry-dotnet#5610) - fix: telemetry dropped by user callbacks and by validation is now counted in client reports by @jamescrosswell in [#5607](getsentry/sentry-dotnet#5607) - fix: exceptions thrown from BeforeBreadcrumb, log filters and BeforeScreenshotCapture no longer reach the application by @jamescrosswell in [#5606](getsentry/sentry-dotnet#5606) - fix: transaction processors no longer discard each other's results by @jamescrosswell in [#5621](getsentry/sentry-dotnet#5621) - fix: Throw ArgumentException for empty traceId in DynamicSamplingContext by @chiliec in [#5599](getsentry/sentry-dotnet#5599) ### Dependencies ⬆️ #### Deps - chore(deps): update Cocoa SDK to v9.30.0 by @github-actions in [#5656](getsentry/sentry-dotnet#5656) - chore(deps): update Cocoa SDK to v9.29.2 by @github-actions in [#5624](getsentry/sentry-dotnet#5624) - chore(deps): update Java SDK to v8.58.0 by @github-actions in [#5622](getsentry/sentry-dotnet#5622) - chore(deps): update Native SDK to v0.17.1 by @github-actions in [#5625](getsentry/sentry-dotnet#5625) - chore(deps): update Native SDK to v0.17.0 by @github-actions in [#5614](getsentry/sentry-dotnet#5614) ### Other - meta: remove duplicate Blank Issue template by @SatvikMishra08 in [#5601](getsentry/sentry-dotnet#5601) Commits viewable in [compare view](getsentry/sentry-dotnet@6.11.1...6.12.0). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This was referenced Oct 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #5557
Summary
On log4net 3.x,
SentryAppendersilently dropped every event property from structured logs — noproperty.*attribute was ever attached to aSentryLog, with no exception and no warning.log4net.Util.PropertiesDictionarygainedIDictionary<string, object?>in 3.x, and that changed what its non-genericIEnumerableyields:KeyValuePair<string, object?>instead ofDictionaryEntry.Sentry.Log4Netis compiled against its 2.0.12 floor and walks exactly that non-generic enumerator, soproperty is DictionaryEntrymatched nothing and the loop body never ran.The fix reads through
GetKeys()+ the indexer, the way the siblingSentryEventpath inGetLoggingEventPropertiesalready does. That API behaves identically on both majors, so the appender is version-agnostic and the package floor stays at 2.0.12 — no new minimum for consumers still on log4net 2.x.Notes for review
New test project,
Sentry.Log4Net.V3.Tests. This is a runtime-only difference — nothing that builds against 2.x can catch it — so covering it needs the appender actually loaded against a 3.x assembly. The new project links the same two appender test files (Compile Include, not copies, so the two majors can't drift) and references log4net 3.4.0, whileSentry.Log4Net.Testsstays on 2.0.15. Both majors now run in CI. I confirmed the new project has teeth: reverting just the appender change reproduces exactly the three failures from the issue (52 passed / 3 failed), and both suites are green with it.The two Verify-based tests are deliberately not linked into the new project — their snapshots are keyed off the original file paths, so running them from two projects would have both writing the same files.
Three
InternalsVisibleToentries were needed for the new assembly (Sentry,Sentry.Log4Net,Sentry.Testing), following the pattern every other test project uses. These don't appear in the API approval snapshots, andApiApprovalTestsis green.🤖 Generated with Claude Code