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

Move cold app start detection and activity timespan generation back t… #4214

Open
wants to merge 3 commits into
base: 7.x.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import io.sentry.TransactionOptions;
import io.sentry.android.core.internal.util.ClassUtil;
import io.sentry.android.core.internal.util.FirstDrawDoneListener;
import io.sentry.android.core.performance.ActivityLifecycleTimeSpan;
import io.sentry.android.core.performance.AppStartMetrics;
import io.sentry.android.core.performance.TimeSpan;
import io.sentry.protocol.MeasurementValue;
Expand Down Expand Up @@ -77,8 +76,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 final @NotNull WeakHashMap<Activity, ActivityLifecycleTimeSpan> activityLifecycleMap =
new WeakHashMap<>();

private @NotNull SentryDate lastPausedTime = new SentryNanotimeDate(new Date(0), 0);
private long lastPausedUptimeMillis = 0;
private @Nullable Future<?> ttfdAutoCloseFuture = null;
Expand All @@ -94,6 +92,7 @@ public ActivityLifecycleIntegration(
final @NotNull Application application,
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull ActivityFramesTracker activityFramesTracker) {

this.application = Objects.requireNonNull(application, "Application is required");
this.buildInfoProvider =
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
Expand Down Expand Up @@ -385,10 +384,6 @@ public void onActivityPreCreated(
? hub.getOptions().getDateProvider().now()
: AndroidDateUtils.getCurrentSentryDateTime();
lastPausedUptimeMillis = SystemClock.uptimeMillis();

final @NotNull ActivityLifecycleTimeSpan timeSpan = new ActivityLifecycleTimeSpan();
timeSpan.getOnCreate().setStartedAt(lastPausedUptimeMillis);
activityLifecycleMap.put(activity, timeSpan);
}

@Override
Expand All @@ -414,36 +409,17 @@ public synchronized void onActivityCreated(

@Override
public void onActivityPostCreated(
final @NotNull Activity activity, final @Nullable Bundle savedInstanceState) {
if (appStartSpan == null) {
activityLifecycleMap.remove(activity);
return;
}

final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.get(activity);
if (timeSpan != null) {
timeSpan.getOnCreate().stop();
timeSpan.getOnCreate().setDescription(activity.getClass().getName() + ".onCreate");
}
@NotNull Activity activity, @Nullable Bundle savedInstanceState) {
// empty override, required to avoid api-level breaking calls
}

@Override
public void onActivityPreStarted(final @NotNull Activity activity) {
if (appStartSpan == null) {
return;
}
final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.get(activity);
if (timeSpan != null) {
timeSpan.getOnStart().setStartedAt(SystemClock.uptimeMillis());
}
public void onActivityPreStarted(@NotNull Activity activity) {
// empty override, required to avoid api-level breaking calls
}

@Override
public synchronized void onActivityStarted(final @NotNull Activity activity) {
if (!isAllActivityCallbacksAvailable) {
onActivityPostCreated(activity, null);
onActivityPreStarted(activity);
}
if (performanceEnabled) {
// The docs on the screen rendering performance tracing
// (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition),
Expand All @@ -456,23 +432,12 @@ public synchronized void onActivityStarted(final @NotNull Activity activity) {
}

@Override
public void onActivityPostStarted(final @NotNull Activity activity) {
final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.remove(activity);
if (appStartSpan == null) {
return;
}
if (timeSpan != null) {
timeSpan.getOnStart().stop();
timeSpan.getOnStart().setDescription(activity.getClass().getName() + ".onStart");
AppStartMetrics.getInstance().addActivityLifecycleTimeSpans(timeSpan);
}
public void onActivityPostStarted(@NotNull Activity activity) {
// empty override, required to avoid api-level breaking calls
}

@Override
public synchronized void onActivityResumed(final @NotNull Activity activity) {
if (!isAllActivityCallbacksAvailable) {
onActivityPostStarted(activity);
}
if (performanceEnabled) {
final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity);
final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity);
Expand Down Expand Up @@ -523,7 +488,6 @@ public void onActivitySaveInstanceState(

@Override
public synchronized void onActivityDestroyed(final @NotNull Activity activity) {
activityLifecycleMap.remove(activity);
if (performanceEnabled) {

// in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid
Expand Down Expand Up @@ -563,7 +527,6 @@ private void clear() {
firstActivityCreated = false;
lastPausedTime = new SentryNanotimeDate(new Date(0), 0);
lastPausedUptimeMillis = 0;
activityLifecycleMap.clear();
}

private void finishSpan(final @Nullable ISpan span) {
Expand Down Expand Up @@ -670,12 +633,6 @@ WeakHashMap<Activity, ITransaction> getActivitiesWithOngoingTransactions() {
return activitiesWithOngoingTransactions;
}

@TestOnly
@NotNull
WeakHashMap<Activity, ActivityLifecycleTimeSpan> getActivityLifecycleMap() {
return activityLifecycleMap;
}

@TestOnly
void setFirstActivityCreated(boolean firstActivityCreated) {
this.firstActivityCreated = firstActivityCreated;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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;
Expand All @@ -25,6 +26,7 @@
import io.sentry.android.core.internal.util.FirstDrawDoneListener;
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import io.sentry.android.core.performance.ActivityLifecycleCallbacksAdapter;
import io.sentry.android.core.performance.ActivityLifecycleTimeSpan;
import io.sentry.android.core.performance.AppStartMetrics;
import io.sentry.android.core.performance.TimeSpan;
import java.io.BufferedReader;
Expand All @@ -33,6 +35,7 @@
import java.io.FileNotFoundException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.util.WeakHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -200,10 +203,51 @@ private void onAppLaunched(
appStartTimespan.setStartedAt(Process.getStartUptimeMillis());
appStartMetrics.registerApplicationForegroundCheck(app);

final AtomicBoolean firstDrawDone = new AtomicBoolean(false);
final @NotNull AtomicBoolean firstDrawDone = new AtomicBoolean(false);
final @NotNull WeakHashMap<Activity, ActivityLifecycleTimeSpan> activityLifecycleMap =
new WeakHashMap<>();

activityCallback =
new ActivityLifecycleCallbacksAdapter() {

@Override
public void onActivityPreCreated(
@NonNull Activity activity, @androidx.annotation.Nullable Bundle savedInstanceState) {
final @Nullable ActivityLifecycleTimeSpan timeSpan = new ActivityLifecycleTimeSpan();
timeSpan.getOnCreate().start();
activityLifecycleMap.put(activity, timeSpan);
}

@Override
public void onActivityCreated(
@NonNull Activity activity, @androidx.annotation.Nullable Bundle savedInstanceState) {
super.onActivityCreated(activity, savedInstanceState);
if (appStartMetrics.getAppStartType() == AppStartMetrics.AppStartType.UNKNOWN) {
appStartMetrics.setAppStartType(
savedInstanceState == null
? AppStartMetrics.AppStartType.COLD
: AppStartMetrics.AppStartType.WARM);
}
}

@Override
public void onActivityPostCreated(
@NonNull Activity activity, @androidx.annotation.Nullable Bundle savedInstanceState) {
final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.get(activity);
if (timeSpan != null) {
timeSpan.getOnCreate().stop();
timeSpan.getOnCreate().setDescription(activity.getClass().getName() + ".onCreate");
}
}

@Override
public void onActivityPreStarted(@NonNull Activity activity) {
final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.get(activity);
if (timeSpan != null) {
timeSpan.getOnStart().setStartedAt(SystemClock.uptimeMillis());
}
}

@Override
public void onActivityStarted(@NonNull Activity activity) {
if (firstDrawDone.get()) {
Expand All @@ -216,6 +260,29 @@ public void onActivityStarted(@NonNull Activity activity) {
new Handler(Looper.getMainLooper()).post(() -> onAppStartDone());
}
}

@Override
public void onActivityPostStarted(@NonNull Activity activity) {
final @Nullable ActivityLifecycleTimeSpan timeSpan = activityLifecycleMap.get(activity);
if (timeSpan != null) {
timeSpan.getOnStart().setStoppedAt(SystemClock.uptimeMillis());
timeSpan.getOnStart().setDescription(activity.getClass().getName() + ".onStop");
AppStartMetrics.getInstance().addActivityLifecycleTimeSpans(timeSpan);
}

// once start is over, we don't need to reference the activity anymore
activityLifecycleMap.remove(activity);
}

@Override
public void onActivityPreResumed(@NonNull Activity activity) {
// empty override, required to avoid api-level breaking calls
}

@Override
public void onActivityPostResumed(@NonNull Activity activity) {
// empty override, required to avoid api-level breaking calls
}
};

app.registerActivityLifecycleCallbacks(activityCallback);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1424,97 +1424,6 @@ class ActivityLifecycleIntegrationTest {
assertEquals(now.nanoTimestamp(), fixture.transaction.startDate.nanoTimestamp())
}

@Test
fun `On activity preCreated onCreate span is created`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
sut.register(fixture.hub, fixture.options)

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

assertTrue(sut.activityLifecycleMap.isEmpty())

val activity = mock<Activity>()
// Activity onCreate date will be used
sut.onActivityPreCreated(activity, fixture.bundle)
// sut.onActivityCreated(activity, fixture.bundle)

assertFalse(sut.activityLifecycleMap.isEmpty())
assertTrue(sut.activityLifecycleMap.values.first().onCreate.hasStarted())
assertFalse(sut.activityLifecycleMap.values.first().onCreate.hasStopped())
}

@Test
fun `Creates activity lifecycle spans`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
val appStartDate = SentryNanotimeDate(Date(1), 0)
val startDate = SentryNanotimeDate(Date(2), 0)
val appStartMetrics = AppStartMetrics.getInstance()
val activity = mock<Activity>()
fixture.options.dateProvider = SentryDateProvider { startDate }
setAppStartTime(appStartDate)

sut.register(fixture.hub, fixture.options)
assertTrue(sut.activityLifecycleMap.isEmpty())

sut.onActivityPreCreated(activity, null)

assertFalse(sut.activityLifecycleMap.isEmpty())
val activityLifecycleSpan = sut.activityLifecycleMap.values.first()
assertTrue(activityLifecycleSpan.onCreate.hasStarted())
assertEquals(startDate.nanoTimestamp(), sut.getProperty<SentryDate>("lastPausedTime").nanoTimestamp())

sut.onActivityCreated(activity, null)
assertNotNull(sut.appStartSpan)

sut.onActivityPostCreated(activity, null)
assertTrue(activityLifecycleSpan.onCreate.hasStopped())

sut.onActivityPreStarted(activity)
assertTrue(activityLifecycleSpan.onStart.hasStarted())

sut.onActivityStarted(activity)
assertTrue(appStartMetrics.activityLifecycleTimeSpans.isEmpty())

sut.onActivityPostStarted(activity)
assertTrue(activityLifecycleSpan.onStart.hasStopped())
assertFalse(appStartMetrics.activityLifecycleTimeSpans.isEmpty())
}

@Test
fun `Creates activity lifecycle spans on API lower than 29`() {
val sut = fixture.getSut(apiVersion = Build.VERSION_CODES.P)
fixture.options.tracesSampleRate = 1.0
val appStartDate = SentryNanotimeDate(Date(1), 0)
val startDate = SentryNanotimeDate(Date(2), 0)
val appStartMetrics = AppStartMetrics.getInstance()
val activity = mock<Activity>()
fixture.options.dateProvider = SentryDateProvider { startDate }
setAppStartTime(appStartDate)

sut.register(fixture.hub, fixture.options)
assertTrue(sut.activityLifecycleMap.isEmpty())

sut.onActivityCreated(activity, null)

assertFalse(sut.activityLifecycleMap.isEmpty())
val activityLifecycleSpan = sut.activityLifecycleMap.values.first()
assertTrue(activityLifecycleSpan.onCreate.hasStarted())
assertEquals(startDate.nanoTimestamp(), sut.getProperty<SentryDate>("lastPausedTime").nanoTimestamp())
assertNotNull(sut.appStartSpan)

sut.onActivityStarted(activity)
assertTrue(activityLifecycleSpan.onCreate.hasStopped())
assertTrue(activityLifecycleSpan.onStart.hasStarted())
assertTrue(appStartMetrics.activityLifecycleTimeSpans.isEmpty())

sut.onActivityResumed(activity)
assertTrue(activityLifecycleSpan.onStart.hasStopped())
assertFalse(appStartMetrics.activityLifecycleTimeSpans.isEmpty())
}

@Test
fun `Does not add activity lifecycle spans when firstActivityCreated is true`() {
val sut = fixture.getSut()
Expand Down
Loading
Loading