Serilog integration tests

CHANGELOG.md

@@ -10,6 +10,7 @@
 ### Fixes
 
 - Fix logging loop with NLog sentry ([#1824](https://github.com/getsentry/sentry-dotnet/pull/1824))
+- Fix logging loop with Serilog sentry ([#1828](https://github.com/getsentry/sentry-dotnet/pull/1828))
 
 ## 3.20.1
 

samples/Sentry.Samples.Serilog/Program.cs

@@ -14,12 +14,15 @@ private static void Main()
             // Other overloads exist, for example, configure the SDK with only the DSN or no parameters at all.
             .WriteTo.Sentry(o =>
             {
-                o.MinimumBreadcrumbLevel = LogEventLevel.Debug; // Debug and higher are stored as breadcrumbs (default os Information)
-                o.MinimumEventLevel = LogEventLevel.Error; // Error and higher is sent as event (default is Error)
+                // Debug and higher are stored as breadcrumbs (default os Information)
+                o.MinimumBreadcrumbLevel = LogEventLevel.Debug;
+                // Error and higher is sent as event (default is Error)
+                o.MinimumEventLevel = LogEventLevel.Error;
                 // If DSN is not set, the SDK will look for an environment variable called SENTRY_DSN. If nothing is found, SDK is disabled.
                 o.Dsn = "https://eb18e953812b41c3aeb042e666fd3b5c@o447951.ingest.sentry.io/5428537";
                 o.AttachStacktrace = true;
-                o.SendDefaultPii = true; // send PII like the username of the user logged in to the device
+                // send PII like the username of the user logged in to the device
+                o.SendDefaultPii = true;
                 // Optional Serilog text formatter used to format LogEvent to string. If TextFormatter is set, FormatProvider is ignored.
                 o.TextFormatter = new MessageTemplateTextFormatter("[{MyTaskId}] {Message}");
                 // Other configuration

src/Sentry.Serilog/LogLevelExtensions.cs

@@ -1,7 +1,20 @@
 namespace Sentry.Serilog;
 
-internal static class LogLevelExtensions
+internal static class SentryExtensions
 {
+    public static bool TryGetSourceContext(this LogEvent logEvent, [NotNullWhen(true)] out string? sourceContext)
+    {
+        if (logEvent.Properties.TryGetValue("SourceContext", out var prop) &&
+            prop is ScalarValue {Value: string sourceContextValue})
+        {
+            sourceContext = sourceContextValue;
+            return true;
+        }
+
+        sourceContext = null;
+        return false;
+    }
+
     public static SentryLevel? ToSentryLevel(this LogEventLevel loggingLevel)
     {
         return loggingLevel switch

src/Sentry.Serilog/Sentry.Serilog.csproj

@@ -10,6 +10,7 @@
     <ProjectReference Include="..\..\src\Sentry\Sentry.csproj" />
 
     <Using Include="System.ComponentModel" />
+    <Using Include="System.Diagnostics.CodeAnalysis" />
     <Using Include="System.IO.Compression" />
     <Using Include="System.Net" />
     <Using Include="Sentry" />

src/Sentry.Serilog/SentrySink.cs

@@ -1,9 +1,3 @@
-using Sentry.Extensibility;
-using Sentry.Infrastructure;
-using Sentry.Reflection;
-using Serilog.Core;
-using Serilog.Events;
-
 namespace Sentry.Serilog;
 
 /// <summary>
@@ -19,6 +13,11 @@ internal sealed class SentrySink : ILogEventSink, IDisposable
     internal static readonly SdkVersion NameAndVersion
         = typeof(SentrySink).Assembly.GetNameAndVersion();
 
+    /// <summary>
+    /// Serilog SDK name.
+    /// </summary>
+    public const string SdkName = "sentry.dotnet.serilog";
+
     private static readonly string ProtocolPackageName = "nuget:" + NameAndVersion.Name;
 
     private readonly Func<IHub> _hubAccessor;
@@ -47,21 +46,35 @@ internal SentrySink(
         _sdkDisposable = sdkDisposable;
     }
 
+    static AsyncLocal<bool> isReentrant = new();
+
     public void Emit(LogEvent logEvent)
     {
-        string? context = null;
+        if (isReentrant.Value)
+        {
+            _options.DiagnosticLogger?.LogError($"Reentrant log event detected. Logging when inside the scope of another log event can cause a StackOverflowException. LogEventInfo.Message: {logEvent.MessageTemplate.Text}");
+            return;
+        }
+
+        isReentrant.Value = true;
+        try
+        {
+            InnerEmit(logEvent);
+        }
+        finally
+        {
+            isReentrant.Value = false;
+        }
+    }
 
-        if (logEvent.Properties.TryGetValue("SourceContext", out var prop)
-            && prop is ScalarValue scalar
-            && scalar.Value is string sourceContextValue)
+    private void InnerEmit(LogEvent logEvent)
+    {
+        if (logEvent.TryGetSourceContext(out var context))
         {
-            if (sourceContextValue.StartsWith("Sentry.")
-                || string.Equals(sourceContextValue, "Sentry", StringComparison.Ordinal))
+            if (IsSentryContext(context))
             {
                 return;
             }
-
-            context = sourceContextValue;
         }
 
         var hub = _hubAccessor();
@@ -89,7 +102,7 @@ public void Emit(LogEvent logEvent)
 
             if (evt.Sdk is { } sdk)
             {
-                sdk.Name = Constants.SdkName;
+                sdk.Name = SdkName;
                 sdk.Version = NameAndVersion.Version;
 
                 if (NameAndVersion.Version is { } version)
@@ -104,30 +117,36 @@ public void Emit(LogEvent logEvent)
         }
 
         // Even if it was sent as event, add breadcrumb so next event includes it
-        if (logEvent.Level >= _options.MinimumBreadcrumbLevel)
+        if (logEvent.Level < _options.MinimumBreadcrumbLevel)
         {
-            Dictionary<string, string>? data = null;
-            if (exception != null && !string.IsNullOrWhiteSpace(formatted))
-            {
-                // Exception.Message won't be used as Breadcrumb message
-                // Avoid losing it by adding as data:
-                data = new Dictionary<string, string>
-                {
-                    {"exception_message", exception.Message}
-                };
-            }
+            return;
+        }
 
-            hub.AddBreadcrumb(
-                _clock,
-                string.IsNullOrWhiteSpace(formatted)
-                    ? exception?.Message ?? ""
-                    : formatted,
-                context,
-                data: data,
-                level: logEvent.Level.ToBreadcrumbLevel());
+        Dictionary<string, string>? data = null;
+        if (exception != null && !string.IsNullOrWhiteSpace(formatted))
+        {
+            // Exception.Message won't be used as Breadcrumb message
+            // Avoid losing it by adding as data:
+            data = new Dictionary<string, string>
+            {
+                {"exception_message", exception.Message}
+            };
         }
+
+        hub.AddBreadcrumb(
+            _clock,
+            string.IsNullOrWhiteSpace(formatted)
+                ? exception?.Message ?? ""
+                : formatted,
+            context,
+            data: data,
+            level: logEvent.Level.ToBreadcrumbLevel());
     }
 
+    private static bool IsSentryContext(string context) =>
+        context.StartsWith("Sentry.") ||
+        string.Equals(context, "Sentry", StringComparison.Ordinal);
+
     private string FormatLogEvent(LogEvent logEvent)
     {
         if (_options.TextFormatter is { } formatter)

test/Directory.Build.props

@@ -46,7 +46,7 @@
     <PackageReference Include="FluentAssertions" Version="6.7.0" />
     <PackageReference Include="xunit" Version="2.4.1" />
     <PackageReference Include="Xunit.SkippableFact" Version="1.4.13" />
-    <PackageReference Include="Verify.Xunit" Version="17.4.2" />
+    <PackageReference Include="Verify.Xunit" Version="17.5.0" />
     <PackageReference Include="Verify.DiffPlex" Version="1.3.0" />
     <PackageReference Include="PublicApiGenerator" Version="10.3.0" />
     <PackageReference Include="GitHubActionsTestLogger" Version="2.0.1" />

test/Sentry.Serilog.Tests/IntegrationTests.LoggingInsideTheContextOfLogging.verified.txt

@@ -0,0 +1,41 @@
+{
+  diagnosticLoggerEntries: [
+    {
+      Level: error,
+      Message: Reentrant log event detected. Logging when inside the scope of another log event can cause a StackOverflowException. LogEventInfo.Message: message from OnEvaluating
+    }
+  ],
+  Envelopes: [
+    {
+      Header: {
+        event_id: Guid_1,
+        sdk: {
+          name: sentry.dotnet,
+          version: 3.20.1
+        }
+      },
+      Items: [
+        {
+          Header: {
+            type: event
+          },
+          Payload: {
+            Source: {
+              Message: {
+                Message: message,
+                Formatted: message
+              },
+              Platform: csharp,
+              SentryExceptions: [],
+              SentryThreads: [],
+              Level: error,
+              Request: {},
+              User: {},
+              Environment: production
+            }
+          }
+        }
+      ]
+    }
+  ]
+}