All exceptions are now added as breadcrumbs on future events (#3584)

getsentry · Sep 11, 2024 · 9ad2caa · 9ad2caa
1 parent ea2dce1
commit 9ad2caa
Show file tree

Hide file tree

Showing 15 changed files with 235 additions and 140 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Features
+
+- All exceptions are now added as breadcrumbs on future events. Previously this was only the case for exceptions captured via the `Sentry.SeriLog` or `Sentry.Extensions.Logging` integrations. ([#3584](https://github.com/getsentry/sentry-dotnet/pull/3584))
+
 ### Fixes
 - On mobile devices, the SDK no longer throws a `FormatException` for `ProcessorFrequency` when trying to report native events ([#3541](https://github.com/getsentry/sentry-dotnet/pull/3541))
 - Add missing org parameter to the CLI release operations ([#3600](https://github.com/getsentry/sentry-dotnet/pull/3600))

diff --git a/src/Sentry.Extensions.Logging/SentryLogger.cs b/src/Sentry.Extensions.Logging/SentryLogger.cs
@@ -55,9 +55,14 @@ public void Log<TState>(
             var @event = CreateEvent(logLevel, eventId, state, exception, message, CategoryName);
 
             _ = _hub.CaptureEvent(@event);
+
+            // Capturing exception events adds a breadcrumb automatically... we don't want to add another one
+            if (exception != null)
+            {
+                return;
+            }
         }
 
-        // Even if it was sent as event, add breadcrumb so next event includes it
         if (ShouldAddBreadcrumb(logLevel, eventId, exception))
         {
             var data = eventId.ToDictionaryOrNull();
@@ -72,7 +77,7 @@ public void Log<TState>(
 
             _hub.AddBreadcrumb(
                 _clock,
-                message ?? exception?.Message!,
+                (message ?? exception?.Message)!,
                 CategoryName,
                 null,
                 data,

diff --git a/src/Sentry.NLog/SentryTarget.cs b/src/Sentry.NLog/SentryTarget.cs
@@ -387,6 +387,12 @@ private void InnerWrite(LogEventInfo logEvent)
             }
 
             _ = hub.CaptureEvent(evt);
+
+            // Capturing exception events adds a breadcrumb automatically... we don't want to add another one
+            if (exception != null)
+            {
+                return;
+            }
         }
 
         // Whether or not it was sent as event, add breadcrumb so the next event includes it

diff --git a/src/Sentry.Serilog/SentrySink.cs b/src/Sentry.Serilog/SentrySink.cs
@@ -114,9 +114,14 @@ private void InnerEmit(LogEvent logEvent)
             evt.SetExtras(GetLoggingEventProperties(logEvent));
 
             hub.CaptureEvent(evt);
+
+            // Capturing exception events adds a breadcrumb automatically... we don't want to add another one
+            if (exception != null)
+            {
+                return;
+            }
         }
 
-        // Even if it was sent as event, add breadcrumb so next event includes it
         if (logEvent.Level < _options.MinimumBreadcrumbLevel)
         {
             return;

diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs
@@ -379,6 +379,44 @@ private void ApplyTraceContextToEvent(SentryEvent evt, SentryPropagationContext
         evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options);
     }
 
+    public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);
+
+    private void AddBreadcrumbForException(SentryEvent evt, Scope scope)
+    {
+        try
+        {
+            if (!IsEnabled || evt.Exception is not { } exception)
+            {
+                return;
+            }
+
+            var exceptionMessage = exception.Message ?? "";
+            var formatted = evt.Message?.Formatted;
+
+            string breadcrumbMessage;
+            Dictionary<string, string>? data = null;
+            if (string.IsNullOrWhiteSpace(formatted))
+            {
+                breadcrumbMessage = exceptionMessage;
+            }
+            else
+            {
+                breadcrumbMessage = formatted;
+                // Exception.Message won't be used as Breadcrumb message
+                // Avoid losing it by adding as data:
+                data = new Dictionary<string, string>
+                {
+                    {"exception_message", exceptionMessage}
+                };
+            }
+            scope.AddBreadcrumb(breadcrumbMessage, "Exception", data: data, level: BreadcrumbLevel.Critical);
+        }
+        catch (Exception e)
+        {
+            _options.LogError(e, "Failure to store breadcrumb for exception event: {0}", evt.EventId);
+        }
+    }
+
     public SentryId CaptureEvent(SentryEvent evt, Action<Scope> configureScope)
         => CaptureEvent(evt, null, configureScope);
 
@@ -394,7 +432,12 @@ public SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Action<Scope> co
             var clonedScope = CurrentScope.Clone();
             configureScope(clonedScope);
 
-            return CaptureEvent(evt, clonedScope, hint);
+            // Although we clone a temporary scope for the configureScope action, for the second scope
+            // argument (the breadcrumbScope) we pass in the current scope... this is because we want
+            // a breadcrumb to be left on the current scope for exception events
+            var eventId = CaptureEvent(evt, hint, clonedScope);
+            AddBreadcrumbForException(evt, CurrentScope);
+            return eventId;
         }
         catch (Exception e)
         {
@@ -403,9 +446,15 @@ public SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Action<Scope> co
         }
     }
 
-    public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);
-
     public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null, SentryHint? hint = null)
+    {
+        scope ??= CurrentScope;
+        var eventId = CaptureEvent(evt, hint, scope);
+        AddBreadcrumbForException(evt, scope);
+        return eventId;
+    }
+
+    private SentryId CaptureEvent(SentryEvent evt, SentryHint? hint, Scope scope)
     {
         if (!IsEnabled)
         {
@@ -414,8 +463,6 @@ public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null, SentryHint? h
 
         try
         {
-            scope ??= CurrentScope;
-
             // We get the span linked to the event or fall back to the current span
             var span = GetLinkedSpan(evt) ?? scope.Span;
             if (span is not null)

diff --git a/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet6_0.verified.txt b/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet6_0.verified.txt
@@ -49,6 +49,13 @@
         Id: Guid_1,
         IpAddress: {{auto}}
       },
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,

diff --git a/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet7_0.verified.txt b/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet7_0.verified.txt
@@ -49,6 +49,13 @@
         Id: Guid_1,
         IpAddress: {{auto}}
       },
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,

diff --git a/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet8_0.verified.txt b/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet8_0.verified.txt
@@ -49,6 +49,13 @@
         Id: Guid_1,
         IpAddress: {{auto}}
       },
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,

diff --git a/...try.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Net4_8.verified.txt b/...try.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Net4_8.verified.txt
@@ -49,6 +49,13 @@
         Id: Guid_1,
         IpAddress: {{auto}}
       },
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,

diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSqlAsync.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSqlAsync.verified.txt
@@ -49,6 +49,13 @@
         Id: Guid_1,
         IpAddress: {{auto}}
       },
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,

diff --git a/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt b/test/Sentry.EntityFramework.Tests/IntegrationTests.Simple.verified.txt
@@ -51,6 +51,13 @@
         IpAddress: {{auto}}
       },
       Environment: production,
+      Breadcrumbs: [
+        {
+          Message: my exception,
+          Category: Exception,
+          Level: critical
+        }
+      ],
       Spans: [
         {
           IsFinished: true,