Fix don't overwrite the span status of unfinished spans

CHANGELOG.md

@@ -25,6 +25,8 @@ Breaking changes:
 - Move enableNdk from SentryOptions to SentryAndroidOptions ([#2793](https://github.com/getsentry/sentry-java/pull/2793))
 - Fix Coroutine Context Propagation using CopyableThreadContextElement, requires `kotlinx-coroutines-core` version `1.6.1` or higher ([#2838](https://github.com/getsentry/sentry-java/pull/2838))
 - Bump min API to 19 ([#2883](https://github.com/getsentry/sentry-java/pull/2883))
+- Fix don't overwrite the span status of unfinished spans ([#2859](https://github.com/getsentry/sentry-java/pull/2859))
+  - If you're using a self hosted version of sentry, sentry self hosted >= 22.12.0 is required
 
 ## Unreleased
 

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -213,14 +213,11 @@
         performanceCollectionData.clear();
       }
 
-      // finish unfinished children
-      for (final Span child : children) {
-        if (!child.isFinished()) {
-          child.setSpanFinishedCallback(
-              null); // reset the callback, as we're already in the finish method
-          child.finish(SpanStatus.DEADLINE_EXCEEDED, finishTimestamp);
-        }
-      }
+      // any un-finished childs will remain unfinished
+      // as relay takes care of setting the end-timestamp + deadline_exceeded
+      // see
+      // https://github.com/getsentry/relay/blob/40697d0a1c54e5e7ad8d183fc7f9543b94fe3839/relay-general/src/store/transactions/processor.rs#L374-L378
+
       root.finish(finishStatus.spanStatus, finishTimestamp);
 
       hub.configureScope(
@@ -292,13 +289,13 @@
               };
           try {
             timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
           } catch (Throwable e) {
             hub.getOptions()
                 .getLogger()
                 .log(SentryLevel.WARNING, "Failed to schedule finish timer", e);
             // if we failed to schedule the finish timer for some reason, we finish it here right
             // away
             onDeadlineTimeoutReached();
           }
         }
       }

sentry/src/main/java/io/sentry/protocol/SentrySpan.java

@@ -61,8 +61,10 @@
     this.tags = tagsCopy != null ? tagsCopy : new ConcurrentHashMap<>();
     // we lose precision here, from potential nanosecond precision down to 10 microsecond precision
     this.timestamp =
-        DateUtils.nanosToSeconds(
-            span.getStartDate().laterDateNanosTimestampByDiff(span.getFinishDate()));
+        span.getFinishDate() == null
+            ? null
+            : DateUtils.nanosToSeconds(
+                span.getStartDate().laterDateNanosTimestampByDiff(span.getFinishDate()));
     // we lose precision here, from potential nanosecond precision down to 10 microsecond precision
     this.startTimestamp = DateUtils.nanosToSeconds(span.getStartDate().nanoTimestamp());
     this.data = data;
@@ -139,7 +141,7 @@
   }
 
   public @Nullable String getOrigin() {
     return origin;
   }
 
   // JsonSerializable

sentry/src/test/java/io/sentry/SentryTracerTest.kt

@@ -538,18 +538,27 @@ class SentryTracerTest {
     }
 
     @Test
-    fun `finishing unfinished spans with the transaction timestamp`() {
+    fun `when finishing, unfinished spans won't have any automatic end-date or status`() {
         val transaction = fixture.getSut(samplingDecision = TracesSamplingDecision(true))
+        // span with no status set
         val span = transaction.startChild("op") as Span
-        transaction.startChild("op2")
+        span.finish(SpanStatus.INVALID_ARGUMENT)
+
+        // span with a status
+        val span1 = transaction.startChild("op2")
+
         transaction.finish(SpanStatus.INVALID_ARGUMENT)
 
         verify(fixture.hub, times(1)).captureTransaction(
             check {
                 assertEquals(2, it.spans.size)
-                assertEquals(transaction.root.finishDate, span.finishDate)
-                assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[0].status)
-                assertEquals(SpanStatus.DEADLINE_EXCEEDED, it.spans[1].status)
+                // span status/timestamp is retained
+                assertNotNull(it.spans[0].status)
+                assertNotNull(it.spans[0].timestamp)
+
+                // span status/timestamp remains untouched
+                assertNull(it.spans[1].status)
+                assertNull(it.spans[1].timestamp)
             },
             anyOrNull<TraceContext>(),
             anyOrNull(),

sentry/src/test/java/io/sentry/protocol/SentrySpanTest.kt

@@ -0,0 +1,35 @@
+package io.sentry.protocol
+
+import io.sentry.IHub
+import io.sentry.SentryLongDate
+import io.sentry.SentryTracer
+import io.sentry.Span
+import io.sentry.SpanOptions
+import io.sentry.TransactionContext
+import org.junit.Test
+import org.mockito.kotlin.mock
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+class SentrySpanTest {
+
+    @Test
+    fun `end timestamps is kept null if not provided`() {
+        // when a span with a start timestamp is generated
+        val span = Span(
+            TransactionContext("name", "op"),
+            mock<SentryTracer>(),
+            mock<IHub>(),
+            SentryLongDate(1000000),
+            SpanOptions()
+        )
+
+        val sentrySpan = SentrySpan(span)
+
+        // then the start timestamp should be correctly set
+        assertEquals(0.001, sentrySpan.startTimestamp)
+
+        // but the end time should remain untouched
+        assertNull(sentrySpan.timestamp)
+    }
+}