From fd54fa67792da6e1b2dbff801a7a59dfc54da02a Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Nov 2022 12:06:06 +0100 Subject: [PATCH 1/2] added ttid span to ActivityLifecycleIntegration --- .../api/sentry-android-core.api | 1 + .../core/ActivityLifecycleIntegration.java | 52 +++++++++++++++++++ .../core/ActivityLifecycleIntegrationTest.kt | 32 ++++++++++++ 3 files changed, 85 insertions(+) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 4bdcfecd27..a7d72b0095 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -15,6 +15,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android public fun onActivityDestroyed (Landroid/app/Activity;)V public fun onActivityPaused (Landroid/app/Activity;)V public fun onActivityPostResumed (Landroid/app/Activity;)V + public fun onActivityPrePaused (Landroid/app/Activity;)V public fun onActivityResumed (Landroid/app/Activity;)V public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V public fun onActivityStarted (Landroid/app/Activity;)V diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index a307b01679..0f4a915457 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -9,8 +9,12 @@ import android.content.Context; import android.os.Build; import android.os.Bundle; +import android.os.Handler; +import android.os.Looper; import android.os.Process; +import androidx.annotation.NonNull; import io.sentry.Breadcrumb; +import io.sentry.DateUtils; import io.sentry.Hint; import io.sentry.IHub; import io.sentry.ISpan; @@ -56,6 +60,9 @@ public final class ActivityLifecycleIntegration private boolean foregroundImportance = false; private @Nullable ISpan appStartSpan; + private @Nullable ISpan ttidSpan; + private @NotNull Date lastPausedTime = DateUtils.getCurrentDateTime(); + private final @NotNull Handler mainHandler = new Handler(Looper.getMainLooper()); // WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the // main-thread @@ -198,8 +205,24 @@ private void startTracing(final @NotNull Activity activity) { appStartSpan = transaction.startChild( getAppStartOp(coldStart), getAppStartDesc(coldStart), appStartTime); + // The first activity ttidSpan should start at the same time as the app start time + ttidSpan = transaction.startChild("TTID", activityName + ".ttid", appStartTime); + } else { + // Other activities (or in case appStartTime is not available) the ttid span should + // start when the previous activity called its onPause method. + ttidSpan = transaction.startChild("TTID", activityName + ".ttid", lastPausedTime); } + // Posting a task to the main thread's handler will make it executed after it finished + // its current job. That is, right after the activity draws the layout. + mainHandler.post( + () -> { + // finishes ttidSpan span + if (ttidSpan != null && !ttidSpan.isFinished()) { + ttidSpan.finish(); + } + }); + // lets bind to the scope so other integrations can pick it up hub.configureScope( scope -> { @@ -258,6 +281,11 @@ private void finishTransaction(final @Nullable ITransaction transaction) { return; } + // in case the ttidSpan isn't completed yet, we finish it as cancelled to avoid memory leak + if (ttidSpan != null && !ttidSpan.isFinished()) { + ttidSpan.finish(SpanStatus.CANCELLED); + } + SpanStatus status = transaction.getStatus(); // status might be set by other integrations, let's not overwrite it if (status == null) { @@ -340,8 +368,20 @@ public synchronized void onActivityPostResumed(final @NotNull Activity activity) } } + @Override + public void onActivityPrePaused(@NonNull Activity activity) { + // only executed if API >= 29 otherwise it happens on onActivityPaused + if (isAllActivityCallbacksAvailable) { + lastPausedTime = DateUtils.getCurrentDateTime(); + } + } + @Override public synchronized void onActivityPaused(final @NotNull Activity activity) { + // only executed if API < 29 otherwise it happens on onActivityPrePaused + if (!isAllActivityCallbacksAvailable) { + lastPausedTime = DateUtils.getCurrentDateTime(); + } addBreadcrumb(activity, "paused"); } @@ -366,12 +406,18 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) { appStartSpan.finish(SpanStatus.CANCELLED); } + // in case the ttidSpan isn't completed yet, we finish it as cancelled to avoid memory leak + if (ttidSpan != null && !ttidSpan.isFinished()) { + ttidSpan.finish(SpanStatus.CANCELLED); + } + // in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it, // we make sure to finish it when the activity gets destroyed. stopTracing(activity, true); // set it to null in case its been just finished as cancelled appStartSpan = null; + ttidSpan = null; // clear it up, so we don't start again for the same activity if the activity is in the activity // stack still. @@ -399,6 +445,12 @@ ISpan getAppStartSpan() { return appStartSpan; } + @TestOnly + @Nullable + ISpan getTtidSpan() { + return ttidSpan; + } + private void setColdStart(final @Nullable Bundle savedInstanceState) { if (!firstActivityCreated) { // if Activity has savedInstanceState then its a warm start diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 7c530b0758..b522f61b41 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -498,6 +498,38 @@ class ActivityLifecycleIntegrationTest { assertNull(sut.appStartSpan) } + @Test + fun `When Activity is destroyed, sets ttidSpan status to cancelled and finish it`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + setAppStartTime() + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) + + val span = fixture.transaction.children.first { it.description?.endsWith(".ttid") == true } + assertEquals(span.status, SpanStatus.CANCELLED) + assertTrue(span.isFinished) + } + + @Test + fun `When Activity is destroyed, sets ttidSpan to null`() { + val sut = fixture.getSut() + fixture.options.tracesSampleRate = 1.0 + sut.register(fixture.hub, fixture.options) + + setAppStartTime() + + val activity = mock() + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityDestroyed(activity) + + assertNull(sut.ttidSpan) + } + @Test fun `When new Activity and transaction is created, finish previous ones`() { val sut = fixture.getSut() From 1d48b5aa0b2e04d15222e2673a16b6dfce052740 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Wed, 16 Nov 2022 17:07:14 +0100 Subject: [PATCH 2/2] SentryFrameMetricsCollector start collecting frameMetrics in onActivityStarted(), not onActivityCreated() anymore --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index af924736a8..d82778f935 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Features +- Add ttid span to ActivityLifecycleIntegration ([#2369](https://github.com/getsentry/sentry-java/pull/2369)) - Update Spring Boot Jakarta to Spring Boot 3.0.0-RC2 ([#2347](https://github.com/getsentry/sentry-java/pull/2347)) ## 6.7.0