fix(core): [Tracing API Cleanup 2] Snapshot tracing options

CHANGELOG.md

@@ -47,6 +47,10 @@
 - Remove deprecated OpenTelemetry integration classes and the legacy `SentrySpanStorage` ([#6243](https://github.com/getsentry/sentry-java/pull/6243))
 - Decide whether foregrounding the app starts a new session on a monotonic clock instead of the wall clock, so that a device time change no longer starts a session that should have been resumed, or resumes one that should have ended. This affects session counts ([#6096](https://github.com/getsentry/sentry-java/pull/6096))
 
+### Fixes
+
+- Prevent changes to reused tracing options from affecting spans and transactions after creation ([#6417](https://github.com/getsentry/sentry-java/pull/6417))
+
 ## 8.62.0
 
 ### Improvements

sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java

@@ -152,11 +152,9 @@ public OtelSpanWrapper(
       @Nullable String description,
       @Nullable SentryDate timestamp,
       @NotNull SpanOptions spanOptions) {
-    if (timestamp != null) {
-      spanOptions.setStartTimestamp(timestamp);
-    }
-
-    return startChild(operation, description, spanOptions);
+    final @NotNull SpanOptions spanOptionsSnapshot = new SpanOptions(spanOptions);
+    spanOptionsSnapshot.setStartTimestamp(timestamp);
+    return startChild(operation, description, spanOptionsSnapshot);
   }
 
   @Override

sentry-opentelemetry/sentry-opentelemetry-core/src/test/kotlin/OtelSpanWrapperTest.kt

@@ -16,12 +16,13 @@ import org.mockito.kotlin.any
 import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 
 class OtelSpanWrapperTest {
   @Test
-  fun `startChild with timestamp and options forwards both`() {
+  fun `startChild with timestamp and options applies timestamp to copy`() {
     val otelSpan = mock<ReadWriteSpan>()
     whenever(otelSpan.spanContext)
       .thenReturn(
@@ -42,19 +43,28 @@ class OtelSpanWrapperTest {
     val parent = OtelSpanWrapper(otelSpan, scopes, SentryLongDate(0), null, null, null, null)
     val expectedChild = mock<ISpan>()
     whenever(spanFactory.createSpan(eq(scopes), any(), any(), eq(parent))).thenReturn(expectedChild)
-    val timestamp = SentryLongDate(1234)
-    val suppliedSpanOptions = SpanOptions()
+    val originalTimestamp = SentryLongDate(1234)
+    val timestamp = SentryLongDate(5678)
+    val suppliedSpanOptions = SpanOptions().apply { startTimestamp = originalTimestamp }
 
     val child = parent.startChild("child-op", "description", timestamp, suppliedSpanOptions)
+    val childWithoutTimestamp =
+      parent.startChild("second-child-op", "second-description", null, suppliedSpanOptions)
 
     val spanOptions = argumentCaptor<SpanOptions>()
     val spanContext = argumentCaptor<io.sentry.SpanContext>()
-    verify(spanFactory)
+    verify(spanFactory, times(2))
       .createSpan(eq(scopes), spanOptions.capture(), spanContext.capture(), eq(parent))
     assertThat(child).isSameInstanceAs(expectedChild)
-    assertThat(spanOptions.firstValue).isSameInstanceAs(suppliedSpanOptions)
-    assertThat(spanOptions.firstValue.startTimestamp).isSameInstanceAs(timestamp)
-    assertThat(spanContext.firstValue.operation).isEqualTo("child-op")
-    assertThat(spanContext.firstValue.description).isEqualTo("description")
+    assertThat(childWithoutTimestamp).isSameInstanceAs(expectedChild)
+    assertThat(spanOptions.allValues[0]).isNotSameInstanceAs(suppliedSpanOptions)
+    assertThat(spanOptions.allValues[0].startTimestamp).isSameInstanceAs(timestamp)
+    assertThat(spanOptions.allValues[1]).isNotSameInstanceAs(suppliedSpanOptions)
+    assertThat(spanOptions.allValues[1].startTimestamp).isNull()
+    assertThat(suppliedSpanOptions.startTimestamp).isSameInstanceAs(originalTimestamp)
+    assertThat(spanContext.allValues[0].operation).isEqualTo("child-op")
+    assertThat(spanContext.allValues[0].description).isEqualTo("description")
+    assertThat(spanContext.allValues[1].operation).isEqualTo("second-child-op")
+    assertThat(spanContext.allValues[1].description).isEqualTo("second-description")
   }
 }

sentry/api/sentry.api

@@ -4371,6 +4371,7 @@ public final class io/sentry/SpanId$Deserializer : io/sentry/JsonDeserializer {
 public class io/sentry/SpanOptions {
 	protected field origin Ljava/lang/String;
 	public fun <init> ()V
+	public fun <init> (Lio/sentry/SpanOptions;)V
 	public fun getOrigin ()Ljava/lang/String;
 	public fun getScopeBindingMode ()Lio/sentry/ScopeBindingMode;
 	public fun getStartTimestamp ()Lio/sentry/SentryDate;

sentry/src/main/java/io/sentry/Scopes.java

@@ -867,7 +867,7 @@ public void flush(long timeoutMillis) {
   public @NotNull ITransaction startTransaction(
       final @NotNull TransactionContext transactionContext,
       final @NotNull TransactionOptions transactionOptions) {
-    return createTransaction(transactionContext, transactionOptions);
+    return createTransaction(transactionContext, new TransactionOptions(transactionOptions));
   }
 
   private @NotNull ITransaction createTransaction(

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

@@ -52,7 +52,10 @@ public final class SentryTracer implements ITransaction {
   private @NotNull TransactionNameSource transactionNameSource;
   private final @NotNull Contexts contexts = new Contexts();
   private final @Nullable CompositePerformanceCollector compositePerformanceCollector;
-  private final @NotNull TransactionOptions transactionOptions;
+  private final boolean waitForChildren;
+  private final @Nullable Long idleTimeout;
+  private final @Nullable Long deadlineTimeout;
+  private final @Nullable TransactionFinishedCallback transactionFinishedCallback;
 
   public SentryTracer(final @NotNull TransactionContext context, final @NotNull IScopes scopes) {
     this(context, scopes, new TransactionOptions(), null);
@@ -81,7 +84,10 @@ public SentryTracer(
     this.compositePerformanceCollector =
         Boolean.TRUE.equals(isSampled()) ? compositePerformanceCollector : null;
     this.transactionNameSource = context.getTransactionNameSource();
-    this.transactionOptions = transactionOptions;
+    this.waitForChildren = transactionOptions.isWaitForChildren();
+    this.idleTimeout = transactionOptions.getIdleTimeout();
+    this.deadlineTimeout = transactionOptions.getDeadlineTimeout();
+    this.transactionFinishedCallback = transactionOptions.getTransactionFinishedCallback();
 
     setDefaultSpanData(root);
 
@@ -97,8 +103,7 @@ public SentryTracer(
       this.compositePerformanceCollector.start(this);
     }
 
-    if (transactionOptions.getIdleTimeout() != null
-        || transactionOptions.getDeadlineTimeout() != null) {
+    if (idleTimeout != null || deadlineTimeout != null) {
       timersEnabled = true;
 
       scheduleDeadlineTimeout();
@@ -110,8 +115,6 @@ public SentryTracer(
   public void scheduleFinish() {
     try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
       if (timersEnabled) {
-        final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();
-
         if (idleTimeout != null) {
           cancelIdleTimer();
           isIdleFinishTimerRunning.set(true);
@@ -145,9 +148,7 @@ private void onIdleTimeoutReached() {
   private void onDeadlineTimeoutReached() {
     final @Nullable SpanStatus status = getStatus();
     forceFinish(
-        (status != null) ? status : SpanStatus.DEADLINE_EXCEEDED,
-        transactionOptions.getIdleTimeout() != null,
-        null);
+        (status != null) ? status : SpanStatus.DEADLINE_EXCEEDED, idleTimeout != null, null);
     isDeadlineTimerRunning.set(false);
   }
 
@@ -195,14 +196,13 @@ public void finish(
 
     // auto-finish any idle spans first
     for (final Span span : children) {
-      if (span.getOptions().isIdle()) {
+      if (span.isIdle()) {
         span.finish((status != null) ? status : getSpanContext().status, finishTimestamp);
       }
     }
 
     this.finishStatus = FinishStatus.finishing(status);
-    if (!root.isFinished()
-        && (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished())) {
+    if (!root.isFinished() && (!waitForChildren || hasAllChildrenFinished())) {
 
       final @NotNull AtomicReference<List<PerformanceCollectionData>> performanceCollectionData =
           new AtomicReference<>();
@@ -218,7 +218,7 @@ public void finish(
             // Let's call the finishCallback here, when the root span has a finished date but it's
             // not finished, yet
             final @Nullable TransactionFinishedCallback finishedCallback =
-                transactionOptions.getTransactionFinishedCallback();
+                transactionFinishedCallback;
             if (finishedCallback != null) {
               finishedCallback.execute(this);
             }
@@ -274,7 +274,7 @@ public void finish(
         }
       }
 
-      if (dropIfNoChildren && children.isEmpty() && transactionOptions.getIdleTimeout() != null) {
+      if (dropIfNoChildren && children.isEmpty() && idleTimeout != null) {
         // if it's an idle transaction which has no children, we drop it to save user's quota
         scopes
             .getOptions()
@@ -302,7 +302,7 @@ private void cancelIdleTimer() {
   }
 
   private void scheduleDeadlineTimeout() {
-    final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();
+    final @Nullable Long deadlineTimeOut = deadlineTimeout;
     if (deadlineTimeOut != null) {
       try (final @NotNull ISentryLifecycleToken ignored = timerLock.acquire()) {
         if (timersEnabled) {
@@ -393,8 +393,9 @@ ISpan startChild(
       final @Nullable String description,
       final @Nullable SentryDate timestamp,
       final @NotNull SpanOptions spanOptions) {
-    spanOptions.setStartTimestamp(timestamp);
-    return createChild(parentSpanId, operation, description, spanOptions);
+    final @NotNull SpanOptions spanOptionsSnapshot = new SpanOptions(spanOptions);
+    spanOptionsSnapshot.setStartTimestamp(timestamp);
+    return createChild(parentSpanId, operation, description, spanOptionsSnapshot);
   }
 
   /**
@@ -449,11 +450,11 @@ private ISpan createChild(
                   compositePerformanceCollector.onSpanFinished(finishingSpan);
                 }
                 final FinishStatus finishStatus = this.finishStatus;
-                if (transactionOptions.getIdleTimeout() != null) {
+                if (idleTimeout != null) {
                   // if it's an idle transaction, no matter the status, we'll reset the timeout here
                   // so the transaction will either idle and finish itself, or a new child will be
                   // added and we'll wait for it again
-                  if (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished()) {
+                  if (!waitForChildren || hasAllChildrenFinished()) {
                     scheduleFinish();
                   }
                 } else if (finishStatus.isFinishing) {
@@ -475,13 +476,13 @@ private ISpan createChild(
       //                  compositePerformanceCollector.onSpanFinished(finishingSpan);
       //                }
       //                final FinishStatus finishStatus = this.finishStatus;
-      //                if (transactionOptions.getIdleTimeout() != null) {
+      //                if (idleTimeout != null) {
       //                  // if it's an idle transaction, no matter the status, we'll reset the
       // timeout here
       //                  // so the transaction will either idle and finish itself, or a new child
       // will be
       //                  // added and we'll wait for it again
-      //                  if (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished()) {
+      //                  if (!waitForChildren || hasAllChildrenFinished()) {
       //                    scheduleFinish();
       //                  }
       //                } else if (finishStatus.isFinishing) {

sentry/src/main/java/io/sentry/Span.java

@@ -40,7 +40,9 @@ public final class Span implements ISpan {
 
   private final @NotNull AtomicBoolean isFinishing = new AtomicBoolean(false);
 
-  private final @NotNull SpanOptions options;
+  private final boolean trimStart;
+  private final boolean trimEnd;
+  private final boolean idle;
 
   private @Nullable SpanFinishedCallback spanFinishedCallback;
 
@@ -59,7 +61,9 @@ public final class Span implements ISpan {
     this.context.setOrigin(options.getOrigin());
     this.transaction = Objects.requireNonNull(transaction, "transaction is required");
     this.scopes = Objects.requireNonNull(scopes, "Scopes are required");
-    this.options = options;
+    this.trimStart = options.isTrimStart();
+    this.trimEnd = options.isTrimEnd();
+    this.idle = options.isIdle();
     this.spanFinishedCallback = spanFinishedCallback;
     final @Nullable SentryDate startTimestamp = options.getStartTimestamp();
     if (startTimestamp != null) {
@@ -78,14 +82,16 @@ public Span(
     this.context.setOrigin(options.getOrigin());
     this.transaction = Objects.requireNonNull(sentryTracer, "sentryTracer is required");
     this.scopes = Objects.requireNonNull(scopes, "scopes are required");
+    this.trimStart = options.isTrimStart();
+    this.trimEnd = options.isTrimEnd();
+    this.idle = options.isIdle();
     this.spanFinishedCallback = null;
     final @Nullable SentryDate startTimestamp = options.getStartTimestamp();
     if (startTimestamp != null) {
       this.startTimestamp = startTimestamp;
     } else {
       this.startTimestamp = scopes.getOptions().getDateProvider().now();
     }
-    this.options = options;
   }
 
   @Override
@@ -188,7 +194,7 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate
 
     this.context.setStatus(status);
     this.timestamp = timestamp == null ? scopes.getOptions().getDateProvider().now() : timestamp;
-    if (options.isTrimStart() || options.isTrimEnd()) {
+    if (trimStart || trimEnd) {
       @Nullable SentryDate minChildStart = null;
       @Nullable SentryDate maxChildEnd = null;
 
@@ -207,12 +213,10 @@ public void finish(final @Nullable SpanStatus status, final @Nullable SentryDate
           maxChildEnd = child.getFinishDate();
         }
       }
-      if (options.isTrimStart()
-          && minChildStart != null
-          && startTimestamp.isBefore(minChildStart)) {
+      if (trimStart && minChildStart != null && startTimestamp.isBefore(minChildStart)) {
         updateStartDate(minChildStart);
       }
-      if (options.isTrimEnd()
+      if (trimEnd
           && maxChildEnd != null
           && (this.timestamp == null || this.timestamp.isAfter(maxChildEnd))) {
         updateEndDate(maxChildEnd);
@@ -431,9 +435,8 @@ private void updateStartDate(@NotNull SentryDate date) {
     this.startTimestamp = date;
   }
 
-  @NotNull
-  SpanOptions getOptions() {
-    return options;
+  boolean isIdle() {
+    return idle;
   }
 
   @NotNull

sentry/src/main/java/io/sentry/SpanOptions.java

@@ -3,12 +3,25 @@
 import static io.sentry.SpanContext.DEFAULT_ORIGIN;
 
 import com.jakewharton.nopen.annotation.Open;
+import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 @Open
 public class SpanOptions {
 
+  public SpanOptions() {}
+
+  @ApiStatus.Internal
+  public SpanOptions(final @NotNull SpanOptions options) {
+    startTimestamp = options.getStartTimestamp();
+    scopeBindingMode = options.getScopeBindingMode();
+    trimStart = options.isTrimStart();
+    trimEnd = options.isTrimEnd();
+    isIdle = options.isIdle();
+    origin = options.getOrigin();
+  }
+
   /** The start timestamp of the transaction */
   private @Nullable SentryDate startTimestamp = null;
 

sentry/src/main/java/io/sentry/TransactionOptions.java

@@ -9,6 +9,19 @@ public final class TransactionOptions extends SpanOptions {
 
   @ApiStatus.Internal public static final long DEFAULT_DEADLINE_TIMEOUT_AUTO_TRANSACTION = 30000;
 
+  public TransactionOptions() {}
+
+  TransactionOptions(final @NotNull TransactionOptions options) {
+    super(options);
+    customSamplingContext = options.getCustomSamplingContext();
+    isAppStartTransaction = options.isAppStartTransaction();
+    waitForChildren = options.isWaitForChildren();
+    idleTimeout = options.getIdleTimeout();
+    deadlineTimeout = options.getDeadlineTimeout();
+    transactionFinishedCallback = options.getTransactionFinishedCallback();
+    spanFactory = options.getSpanFactory();
+  }
+
   /**
    * Arbitrary data used in {@link SamplingContext} to determine if transaction is going to be
    * sampled.

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

@@ -1870,6 +1870,28 @@ class ScopesTest {
     assertEquals(contexts, transaction.root.spanContext)
   }
 
+  @Test
+  fun `when startTransaction, passes a snapshot of transaction options to span factory`() {
+    val scopes = generateScopes()
+    val spanFactory = mock<ISpanFactory>()
+    val transaction = mock<ITransaction>()
+    whenever(spanFactory.createTransaction(any(), any(), any(), anyOrNull()))
+      .thenReturn(transaction)
+    val transactionOptions =
+      TransactionOptions().apply {
+        isWaitForChildren = true
+        setSpanFactory(spanFactory)
+      }
+
+    scopes.startTransaction(TransactionContext("name", "op"), transactionOptions)
+    transactionOptions.isWaitForChildren = false
+
+    val optionsCaptor = argumentCaptor<TransactionOptions>()
+    verify(spanFactory).createTransaction(any(), eq(scopes), optionsCaptor.capture(), anyOrNull())
+    assertThat(optionsCaptor.firstValue).isNotSameInstanceAs(transactionOptions)
+    assertThat(optionsCaptor.firstValue.isWaitForChildren).isTrue()
+  }
+
   @Test
   fun `when startTransaction with bindToScope set to false, transaction is not attached to the scope`() {
     val scopes = generateScopes()