From 41654d88c73e636568c0ee86d4b3bf9f230368cf Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Thu, 19 Aug 2021 15:34:42 +0200 Subject: [PATCH] Fix: Remove onActivityPreCreated call in favor of onActivityCreated (#1661) (#1661) --- CHANGELOG.md | 1 + .../api/sentry-android-core.api | 1 - .../core/ActivityLifecycleIntegration.java | 32 +--------- .../core/ActivityLifecycleIntegrationTest.kt | 60 ++++++++----------- 4 files changed, 28 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd240a93f9..b7ed3351f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +* Fix: Remove onActivityPreCreated call in favor of onActivityCreated (#1661) * Fix: Do not crash if SENSOR_SERVICE throws (#1655) * Feat: Add support for async methods in Spring MVC (#1652) * Feat: Add secondary constructor taking IHub to SentryOkHttpInterceptor (#1657) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 315475a92e..4ca815023e 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -5,7 +5,6 @@ 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 onActivityPreCreated (Landroid/app/Activity;Landroid/os/Bundle;)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 744534f5ff..2723342cdb 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 @@ -224,40 +224,14 @@ private void finishTransaction(final @Nullable ITransaction transaction) { } } - @Override - public synchronized void onActivityPreCreated( - final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { - - // only executed if API >= 29 otherwise it happens on onActivityCreated - if (isAllActivityCallbacksAvailable) { - // start collecting frame metrics for transaction - activityFramesTracker.addActivity(activity); - - setColdStart(savedInstanceState); - - // if activity has global fields being init. and - // they are slow, this won't count the whole fields/ctor initialization time, but only - // when onCreate is actually called. - startTracing(activity); - } - } - @Override public synchronized void onActivityCreated( final @NonNull Activity activity, final @Nullable Bundle savedInstanceState) { - if (!isAllActivityCallbacksAvailable) { - // start collecting frame metrics for transaction - activityFramesTracker.addActivity(activity); - - setColdStart(savedInstanceState); - } - + activityFramesTracker.addActivity(activity); + setColdStart(savedInstanceState); addBreadcrumb(activity, "created"); + startTracing(activity); - // fallback call for API < 29 compatibility, otherwise it happens on onActivityPreCreated - if (!isAllActivityCallbacksAvailable) { - startTracing(activity); - } firstActivityCreated = true; } 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 c72a2c084f..4c4bdef486 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 @@ -229,7 +229,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub, never()).startTransaction(any(), any(), anyOrNull(), any(), any()) } @@ -241,8 +241,8 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(any(), any(), anyOrNull(), any(), any()) } @@ -256,7 +256,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime() val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(any(), check { assertEquals("ui.load", it) @@ -270,7 +270,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.activityFramesTracker).addActivity(eq(activity)) } @@ -284,7 +284,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime() val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(check { assertEquals("Activity", it) @@ -307,7 +307,7 @@ class ActivityLifecycleIntegrationTest { } val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) } @Test @@ -328,7 +328,7 @@ class ActivityLifecycleIntegrationTest { } val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) } @Test @@ -338,7 +338,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) verify(fixture.hub).captureTransaction(check { @@ -353,7 +353,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) fixture.transaction.status = SpanStatus.UNKNOWN_ERROR @@ -372,7 +372,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) verify(fixture.hub, never()).captureTransaction(any()) @@ -396,7 +396,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) sut.onActivityDestroyed(activity) verify(fixture.hub).captureTransaction(any()) @@ -409,7 +409,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) assertFalse(sut.activitiesWithOngoingTransactions.isEmpty()) } @@ -421,7 +421,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) sut.onActivityDestroyed(activity) assertTrue(sut.activitiesWithOngoingTransactions.isEmpty()) @@ -433,24 +433,12 @@ class ActivityLifecycleIntegrationTest { fixture.options.tracesSampleRate = 1.0 sut.register(fixture.hub, fixture.options) - sut.onActivityPreCreated(mock(), mock()) + sut.onActivityCreated(mock(), mock()) - sut.onActivityPreCreated(mock(), fixture.bundle) + sut.onActivityCreated(mock(), fixture.bundle) verify(fixture.hub).captureTransaction(any()) } - @Test - fun `do not start transaction on created if API 29`() { - val sut = fixture.getSut() - fixture.options.tracesSampleRate = 1.0 - sut.register(fixture.hub, fixture.options) - - val activity = mock() - sut.onActivityCreated(activity, mock()) - - verify(fixture.hub, never()).startTransaction(any(), any(), anyOrNull(), any(), any()) - } - @Test fun `do not stop transaction on resumed if API 29`() { val sut = fixture.getSut() @@ -458,7 +446,7 @@ class ActivityLifecycleIntegrationTest { sut.register(fixture.hub, fixture.options) val activity = mock() - sut.onActivityPreCreated(activity, mock()) + sut.onActivityCreated(activity, mock()) sut.onActivityResumed(activity) verify(fixture.hub, never()).captureTransaction(any()) @@ -556,7 +544,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime(date) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) // call only once verify(fixture.hub).startTransaction(any(), any(), eq(date), any(), any()) @@ -572,7 +560,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime(date) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) val span = fixture.transaction.children.first() assertEquals(span.operation, "app.start.warm") @@ -589,7 +577,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime(date) val activity = mock() - sut.onActivityPreCreated(activity, null) + sut.onActivityCreated(activity, null) val span = fixture.transaction.children.first() assertEquals(span.operation, "app.start.cold") @@ -606,7 +594,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime(date) val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) val span = fixture.transaction.children.first() assertEquals(span.description, "Warm Start") @@ -623,7 +611,7 @@ class ActivityLifecycleIntegrationTest { setAppStartTime(date) val activity = mock() - sut.onActivityPreCreated(activity, null) + sut.onActivityCreated(activity, null) val span = fixture.transaction.children.first() assertEquals(span.description, "Cold Start") @@ -640,14 +628,14 @@ class ActivityLifecycleIntegrationTest { setAppStartTime() val activity = mock() - sut.onActivityPreCreated(activity, fixture.bundle) + sut.onActivityCreated(activity, fixture.bundle) verify(fixture.hub).startTransaction(any(), any(), eq(date), any(), any()) sut.onActivityCreated(activity, fixture.bundle) sut.onActivityPostResumed(activity) val newActivity = mock() - sut.onActivityPreCreated(newActivity, fixture.bundle) + sut.onActivityCreated(newActivity, fixture.bundle) val nullDate: Date? = null verify(fixture.hub).startTransaction(any(), any(), eq(nullDate), any(), any())