Finish unfinished Spans on Transaction completion

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Finish unfinished Spans on Transaction completion ([#1296](https://github.com/getsentry/sentry-dotnet/pull/1296))
+
 ## 3.11.1
 
 ### Fixes

src/Sentry/SentryEventExtensions.cs

@@ -8,6 +8,8 @@ namespace Sentry
     public static class SentryEventExtensions
     {
         internal static bool IsErrored(this SentryEvent @event)
-            => @event.Exception is not null || @event.SentryExceptions?.Any() == true;
+            => @event.Level >= SentryLevel.Error ||
+               @event.Exception is not null ||
+               @event.SentryExceptionValues?.Values.Any() == true;
     }
 }

src/Sentry/Transaction.cs

@@ -225,7 +225,7 @@ public Transaction(ITransaction tracer)
             _breadcrumbs = tracer.Breadcrumbs.ToList();
             _extra = tracer.Extra.ToDictionary();
             _tags = tracer.Tags.ToDictionary();
-            _spans = tracer.Spans.Where(s => s.IsFinished).Select(s => new Span(s)).ToArray();
+            _spans = tracer.Spans.Select(s => new Span(s)).ToArray();
         }
 
         /// <inheritdoc />

src/Sentry/TransactionTracer.cs

@@ -225,6 +225,14 @@ public void Finish()
             Status ??= SpanStatus.UnknownError;
             EndTimestamp = DateTimeOffset.UtcNow;
 
+            foreach (var span in _spans)
+            {
+                if (!span.IsFinished)
+                {
+                    span.Finish(SpanStatus.DeadlineExceeded);
+                }
+            }
+
             // Clear the transaction from the scope
             _hub.ConfigureScope(scope => scope.ResetTransaction(this));
 

test/Sentry.Tests/HubTests.cs

@@ -284,7 +284,7 @@ public void CaptureEvent_ExceptionWithOpenSpan_SpanFinishedWithInternalError()
 
             var hub = new Hub(new SentryOptions
             {
-                Dsn = DsnSamples.ValidDsnWithSecret,
+                Dsn = DsnSamples.ValidDsnWithSecret
             }, client);
             var scope = new Scope();
             scope.Transaction = hub.StartTransaction("transaction", "operation");
@@ -1016,5 +1016,56 @@ public void ResumeSession_NoPausedSession_DoesNothing()
             // Assert
             client.DidNotReceive().CaptureSession(Arg.Is<SessionUpdate>(s => s.EndStatus != null));
         }
+
+        [Theory]
+        [InlineData(SentryLevel.Error)]
+        [InlineData(SentryLevel.Fatal)]
+        public void CaptureEvent_ErroredMessageWithoutException_ClosesCurrentSpanAsInternalError(SentryLevel level)
+        {
+            // Arrange
+            var client = Substitute.For<ISentryClient>();
+
+            var hub = new Hub(new SentryOptions
+            {
+                Dsn = DsnSamples.ValidDsnWithSecret
+            }, client);
+            var scope = new Scope();
+            scope.Transaction = hub.StartTransaction("transaction", "operation");
+
+            var child = scope.Transaction.StartChild("child", "child");
+
+            // Act
+            hub.CaptureEvent(new SentryEvent() { Message = "Logger error", Level = level }, scope);
+
+            // Assert
+            Assert.Equal(SpanStatus.InternalError, child.Status);
+            Assert.True(child.IsFinished);
+        }
+
+        [Theory]
+        [InlineData(SentryLevel.Warning)]
+        [InlineData(SentryLevel.Info)]
+        [InlineData(SentryLevel.Debug)]
+        public void CaptureEvent_InfoMessageWithoutException_SpanNotChanged(SentryLevel level)
+        {
+            // Arrange
+            var client = Substitute.For<ISentryClient>();
+
+            var hub = new Hub(new SentryOptions
+            {
+                Dsn = DsnSamples.ValidDsnWithSecret
+            }, client);
+            var scope = new Scope();
+            scope.Transaction = hub.StartTransaction("transaction", "operation");
+
+            var child = scope.Transaction.StartChild("child", "child");
+
+            // Act
+            hub.CaptureEvent(new SentryEvent() { Message = "Logger error", Level = level }, scope);
+
+            // Assert
+            Assert.Null(child.Status);
+            Assert.False(child.IsFinished);
+        }
     }
 }

test/Sentry.Tests/Protocol/TransactionTests.cs

@@ -235,46 +235,43 @@ public void Finish_RecordsTime()
         }
 
         [Fact]
-        public void Finish_CapturesTransaction()
+        public void Finish_UnfinishedSpansGetsFinishedWithDeadlineStatus()
         {
             // Arrange
-            var client = Substitute.For<ISentryClient>();
-            var options = new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret };
-            var hub = new Hub(options, client);
-
-            var transaction = new TransactionTracer(hub, "my name", "my op");
+            var transaction = new TransactionTracer(DisabledHub.Instance, "my name", "my op");
+            transaction.StartChild("children1");
+            transaction.StartChild("children2");
+            transaction.StartChild("children3.finished").Finish(SpanStatus.Ok);
+            transaction.StartChild("children4");
 
             // Act
             transaction.Finish();
 
             // Assert
-            client.Received(1).CaptureTransaction(Arg.Any<Transaction>());
+
+            Assert.All(transaction.Spans.Where(span => !span.Operation.EndsWith("finished")), span =>
+            {
+                Assert.True(span.IsFinished);
+                Assert.Equal(SpanStatus.DeadlineExceeded, span.Status);
+            });
+            Assert.Single(transaction.Spans.Where(span => span.Operation.EndsWith("finished") && span.Status == SpanStatus.Ok));
         }
 
         [Fact]
-        public void Finish_DropsUnfinishedSpans()
+        public void Finish_CapturesTransaction()
         {
             // Arrange
             var client = Substitute.For<ISentryClient>();
-            var hub = new Hub(new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret }, client);
+            var options = new SentryOptions { Dsn = DsnSamples.ValidDsnWithoutSecret };
+            var hub = new Hub(options, client);
 
             var transaction = new TransactionTracer(hub, "my name", "my op");
 
-            transaction.StartChild("op1").Finish();
-            transaction.StartChild("op2");
-            transaction.StartChild("op3").Finish();
-
             // Act
             transaction.Finish();
 
             // Assert
-            client.Received(1).CaptureTransaction(
-                Arg.Is<Transaction>(t =>
-                    t.Spans.Count == 2 &&
-                    t.Spans.Any(s => s.Operation == "op1") &&
-                    t.Spans.All(s => s.Operation != "op2") &&
-                    t.Spans.Any(s => s.Operation == "op3")
-                ));
+            client.Received(1).CaptureTransaction(Arg.Any<Transaction>());
         }
 
         [Fact]