Skip to content

fix(logging): warn when ClearProviders() removes the logger providers UseSentry added - #5789

Merged
jamescrosswell merged 4 commits into
version7from
fix/warn-when-logger-providers-removed
Oct 27, 2026
Merged

jamescrosswell merged 4 commits into
version7from
fix/warn-when-logger-providers-removed

Conversation

@ric-oliv

@ric-oliv ric-oliv commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

With Debug = true, the SDK now warns when the app removed the logger providers that UseSentry added, for example with ClearProviders(), and didn't add them back with builder.Logging.AddSentry(). Until now LogError calls silently stopped reaching Sentry.

The generic host checks during Build(), in the hook from #5772, so it also covers hosts that are never started. ASP.NET Core and Google Cloud Functions check in app.UseSentry() at startup. With both UseSentry calls, only ASP.NET Core checks.

The check reads the service collection first and stops if the host's providers are still there, so apps that replace the logger factory, as AddSerilog() does, get no providers created. Otherwise it looks for any Sentry logger provider, which covers Logging.AddSentry() after the clear.

Limitations:

  • The warning only shows with Debug = true, so NLog, Serilog and log4net apps that clear on purpose don't see it by default.
  • In ASP.NET Core it goes through the app's own ILogger, so an app with no providers left won't see it.
  • MAUI and Blazor WebAssembly have no check.
  • On the legacy WebHostBuilder, a clear inside Startup.ConfigureServices isn't detected, because that builder copies the service collection first.

Stacked on #5776.

Refs #5755

@codecov

codecov Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 76.21%. Comparing base (2194c92) to head (245cf66).
⚠️ Report is 3 commits behind head on version7.

Files with missing lines Patch % Lines
...c/Sentry.Extensions.Logging/HostLoggerProviders.cs 90.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           version7    #5789      +/-   ##
============================================
+ Coverage     76.17%   76.21%   +0.03%     
============================================
  Files           526      528       +2     
  Lines         19265    19318      +53     
  Branches       3729     3737       +8     
============================================
+ Hits          14676    14724      +48     
- Misses         3734     3735       +1     
- Partials        855      859       +4     

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

@ric-oliv
ric-oliv marked this pull request as ready for review October 23, 2026 23:47
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Oct 23, 2026
Base automatically changed from fix/logging-addsentry-after-clearproviders to version7 October 26, 2026 23:12
ric-oliv and others added 4 commits October 27, 2026 12:24
After UseSentry, ClearProviders() removes Sentry's logger providers, and
without a later Logging.AddSentry() nothing says that ILogger entries no
longer reach Sentry.

The generic host now checks while the host is built, in the hook that
starts the SDK, and ASP.NET Core checks in app.UseSentry() when the host
starts. If the host's providers are gone and no Sentry logger provider
is registered, the SDK logs a warning through its diagnostic logger, so
it shows with Debug = true. The check reads the service collection first,
so apps that keep the registrations, such as those that replace the
logger factory with Serilog, never have providers created by it.

HostLoggerProviders now keeps the service collection it was added to,
which lets app.UseSentry() run the check and spares Logging.AddSentry()
capturing it.

Refs #5755

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ction

The legacy WebHostBuilder copies the service collection before
Startup.ConfigureServices, so checking the collection UseSentry used
brought back NullLoggerProviders there. Only the warning uses it now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The generic host now leaves the check to the other host integration,
as it already does for the logger providers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IHostBuilder apps use ConfigureLogging(), so the warning now names the
logging builder and gives builder.Logging.AddSentry() as an example.

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

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

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