Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/Sentry.Serilog/SentrySinkExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public static class SentrySinkExtensions
/// <param name="deduplicateMode">What modes to use for event automatic de-duplication. <seealso cref="SentryOptions.DeduplicateMode"/></param>
/// <param name="defaultTags">Default tags to add to all events. <seealso cref="SentryOptions.DefaultTags"/></param>
/// <param name="enableLogs">Whether to send structured logs. <seealso cref="SentryOptions.EnableLogs"/></param>
/// <param name="restrictedToMinimumLevel">The minimum level for events passed through the sink. Ignored when <paramref name="levelSwitch"/> is specified.</param>
/// <param name="levelSwitch">A switch allowing the pass-through minimum level to be changed at runtime.</param>
/// <returns><see cref="LoggerConfiguration"/></returns>
/// <example>This sample shows how each item may be set from within a configuration file:
/// <code>
Expand Down Expand Up @@ -106,7 +108,9 @@ public static LoggerConfiguration Sentry(
ReportAssembliesMode? reportAssembliesMode = null,
DeduplicateMode? deduplicateMode = null,
Dictionary<string, string>? defaultTags = null,
bool? enableLogs = null)
bool? enableLogs = null,
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
LoggingLevelSwitch? levelSwitch = null)
{
return loggerConfiguration.Sentry(o => ConfigureSentrySerilogOptions(o,
dsn,
Expand All @@ -132,7 +136,9 @@ public static LoggerConfiguration Sentry(
reportAssembliesMode,
deduplicateMode,
defaultTags,
enableLogs));
enableLogs),
restrictedToMinimumLevel,
levelSwitch);
}

/// <summary>
Expand All @@ -147,6 +153,8 @@ public static LoggerConfiguration Sentry(
/// <param name="minimumBreadcrumbLevel">Minimum log level to record a breadcrumb. <seealso cref="SentrySerilogOptions.MinimumBreadcrumbLevel"/></param>
/// <param name="formatProvider">The Serilog format provider. <seealso cref="IFormatProvider"/></param>
/// <param name="textFormatter">The Serilog text formatter. <seealso cref="ITextFormatter"/></param>
/// <param name="restrictedToMinimumLevel">The minimum level for events passed through the sink. Ignored when <paramref name="levelSwitch"/> is specified.</param>
/// <param name="levelSwitch">A switch allowing the pass-through minimum level to be changed at runtime.</param>
/// <returns><see cref="LoggerConfiguration"/></returns>
/// <example>This sample shows how each item may be set from within a configuration file:
/// <code>
Expand Down Expand Up @@ -174,15 +182,18 @@ public static LoggerConfiguration Sentry(
LogEventLevel? minimumEventLevel = null,
LogEventLevel? minimumBreadcrumbLevel = null,
IFormatProvider? formatProvider = null,
ITextFormatter? textFormatter = null
)
ITextFormatter? textFormatter = null,
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
LoggingLevelSwitch? levelSwitch = null)
{
return loggerConfiguration.Sentry(o => ConfigureSentrySerilogOptions(o,
null,
minimumEventLevel,
minimumBreadcrumbLevel,
formatProvider,
textFormatter));
textFormatter),
restrictedToMinimumLevel,
levelSwitch);
}

internal static void ConfigureSentrySerilogOptions(
Expand Down Expand Up @@ -351,9 +362,13 @@ internal static void ConfigureSentrySerilogOptions(
/// </summary>
/// <param name="loggerConfiguration">The logger configuration.</param>
/// <param name="configureOptions">The configure options callback.</param>
/// <param name="restrictedToMinimumLevel">The minimum level for events passed through the sink. Ignored when <paramref name="levelSwitch"/> is specified.</param>
/// <param name="levelSwitch">A switch allowing the pass-through minimum level to be changed at runtime.</param>
public static LoggerConfiguration Sentry(
this LoggerSinkConfiguration loggerConfiguration,
Action<SentrySerilogOptions> configureOptions)
Action<SentrySerilogOptions> configureOptions,
LogEventLevel restrictedToMinimumLevel = LevelAlias.Minimum,
LoggingLevelSwitch? levelSwitch = null)
{
var options = new SentrySerilogOptions();
configureOptions?.Invoke(options);
Expand All @@ -364,6 +379,6 @@ public static LoggerConfiguration Sentry(
sdkDisposable = SentrySdk.Init(options);
}

return loggerConfiguration.Sink(new SentrySink(options, sdkDisposable));
return loggerConfiguration.Sink(new SentrySink(options, sdkDisposable), restrictedToMinimumLevel, levelSwitch);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ namespace Serilog
{
public static class SentrySinkExtensions
{
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(
this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration,
string dsn,
Expand All @@ -47,6 +47,8 @@ namespace Serilog
Sentry.ReportAssembliesMode? reportAssembliesMode = default,
Sentry.DeduplicateMode? deduplicateMode = default,
System.Collections.Generic.Dictionary<string, string>? defaultTags = null,
bool? enableLogs = default) { }
bool? enableLogs = default,
Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0,
Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ namespace Serilog
{
public static class SentrySinkExtensions
{
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(
this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration,
string dsn,
Expand All @@ -47,6 +47,8 @@ namespace Serilog
Sentry.ReportAssembliesMode? reportAssembliesMode = default,
Sentry.DeduplicateMode? deduplicateMode = default,
System.Collections.Generic.Dictionary<string, string>? defaultTags = null,
bool? enableLogs = default) { }
bool? enableLogs = default,
Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0,
Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ namespace Serilog
{
public static class SentrySinkExtensions
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding optional parameters to public Sentry() overloads is a binary-breaking change

Adding new optional parameters (restrictedToMinimumLevel, levelSwitch) to the existing public Sentry() extension methods changes their metadata signatures. While source-compatible, this is binary-breaking in .NET: consumers compiled against a previous version of Sentry.Serilog will get a MissingMethodException at runtime until they recompile. For a public library this is an API contract change that warrants senior engineer review and, ideally, preservation of the prior signatures via additional overloads instead of in-place parameter additions.

Verification

Read src/Sentry.Serilog/SentrySinkExtensions.cs and confirmed the two affected overloads (lines 86-113 and 180-187) had new optional parameters appended rather than being introduced as new overloads. Verified that the API approval snapshot reflects the same signature change at lines 24 and 49-53 of the .verified.txt hunk. .NET's binding to optional parameters is resolved at the call site at compile time, so adding optional parameters changes the method's metadata token and breaks binary compatibility, even though source compiles unchanged.

Identified by Warden code-review · PB4-XBA

Copy link
Copy Markdown
Collaborator Author

@jamescrosswell jamescrosswell May 7, 2026

Choose a reason for hiding this comment

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

@Flash0ver I'd rather go with a binary breaking change here (it's source compatible). The signatures get really messy if we try to add overloads (there are domino effects).

I don't think there are many people swapping out the Sentry dll without recompiling their apps when they upgrade (I'd venture to say probably none)... and accessing functionality like this via reflection also seems highly improbable.

How do you feel about that?

public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, System.Action<Sentry.Serilog.SentrySerilogOptions> configureOptions, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration, Serilog.Events.LogEventLevel? minimumEventLevel = default, Serilog.Events.LogEventLevel? minimumBreadcrumbLevel = default, System.IFormatProvider? formatProvider = null, Serilog.Formatting.ITextFormatter? textFormatter = null, Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0, Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
public static Serilog.LoggerConfiguration Sentry(
this Serilog.Configuration.LoggerSinkConfiguration loggerConfiguration,
string dsn,
Expand All @@ -47,6 +47,8 @@ namespace Serilog
Sentry.ReportAssembliesMode? reportAssembliesMode = default,
Sentry.DeduplicateMode? deduplicateMode = default,
System.Collections.Generic.Dictionary<string, string>? defaultTags = null,
bool? enableLogs = default) { }
bool? enableLogs = default,
Serilog.Events.LogEventLevel restrictedToMinimumLevel = 0,
Serilog.Core.LoggingLevelSwitch? levelSwitch = null) { }
}
}
34 changes: 34 additions & 0 deletions test/Sentry.Serilog.Tests/SentrySerilogSinkExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,40 @@
Assert.Equal(_fixture.MinimumBreadcrumbLevel, sut.MinimumBreadcrumbLevel);
}

[Fact]
public void Sentry_WithRestrictedToMinimumLevel_FiltersLogsBelow()
{
using var logger = new LoggerConfiguration()
.WriteTo.Sentry(o =>
{
o.InitializeSdk = false;
o.MinimumBreadcrumbLevel = LogEventLevel.Debug;
o.MinimumEventLevel = LogEventLevel.Debug;
}, restrictedToMinimumLevel: LogEventLevel.Error)
.CreateLogger();

// Below restrictedToMinimumLevel — should not reach the sink
logger.Warning("This should be filtered by Serilog before reaching the sink");

// At or above restrictedToMinimumLevel — should reach the sink

Check warning on line 143 in test/Sentry.Serilog.Tests/SentrySerilogSinkExtensionsTests.cs

View check run for this annotation

@sentry/warden / warden: code-review

Filter test has no assertions verifying filtering behavior

`Sentry_WithRestrictedToMinimumLevel_FiltersLogsBelow` logs a Warning and an Error but never asserts that the Warning was filtered out by Serilog or that the Error was delivered to the sink. With `InitializeSdk = false` and no spy/mock sink capturing log events, the test will pass even if `restrictedToMinimumLevel` is completely ignored. As a result, the new pass-through level functionality is not actually covered by a behavioral test, only by a does-not-throw smoke test.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
Outdated
logger.Error("This should reach the sink");
}

[Fact]
public void Sentry_WithRestrictedToMinimumLevel_NoDsn_ParameterIsAccepted()
{
// Verify the no-DSN overload accepts restrictedToMinimumLevel without throwing
var ex = Record.Exception(() =>
new LoggerConfiguration()
.WriteTo.Sentry(
minimumBreadcrumbLevel: LogEventLevel.Verbose,
minimumEventLevel: LogEventLevel.Error,
restrictedToMinimumLevel: LogEventLevel.Warning)
.CreateLogger());

Assert.Null(ex);
}

private static void AssertEqualDeep(object expected, object actual)
{
var serializedLeftObject = JsonSerializer.Serialize(expected);
Expand Down
Loading