Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Ensure app start type is set, even when ActivityLifecycleIntegration is not running #4216

Open
wants to merge 17 commits into
base: 7.x.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Fix Ensure app start type is set, even when ActivityLifecycleIntegration is not running ([#4216](https://github.com/getsentry/sentry-java/pull/4216))

## 7.22.0

### Fixes
Expand Down
1 change: 1 addition & 0 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr
public fun setAppStartType (Lio/sentry/android/core/performance/AppStartMetrics$AppStartType;)V
public fun setClassLoadedUptimeMs (J)V
public fun shouldSendStartMeasurements ()Z
public fun updateAppStartType (ZJ)V
}

public final class io/sentry/android/core/performance/AppStartMetrics$AppStartType : java/lang/Enum {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ public synchronized void onActivityCreated(
if (!isAllActivityCallbacksAvailable) {
onActivityPreCreated(activity, savedInstanceState);
}
setColdStart(savedInstanceState);
setColdStart(savedInstanceState != null);
if (hub != null && options != null && options.isEnableScreenTracking()) {
final @Nullable String activityClassName = ClassUtil.getClassName(activity);
hub.configureScope(scope -> scope.setScreen(activityClassName));
Expand Down Expand Up @@ -705,24 +705,9 @@ WeakHashMap<Activity, ISpan> getTtfdSpanMap() {
return ttfdSpanMap;
}

private void setColdStart(final @Nullable Bundle savedInstanceState) {
private void setColdStart(final boolean hasBundle) {
if (!firstActivityCreated) {
final @NotNull TimeSpan appStartSpan = AppStartMetrics.getInstance().getAppStartTimeSpan();
// If the app start span already started and stopped, it means the app restarted without
// killing the process, so we are in a warm start
// If the app has an invalid cold start, it means it was started in the background, like
// via BroadcastReceiver, so we consider it a warm start
if ((appStartSpan.hasStarted() && appStartSpan.hasStopped())
|| (!AppStartMetrics.getInstance().isColdStartValid())) {
AppStartMetrics.getInstance().restartAppStart(lastPausedUptimeMillis);
AppStartMetrics.getInstance().setAppStartType(AppStartMetrics.AppStartType.WARM);
} else {
AppStartMetrics.getInstance()
.setAppStartType(
savedInstanceState == null
? AppStartMetrics.AppStartType.COLD
: AppStartMetrics.AppStartType.WARM);
}
AppStartMetrics.getInstance().updateAppStartType(hasBundle, lastPausedUptimeMillis);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import android.content.pm.ProviderInfo;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
import android.os.Process;
import android.os.SystemClock;
import android.util.Log;
import androidx.annotation.NonNull;
import io.sentry.ILogger;
import io.sentry.ITransactionProfiler;
Expand All @@ -34,6 +36,7 @@
import java.io.InputStreamReader;
import java.io.Reader;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -51,6 +54,8 @@ public final class SentryPerformanceProvider extends EmptySecureContentProvider

private final @NotNull ILogger logger;
private final @NotNull BuildInfoProvider buildInfoProvider;
private final AtomicInteger activeActivitiesCounter = new AtomicInteger();
private final AtomicBoolean firstDrawDone = new AtomicBoolean(false);

@TestOnly
SentryPerformanceProvider(
Expand Down Expand Up @@ -200,12 +205,27 @@ private void onAppLaunched(
appStartTimespan.setStartedAt(Process.getStartUptimeMillis());
appStartMetrics.registerApplicationForegroundCheck(app);

final AtomicBoolean firstDrawDone = new AtomicBoolean(false);

activityCallback =
new ActivityLifecycleCallbacksAdapter() {

@Override
public void onActivityStarted(@NonNull Activity activity) {
public void onActivityCreated(
@NotNull Activity activity, @Nullable Bundle savedInstanceState) {
Log.d("TAG", "onActivityCreated");
activeActivitiesCounter.incrementAndGet();

// In case the SDK gets initialized async or the
// ActivityLifecycleIntegration is not enabled (e.g on RN due to Context not being
// instanceof Application)
// the app start type never gets set
if (!firstDrawDone.get()) {
final long now = SystemClock.uptimeMillis();
AppStartMetrics.getInstance().updateAppStartType(savedInstanceState != null, now);
}
}

@Override
public void onActivityStarted(@NotNull Activity activity) {
if (firstDrawDone.get()) {
return;
}
Expand All @@ -216,20 +236,26 @@ public void onActivityStarted(@NonNull Activity activity) {
new Handler(Looper.getMainLooper()).post(() -> onAppStartDone());
}
}

@Override
public void onActivityDestroyed(@NonNull Activity activity) {
final int remainingActivities = activeActivitiesCounter.decrementAndGet();
// if the app is moving into background, reset firstDrawDone
// as the next Activity is considered like a new app start
if (remainingActivities == 0 && !activity.isChangingConfigurations()) {
firstDrawDone.set(false);
}
}
};

app.registerActivityLifecycleCallbacks(activityCallback);
}

synchronized void onAppStartDone() {
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
appStartMetrics.getSdkInitTimeSpan().stop();
appStartMetrics.getAppStartTimeSpan().stop();

if (app != null) {
if (activityCallback != null) {
app.unregisterActivityLifecycleCallbacks(activityCallback);
}
if (!firstDrawDone.getAndSet(true)) {
final @NotNull AppStartMetrics appStartMetrics = AppStartMetrics.getInstance();
appStartMetrics.getSdkInitTimeSpan().stop();
appStartMetrics.getAppStartTimeSpan().stop();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,26 @@ public static void onContentProviderPostCreate(final @NotNull ContentProvider co
measurement.setStoppedAt(now);
}
}

/**
* @param hasBundle true if the activity onCreate had a non-null bundle
* @param lastKnownStart in case the app start is too long, resets the app start timestamp to this
* value
*/
public void updateAppStartType(final boolean hasBundle, final long lastKnownStart) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* @param hasBundle true if the activity onCreate had a non-null bundle
* @param lastKnownStart in case the app start is too long, resets the app start timestamp to this
* value
*/
public void updateAppStartType(final boolean hasBundle, final long lastKnownStart) {
/**
* @param hasBundle true if the activity onCreate had a non-null bundle
* @param startUptimeMillis in case the app start is too long, resets the app start timestamp to this
* value, retrieved via System.uptimeMillis()
*/
public void updateAppStartType(final boolean hasBundle, final long startUptimeMillis) {

Just make it clear we want a time from System.uptimeMillis()

final @NotNull TimeSpan appStartSpan = getInstance().getAppStartTimeSpan();
// If the app start span already started and stopped, it means the app restarted without
// killing the process, so we are in a warm start
// If the app has an invalid cold start, it means it was started in the background, like
// via BroadcastReceiver, so we consider it a warm start
if ((appStartSpan.hasStarted() && appStartSpan.hasStopped())
|| (!getInstance().isColdStartValid())) {
getInstance().restartAppStart(lastKnownStart);
getInstance().setAppStartType(AppStartMetrics.AppStartType.WARM);
} else {
getInstance()
.setAppStartType(
hasBundle ? AppStartMetrics.AppStartType.WARM : AppStartMetrics.AppStartType.COLD);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package io.sentry.android.core

import android.app.Activity
import android.app.Application
import android.app.Application.ActivityLifecycleCallbacks
import android.content.pm.ProviderInfo
import android.os.Build
import android.os.Bundle
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.sentry.ILogger
import io.sentry.JsonSerializer
Expand All @@ -26,6 +29,7 @@ import java.nio.file.Files
import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertNotNull
Expand All @@ -48,6 +52,7 @@ class SentryPerformanceProviderTest {
val providerInfo = ProviderInfo()
val logger = mock<ILogger>()
lateinit var configFile: File
var activityLifecycleCallback: ActivityLifecycleCallbacks? = null

fun getSut(sdkVersion: Int = Build.VERSION_CODES.S, authority: String = AUTHORITY, handleFile: ((config: File) -> Unit)? = null): SentryPerformanceProvider {
val buildInfoProvider: BuildInfoProvider = mock()
Expand All @@ -56,7 +61,10 @@ class SentryPerformanceProviderTest {
whenever(mockContext.applicationContext).thenReturn(mockContext)
configFile = File(sentryCache, Sentry.APP_START_PROFILING_CONFIG_FILE_NAME)
handleFile?.invoke(configFile)

whenever(mockContext.registerActivityLifecycleCallbacks(any())).then {
activityLifecycleCallback = it.arguments[0] as ActivityLifecycleCallbacks
return@then Unit
}
providerInfo.authority = authority
return SentryPerformanceProvider(logger, buildInfoProvider).apply {
attachInfo(mockContext, providerInfo)
Expand Down Expand Up @@ -232,6 +240,73 @@ class SentryPerformanceProviderTest {
assertFalse(AppStartMetrics.getInstance().appStartProfiler!!.isRunning)
}

@Test
fun `Sets app launch type to cold`() {
val provider = fixture.getSut()
val activity = mock<Activity>()
provider.onCreate()

assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType)

// when the first activity has no bundle
fixture.activityLifecycleCallback!!.onActivityCreated(activity, null)

// then the app start is considered cold
assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType)

// when any subsequent activity launches
fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock<Bundle>())

// then the app start is still considered cold
assertEquals(AppStartMetrics.AppStartType.COLD, AppStartMetrics.getInstance().appStartType)
}

@Test
fun `Sets app launch type to warm if process init is too old`() {
val provider = fixture.getSut()
val activity = mock<Activity>()
provider.onCreate()

assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType)

AppStartMetrics.getInstance().appStartTimeSpan.setStartedAt(
AppStartMetrics.getInstance().appStartTimeSpan.startUptimeMs - 20000
)

// when the first activity has no bundle
fixture.activityLifecycleCallback!!.onActivityCreated(activity, null)

// then the app start is considered warm
assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType)

// when any subsequent activity launches
fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock<Bundle>())

// then the app start is still considered warm
assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType)
}

@Test
fun `Sets app launch type to warm`() {
val provider = fixture.getSut()
val activity = mock<Activity>()
provider.onCreate()

assertEquals(AppStartMetrics.AppStartType.UNKNOWN, AppStartMetrics.getInstance().appStartType)

// when the first activity has a bundle
fixture.activityLifecycleCallback!!.onActivityCreated(activity, mock<Bundle>())

// then the app start is considered WARM
assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType)

// when any subsequent activity launches
fixture.activityLifecycleCallback!!.onActivityCreated(activity, null)

// then the app start is still considered warm
assertEquals(AppStartMetrics.AppStartType.WARM, AppStartMetrics.getInstance().appStartType)
}

private fun writeConfig(
configFile: File,
profilingEnabled: Boolean = true,
Expand Down
Loading