Skip to content
Merged
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
Contributor

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