Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ActivityLifecycleIntegration now reads first activity creation on cre…
…ate instead of class instantiation

AppStartMetrics callback is now registered only once
AppStartMetrics stops app start profiler if no activity is being created
  • Loading branch information
stefanosiano committed Jul 16, 2024
commit 93a996299c9d3ba9cee025615489dae956e592fd
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import io.sentry.NoOpTransaction;
import io.sentry.SentryDate;
import io.sentry.SentryLevel;
import io.sentry.SentryNanotimeDate;
import io.sentry.SentryOptions;
import io.sentry.SpanStatus;
import io.sentry.TracesSamplingDecision;
Expand All @@ -37,6 +38,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.lang.ref.WeakReference;
import java.util.Date;
import java.util.Map;
import java.util.WeakHashMap;
import java.util.concurrent.Future;
Expand Down Expand Up @@ -75,7 +77,7 @@ public final class ActivityLifecycleIntegration
private @Nullable ISpan appStartSpan;
private final @NotNull WeakHashMap<Activity, ISpan> ttidSpanMap = new WeakHashMap<>();
private final @NotNull WeakHashMap<Activity, ISpan> ttfdSpanMap = new WeakHashMap<>();
private @NotNull SentryDate lastPausedTime = AndroidDateUtils.getCurrentSentryDateTime();
private @NotNull SentryDate lastPausedTime = new SentryNanotimeDate(new Date(0), 0);
private final @NotNull Handler mainHandler = new Handler(Looper.getMainLooper());
private @Nullable Future<?> ttfdAutoCloseFuture = null;

Expand Down Expand Up @@ -627,6 +629,14 @@ WeakHashMap<Activity, ISpan> getTtfdSpanMap() {
}

