Skip to content

feat: Logs sent via SentrySdk.Logger no longer require EnableLogs - #5512

Merged
jamescrosswell merged 3 commits into
mainfrom
feat/logs-api-ignores-enable-logs
Aug 27, 2026
Merged

feat: Logs sent via SentrySdk.Logger no longer require EnableLogs#5512
jamescrosswell merged 3 commits into
mainfrom
feat/logs-api-ignores-enable-logs

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

Phase 1 of #5479, split out of #5504.

Summary

Reaching for SentrySdk.Logger (or any SentryStructuredLogger) is already an explicit statement that you want those logs in Sentry, so requiring EnableLogs = true on top of it was an unnecessary hoop. That gate is gone.

The whole change is one branch in SentryStructuredLogger.Create:

-return options.EnableLogs
-    ? new DefaultSentryStructuredLogger(hub, options, clock, batchCount, batchInterval)
-    : DisabledSentryStructuredLogger.Instance;
+return new DefaultSentryStructuredLogger(hub, options, clock, batchCount, batchInterval);

What this deliberately does not change

SentryOptions.EnableLogs stays, still defaults to false, and is not obsolete. The logging integrations — Sentry.Extensions.Logging, Sentry.Serilog, Sentry.NLog, Sentry.Log4Net — each check EnableLogs themselves before calling hub.Logger.CaptureLog(...), so they are entirely unaffected and continue to honour the option.

That is what makes this seam clean: the integrations sit in front of the gate being removed, not behind it. The evidence is that not a single Verify snapshot or API-approval file changed — if any integration had started emitting logs, the Simple / Versioning snapshots would have moved, as they did in #5504.

Leaving them alone is deliberate rather than incidental. Those integrations instrument a general-purpose ILogger-style surface where "I configured logging" says much less about whether you want the logs in Sentry, and each one produces three different things from a single log call — breadcrumbs, events, and structured logs. Changing that is a breaking change, so it belongs in the next major version.

The option's XML doc now says exactly which of the two it governs.

Notes for review

  • Sentry.Samples.Console.Basic used the direct API only, and its comment claimed the option "enables Sentry Logs created via SentrySdk.Logger" — no longer true, so both are gone. Every other sample uses a logging integration and keeps EnableLogs = true untouched.
  • Two HubTests cases described the option toggling this API and no longer have any meaning; Logger_IsDisabled_DoesNotCaptureLog is replaced by Logger_EnableLogsDisabled_StillCapturesLog, asserting the new behaviour directly. Create_EnableLogsDisabled_NewDefaultInstance covers the same at the factory.
  • The existing integration tests that assert EnableLogs still gates them (SentrySinkTests.Emit_StructuredLogging_IsEnabled, SentryAppenderTests.DoAppend_StructuredLogging_IsEnabled, SentryStructuredLoggerTests.IsEnabled_HubAndOptions_Returns) are untouched and now serve as the guard against this phase leaking into the next.
  • No CHANGELOG.md entry — scripts/verify-changelog.sh rejects manual ## Unreleased sections, so the commit subject is the changelog line.

🤖 Generated with Claude Code

Reaching for `SentrySdk.Logger` (or any `SentryStructuredLogger`) is already an
explicit statement that you want those logs in Sentry, so requiring
`EnableLogs = true` on top of it was an unnecessary hoop.

Deliberately scoped to that API. The logging integrations
(`Sentry.Extensions.Logging`, `Sentry.Serilog`, `Sentry.NLog`, `Sentry.Log4Net`)
each gate on `EnableLogs` themselves before calling `hub.Logger.CaptureLog`, so
they are unaffected and continue to honour the option. Those instrument a
general-purpose `ILogger`-style surface where intent is far less clear, and they
also produce breadcrumbs and events - so changing them is a breaking change,
left for the next major version.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.72%. Comparing base (5cd6ad6) to head (b4c3769).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5512      +/-   ##
==========================================
- Coverage   74.74%   74.72%   -0.03%     
==========================================
  Files         513      513              
  Lines       18829    18827       -2     
  Branches     3682     3681       -1     
==========================================
- Hits        14074    14068       -6     
- Misses       3875     3878       +3     
- Partials      880      881       +1     

☔ View full report in Codecov by Harness.
📢 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.

Comment thread samples/Sentry.Samples.Console.Basic/Program.cs Outdated
Comment thread src/Sentry/SentryOptions.cs Outdated
Comment thread src/Sentry/SentryStructuredLogger.cs Outdated
jamescrosswell and others added 2 commits August 27, 2026 13:01
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
6c8fb7a removed the first line of the "To filter or drop those, use
SetBeforeSendLog and return null" sentence but left the second, so the remarks
ended mid-sentence with "and return null.". Completing that removal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review August 27, 2026 04:22
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Aug 27, 2026
@dingsdax
dingsdax self-requested a review August 27, 2026 05:18
@jamescrosswell
jamescrosswell merged commit 0f74c1e into main Aug 27, 2026
55 of 56 checks passed
@jamescrosswell
jamescrosswell deleted the feat/logs-api-ignores-enable-logs branch August 27, 2026 09:49
jamescrosswell added a commit to mentaldesk/truman-sentry that referenced this pull request Sep 2, 2026
Truman reaches structured logs through the Microsoft.Extensions.Logging
integration rather than by calling SentrySdk.Logger, so getsentry/sentry-dotnet#5512
— which dropped the EnableLogs requirement for the direct API — does not apply
here. The integrations still check the option in 6.4.1, in 6.9.0 and on main.

Records that, and the fact that Sentry.AspNetCore builds on
Sentry.Extensions.Logging (so the API needs no extra package), so nobody reads
this later and assumes the option is redundant before it actually is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants