feat(log4net)!: the Sentry appender no longer initializes the SDK - #5592
Conversation
The Sentry sink for Serilog now only configures the sink. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - SentrySerilogOptions no longer derives from SentryOptions and only carries sink settings; InitializeSdk is removed - Remove the WriteTo.Sentry(string dsn, ...) overload - Rename ApplySerilogScopeToEvents() to UseSerilog(), make it idempotent - The sink logs a one-time diagnostic warning when UseSerilog() was not called on the options used to initialize Sentry Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into feat/no-init-from-logging-5245
The Sentry target for NLog now only configures the target. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - SentryNLogOptions no longer derives from SentryOptions and only carries target settings; FlushTimeout moves onto it directly - Remove InitializeSdk, Dsn/DsnLayout, Release/ReleaseLayout, Environment/EnvironmentLayout and ShutdownTimeoutSeconds. Events take release and environment from the SDK options - Collapse the AddSentry overloads into AddSentry(optionsConfig, targetName); the dsn overloads are removed - The target no longer routes SDK diagnostics to NLog's InternalLogger Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
Remove SentryTarget.FlushTimeoutSeconds and SentryNLogOptions.FlushTimeout. When NLog flushes the target, the hub is now flushed with the FlushTimeout from the options used to initialize Sentry, since the target no longer owns the SDK. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Sentry appender for log4net now only sends log events to Sentry. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - Remove SentryAppender.Dsn and the lazy SDK initialization on first append - Remove SentryAppender.Environment; events take the environment from the options used to initialize Sentry - Remove the OnClose override, which only disposed the SDK the appender had initialized Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/no-init-from-logging-nlog-5245 #5592 +/- ##
=======================================================================
- Coverage 75.00% 74.96% -0.04%
=======================================================================
Files 515 515
Lines 18860 18832 -28
Branches 3660 3655 -5
=======================================================================
- Hits 14145 14118 -27
Misses 3855 3855
+ Partials 860 859 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // Honor the appender-level settings, overriding the scope/options defaults, to match the SentryEvent path. | ||
| if (!string.IsNullOrWhiteSpace(environment)) | ||
| { | ||
| log.SetAttribute("sentry.environment", environment!); |
There was a problem hiding this comment.
This gets handled here now (just a couple of lines up):
The sample sets the DSN in code via UseSentry, so the commented-out Dsn entry is misleading. EnableTracing is declared on BindableSentryOptions but never applied, so setting it has no effect. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Emit can run concurrently, so the check-then-set on the warned flag could let more than one thread log the warning. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into feat/no-init-from-logging-nlog-5245
Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The sink identifies itself through the log origin (auto.log.serilog) instead. See #5497. Events are no longer stamped with sentry.dotnet.serilog, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…logging-nlog-5245
Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The target identifies itself through the log origin (auto.log.nlog) instead. See #5497. Events are no longer stamped with sentry.dotnet.nlog, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. With no remaining callers, Constants is deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…from-logging-log4net-5245
Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The appender identifies itself through the log origin (auto.log.log4net) instead. See #5497. Events are no longer stamped with sentry.dotnet.log4net, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…on error Serilog configuration providers bind sink arguments by parameter name, so removing the dsn-first overload made them drop `dsn` silently: the sink still binds, Sentry is never initialized, and nothing is reported. Keeping the overload as an [Obsolete(error: true)] tombstone that throws makes both Serilog.Settings.Configuration (appsettings.json) and Serilog.Settings.AppSettings (app.config) fail loudly with migration guidance, while code callers get a compile error instead of a type mismatch on the second argument. The overload mirrors the surviving overload's parameters plus `dsn`. With only `string dsn` it loses Serilog's overload ranking whenever a configuration supplies two or more of the surviving arguments, which would restore the silent behaviour. Part of #5245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ration The migration guard works only because of Serilog's overload ranking, and nothing exercised that path. These tests bind a sink from IConfiguration the way a provider does, so a Serilog change that stops selecting the tombstone fails here rather than silently dropping the DSN again. Verified they fail without the tombstone overload. Selection behaves the same on Serilog.Settings.Configuration 3.4.0 (Serilog 2.12) and 10.0.1 (Serilog 4.3); 3.4.0 is referenced to avoid bumping Serilog in the tests. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into feat/no-init-from-logging-nlog-5245
…t-5245' into feat/no-init-from-logging-log4net-5245
Restores the DSN comment dropped from the Serilog sample's appsettings.json, pointing at where this sample actually sets it, and records in AGENTS.md that "prefer no comments" covers the library rather than samples - including their JSON configuration files. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…try is not initialized The tombstoned overloads catch everyone who passes a DSN to the sink, but they cannot see the `WriteTo.Sentry(o => ...)` callback that only sets sink options and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute. On 6.x that overload initialized the SDK itself; now it compiles, nothing calls Init, and the sink drops everything silently. Warn once, on the first event at or above MinimumEventLevel, when the hub is disabled and a DSN can still be found. There is no DiagnosticLogger to write to in that state, so the warning goes to Serilog's SelfLog and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into feat/no-init-from-logging-nlog-5245
…ry is not initialized Mirrors the Serilog sink: the tombstoned Dsn/InitializeSdk properties cannot see an AddSentry(o => ...) call that only sets target options and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute, so warn once on the first event at or above MinimumEventLevel when the hub is disabled and a DSN can still be found. The warning goes to NLog's InternalLogger and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…245' into feat/no-init-from-logging-log4net-5245
… Sentry is not initialized Mirrors the Serilog sink and the NLog target: the tombstoned Dsn property cannot see an appender that is configured with only appender settings and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute, so warn once on the first event that would have become a Sentry event when the hub is disabled and a DSN can still be found. The warning goes to log4net's LogLog and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…-logging-nlog-5245 version7 now has main merged into it, and #5573 has landed there, so this picks both up. All four conflicts took version7's side, which is strictly newer in each case: - AGENTS.md, samples/Sentry.Samples.AspNetCore.Serilog/appsettings.json and test/Sentry.Serilog.Tests/SentrySinkTests.cs were resolved when main was merged into version7; this branch only carried the pre-merge side of them. - src/Sentry.Serilog/SentryOptionExtensions.cs: the UseSerilog() doc comment here still said the sink "cannot do this for you", which stopped being true when #5612 made the sink register the scope processor itself. version7 has the corrected wording. The merge also brings #5523's breadcrumb hint to the NLog target (hint: exception.ToHint()) and its test, which don't overlap with anything in this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…245' into feat/no-init-from-logging-log4net-5245 Picks up version7 (which now has main merged in) and #5573 via the NLog branch. No conflicts. main added test/Sentry.Log4Net.V3.Tests, which shares the appender test sources by <Compile Include> and runs them against log4net 3.4.0 instead of 2.0.12. The new SentryAppenderUninitializedSdkTests.cs is now included there too, so the runtime warning is covered on both log4net majors. That also confirms LogLog.Warn(Type, string) is binary compatible across the two, which the appender relies on: Sentry.Log4Net compiles against 2.0.12 but the V3 tests load it against 3.4.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This one is not a problem, but the runtime warning prints twice on stderr:
|
There was a problem hiding this comment.
This new test does not run against log4net v3 (not referenced, and also fails due to a potential null reference).
I've opened #5651 on top of this branch to fix this :)
…5651) The V3 test project links the other test files added in #5592, but not SentryAppenderConfigurationBindingTests. That file didn't compile against log4net 3, which marks the XmlElement parameter of XmlConfigurator.Configure as non-nullable. With a null-forgiving operator on DocumentElement it compiles, so the V3 project now links it and the Dsn tombstone behavior is pinned on both log4net versions. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* fix(nlog): Report a stale dsn and accept initializeSdk=false Both tombstones could leave an upgraded app worse off than it needed to be. A dsn left in NLog.config was silent with NLog's default settings. NLog swallows the setter's exception unless throwConfigExceptions is on, so the target attached, Sentry was never initialized and nothing was printed. The runtime warning also stayed quiet, because it only looks for a DSN in the environment or an assembly attribute. The Dsn setter now writes the migration message to standard error before it throws. initializeSdk="false" was the recommended v6 setting next to UseSentry, and it already matches the new behavior. With throwConfigExceptions on, it still threw, NLog rejected the whole configuration and the app lost every NLog target. The setter now only throws for true, like the Microsoft.Extensions.Logging tombstone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nlog): report a stale initializeSdk="true" as well The stale dsn message only came from the Dsn setter, so a config carrying initializeSdk="true" instead was still silent with NLog's default throwConfigExceptions: the setter threw, NLog discarded it, the target attached and nothing was printed. Both setters now go through one report-and-throw helper. Reporting from both setters means a v6 config carrying dsn and initializeSdk="true" together would print the same message twice, so the helper reports at most once per target. A configuration reload builds a new target, and so reports again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
…245' into feat/no-init-from-logging-log4net-5245 Picks up #5652: a stale dsn or initializeSdk="true" in NLog.config is now reported to standard error, and initializeSdk="false" is accepted rather than failing the configuration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t-5245' into feat/no-init-from-logging-log4net-5245 Picks up #5651, which runs the appender configuration binding tests against log4net 3 as well. It adds its own <Compile Include> to Sentry.Log4Net.V3.Tests beside the one for the uninitialized-SDK tests, so the two merged cleanly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…itializes the SDK (#5595) * feat: Serilog sink no longer initializes the SDK The Sentry sink for Serilog now only configures the sink. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - SentrySerilogOptions no longer derives from SentryOptions and only carries sink settings; InitializeSdk is removed - Remove the WriteTo.Sentry(string dsn, ...) overload - Rename ApplySerilogScopeToEvents() to UseSerilog(), make it idempotent - The sink logs a one-time diagnostic warning when UseSerilog() was not called on the options used to initialize Sentry Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Accept API verifier changes * Tweak comments in the samples * feat: NLog target no longer initializes the SDK The Sentry target for NLog now only configures the target. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - SentryNLogOptions no longer derives from SentryOptions and only carries target settings; FlushTimeout moves onto it directly - Remove InitializeSdk, Dsn/DsnLayout, Release/ReleaseLayout, Environment/EnvironmentLayout and ShutdownTimeoutSeconds. Events take release and environment from the SDK options - Collapse the AddSentry overloads into AddSentry(optionsConfig, targetName); the dsn overloads are removed - The target no longer routes SDK diagnostics to NLog's InternalLogger Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Tweaked wording Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com> * feat: NLog target flushes using the SDK's FlushTimeout Remove SentryTarget.FlushTimeoutSeconds and SentryNLogOptions.FlushTimeout. When NLog flushes the target, the hub is now flushed with the FlushTimeout from the options used to initialize Sentry, since the target no longer owns the SDK. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: log4net appender no longer initializes the SDK The Sentry appender for log4net now only sends log events to Sentry. Sentry must be initialized separately (SentrySdk.Init, UseSentry, etc). - Remove SentryAppender.Dsn and the lazy SDK initialization on first append - Remove SentryAppender.Environment; events take the environment from the options used to initialize Sentry - Remove the OnClose override, which only disposed the SDK the appender had initialized Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: Microsoft.Extensions.Logging integration no longer initializes the SDK Completes the logging-integration part of #5245. The MEL integration now only wires up the logger providers; Sentry has to be initialized separately. Unlike Serilog, NLog and log4net, SentryLoggingOptions keeps deriving from SentryOptions, because SentryAspNetCoreOptions, SentryMauiOptions and SentryBlazorOptions derive from it and those integrations do initialize the SDK. InitializeSdk therefore stays as internal plumbing, now defaulting to false and opted into by the framework integrations that own it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: SentryHostOptions replaces SentryLoggingOptions as the base for framework options SentryLoggingOptions no longer derives from SentryOptions, matching the Serilog and NLog options: it carries only the log levels and entry filters. SentryAspNetCoreOptions, SentryMauiOptions and SentryBlazorOptions now derive from a new abstract SentryHostOptions, which keeps MinimumEventLevel, MinimumBreadcrumbLevel, ConfigureScope and AddLogEntryFilter by passing them through to an inner SentryLoggingOptions, so existing UseSentry callbacks and configuration keys keep working. InitializeSdk is removed. Integrations that initialise through DI call AddSentry<TOptions>; MAUI, which initialises in SentryMauiInitializer, uses an internal non-initialising overload. ConfigureScope callbacks are applied right after the SDK is initialised instead of when the MEL logger provider is built. Also fixes Blazor WebAssembly's logger ignoring the logging settings from UseSentry (it was built from a separate, default IOptions<SentryLoggingOptions>), and structured logs from plain MEL taking default attributes from options the SDK was not initialised with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: drop unused Sentry settings from the Serilog sample appsettings The sample sets the DSN in code via UseSentry, so the commented-out Dsn entry is misleading. EnableTracing is declared on BindableSentryOptions but never applied, so setting it has no effect. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(serilog): make the UseSerilog warning check atomic Emit can run concurrently, so the check-then-set on the warned flag could let more than one thread log the warning. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: the logging integration no longer stamps the SDK name or disposes the hub Sdk.Name and Sdk.Version should identify the integration that initialised the hub, which after this PR can no longer be a logging integration. The logging integration identifies itself through the log origin (auto.log.*) instead. See #5497. With the SDK name gone, and ConfigureScope callbacks now applied at init, the scope the provider pushed has nothing left to hold, so it goes too. Disposing the hub goes as well: whoever initialises the hub owns it, and the provider is now always handed HubAdapter, which is not IDisposable. The one fixture that handed it a real Hub now disposes the Hub it created. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: the Serilog sink no longer sets the SDK name Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The sink identifies itself through the log origin (auto.log.serilog) instead. See #5497. Events are no longer stamped with sentry.dotnet.serilog, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: the NLog target no longer sets the SDK name Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The target identifies itself through the log origin (auto.log.nlog) instead. See #5497. Events are no longer stamped with sentry.dotnet.nlog, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. With no remaining callers, Constants is deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: the log4net appender no longer sets the SDK name Sdk.Name should identify the integration that initialised the hub, which after this change can no longer be a logging integration. The appender identifies itself through the log origin (auto.log.log4net) instead. See #5497. Events are no longer stamped with sentry.dotnet.log4net, and structured logs no longer carry it as sentry.sdk.name; both now report the SDK that initialised Sentry. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor: structured logs from Microsoft.Extensions.Logging no longer set the SDK name Completes the change across the four logging integrations: the SDK name on a log should identify the integration that initialised the hub, and the logging integration identifies itself through the origin (auto.log.extensions_logging). See #5497. The ASP.NET Core and MAUI structured logger providers keep passing their own SDK version: those integrations do initialise the SDK, so the name is theirs to set. With no remaining callers, Constants and SentryLoggerProvider.NameAndVersion are deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(serilog): configuring a DSN on the sink now fails with a migration error Serilog configuration providers bind sink arguments by parameter name, so removing the dsn-first overload made them drop `dsn` silently: the sink still binds, Sentry is never initialized, and nothing is reported. Keeping the overload as an [Obsolete(error: true)] tombstone that throws makes both Serilog.Settings.Configuration (appsettings.json) and Serilog.Settings.AppSettings (app.config) fail loudly with migration guidance, while code callers get a compile error instead of a type mismatch on the second argument. The overload mirrors the surviving overload's parameters plus `dsn`. With only `string dsn` it loses Serilog's overload ranking whenever a configuration supplies two or more of the surviving arguments, which would restore the silent behaviour. Part of #5245 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(serilog): pin the DSN tombstone against Serilog.Settings.Configuration The migration guard works only because of Serilog's overload ranking, and nothing exercised that path. These tests bind a sink from IConfiguration the way a provider does, so a Serilog change that stops selecting the tombstone fails here rather than silently dropping the DSN again. Verified they fail without the tombstone overload. Selection behaves the same on Serilog.Settings.Configuration 3.4.0 (Serilog 2.12) and 10.0.1 (Serilog 4.3); 3.4.0 is referenced to avoid bumping Serilog in the tests. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(nlog): configuring a DSN on the target now fails with a migration error Mirrors the Serilog guard (#5611). The v6 AddSentry(dsn, ...) overloads and the SentryTarget.Dsn / InitializeSdk properties come back as tombstones: obsolete-as-error for code callers, throwing NotSupportedException so NLog.config bindings fail loudly with migration guidance instead of reporting an unknown property. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(serilog): reword the DSN migration error Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(nlog): reword the DSN migration error to match Serilog Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(log4net): configuring a DSN on the appender now reports a migration error Brings back SentryAppender.Dsn as a tombstone: [Obsolete(error: true)] with a setter that throws NotSupportedException carrying migration guidance, so code callers get a compile error and XML configs report the message instead of log4net's "Cannot find Property [Dsn]". Unlike Serilog and NLog, this cannot fail configuration loading: log4net catches exceptions thrown while setting a parameter, so the appender is still attached and the message surfaces through log4net's internal logging. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(mel): configuring a DSN through the logging integration now fails with a migration error Mirrors the Serilog (#5611), NLog and log4net guards. The v6 AddSentry(dsn) overload and the SentryLoggingOptions.Dsn / InitializeSdk properties come back as tombstones: obsolete-as-error for code callers, throwing NotSupportedException so configuration fails loudly with migration guidance instead of being ignored. Both binding paths are covered. On .NET 6 and later the Sentry section binds through BindableSentryLoggingOptions, which now carries these keys and throws when either is present; on netstandard2.0 the configuration binder sets the properties directly and the setters throw. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(mel): only treat a supplied Dsn or InitializeSdk as a migration error The tombstone setters threw unconditionally, which broke every bind of SentryLoggingOptions on netstandard2.0: ConfigurationBinder reads each property and writes the value back, so InitializeSdk's own `false` tripped the guard even when the key was absent. That failed four tests on net48, three of them pre-existing. The setters now throw only for a value that asks for something the integration can no longer do, and BindableSentryLoggingOptions matches, so both binding paths behave the same: a Dsn or InitializeSdk=true is an error, InitializeSdk=false is accepted because not initializing is what now always happens. Covered by a test that binds onto the options directly, which reproduces the netstandard2.0 write-back on every target framework. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(serilog): the Sentry sink registers the Serilog scope event processor automatically (#5612) * fix: make the SentryOptions processor collections thread safe SentryClient enumerates these collections lazily for the whole duration of a capture, and AddEventProcessor is documented as supporting registration after the SDK is initialised. They were plain Lists, so appending to one while a capture was in flight threw InvalidOperationException - which the SDK catches and logs at Debug, silently dropping the event. Swap them for ConcurrentBagLite, which snapshots on enumeration. Scope.EventProcessors already uses it for the same reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(serilog): register the Serilog scope event processor automatically The sink no longer initialises the SDK, so integrators have to call UseSerilog() on the options used to initialise Sentry. Forgetting it was only reported as a warning gated behind Debug and DiagnosticLevel, so in practice it was silent. The sink now registers SerilogScopeEventProcessor itself: at construction when Sentry is already initialised, otherwise on the first log event. The sink and the processor live in the same assembly, so no reflection is needed and this stays AOT safe. UseSerilog() is still the better option - it applies from the first event rather than from the first log line - and the warning now says so. Also fixes a feedback loop this exposed. Emit answered a reentrant log event with another diagnostic, which Serilog routed straight back into the sink, each message embedding the last. With DiagnosticLevel at Info that produced 55 MB of logs in 17 seconds and the app stopped serving requests. The SDK-namespace filter that breaks the cycle now runs before the reentrancy check instead of after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Removed unnecessary comments Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com> * fix(serilog): register the scope event processor atomically Sinks sharing one set of SentryOptions can reach registration concurrently - each sink's guard is per-instance - so the check and the add have to happen under a lock, not as check-then-act. The sink now learns from the result whether it was the one that registered, which is what the warning reports. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(serilog): use the Lock shim for the registration lock Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Tidy comments Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com> * docs: samples are exempt from the no-comments rule Restores the DSN comment dropped from the Serilog sample's appsettings.json, pointing at where this sample actually sets it, and records in AGENTS.md that "prefer no comments" covers the library rather than samples - including their JSON configuration files. Part of #5245 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Apply suggestion from @jamescrosswell * feat(serilog): warn at runtime when the sink drops events because Sentry is not initialized The tombstoned overloads catch everyone who passes a DSN to the sink, but they cannot see the `WriteTo.Sentry(o => ...)` callback that only sets sink options and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute. On 6.x that overload initialized the SDK itself; now it compiles, nothing calls Init, and the sink drops everything silently. Warn once, on the first event at or above MinimumEventLevel, when the hub is disabled and a DSN can still be found. There is no DiagnosticLogger to write to in that state, so the warning goes to Serilog's SelfLog and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(nlog): warn at runtime when the target drops events because Sentry is not initialized Mirrors the Serilog sink: the tombstoned Dsn/InitializeSdk properties cannot see an AddSentry(o => ...) call that only sets target options and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute, so warn once on the first event at or above MinimumEventLevel when the hub is disabled and a DSN can still be found. The warning goes to NLog's InternalLogger and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(log4net): warn at runtime when the appender drops events because Sentry is not initialized Mirrors the Serilog sink and the NLog target: the tombstoned Dsn property cannot see an appender that is configured with only appender settings and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute, so warn once on the first event that would have become a Sentry event when the hub is disabled and a DSN can still be found. The warning goes to log4net's LogLog and to standard error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(mel): warn at runtime when the integration drops events because Sentry is not initialized Completes the cascade from Serilog, NLog and log4net: the tombstoned Dsn/InitializeSdk properties cannot see an AddSentry(o => ...) call that only sets logging options and gets its DSN from SENTRY_DSN or a [Dsn] assembly attribute, so warn once on the first log event that would have become a Sentry event when the hub is disabled and a DSN can still be found. Microsoft.Extensions.Logging has no self-diagnostics channel, so the warning only goes to standard error. The warning is owned by SentryLoggerProvider so that it is shared by every category's logger rather than repeated per category. SentryLogger.Log now reads IHub.IsEnabled once per call instead of twice, via the level-only IsEnabledForLevel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(mel): match the other integrations' migration-error phrasing The MEL tombstone said "(or an integration such as UseSentry)" where Serilog, NLog and log4net all say "(or UseSentry via one of the integrations)". Same wording everywhere now. Only Sentry.Extensions.Logging's API snapshots carry the message, since the tombstoned members are on SentryLoggingOptions. Net4_8 can't regenerate on macOS, so it got the same substitution by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: UseSentry now initializes Sentry even when Logging.AddSentry ran first The logging integration registers a non-initializing Func<IHub>, and the initializing one was registered with TryAdd, so whichever ran first won. Calling builder.Logging.AddSentry() before UseSentry therefore left the SDK disabled with no indication, where v6 failed loudly at startup with "You must supply a DSN". The initializing registration now replaces any existing accessor, so the order of the two calls no longer matters. Tests cover both orders; the AddSentry-first one fails without this change. Calling both is still redundant and still registers two pairs of logger providers, which is unchanged from v6. Reported by Cursor Bugbot on #5595. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(log4net): Run the configuration binding tests against log4net 3 (#5651) The V3 test project links the other test files added in #5592, but not SentryAppenderConfigurationBindingTests. That file didn't compile against log4net 3, which marks the XmlElement parameter of XmlConfigurator.Configure as non-nullable. With a null-forgiving operator on DocumentElement it compiles, so the V3 project now links it and the Dsn tombstone behavior is pinned on both log4net versions. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> * fix(nlog): report a stale dsn and accept initializeSdk=false (#5652) * fix(nlog): Report a stale dsn and accept initializeSdk=false Both tombstones could leave an upgraded app worse off than it needed to be. A dsn left in NLog.config was silent with NLog's default settings. NLog swallows the setter's exception unless throwConfigExceptions is on, so the target attached, Sentry was never initialized and nothing was printed. The runtime warning also stayed quiet, because it only looks for a DSN in the environment or an assembly attribute. The Dsn setter now writes the migration message to standard error before it throws. initializeSdk="false" was the recommended v6 setting next to UseSentry, and it already matches the new behavior. With throwConfigExceptions on, it still threw, NLog rejected the whole configuration and the app lost every NLog target. The setter now only throws for true, like the Microsoft.Extensions.Logging tombstone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nlog): report a stale initializeSdk="true" as well The stale dsn message only came from the Dsn setter, so a config carrying initializeSdk="true" instead was still silent with NLog's default throwConfigExceptions: the setter threw, NLog discarded it, the target attached and nothing was printed. Both setters now go through one report-and-throw helper. Reporting from both setters means a v6 config carrying dsn and initializeSdk="true" together would print the same message twice, so the helper reports at most once per target. A configuration reload builds a new target, and so reports again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com> * test: keep the real environment out of the disabled-SDK tests SentryConstants.DisableSdkDsnValue is an empty string, and SettingLocator.GetDsn treats an empty Dsn as unset and falls back to SENTRY_DSN. So on a machine with that variable set, these two tests initialised a working SDK and captured the event they assert is never captured. FakeSettings() swaps in a locator that reads only from its own dictionary, so the fallback can't reach the real environment. Both tests now pass with and without SENTRY_DSN set. The fallback itself predates this PR and is also on main: options.Dsn = "" does not disable the SDK when SENTRY_DSN is set. Reported by @ric-oliv on #5595. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: read core SDK settings only from the Sentry configuration section The logging integrations no longer initialize the SDK, so the logging provider section is no longer a place where a DSN or any other core SDK setting has an effect. Reading them there would silently ignore them. The root 'Sentry' section now configures the whole of SentryHostOptions, and 'Logging:Sentry' configures only the logging half. Any other key there throws, naming the offending keys, so an app that upgrades with core settings in the logging section fails at startup instead of reporting nothing. A single SentryLoggingConfiguration binds the logging section for both the standalone MEL options and the host options, on every target framework. That replaces BindableSentryLoggingOptions and the separate netstandard2.0 reflection path. The integrations that initialize Sentry read 'Logging:Sentry' from the app's configuration rather than through ILoggerProviderConfiguration: an app that passes the whole configuration to AddConfiguration, as Google Cloud Functions does, makes the root 'Sentry' section part of the provider section, and every core setting in it would then be rejected. Plain MEL keeps the provider configuration, where rejecting core settings is correct. Also fixes ASP.NET Core registering SentryAspNetCoreOptionsSetup twice - once with the 'Sentry' section and once with the logging provider configuration - which applied everything its Configure adds beyond binding twice: two Kestrel deduplication log filters and two TraceIgnoreStatusCodeTransactionProcessor instances. The IConfiguration substitute in SentryWebHostBuilderExtensionsTests returned an empty string for every key, where configuration returns null for an absent one. Returning null removes a NET8_0-only workaround. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(blazor): bind configuration before the UseSentry callback Blazor WebAssembly sets DetectStartupTime, RequestBodyCompressionLevel and IsGlobalModeEnabled because the platform requires them, in the same Configure delegate that runs the UseSentry callback. That delegate was registered before the configuration binding, and IConfigureOptions<T> run in registration order, so a Sentry:DetectStartupTime key in wwwroot/appsettings.json won over the platform default - reintroducing the PlatformNotSupportedException the default is there to avoid - and configuration won over anything set in UseSentry, the opposite of ASP.NET Core. The two setups are now registered first, so precedence runs from the Sentry section, through Logging:Sentry, to the callback and the platform defaults. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Format code --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io> Co-authored-by: Ricardo Colombo Oliveira <github@ricoliv.com>
The log4net portion of #5245, stacked on #5585 (NLog) and following the same design. The Sentry appender for log4net now only sends log events to Sentry; Sentry has to be initialized separately via
SentrySdk.Init,UseSentry, etc.Part of #5245
Breaking changes
SentryAppender.Dsnremains as a tombstone —[Obsolete(..., error: true)]with a setter that throwsNotSupportedExceptioncarrying migration guidance, matching feat(serilog): configuring a DSN on the sink now fails with a migration error #5611 (Serilog) and feat(nlog)!: the Sentry target no longer initializes the SDK #5585 (NLog).SentryAppender.Environmentis removed. Events and structured logs take the environment from the options used to initialize Sentry.SentryAppenderno longer overridesOnClose. It only existed to dispose the SDK the appender had initialized, so closing the log4net repository no longer flushes Sentry; disposing the handle fromSentrySdk.Initdoes.SENTRY_DSNor a[Dsn]assembly attribute. The appender therefore warns at runtime — on the first event that would have become a Sentry event, if Sentry is not initialized but a DSN can still be found, it writes one line to log4net'sLogLogand to standard error, at most once per appender. That matters more here than elsewhere, since log4net swallows the tombstone setter's exception and loads the config anyway. The shared policy lives inSentry.Internal.UninitializedSdkWarning, added in feat(serilog)!: the Sentry sink no longer initializes the SDK #5573.Before:
After:
The tombstone behaves differently here than in Serilog and NLog
log4net cannot be made to fail configuration loading.
XmlHierarchyConfigurator.SetParametercatches exceptions thrown while setting a parameter, so the throwing setter does not propagate: the config still loads and the appender is still attached. Verified, rather than assumed:app.configwith<Dsn value="…" />log4net:ERROR … Cannot find Property [Dsn] to set object on [Sentry.Log4Net.SentryAppender], config loads, appender attachedlog4net:ERROR Failed to set parameter [Dsn] … NotSupportedException: <migration message>, config loads, appender attachedappender.Dsn = "…"in codeCS1061: SentryAppender does not contain a definition for 'Dsn'CS0619carrying the migration messageapp.configwithout<Dsn>So the gain is a better message on the XML path and a compile error carrying migration guidance on the code path — not the hard startup failure Serilog and NLog get. Both messages go to
Console.Errorvia log4net's internal logging unless quiet mode is on.If a hard failure matters more than keeping the appender working, the alternative is to record the attempt in the setter and throw from
ActivateOptions.ParseAppendercatches that too, but it drops the appender entirely, so nothing reaches Sentry at all — worse for anyone who initializes correctly but leaves a stale<Dsn>behind. I went with the setter; say the word if you'd rather have the louder one.Notes for review
Dsnwas set.IsEnabled == false, and events were only captured because the mocked init had run.Environmentgets no tombstone, following feat(nlog)!: the Sentry target no longer initializes the SDK #5585, which tombstoned only the SDK-init settings. It's a one-line addition if you'd rather it carried a message too — log4net reports the removal either way, just less usefully.UseLog4Net(): identity and properties are applied by the appender to the events and logs it creates.ApiApprovalTests.Run.Net4_8andDotNet10_0are copies of the regeneratedDotNet11_0snapshot, which all three matched byte-for-byte beforehand.Net4_8can't regenerate on macOS, andDotNet10_0isn't run at all since the test project only targets the latest TFM.🤖 Generated with Claude Code