feat: Logs are always enabled and `SentryOptions.EnableLogs` is ignored by jamescrosswell · Pull Request #5504 · getsentry/sentry-dotnet

jamescrosswell · 2026-08-24T08:58:22Z

Resolves #5479

Summary

Aligns the .NET SDK with the other SDKs (see sentry-cocoa #8769) by removing the explicit opt-in for logs.

Adding one of the logging integrations — Sentry.Extensions.Logging, Sentry.Serilog, Sentry.NLog, Sentry.Log4Net — is already an explicit statement that you want your logs in Sentry. Requiring EnableLogs = true on top of that was just an extra hoop, so every branch that gated on the option now assumes it's true and structured logs are captured unconditionally.

Deprecate rather than remove

Cocoa deleted enableLogs outright. Here it's marked [Obsolete] and ignored instead, which keeps this source- and binary-compatible — nobody's build breaks, nobody's appsettings.json stops binding:

SentryOptions.EnableLogs and SentryTarget.EnableLogs (NLog) — getter always returns true, setter is a no-op.
BindableSentryOptions.EnableLogs still binds, so an existing "EnableLogs": true in configuration is accepted and ignored rather than throwing.
The enableLogs parameter on the Serilog WriteTo.Sentry(...) overloads stays in place (it can't be attributed) but is documented as ignored and no longer applied.

The escape hatch for anyone who genuinely wants logs off is SetBeforeSendLog(log => null) — same guidance Cocoa gives.

Notes for review

Changelog. scripts/verify-changelog.sh fails PRs that add a manual ## Unreleased section, so the rationale the issue asks for lives in this description and the commit body rather than in CHANGELOG.md. Happy to add a hand-written note at release time if you'd rather it be in the file.
Serilog.SentrySink.IsEnabled is gone. It short-circuited Emit when an event was below both the breadcrumb and event minimums and logs were off. With logs always on, it could only ever return true, so it was removed rather than left as a vacuous check.
Three tests changed meaning, not just setup, because "nothing is sent" is no longer true once logs are on by default:
- WebIntegrationTests.PreFlightIgnoresTransaction now asserts no non-log envelope items, which is what #1835 was actually about.
- Google.Cloud.Functions.IntegrationTests.SentryIntegrationTest_CaptureUnhandledException asserted the sentry.dotnet.google-cloud-function SDK name on every request; it now asserts it on the envelopes carrying the error. The SDK-name override is an ISentryEventProcessor, so it doesn't apply to log envelopes — pre-existing for anyone who had EnableLogs = true, but now visible by default. Worth a follow-up if we want log envelopes attributed to the wrapping SDK.
- Several Verify snapshots gain a log envelope. Only the DotNet10_0 variants could be regenerated locally (no net8/net9 runtimes on this machine), so the rest were updated by hand and CI checked them: the first run confirmed the hand-edited Serilog/NLog/log4net Simple and ApiApprovalTests snapshots (net8/net9/net48 all green) and caught one family I had missed entirely, WebIntegrationTests.Versioning. Its net8.0/net9.0 snapshots are now taken byte-for-byte from the CI artifacts, which produced an identical delta on all seven platforms.
Native Cocoa's own enableLogs is untouched. We never set it from the managed side, so native Cocoa logging stays as it was.

🤖 Generated with Claude Code

Adding one of the logging integrations (`Sentry.Extensions.Logging`, `Sentry.Serilog`, `Sentry.NLog`, `Sentry.Log4Net`) is already an explicit opt-in to sending logs, so requiring `EnableLogs = true` on top of that was just an extra hoop. Structured logs are now captured unconditionally. `SentryOptions.EnableLogs` and `SentryTarget.EnableLogs` are marked obsolete rather than removed: the getters always return `true`, the setters are ignored, and existing configuration (including `"EnableLogs"` in appsettings.json and `enableLogs` on the Serilog sink) still binds without error. To drop logs, use `SetBeforeSendLog` and return `null`. Resolves #5479 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

jamescrosswell · 2026-08-24T09:18:58Z

            Assert.True(requests.Any(p => p.Contains(ExpectedMessage)),
                "Expected error to be captured");
-            Assert.True(requests.All(p => p.Contains("sentry.dotnet.google-cloud-function")),
+            // Structured logs are always captured, and those envelopes carry the ASP.NET Core SDK name,


The sentry.dotnet.google-cloud-function SDK name is applied by SentryGoogleCloudFunctionEventProcessor, an ISentryEventProcessor - so it's not being applied to logs (they're not events).

This was true before but with logs on by default, it's now visible.

For this PR, assertion scoped to the envelopes carrying the error, which is what it was checking in substance. It does need fixing properly though.

Opened #5506 for this.

Events, logs and metrics each read their SdkVersion from a different source (event-processor chain / the registered ILoggerProvider / scope.Sdk), so an integration that overrides only the event path gets the other two wrong. Sentry.AspNetCore.Grpc has the same issue as Google Cloud Functions, and Sentry.AspNet does too (though only with a logging integration wired up).

MAUI and the Serilog/NLog/log4net sinks are fine — maybe a model to copy there.

Leaving the scoped assertion here so as not to block this PR.

Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>

Belt-and-braces: `DefaultSentryStructuredLogger` asserts `hub.IsEnabled`, so `Create` now honours that by construction and returns the disabled logger otherwise. Restores the disabled-path coverage that the `EnableLogs` test removed, keyed off the Hub instead of the option. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`WebIntegrationTests.Versioning` has per-TFM snapshots; only the net10.0 one was regenerated locally (no net8/net9 runtimes on the dev machine). Taken byte-for-byte from the CI artifacts, which produced an identical delta on every platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

codecov · 2026-08-24T11:00:19Z

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.78%. Comparing base (2be688c) to head (47c133d).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/Sentry.Log4Net/SentryAppender.cs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5504      +/-   ##
==========================================
- Coverage   74.87%   74.78%   -0.09%     
==========================================
  Files         513      513              
  Lines       18659    18749      +90     
  Branches     3636     3665      +29     
==========================================
+ Hits        13970    14022      +52     
- Misses       3819     3857      +38     
  Partials      870      870

☔ 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.

Review feedback: the hub isn't disabled, so logs are captured as usual — excluding logs was the wrong way round. Assert directly that no transaction envelope item is sent, which is the only thing the test needs to show. Uses the existing EnvelopeItem.TryGetType() and TypeValueTransaction rather than a hand-rolled header lookup. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This reverts commit 12ec118.

jamescrosswell · 2026-08-26T23:55:57Z

Superseded in part by #5512, and converting this back to a draft so it doesn't merge by accident.

Per updated guidance, #5479 is being split into two phases:

Phase 1 — now (feat: Logs sent via SentrySdk.Logger no longer require EnableLogs #5512). EnableLogs is ignored for logs created directly via SentryStructuredLogger / SentrySdk.Logger. Non-breaking, so it can ship in a minor. The whole change is one branch in SentryStructuredLogger.Create; the logging integrations gate on EnableLogs before calling hub.Logger.CaptureLog, so they sit in front of that gate and are untouched — no Verify snapshot or API-approval file moves.
Phase 2 — 7.0.0 (this PR). Removing the option outright and taking the gate out of the four logging integrations. Breaking, and more nuanced than it looked: those integrations instrument a general-purpose ILogger-style surface where configuring logging says much less about whether you want the logs in Sentry, and each produces breadcrumbs and events and structured logs from one call.

Everything here still applies to phase 2 and CI was green at 47c133d, so this should be a case of rebasing onto main when 7.0.0 comes around rather than rewriting. The review threads are all resolved and the findings are worth keeping — in particular #5506 (logs and metrics not attributed to the wrapping SDK), which phase 2 will surface again.