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 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
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
6 changes: 3 additions & 3 deletions sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -447,15 +447,15 @@ public class io/sentry/android/core/performance/AppStartMetrics : io/sentry/andr
public static fun getInstance ()Lio/sentry/android/core/performance/AppStartMetrics;
public fun getSdkInitTimeSpan ()Lio/sentry/android/core/performance/TimeSpan;
public fun isAppLaunchedInForeground ()Z
public fun isColdStartValid ()Z
public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V
public fun onActivityDestroyed (Landroid/app/Activity;)V
public fun onActivityStarted (Landroid/app/Activity;)V
public fun onAppStartSpansSent ()V
public static fun onApplicationCreate (Landroid/app/Application;)V
public static fun onApplicationPostCreate (Landroid/app/Application;)V
public static fun onContentProviderCreate (Landroid/content/ContentProvider;)V
public static fun onContentProviderPostCreate (Landroid/content/ContentProvider;)V
public fun registerApplicationForegroundCheck (Landroid/app/Application;)V
public fun restartAppStart (J)V
public fun registerLifecycleCallbacks (Landroid/app/Application;)V
public fun setAppLaunchedInForeground (Z)V
public fun setAppStartProfiler (Lio/sentry/ITransactionProfiler;)V
public fun setAppStartSamplingDecision (Lio/sentry/TracesSamplingDecision;)V
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,6 @@ public synchronized void onActivityCreated(
if (!isAllActivityCallbacksAvailable) {
onActivityPreCreated(activity, savedInstanceState);
}
setColdStart(savedInstanceState);
if (hub != null && options != null && options.isEnableScreenTracking()) {
final @Nullable String activityClassName = ClassUtil.getClassName(activity);
hub.configureScope(scope -> scope.setScreen(activityClassName));
Expand Down Expand Up @@ -554,15 +553,13 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) {
// if the activity is opened again and not in memory, transactions will be created normally.
activitiesWithOngoingTransactions.remove(activity);

if (activitiesWithOngoingTransactions.isEmpty()) {
if (activitiesWithOngoingTransactions.isEmpty() && !activity.isChangingConfigurations()) {
clear();
}
}

private void clear() {
firstActivityCreated = false;
lastPausedTime = new SentryNanotimeDate(new Date(0), 0);
lastPausedUptimeMillis = 0;
activityLifecycleMap.clear();
}

Expand Down Expand Up @@ -705,27 +702,6 @@ WeakHashMap<Activity, ISpan> getTtfdSpanMap() {
return ttfdSpanMap;
}

private void setColdStart(final @Nullable Bundle savedInstanceState) {
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);
}
}
}

private @NotNull String getTtidDesc(final @NotNull String activityName) {
return activityName + " initial display";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public static synchronized void init(
}
}
if (context.getApplicationContext() instanceof Application) {
appStartMetrics.registerApplicationForegroundCheck(
appStartMetrics.registerLifecycleCallbacks(
(Application) context.getApplicationContext());
}
final @NotNull TimeSpan sdkInitTimeSpan = appStartMetrics.getSdkInitTimeSpan();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@
import static io.sentry.Sentry.APP_START_PROFILING_CONFIG_FILE_NAME;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.Application;
import android.content.Context;
import android.content.pm.ProviderInfo;
import android.net.Uri;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.os.Process;
import android.os.SystemClock;
import androidx.annotation.NonNull;
import io.sentry.ILogger;
import io.sentry.ITransactionProfiler;
import io.sentry.JsonSerializer;
Expand All @@ -22,9 +18,7 @@
import io.sentry.SentryLevel;
import io.sentry.SentryOptions;
import io.sentry.TracesSamplingDecision;
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.AppStartMetrics;
import io.sentry.android.core.performance.TimeSpan;
import java.io.BufferedReader;
Expand All @@ -33,7 +27,6 @@
import java.io.FileNotFoundException;
import java.io.InputStreamReader;
import java.io.Reader;
import java.util.concurrent.atomic.AtomicBoolean;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -185,8 +178,9 @@ private void onAppLaunched(

// performance v2: Uses Process.getStartUptimeMillis()
// requires API level 24+
if (buildInfoProvider.getSdkInfoVersion() < android.os.Build.VERSION_CODES.N) {
return;
if (buildInfoProvider.getSdkInfoVersion() >= android.os.Build.VERSION_CODES.N) {
final @NotNull TimeSpan appStartTimespan = appStartMetrics.getAppStartTimeSpan();
appStartTimespan.setStartedAt(Process.getStartUptimeMillis());
}

if (context instanceof Application) {
Expand All @@ -196,40 +190,6 @@ private void onAppLaunched(
return;
}

final @NotNull TimeSpan appStartTimespan = appStartMetrics.getAppStartTimeSpan();
appStartTimespan.setStartedAt(Process.getStartUptimeMillis());
appStartMetrics.registerApplicationForegroundCheck(app);

final AtomicBoolean firstDrawDone = new AtomicBoolean(false);

activityCallback =
new ActivityLifecycleCallbacksAdapter() {
@Override
public void onActivityStarted(@NonNull Activity activity) {
if (firstDrawDone.get()) {
return;
}
if (activity.getWindow() != null) {
FirstDrawDoneListener.registerForNextDraw(
activity, () -> onAppStartDone(), buildInfoProvider);
} else {
new Handler(Looper.getMainLooper()).post(() -> onAppStartDone());
}
}
};

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);
}
}
appStartMetrics.registerLifecycleCallbacks(app);
Copy link
Member

Choose a reason for hiding this comment

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

this is a behavioural change, as we were not doing it on API < 24, while now we would. I'd keep the previous check that returns early

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's on purpose - otherwise we would need to duplicate half of the logic within AppStartMetrics again, app start type, foreground importance, etc..

}
}
Loading
Loading