fix(core): Default trace context status to OK

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Breaking Changes
 
+- Default trace context status to `ok` when an active span has no explicit status ([#6418](https://github.com/getsentry/sentry-java/pull/6418))
 - Remove the deprecated user feedback APIs ([#6337](https://github.com/getsentry/sentry-java/pull/6337))
   - `UserFeedback` and `Sentry.captureUserFeedback(...)`: use `Sentry.feedback().capture(...)` with `io.sentry.protocol.Feedback` instead
   - `Sentry.captureFeedback(...)`: use `Sentry.feedback().capture(...)` instead

sentry/src/main/java/io/sentry/Scope.java

@@ -300,7 +300,7 @@ public void setTransaction(final @Nullable ITransaction transaction) {
       for (final IScopeObserver observer : options.getScopeObservers()) {
         if (transaction != null) {
           observer.setTransaction(transaction.getName());
-          observer.setTrace(transaction.getSpanContext(), this);
+          observer.setTrace(transaction.getSpanContext().withDefaultStatus(), this);
         } else {
           observer.setTransaction(null);
           observer.setTrace(null, this);
@@ -1246,7 +1246,7 @@ public void assignTraceContext(final @NotNull SentryEvent event) {
         if (event.getContexts().getTrace() == null && spanWeakRef != null) {
           final ISpan span = spanWeakRef.get();
           if (span != null) {
-            event.getContexts().setTrace(span.getSpanContext());
+            event.getContexts().setTrace(span.getSpanContext().withDefaultStatus());
           }
         }
         final String transactionName = pair.getSecond();

sentry/src/main/java/io/sentry/SentryClient.java

@@ -1442,7 +1442,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri
               .getContexts()
               .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext()));
         } else {
-          event.getContexts().setTrace(span.getSpanContext());
+          event.getContexts().setTrace(span.getSpanContext().withDefaultStatus());
         }
       }
 
@@ -1487,7 +1487,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri
             .getContexts()
             .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext()));
       } else {
-        event.getContexts().setTrace(span.getSpanContext());
+        event.getContexts().setTrace(span.getSpanContext().withDefaultStatus());
       }
     }
 
@@ -1505,7 +1505,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri
               .getContexts()
               .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext()));
         } else {
-          checkIn.getContexts().setTrace(span.getSpanContext());
+          checkIn.getContexts().setTrace(span.getSpanContext().withDefaultStatus());
         }
       }
     }
@@ -1546,7 +1546,7 @@ public void captureBatchedMetricsEvents(final @NotNull SentryMetricsEvents metri
               .getContexts()
               .setTrace(TransactionContext.fromPropagationContext(scope.getPropagationContext()));
         } else {
-          replayEvent.getContexts().setTrace(span.getSpanContext());
+          replayEvent.getContexts().setTrace(span.getSpanContext().withDefaultStatus());
         }
       }
     }

sentry/src/main/java/io/sentry/SpanContext.java

@@ -194,6 +194,16 @@ public SpanId getParentSpanId() {
     return status;
   }
 
+  @NotNull
+  SpanContext withDefaultStatus() {
+    final @NotNull SpanContext traceContext = new SpanContext(this);
+    traceContext.setOrigin(getOrigin());
+    if (traceContext.getStatus() == null) {
+      traceContext.setStatus(SpanStatus.OK);
+    }
+    return traceContext;
+  }
+
   public @NotNull Map<String, String> getTags() {
     return tags;
   }

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

@@ -849,13 +849,19 @@ class ScopeTest {
     val observer = mock<IScopeObserver>()
     val options = SentryOptions().apply { addScopeObserver(observer) }
     val scope = Scope(options)
+    val spanContext = SpanContext("ui.load")
 
     scope.transaction = mock {
       whenever(mock.name).thenReturn("main")
-      whenever(mock.spanContext).thenReturn(SpanContext("ui.load"))
+      whenever(mock.spanContext).thenReturn(spanContext)
     }
     verify(observer).setTransaction(eq("main"))
-    verify(observer).setTrace(argThat { operation == "ui.load" }, eq(scope))
+    verify(observer)
+      .setTrace(
+        argThat { operation == "ui.load" && status == SpanStatus.OK },
+        eq(scope),
+      )
+    assertThat(spanContext.status).isNull()
   }
 
   @Test

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

@@ -415,7 +415,7 @@ class ScopesTest {
 
     val hints = HintUtils.createWithTypeCheckHint({})
     sut.captureEvent(event, hints)
-    assertEquals(span.spanContext, event.contexts.trace)
+    assertTraceContextDefaultsToOk(span.spanContext, event.contexts.trace)
     verify(mockClient).captureEvent(eq(event), any(), eq(hints))
   }
 
@@ -431,7 +431,7 @@ class ScopesTest {
 
     val hints = HintUtils.createWithTypeCheckHint({})
     sut.captureEvent(event, hints)
-    assertEquals(span.spanContext, event.contexts.trace)
+    assertTraceContextDefaultsToOk(span.spanContext, event.contexts.trace)
     verify(mockClient).captureEvent(eq(event), any(), eq(hints))
   }
 