private void setColdStart(final @Nullable Bundle savedInstanceState) {
// The very first activity start timestamp cannot be set to the class instantiation time, as it
// may happen before an activity is started (service, broadcast receiver, etc). So we set it
// here.
if (hub != null && lastPausedTime.nanoTimestamp() == 0) {
lastPausedTime = hub.getOptions().getDateProvider().now();
} else if (lastPausedTime.nanoTimestamp() == 0) {
lastPausedTime = AndroidDateUtils.getCurrentSentryDateTime();
}
if (!firstActivityCreated) {
// if Activity has savedInstanceState then its a warm start
// https://developer.android.com/topic/performance/vitals/launch-time#warm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public enum AppStartType {
private @Nullable TracesSamplingDecision appStartSamplingDecision = null;
private @Nullable SentryDate onCreateTime = null;
private boolean appLaunchTooLong = false;
private boolean isCallbackRegistered = false;

public static @NotNull AppStartMetrics getInstance() {

Expand Down Expand Up @@ -186,6 +187,7 @@ public void clear() {
appLaunchTooLong = false;
appLaunchedInForeground = false;
onCreateTime = null;
isCallbackRegistered = false;
}

public @Nullable ITransactionProfiler getAppStartProfiler() {
Expand Down Expand Up @@ -223,8 +225,6 @@ public static void onApplicationCreate(final @NotNull Application application) {
final @NotNull AppStartMetrics instance = getInstance();
if (instance.applicationOnCreate.hasNotStarted()) {
instance.applicationOnCreate.setStartedAt(now);
instance.appLaunchedInForeground =
instance.appLaunchedInForeground || ContextUtils.isForegroundImportance();
instance.registerApplicationForegroundCheck(application);
}
}
Expand All @@ -235,6 +235,11 @@ public static void onApplicationCreate(final @NotNull Application application) {
* @param application The application object to register the callback to
*/
public void registerApplicationForegroundCheck(final @NotNull Application application) {
if (isCallbackRegistered) {
return;
}
isCallbackRegistered = true;
appLaunchedInForeground = appLaunchedInForeground || ContextUtils.isForegroundImportance();
application.registerActivityLifecycleCallbacks(instance);
new Handler(Looper.getMainLooper())
.post(
Expand All @@ -244,6 +249,11 @@ public void registerApplicationForegroundCheck(final @NotNull Application applic
appLaunchedInForeground = false;
}
application.unregisterActivityLifecycleCallbacks(instance);
// we stop the app start profiler, as it's useless and likely to timeout
if (appStartProfiler != null && appStartProfiler.isRunning()) {
appStartProfiler.close();
appStartProfiler = null;
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -711,15 +711,19 @@ class ActivityLifecycleIntegrationTest {
sut.register(fixture.hub, fixture.options)

val date = SentryNanotimeDate(Date(1), 0)
val date2 = SentryNanotimeDate(Date(2), 2)
setAppStartTime(date)

val activity = mock<Activity>()
// The activity onCreate date will be ignored
fixture.options.dateProvider = SentryDateProvider { date2 }
sut.onActivityCreated(activity, fixture.bundle)

verify(fixture.hub).startTransaction(
any(),
check<TransactionOptions> {
assertEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp())
assertNotEquals(date2.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp())
assertFalse(it.isAppStartTransaction)
}
)
Expand Down Expand Up @@ -758,6 +762,30 @@ class ActivityLifecycleIntegrationTest {
)
}

@Test
fun `When firstActivityCreated is true and no app start time is set, default to onActivityCreated time`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
sut.register(fixture.hub, fixture.options)

// usually set by SentryPerformanceProvider
val date = SentryNanotimeDate(Date(1), 0)
val date2 = SentryNanotimeDate(Date(2), 2)

val activity = mock<Activity>()
// Activity onCreate date will be used
fixture.options.dateProvider = SentryDateProvider { date2 }
sut.onActivityCreated(activity, fixture.bundle)

verify(fixture.hub).startTransaction(
any(),
check<TransactionOptions> {
assertEquals(date2.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp())
assertNotEquals(date.nanoTimestamp(), it.startTimestamp!!.nanoTimestamp())
}
)
}

@Test
fun `Create and finish app start span immediately in case SDK init is deferred`() {
val sut = fixture.getSut(importance = RunningAppProcessInfo.IMPORTANCE_FOREGROUND)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ import android.content.ContentProvider
import android.os.Build
import android.os.Looper
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.sentry.ITransactionProfiler
import io.sentry.android.core.SentryAndroidOptions
import io.sentry.android.core.SentryShadowProcess
import org.junit.Before
import org.junit.runner.RunWith
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.robolectric.Shadows
import org.robolectric.annotation.Config
import java.util.concurrent.TimeUnit
Expand All @@ -36,7 +39,6 @@ class AppStartMetricsTest {
AppStartMetrics.getInstance().clear()
SentryShadowProcess.setStartUptimeMillis(42)
AppStartMetrics.getInstance().isAppLaunchedInForeground = true
AppStartMetrics.onApplicationCreate(mock())
}

@Test
Expand Down Expand Up @@ -170,23 +172,30 @@ class AppStartMetricsTest {

@Test
fun `if activity is never started, returns an empty span`() {
AppStartMetrics.getInstance().registerApplicationForegroundCheck(mock())
val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan
appStartTimeSpan.start()
appStartTimeSpan.stop()
appStartTimeSpan.setStartedAt(1)
appStartTimeSpan.setStoppedAt(TimeUnit.MINUTES.toMillis(1) + 1)
assertTrue(appStartTimeSpan.hasStarted())
// Job on main thread checks if activity was launched
Shadows.shadowOf(Looper.getMainLooper()).idle()

val options = SentryAndroidOptions().apply {
isEnablePerformanceV2 = true
}

val timeSpan = AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(options)
val timeSpan = AppStartMetrics.getInstance().getAppStartTimeSpanWithFallback(SentryAndroidOptions())
assertFalse(timeSpan.hasStarted())
}

@Test
fun `if activity is never started, stops app start profiler if running`() {
val profiler = mock<ITransactionProfiler>()
whenever(profiler.isRunning).thenReturn(true)
AppStartMetrics.getInstance().appStartProfiler = profiler

AppStartMetrics.getInstance().registerApplicationForegroundCheck(mock())
// Job on main thread checks if activity was launched
Shadows.shadowOf(Looper.getMainLooper()).idle()

verify(profiler).close()
}

@Test
fun `if app start span is longer than 1 minute, appStartTimeSpanWithFallback returns an empty span`() {
val appStartTimeSpan = AppStartMetrics.getInstance().appStartTimeSpan
Expand All @@ -205,6 +214,14 @@ class AppStartMetricsTest {
assertFalse(timeSpan.hasStarted())
}

@Test
fun `when multiple registerApplicationForegroundCheck, only one callback is registered to application`() {
val application = mock<Application>()
AppStartMetrics.getInstance().registerApplicationForegroundCheck(application)
AppStartMetrics.getInstance().registerApplicationForegroundCheck(application)
verify(application, times(1)).registerActivityLifecycleCallbacks(eq(AppStartMetrics.getInstance()))
}

@Test
fun `when registerApplicationForegroundCheck, a callback is registered to application`() {
val application = mock<Application>()
Expand Down