Fix slow/frozen frames were not reported with transactions

CHANGELOG.md

@@ -12,6 +12,7 @@
 - Remove code that set `tracesSampleRate` to `0.0` for Spring Boot if not set ([#2800](https://github.com/getsentry/sentry-java/pull/2800))
   - This used to enable performance but not send any transactions by default.
   - Performance is now disabled by default.
+- Fix slow/frozen frames were not reported with transactions ([#2811](https://github.com/getsentry/sentry-java/pull/2811))
 
 ### Dependencies
 

sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt

@@ -25,6 +25,7 @@ import io.sentry.SentryOptions
 import io.sentry.SentryTracer
 import io.sentry.Span
 import io.sentry.SpanStatus
+import io.sentry.SpanStatus.OK
 import io.sentry.TraceContext
 import io.sentry.TransactionContext
 import io.sentry.TransactionFinishedCallback
@@ -94,7 +95,7 @@ class ActivityLifecycleIntegrationTest {
             // We let the ActivityLifecycleIntegration create the proper transaction here
             val argumentCaptor = argumentCaptor<TransactionOptions>()
             whenever(hub.startTransaction(any(), argumentCaptor.capture())).thenAnswer {
-                val t = SentryTracer(context, hub, argumentCaptor.lastValue, transactionFinishedCallback)
+                val t = SentryTracer(context, hub, argumentCaptor.lastValue)
                 transaction = t
                 return@thenAnswer t
             }
@@ -1442,6 +1443,19 @@ class ActivityLifecycleIntegrationTest {
         assertNotSame(propagationContextAfterNewTrace, scope.propagationContext)
     }
 
+    @Test
+    fun `when transaction is finished, sets frame metrics`() {
+        val sut = fixture.getSut()
+        fixture.options.tracesSampleRate = 1.0
+        sut.register(fixture.hub, fixture.options)
+
+        val activity = mock<Activity>()
+        sut.onActivityCreated(activity, fixture.bundle)
+
+        fixture.transaction.forceFinish(OK, false)
+        verify(fixture.activityFramesTracker).setMetrics(activity, fixture.transaction.eventId)
+    }
+
     private fun runFirstDraw(view: View) {
         // Removes OnDrawListener in the next OnGlobalLayout after onDraw
         view.viewTreeObserver.dispatchOnDraw()

sentry/api/sentry.api

@@ -1933,7 +1933,7 @@ public final class io/sentry/SentryTraceHeader {
 
 public final class io/sentry/SentryTracer : io/sentry/ITransaction {
 	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;)V
-	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;Lio/sentry/TransactionOptions;Lio/sentry/TransactionFinishedCallback;)V
+	public fun <init> (Lio/sentry/TransactionContext;Lio/sentry/IHub;Lio/sentry/TransactionOptions;)V
 	public fun finish ()V
 	public fun finish (Lio/sentry/SpanStatus;)V
 	public fun finish (Lio/sentry/SpanStatus;Lio/sentry/SentryDate;)V

sentry/src/main/java/io/sentry/Hub.java

@@ -727,7 +727,7 @@ public void flush(long timeoutMillis) {
 
       transaction =
           new SentryTracer(
-              transactionContext, this, transactionOptions, null, transactionPerformanceCollector);
+              transactionContext, this, transactionOptions, transactionPerformanceCollector);
 
       // The listener is called only if the transaction exists, as the transaction is needed to
       // stop it

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

@@ -37,12 +37,6 @@ public final class SentryTracer implements ITransaction {
    */
   private @NotNull FinishStatus finishStatus = FinishStatus.NOT_FINISHED;
 
-  /**
-   * When `waitForChildren` is set to `true` and this callback is set, it's called before the
-   * transaction is captured.
-   */
-  private final @Nullable TransactionFinishedCallback transactionFinishedCallback;
-
   private volatile @Nullable TimerTask timerTask;
   private volatile @Nullable Timer timer = null;
   private final @NotNull Object timerLock = new Object();
@@ -57,22 +51,20 @@ public final class SentryTracer implements ITransaction {
   private final @NotNull TransactionOptions transactionOptions;
 
   public SentryTracer(final @NotNull TransactionContext context, final @NotNull IHub hub) {
-    this(context, hub, new TransactionOptions(), null, null);
+    this(context, hub, new TransactionOptions(), null);
   }
 
   public SentryTracer(
       final @NotNull TransactionContext context,
       final @NotNull IHub hub,
-      final @NotNull TransactionOptions transactionOptions,
-      final @Nullable TransactionFinishedCallback transactionFinishedCallback) {
-    this(context, hub, transactionOptions, transactionFinishedCallback, null);
+      final @NotNull TransactionOptions transactionOptions) {
+    this(context, hub, transactionOptions, null);
   }
 
   SentryTracer(
       final @NotNull TransactionContext context,
       final @NotNull IHub hub,
       final @NotNull TransactionOptions transactionOptions,
-      final @Nullable TransactionFinishedCallback transactionFinishedCallback,
       final @Nullable TransactionPerformanceCollector transactionPerformanceCollector) {
     Objects.requireNonNull(context, "context is required");
     Objects.requireNonNull(hub, "hub is required");
@@ -84,7 +76,6 @@ public SentryTracer(
     this.name = context.getName();
     this.instrumenter = context.getInstrumenter();
     this.hub = hub;
-    this.transactionFinishedCallback = transactionFinishedCallback;
     this.transactionPerformanceCollector = transactionPerformanceCollector;
     this.transactionNameSource = context.getTransactionNameSource();
     this.transactionOptions = transactionOptions;
@@ -222,8 +213,10 @@ public void finish(
                 });
           });
       final SentryTransaction transaction = new SentryTransaction(this);
-      if (transactionFinishedCallback != null) {
-        transactionFinishedCallback.execute(this);
+      final TransactionFinishedCallback finishedCallback =
+          transactionOptions.getTransactionFinishedCallback();
+      if (finishedCallback != null) {
+        finishedCallback.execute(this);
       }
 
       if (timer != null) {

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

@@ -55,7 +55,8 @@ class SentryTracerTest {
             transactionOptions.isWaitForChildren = waitForChildren
             transactionOptions.idleTimeout = idleTimeout
             transactionOptions.isTrimEnd = trimEnd
-            return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, transactionOptions, transactionFinishedCallback, performanceCollector)
+            transactionOptions.transactionFinishedCallback = transactionFinishedCallback
+            return SentryTracer(TransactionContext("name", "op", samplingDecision), hub, transactionOptions, performanceCollector)
         }
     }
 

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

@@ -41,7 +41,7 @@ class SpanTest {
         }
 
         fun getRootSut(options: TransactionOptions = TransactionOptions()): Span {
-            return SentryTracer(TransactionContext("name", "op"), hub, options, null).root
+            return SentryTracer(TransactionContext("name", "op"), hub, options).root
         }
     }