@@ -448,7 +448,7 @@ class ScopesTest {
 
     val hints = HintUtils.createWithTypeCheckHint({})
     sut.captureEvent(event, hints)
-    assertEquals(span.spanContext, event.contexts.trace)
+    assertTraceContextDefaultsToOk(span.spanContext, event.contexts.trace)
     verify(mockClient).captureEvent(eq(event), any(), eq(hints))
   }
 
@@ -729,7 +729,7 @@ class ScopesTest {
     verify(mockClient)
       .captureEvent(
         check {
-          assertEquals(span.spanContext, it.contexts.trace)
+          assertTraceContextDefaultsToOk(span.spanContext, it.contexts.trace)
           assertEquals("tx-name", it.transaction)
         },
         any(),
@@ -2153,7 +2153,7 @@ class ScopesTest {
 
     verify(mockClient)
       .captureEvent(
-        check { assertEquals(span.spanContext, it.contexts.trace) },
+        check { assertTraceContextDefaultsToOk(span.spanContext, it.contexts.trace) },
         anyOrNull(),
         anyOrNull(),
       )
@@ -4442,6 +4442,17 @@ class ScopesTest {
     return fileHashFolder.absolutePath
   }
 
+  private fun assertTraceContextDefaultsToOk(
+    expectedContext: SpanContext,
+    actualContext: SpanContext?,
+  ) {
+    assertThat(actualContext).isNotNull()
+    assertThat(actualContext!!.traceId).isEqualTo(expectedContext.traceId)
+    assertThat(actualContext.spanId).isEqualTo(expectedContext.spanId)
+    assertThat(actualContext.status).isEqualTo(SpanStatus.OK)
+    assertThat(expectedContext.status).isNull()
+  }
+
   private fun equalTraceContext(expectedContext: TraceContext?): TraceContext? {
     expectedContext ?: return eq<TraceContext?>(null)
 

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

@@ -2239,7 +2239,10 @@ class SentryClientTest {
     transaction.finish()
     sut.captureEvent(event, scope)
     assertNotNull(event.contexts.trace)
-    assertEquals(transaction.root.spanContext, event.contexts.trace)
+    assertThat(event.contexts.trace!!.traceId).isEqualTo(transaction.root.spanContext.traceId)
+    assertThat(event.contexts.trace!!.spanId).isEqualTo(transaction.root.spanContext.spanId)
+    assertThat(event.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
+    assertThat(transaction.root.spanContext.status).isNull()
   }
 
   @Test
@@ -2252,7 +2255,10 @@ class SentryClientTest {
     val span = transaction.startChild("op")
     sut.captureEvent(event, scope)
     assertNotNull(event.contexts.trace)
-    assertEquals(span.spanContext, event.contexts.trace)
+    assertThat(event.contexts.trace!!.traceId).isEqualTo(span.spanContext.traceId)
+    assertThat(event.contexts.trace!!.spanId).isEqualTo(span.spanContext.spanId)
+    assertThat(event.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
+    assertThat(span.spanContext.status).isNull()
   }
 
   @Test
@@ -3288,6 +3294,8 @@ class SentryClientTest {
 
     assertEquals(spanContext.traceId, sentryEvent.contexts.trace!!.traceId)
     assertEquals(spanContext.spanId, sentryEvent.contexts.trace!!.spanId)
+    assertThat(sentryEvent.contexts.trace!!.status).isEqualTo(SpanStatus.OK)
+    assertThat(spanContext.status).isNull()
     assertNotEquals(scopePropagationContext.traceId, sentryEvent.contexts.trace!!.traceId)
     assertNotEquals(scopePropagationContext.spanId, sentryEvent.contexts.trace!!.spanId)
   }

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

@@ -1,5 +1,6 @@
 package io.sentry
 
+import com.google.common.truth.Truth.assertThat
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -21,6 +22,30 @@ class SpanContextTest {
     assertNotNull(trace.data[SpanDataConvention.THREAD_NAME])
   }
 
+  @Test
+  fun `withDefaultStatus returns a copy with the default status without modifying the original`() {
+    val spanContext = SpanContext("op").apply { origin = "auto.test" }
+
+    val traceContext = spanContext.withDefaultStatus()
+
+    assertThat(traceContext).isNotSameInstanceAs(spanContext)
+    assertThat(traceContext.traceId).isEqualTo(spanContext.traceId)
+    assertThat(traceContext.spanId).isEqualTo(spanContext.spanId)
+    assertThat(traceContext.origin).isEqualTo(spanContext.origin)
+    assertThat(traceContext.status).isEqualTo(SpanStatus.OK)
+    assertThat(spanContext.status).isNull()
+  }
+
+  @Test
+  fun `withDefaultStatus preserves an explicitly set status`() {
+    val spanContext = SpanContext("op").apply { status = SpanStatus.INTERNAL_ERROR }
+
+    val traceContext = spanContext.withDefaultStatus()
+
+    assertThat(traceContext).isNotSameInstanceAs(spanContext)
+    assertThat(traceContext.status).isEqualTo(SpanStatus.INTERNAL_ERROR)
+  }
+
   @Test
   fun `sets tag`() {
     val trace = SpanContext("op")