Repository navigation
fix(maui)!: options set in UseSentry now take precedence over appsettings.json - #5658
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>
…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>
… 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>
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>
…sposes 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>
…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>
…it-from-logging-mel-5245
… 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>
…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
…n 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>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into feat/no-init-from-logging-nlog-5245
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…245' into feat/no-init-from-logging-log4net-5245
…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>
…t-5245' into feat/no-init-from-logging-mel-5245
…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>
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>
…-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>
…t-5245' into feat/no-init-from-logging-mel-5245 Picks up version7 (which now has main merged in) plus #5573 and the NLog and log4net branches. test/Sentry.Extensions.Logging.Tests/SentryLoggingOptionsSetupTests.cs was the only textual conflict, and kept this branch's side: the test asserts only MinimumBreadcrumbLevel and MinimumEventLevel because SentryLoggingOptions no longer derives from SentryOptions, so the core SDK properties the other side binds don't exist on it. version7's only change to that file was #5631 removing three assertions this version never makes. Two things merged cleanly but did not compile, where main's new log entry filters meet this branch's options split: - Main moved the category, EF and filter checks out of ShouldCaptureEvent into their own early return, leaving it level-only, so the runtime warning's gate no longer had a 3-argument overload to call. It now goes through WouldCaptureEvent, which mirrors the real path. - SentryLogger reported a failing filter callback via _options.LogError, which only resolves while SentryLoggingOptions is a SentryOptions. It now reports through the hub's options, matching how the structured logger reads its defaults on this branch. The two tests covering it set the diagnostic logger substitute on the hub's options instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… 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>
…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>
…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>
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>
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>
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>
…ings.json SentryMauiOptionsSetup both bound the Sentry configuration section and applied the settings MAUI forces (global mode, the debug logger, the screenshot and SDK event processors). It was registered after the UseSentry callback, and IConfigureOptions<T> run in registration order, so a key in appsettings.json overrode the same setting passed to UseSentry - the reverse of ASP.NET Core. Moving the whole setup ahead of the callback would have broken the forced half, which reads Debug and AttachScreenshot as the callback leaves them. The binding now lives in SentryMauiConfigurationOptionsSetup, registered before the callback; SentryMauiOptionsSetup keeps the forced settings and still runs last. Fixes #5657 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 @@
## version7 #5658 +/- ##
===========================================
Coverage ? 75.09%
===========================================
Files ? 520
Lines ? 18880
Branches ? 3658
===========================================
Hits ? 14177
Misses ? 3840
Partials ? 863 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
How the MAUI options pieces fit togetherThis is for review: it explains why
sequenceDiagram
participant App as MauiProgram
participant UseSentry
participant Init as MauiApp.Build<br/>(SentryMauiInitializer)
participant Factory as Options factory
App->>UseSentry: UseSentry(callback)
Note over UseSentry: Registers, in this order:<br/>1. SentryMauiConfigurationOptionsSetup<br/>2. the callback (services.Configure)<br/>3. SentryMauiOptionsSetup
App->>Init: Build()
Init->>Factory: IOptions.Value
Note over Factory: new SentryMauiOptions()<br/>constructor defaults
Factory->>Factory: SentryMauiConfigurationOptionsSetup<br/>binds the "Sentry" section
Factory->>Factory: the UseSentry callback
Factory->>Factory: SentryMauiOptionsSetup<br/>forced settings
Factory-->>Init: options
Init->>Init: SentrySdk.Init(options)
Precedence: constructor defaults → Before this PR both jobs lived in Two existing behaviours this PR doesn't change:
|
Summary
In MAUI, a setting in
appsettings.jsonoverrode the same setting passed toUseSentry(options => ...). ASP.NET Core does the reverse: configuration supplies the defaults and the callback overrides them.SentryMauiOptionsSetupdid two jobs. It bound theSentrysection, and it applied the settings MAUI forces: global mode, the debug logger, the SDK and screenshot event processors, andNative.AttachScreenshot. It was registered after the callback, so the binding ran last.The issue suggests registering that setup before the callback. That would break the forced half, which reads
DebugandAttachScreenshotafter the callback has set them. For example,UseSentry(o => o.AttachScreenshot = true)would never add the screenshot processor. So this PR splits the class instead:SentryMauiConfigurationOptionsSetupbinds theSentrysection. It is registered before the callback.SentryMauiOptionsSetupkeeps the forced settings and still runs after it.Precedence is now:
Sentryconfiguration section →UseSentrycallback → settings MAUI forces.For reviewers
There's a bit of a blurb on what's going on in this comment:
Notes
appsettings.jsonoverriding the callback, hence the!. The stack lands onversion7.Services.Configure<SentryMauiOptions>(...)registered beforeUseSentrystill runs before the configuration binding, as it did before. Only the callback has moved.UseSentry_DebugTrueInConfiguration_ConsoleAndTracingDiagnosticsLoggerchecks the split: a setting from configuration still has to reach the forced settings.Closes #5657
🤖 Generated with Claude Code