test(log4net): run the configuration binding tests against log4net 3 - #5651
Merged
jamescrosswell merged 1 commit intoSep 30, 2026
Conversation
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>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/no-init-from-logging-log4net-5245 #5651 +/- ##
=======================================================================
Coverage 74.96% 74.96%
=======================================================================
Files 515 515
Lines 18826 18826
Branches 3653 3653
=======================================================================
Hits 14112 14112
Misses 3855 3855
Partials 859 859 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jamescrosswell
merged commit Sep 30, 2026
248c671
into
feat/no-init-from-logging-log4net-5245
49 of 50 checks passed
jamescrosswell
added a commit
that referenced
this pull request
Sep 30, 2026
…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>
jamescrosswell
added a commit
that referenced
this pull request
Sep 30, 2026
…t-5245' into feat/no-init-from-logging-mel-5245 Picks up #5652 (a stale dsn or initializeSdk="true" in NLog.config is reported to standard error, and initializeSdk="false" is accepted rather than failing the configuration) and #5651 (the appender configuration binding tests also run against log4net 3), via the log4net branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jamescrosswell
added a commit
that referenced
this pull request
Oct 1, 2026
) * 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> * 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 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> * 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> * 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> * 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> --------- 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>
jamescrosswell
added a commit
that referenced
this pull request
Oct 1, 2026
…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>
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
2 participants
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.
SentryAppenderConfigurationBindingTestsfrom #5592 now also runs against log4net 3. The V3 test project links the other new test files but left this one out, because it didn't compile there. log4net 3 marks theXmlElementparameter ofXmlConfigurator.Configureas non-nullable, so the test now passesdocument.DocumentElement!. The four tests pass on log4net 3.4.0 as they do on 2.0.15, so both versions now cover theDsntombstone behavior.#skip-changelog
🤖 Generated with Claude Code