Logging loop with n log sentry

CHANGELOG.md

@@ -7,6 +7,10 @@
 - Added 'integrations' to SdkVersion ([#1820](https://github.com/getsentry/sentry-dotnet/pull/1820))
 - Updated Sentry Android SDK to version 6.3.0 ([#1826](https://github.com/getsentry/sentry-dotnet/pull/1826))
 
+### Fixes
+
+- Fix logging loop with NLog sentry ([#1824](https://github.com/getsentry/sentry-dotnet/pull/1824))
+
 ## 3.20.1
 
 ### Fixes

src/Sentry.NLog/SentryTarget.cs

@@ -283,7 +283,36 @@ protected override void FlushAsync(AsyncContinuation asyncContinuation)
     /// <inheritdoc />
     protected override void Write(LogEventInfo logEvent)
     {
-        if (logEvent?.Message == null)
+        //TODO: check if can really be null
+        // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract
+        if (logEvent == null)
+        {
+            return;
+        }
+
+        const string sentryContext = "__sentry__";
+        if (GetContextNdlc(logEvent)?.Contains(sentryContext) is true)
+        {
+            Options.DiagnosticLogger?.LogError($"Reentrant log event detected. Logging when inside the scope of another log event can cause a StackOverflowException. LogEventInfo.Message:{logEvent.Message}");
+            return;
+        }
+
+        using var _ = NestedDiagnosticsLogicalContext.Push(sentryContext);
+
+        try
+        {
+            InnerWrite(logEvent);
+        }
+        catch (Exception exception)
+        {
+            Options.DiagnosticLogger?.LogError("Failed to write log event", exception);
+            throw;
+        }
+    }
+
+    private void InnerWrite(LogEventInfo logEvent)
+    {
+        if (logEvent.Message == null)
         {
             return;
         }
@@ -347,6 +376,7 @@ protected override void Write(LogEventInfo logEvent)
                     {
                         overridenFingerprint.Add("{{ default }}");
                     }
+
                     overridenFingerprint.Add(groupingKey);
 
                     evt.SetFingerprint(overridenFingerprint);
@@ -377,8 +407,8 @@ protected override void Write(LogEventInfo logEvent)
                 // Exception won't be used as Breadcrumb message. Avoid losing it by adding as data:
                 data = new Dictionary<string, string>
                 {
-                    { "exception_type", exception.GetType().ToString() },
-                    { "exception_message", exception.Message },
+                    {"exception_type", exception.GetType().ToString()},
+                    {"exception_message", exception.Message},
                 };
             }
 

src/Sentry/Properties/AssemblyInfo.cs

@@ -5,6 +5,7 @@
 [assembly: InternalsVisibleTo("Sentry.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
 [assembly: InternalsVisibleTo("Sentry.Testing, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
 [assembly: InternalsVisibleTo("Sentry.Log4Net.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
+[assembly: InternalsVisibleTo("Sentry.NLog.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
 [assembly: InternalsVisibleTo("Sentry.Serilog.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
 [assembly: InternalsVisibleTo("Sentry.AspNet, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]
 [assembly: InternalsVisibleTo("Sentry.AspNet.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010059964a931488bcdbd14657f1ee0df32df61b57b3d14d7290c262c2cc9ddaad6ec984044f761f778e1823049d2cb996a4f58c8ea5b46c37891414cb34b4036b1c178d7b582289d2eef3c0f1e9b692c229a306831ee3d371d9e883f0eb0f74aeac6c6ab8c85fd1ec04b267e15a31532c4b4e2191f5980459db4dce0081f1050fb8")]

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

@@ -0,0 +1,42 @@
+{
+  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
+              },
+              Logger: Sentry.NLog.Tests.IntegrationTests,
+              Platform: csharp,
+              SentryExceptions: [],
+              SentryThreads: [],
+              Level: error,
+              Request: {},
+              User: {},
+              Environment: production
+            }
+          }
+        }
+      ]
+    }
+  ]
+}

test/Sentry.NLog.Tests/IntegrationTests.Simple.verified.txt

@@ -0,0 +1,67 @@
+[
+  {
+    Header: {
+      event_id: Guid_1,
+      sdk: {
+        name: sentry.dotnet,
+        version: 3.20.1
+      }
+    },
+    Items: [
+      {
+        Header: {
+          type: event
+        },
+        Payload: {
+          Source: {
+            Exception: {
+              $type: Exception,
+              Type: Exception,
+              Message: Exception message
+            },
+            Message: {
+              Message: message = {arg},
+              Formatted: message = "arg value"
+            },
+            Logger: Sentry.NLog.Tests.IntegrationTests,
+            Platform: csharp,
+            SentryExceptions: [
+              {
+                Type: System.Exception,
+                Value: Exception message,
+                Stacktrace: {
+                  Frames: [
+                    {
+                      Function: Task IntegrationTests.Simple(),
+                      InApp: false
+                    }
+                  ]
+                },
+                Mechanism: {}
+              }
+            ],
+            SentryThreads: [],
+            Level: error,
+            Request: {},
+            User: {
+              Email: ,
+              Id: myId,
+              IpAddress: ,
+              Username: ,
+              Other: {
+                mood: joyous
+              }
+            },
+            Environment: production,
+            Extra: {
+              arg: arg value
+            },
+            Tags: {
+              logger: Sentry.NLog.Tests.IntegrationTests
+            }
+          }
+        }
+      }
+    ]
+  }
+]

test/Sentry.NLog.Tests/IntegrationTests.cs

@@ -0,0 +1,105 @@
+using NLog;
+using NLog.Config;
+using NLog.Targets;
+using Sentry.Testing;
+
+namespace Sentry.NLog.Tests;
+
+[UsesVerify]
+public class IntegrationTests
+{
+    [Fact]
+    public Task Simple()
+    {
+        var transport = new RecordingTransport();
+
+        var configuration = new LoggingConfiguration();
+
+        configuration.AddSentry(
+            options =>
+            {
+                options.TracesSampleRate = 1;
+                options.Layout = "${message}";
+                options.Transport = transport;
+                options.DiagnosticLevel = SentryLevel.Debug;
+                options.IncludeEventDataOnBreadcrumbs = true;
+                options.MinimumBreadcrumbLevel = LogLevel.Debug;
+                options.Dsn = ValidDsn;
+                options.User = new SentryNLogUser
+                {
+                    Id = "${mdlc:item=id}",
+                    Username = "${mdlc:item=username}",
+                    Email = "${mdlc:item=email}",
+                    IpAddress = "${mdlc:item=ipAddress}",
+                    Other =
+                    {
+                        new TargetPropertyWithContext("mood", "joyous")
+                    },
+                };
+
+                options.AddTag("logger", "${logger}");
+            });
+
+        LogManager.Configuration = configuration;
+
+        var log = LogManager.GetCurrentClassLogger();
+
+        using (MappedDiagnosticsLogicalContext.SetScoped("id", "myId"))
+        {
+            try
+            {
+                throw new("Exception message");
+            }
+            catch (Exception exception)
+            {
+                log.Error(exception, "message = {arg}", "arg value");
+            }
+        }
+
+        LogManager.Flush();
+
+        return Verify(transport.Envelopes)
+            .IgnoreStandardSentryMembers();
+    }
+
+    [Fact]
+    public Task LoggingInsideTheContextOfLogging()
+    {
+        var transport = new RecordingTransport();
+
+        var configuration = new LoggingConfiguration();
+
+        var diagnosticLogger = new InMemoryDiagnosticLogger();
+        configuration.AddSentry(
+            options =>
+            {
+                options.TracesSampleRate = 1;
+                options.Debug = true;
+                options.DiagnosticLogger = diagnosticLogger;
+                options.Transport = transport;
+                options.Dsn = ValidDsn;
+            });
+
+        LogManager.Configuration = configuration;
+
+        var log = LogManager.GetCurrentClassLogger();
+
+        SentrySdk.ConfigureScope(
+            scope =>
+            {
+                scope.OnEvaluating += (_, _) => log.Error("message from OnEvaluating");
+                log.Error("message");
+            });
+        LogManager.Flush();
+
+        return Verify(
+                new
+                {
+                    diagnosticLoggerEntries = diagnosticLogger
+                        .Entries
+                        .Where(_ => _.Level == SentryLevel.Error),
+                    transport.Envelopes
+                })
+            .IgnoreStandardSentryMembers();
+    }
+}

test/Sentry.NLog.Tests/ModuleInit.cs

@@ -0,0 +1,10 @@
+using System.Runtime.CompilerServices;
+
+public static class ModuleInit
+{
+    [ModuleInitializer]
+    public static void Init()
+    {
+        VerifyDiffPlex.Initialize();
+    }
+}

test/Sentry.NLog.Tests/ModuleInitializerAttribute.cs

@@ -0,0 +1,10 @@
+#if !NET5_0_OR_GREATER
+// ReSharper disable once CheckNamespace - It's a polyfill to a type on this location
+namespace System.Runtime.CompilerServices
+{
+    [AttributeUsage(AttributeTargets.Method, Inherited = false)]
+    public sealed class ModuleInitializerAttribute : Attribute
+    {
+    }
+}
+#endif

test/Sentry.Testing/VerifyExtensions.cs

@@ -9,10 +9,25 @@ public static SettingsTask IgnoreStandardSentryMembers(this SettingsTask setting
             .IgnoreMembersWithType<DateTimeOffset>()
             .IgnoreMembersWithType<SpanId>()
             .IgnoreMembersWithType<SentryId>()
-            .IgnoreMembers<SentryEvent>(e => e.Modules, e => e.Release)
-            .IgnoreMembers<Request>(e => e.Env, e => e.Url, e => e.Headers)
-            .IgnoreMembers<Transaction>(t => t.Release)
-            .IgnoreMembers<SentryException>(e => e.Module, e => e.ThreadId);
+            .IgnoreMembers<SentryEvent>(
+                _ => _.Modules,
+                _ => _.Release)
+            .IgnoreMembers<Request>(
+                _ => _.Env,
+                _ => _.Url,
+                _ => _.Headers)
+            .IgnoreMembers<Transaction>(
+                _ => _.Release)
+            .IgnoreMembers<SentryException>(
+                _ => _.Module,
+                _ => _.ThreadId)
+            .IgnoreMembers<SentryStackFrame>(
+                _ => _.FileName,
+                _ => _.LineNumber,
+                _ => _.ColumnNumber,
+                _ => _.InstructionOffset,
+                _ => _.Package)
+            .IgnoreStackTrace();
     }
 
     class SpansConverter : WriteOnlyJsonConverter<IReadOnlyCollection<Span>>