All exceptions are now added as breadcrumbs on future events

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))
 

src/Sentry.Extensions.Logging/SentryLogger.cs

@@ -55,9 +55,11 @@ 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
+            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 +74,7 @@ public void Log<TState>(
 
             _hub.AddBreadcrumb(
                 _clock,
-                message ?? exception?.Message!,
+                (message ?? exception?.Message)!,
                 CategoryName,
                 null,
                 data,

src/Sentry.Serilog/SentrySink.cs

@@ -114,9 +114,11 @@ 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
+            return;
         }
 
-        // Even if it was sent as event, add breadcrumb so next event includes it
         if (logEvent.Level < _options.MinimumBreadcrumbLevel)
         {
             return;

src/Sentry/Internal/Hub.cs

@@ -379,6 +379,8 @@ private void ApplyTraceContextToEvent(SentryEvent evt, SentryPropagationContext
         evt.DynamicSamplingContext = propagationContext.GetOrCreateDynamicSamplingContext(_options);
     }
 
+    public bool CaptureEnvelope(Envelope envelope) => CurrentClient.CaptureEnvelope(envelope);
+
     public SentryId CaptureEvent(SentryEvent evt, Action<Scope> configureScope)
         => CaptureEvent(evt, null, configureScope);
 
@@ -394,7 +396,10 @@ 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
+            return CaptureEvent(evt, clonedScope, CurrentScope, hint);
         }
         catch (Exception e)
         {
@@ -403,9 +408,10 @@ 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)
+        => CaptureEvent(evt, scope ?? CurrentScope, scope ?? CurrentScope, hint);
+
+    private SentryId CaptureEvent(SentryEvent evt, Scope eventScope, Scope breadcrumbScope, SentryHint? hint)
     {
         if (!IsEnabled)
         {
@@ -414,10 +420,8 @@ 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;
+            var span = GetLinkedSpan(evt) ?? eventScope.Span;
             if (span is not null)
             {
                 if (span.IsSampled is not false)
@@ -428,22 +432,46 @@ public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null, SentryHint? h
             else
             {
                 // If there is no span on the scope (and not just no sampled one), fall back to the propagation context
-                ApplyTraceContextToEvent(evt, scope.PropagationContext);
+                ApplyTraceContextToEvent(evt, eventScope.PropagationContext);
             }
 
             // Now capture the event with the Sentry client on the current scope.
-            var id = CurrentClient.CaptureEvent(evt, scope, hint);
-            scope.LastEventId = id;
-            scope.SessionUpdate = null;
+            var id = CurrentClient.CaptureEvent(evt, eventScope, hint);
+            eventScope.LastEventId = id;
+            eventScope.SessionUpdate = null;
 
-            if (evt.HasTerminalException() && scope.Transaction is { } transaction)
+            if (evt.HasTerminalException() && eventScope.Transaction is { } transaction)
             {
                 // Event contains a terminal exception -> finish any current transaction as aborted
                 // Do this *after* the event was captured, so that the event is still linked to the transaction.
                 _options.LogDebug("Ending transaction as Aborted, due to unhandled exception.");
                 transaction.Finish(SpanStatus.Aborted);
             }
 
+            if (evt.Exception is { } exception)
+            {
+                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}
+                    };
+                }
+                breadcrumbScope.AddBreadcrumb(breadcrumbMessage, "Exception", data: data, level: BreadcrumbLevel.Critical);
+            }
+
             return id;
         }
         catch (Exception e)

test/Sentry.Extensions.Logging.Tests/SentryLoggerTests.cs

@@ -33,22 +33,17 @@ public void Log_InvokesSdkIsEnabled()
     }
 
     [Fact]
-    public void Log_WithException_BreadcrumbFromException()
+    public void Log_WithException_NoBreadcrumbFromException()
     {
         var expectedException = new Exception("expected message");
-        const BreadcrumbLevel expectedLevel = BreadcrumbLevel.Critical;
 
         var sut = _fixture.GetSut();
 
         sut.Log<object>(LogLevel.Critical, default, null, expectedException, null);
 
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Equal(expectedException.Message, b.Message);
-        Assert.Equal(DateTimeOffset.MaxValue, b.Timestamp);
-        Assert.Equal(_fixture.CategoryName, b.Category);
-        Assert.Equal(expectedLevel, b.Level);
-        Assert.Equal(BreadcrumbType, b.Type);
-        Assert.Null(b.Data);
+        // Breadcrumbs get created automatically by the hub for captured exceptions... we don't want
+        // our logging integration to be creating these also
+        _fixture.Scope.Breadcrumbs.Should().BeEmpty();
     }
 
     [Fact]
@@ -141,25 +136,6 @@ public void Log_WithEmptyGuidProperty_DoesntSetTagInEvent()
                 e => e.Tags.Count == 0));
     }
 
-    [Fact]
-    public void Log_WithEventId_EventIdAsBreadcrumbData()
-    {
-        var expectedEventId = new EventId(10, "EventId-!@#$%^&*(");
-        const BreadcrumbLevel expectedLevel = BreadcrumbLevel.Critical;
-
-        var sut = _fixture.GetSut();
-
-        sut.Log<object>(LogLevel.Critical, expectedEventId, null, null, null);
-
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Equal(expectedEventId.ToString(), b.Data![EventIdExtensions.DataKey]);
-        Assert.Equal(DateTimeOffset.MaxValue, b.Timestamp);
-        Assert.Equal(_fixture.CategoryName, b.Category);
-        Assert.Equal(expectedLevel, b.Level);
-        Assert.Equal(BreadcrumbType, b.Type);
-        Assert.Null(b.Message);
-    }
-
     [Fact]
     public void LogCritical_MatchingFilter_DoesNotCapturesEvent()
     {
@@ -206,26 +182,6 @@ public void LogCritical_NotMatchingFilter_CapturesEvent()
             .CaptureEvent(Arg.Any<SentryEvent>());
     }
 
-    [Fact]
-    public void LogCritical_NotMatchingFilter_AddsBreadcrumb()
-    {
-        var scope = new Scope();
-        _fixture.Hub.ConfigureScope(Arg.Invoke(scope));
-
-        const string expected = "message";
-        _fixture.Options.AddLogEntryFilter((_, _, _, _) => false);
-        _fixture.Options.AddLogEntryFilter((_, _, _, _) => false);
-
-        var sut = _fixture.GetSut();
-
-        sut.LogCritical(expected);
-
-        _fixture.Hub.Received(1)
-            .ConfigureScope(Arg.Any<Action<Scope>>());
-
-        _ = Assert.Single(scope.Breadcrumbs);
-    }
-
     [Fact]
     public void LogCritical_DefaultOptions_CapturesEvent()
     {
@@ -290,81 +246,6 @@ public void Log_SentrySomethingCategory_SendEvent()
             .CaptureEvent(Arg.Any<SentryEvent>());
     }
 
-    [Fact]
-    public void LogCritical_DefaultOptions_RecordsBreadcrumbs()
-    {
-        const string expectedMessage = "message";
-        const BreadcrumbLevel expectedLevel = BreadcrumbLevel.Critical;
-
-        var sut = _fixture.GetSut();
-
-        sut.LogCritical(expectedMessage);
-
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Equal(expectedMessage, b.Message);
-        Assert.Equal(DateTimeOffset.MaxValue, b.Timestamp);
-        Assert.Equal(_fixture.CategoryName, b.Category);
-        Assert.Equal(expectedLevel, b.Level);
-        Assert.Equal(BreadcrumbType, b.Type);
-        Assert.Null(b.Data);
-    }
-
-    [Fact]
-    public void LogCritical_ExceptionAndMessage_ExceptionMessageAsBreadcrumbData()
-    {
-        const string expectedMessage = "message";
-        var exception = new Exception("exception message");
-
-        var sut = _fixture.GetSut();
-
-        sut.LogCritical(exception, expectedMessage);
-
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Contains(b.Data,
-            pair => pair.Key == "exception_message" && pair.Value == exception.Message);
-        Assert.Equal(expectedMessage, b.Message);
-    }
-
-    [Fact]
-    public void LogCritical_ExceptionAndMessageAndEventId_ExceptionMessageAndEventIdAsBreadcrumbData()
-    {
-        const string expectedMessage = "message";
-        const int expectedEventId = 1;
-        var exception = new Exception("exception message");
-
-        var sut = _fixture.GetSut();
-
-        sut.LogCritical(expectedEventId, exception, expectedMessage);
-
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Contains(b.Data,
-            pair => pair.Key == "exception_message"
-                    && pair.Value == exception.Message);
-        Assert.Contains(b.Data,
-            pair => pair.Key == EventIdExtensions.DataKey
-                    && pair.Value == expectedEventId.ToString());
-        Assert.Equal(expectedMessage, b.Message);
-    }
-
-    [Fact]
-    public void LogError_DefaultOptions_RecordsBreadcrumbs()
-    {
-        const string expectedMessage = "message";
-        const BreadcrumbLevel expectedLevel = BreadcrumbLevel.Error;
-
-        var sut = _fixture.GetSut();
-
-        sut.LogError(expectedMessage);
-
-        var b = _fixture.Scope.Breadcrumbs.First();
-        Assert.Equal(expectedMessage, b.Message);
-        Assert.Equal(DateTimeOffset.MaxValue, b.Timestamp);
-        Assert.Equal(_fixture.CategoryName, b.Category);
-        Assert.Equal(expectedLevel, b.Level);
-        Assert.Equal(BreadcrumbType, b.Type);
-        Assert.Null(b.Data);
-    }
-
     [Fact]
     public void LogWarning_DefaultOptions_RecordsBreadcrumbs()
     {

test/Sentry.Serilog.Tests/IntegrationTests.Simple.DotNet6_0.verified.txt

@@ -86,12 +86,6 @@
               IpAddress: {{auto}}
             },
             Environment: production,
-            Breadcrumbs: [
-              {
-                Message: [42] Debug message stored as breadcrumb.,
-                Level: debug
-              }
-            ],
             Extra: {
               inventory: { SmallPotion = 3, BigPotion = 0, CheeseWheels = 512 },
               MyTaskId: 65
@@ -140,16 +134,6 @@
               IpAddress: {{auto}}
             },
             Environment: production,
-            Breadcrumbs: [
-              {
-                Message: [42] Debug message stored as breadcrumb.,
-                Level: debug
-              },
-              {
-                Message: [65] Message with a different MyTaskId,
-                Level: debug
-              }
-            ],
             Extra: {
               inventory: { SmallPotion = 3, BigPotion = 0, CheeseWheels = 512 },
               MyTaskId: 42
@@ -254,20 +238,6 @@
               IpAddress: {{auto}}
             },
             Environment: production,
-            Breadcrumbs: [
-              {
-                Message: [42] Debug message stored as breadcrumb.,
-                Level: debug
-              },
-              {
-                Message: [65] Message with a different MyTaskId,
-                Level: debug
-              },
-              {
-                Message: [42] Some event that includes the previous breadcrumbs,
-                Level: error
-              }
-            ],
             Extra: {
               inventory: { SmallPotion = 3, BigPotion = 0, CheeseWheels = 512 },
               MyTaskId: 42