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

Add ttid span to ActivityLifecycleIntegration #2369

Merged
merged 15 commits into from Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 10 additions & 4 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Features

- Add ttid span to ActivityLifecycleIntegration ([#2369](https://github.com/getsentry/sentry-java/pull/2369))
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved

## 6.9.1

### Fixes
Expand Down Expand Up @@ -28,15 +34,15 @@

## 6.8.0

### Features

- Add FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342))

### Fixes

- Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348))
- Fix ensure all options are processed before integrations are loaded ([#2377](https://github.com/getsentry/sentry-java/pull/2377))

### Features

- Add FrameMetrics to Android profiling data ([#2342](https://github.com/getsentry/sentry-java/pull/2342))

## 6.7.1

### Fixes
Expand Down
1 change: 1 addition & 0 deletions sentry-android-core/api/sentry-android-core.api
Expand Up @@ -15,6 +15,7 @@ public final class io/sentry/android/core/ActivityLifecycleIntegration : android
public fun onActivityDestroyed (Landroid/app/Activity;)V
public fun onActivityPaused (Landroid/app/Activity;)V
public fun onActivityPostResumed (Landroid/app/Activity;)V
public fun onActivityPrePaused (Landroid/app/Activity;)V
public fun onActivityResumed (Landroid/app/Activity;)V
public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V
public fun onActivityStarted (Landroid/app/Activity;)V
Expand Down
Expand Up @@ -3,14 +3,20 @@
import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND;
import static io.sentry.TypeCheckHint.ANDROID_ACTIVITY;

import android.annotation.SuppressLint;
import android.app.Activity;
import android.app.ActivityManager;
import android.app.Application;
import android.content.Context;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.os.Looper;
import android.os.Process;
import android.view.View;
import androidx.annotation.NonNull;
import io.sentry.Breadcrumb;
import io.sentry.DateUtils;
import io.sentry.Hint;
import io.sentry.IHub;
import io.sentry.ISpan;
Expand All @@ -23,6 +29,7 @@
import io.sentry.SpanStatus;
import io.sentry.TransactionContext;
import io.sentry.TransactionOptions;
import io.sentry.android.core.internal.util.FirstDrawDoneListener;
import io.sentry.protocol.TransactionNameSource;
import io.sentry.util.Objects;
import java.io.Closeable;
Expand All @@ -45,6 +52,7 @@ public final class ActivityLifecycleIntegration
static final String APP_START_COLD = "app.start.cold";

private final @NotNull Application application;
private final @NotNull BuildInfoProvider buildInfoProvider;
private @Nullable IHub hub;
private @Nullable SentryAndroidOptions options;

Expand All @@ -57,6 +65,9 @@ public final class ActivityLifecycleIntegration
private boolean foregroundImportance = false;

private @Nullable ISpan appStartSpan;
private final @NotNull WeakHashMap<Activity, ISpan> ttidSpanMap = new WeakHashMap<>();
private @NotNull Date lastPausedTime = DateUtils.getCurrentDateTime();
private final @NotNull Handler mainHandler = new Handler(Looper.getMainLooper());

// WeakHashMap isn't thread safe but ActivityLifecycleCallbacks is only called from the
// main-thread
Expand All @@ -70,7 +81,8 @@ public ActivityLifecycleIntegration(
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull ActivityFramesTracker activityFramesTracker) {
this.application = Objects.requireNonNull(application, "Application is required");
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
this.buildInfoProvider =
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
this.activityFramesTracker =
Objects.requireNonNull(activityFramesTracker, "ActivityFramesTracker is required");

Expand Down Expand Up @@ -146,7 +158,8 @@ private void stopPreviousTransactions() {
for (final Map.Entry<Activity, ITransaction> entry :
activitiesWithOngoingTransactions.entrySet()) {
final ITransaction transaction = entry.getValue();
finishTransaction(transaction);
final ISpan ttidSpan = ttidSpanMap.get(entry.getKey());
finishTransaction(transaction, ttidSpan);
}
}

Expand Down Expand Up @@ -202,6 +215,18 @@ private void startTracing(final @NotNull Activity activity) {
getAppStartDesc(coldStart),
appStartTime,
Instrumenter.SENTRY);
// The first activity ttidSpan should start at the same time as the app start time
ttidSpanMap.put(
activity,
transaction.startChild(
"TTID", activityName + ".ttid", appStartTime, Instrumenter.SENTRY));
} else {
// Other activities (or in case appStartTime is not available) the ttid span should
// start when the previous activity called its onPause method.
ttidSpanMap.put(
activity,
transaction.startChild(
"TTID", activityName + ".ttid", lastPausedTime, Instrumenter.SENTRY));
}

// lets bind to the scope so other integrations can pick it up
Expand Down Expand Up @@ -250,18 +275,25 @@ private boolean isRunningTransaction(final @NotNull Activity activity) {
private void stopTracing(final @NotNull Activity activity, final boolean shouldFinishTracing) {
if (performanceEnabled && shouldFinishTracing) {
final ITransaction transaction = activitiesWithOngoingTransactions.get(activity);
finishTransaction(transaction);
final ISpan ttidSpan = ttidSpanMap.get(activity);
finishTransaction(transaction, ttidSpan);
}
}

private void finishTransaction(final @Nullable ITransaction transaction) {
private void finishTransaction(
final @Nullable ITransaction transaction, final @Nullable ISpan ttidSpan) {
if (transaction != null) {
// if io.sentry.traces.activity.auto-finish.enable is disabled, transaction may be already
// finished manually when this method is called.
if (transaction.isFinished()) {
return;
}

// in case the ttidSpan isn't completed yet, we finish it as cancelled to avoid memory leak
if (ttidSpan != null && !ttidSpan.isFinished()) {
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
ttidSpan.finish(SpanStatus.CANCELLED);
}

SpanStatus status = transaction.getStatus();
// status might be set by other integrations, let's not overwrite it
if (status == null) {
Expand Down Expand Up @@ -301,6 +333,7 @@ public synchronized void onActivityStarted(final @NotNull Activity activity) {
addBreadcrumb(activity, "started");
}

@SuppressLint("NewApi")
@Override
public synchronized void onActivityResumed(final @NotNull Activity activity) {
if (!firstActivityResumed) {
Expand All @@ -326,6 +359,30 @@ public synchronized void onActivityResumed(final @NotNull Activity activity) {
firstActivityResumed = true;
}

final ISpan ttidSpan = ttidSpanMap.get(activity);
final View rootView = activity.findViewById(android.R.id.content);
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN
&& rootView != null) {
FirstDrawDoneListener.registerForNextDraw(
rootView,
() -> {
// finishes ttidSpan span
if (ttidSpan != null && !ttidSpan.isFinished()) {
ttidSpan.finish();
}
},
buildInfoProvider);
} else {
// Posting a task to the main thread's handler will make it executed after it finished
// its current job. That is, right after the activity draws the layout.
mainHandler.post(
() -> {
// finishes ttidSpan span
if (ttidSpan != null && !ttidSpan.isFinished()) {
ttidSpan.finish();
}
});
}
addBreadcrumb(activity, "resumed");

// fallback call for API < 29 compatibility, otherwise it happens on onActivityPostResumed
Expand All @@ -344,8 +401,20 @@ public synchronized void onActivityPostResumed(final @NotNull Activity activity)
}
}

@Override
public void onActivityPrePaused(@NonNull Activity activity) {
// only executed if API >= 29 otherwise it happens on onActivityPaused
if (isAllActivityCallbacksAvailable) {
lastPausedTime = DateUtils.getCurrentDateTime();
}
}

@Override
public synchronized void onActivityPaused(final @NotNull Activity activity) {
// only executed if API < 29 otherwise it happens on onActivityPrePaused
if (!isAllActivityCallbacksAvailable) {
lastPausedTime = DateUtils.getCurrentDateTime();
}
addBreadcrumb(activity, "paused");
}

Expand Down Expand Up @@ -376,6 +445,8 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) {

// set it to null in case its been just finished as cancelled
appStartSpan = null;
// ttidSpan is finished in the stopTracing() method
ttidSpanMap.remove(activity);

// clear it up, so we don't start again for the same activity if the activity is in the activity
// stack still.
Expand Down Expand Up @@ -403,6 +474,12 @@ ISpan getAppStartSpan() {
return appStartSpan;
}

@TestOnly
@NotNull
WeakHashMap<Activity, ISpan> getTtidSpanMap() {
return ttidSpanMap;
}

private void setColdStart(final @Nullable Bundle savedInstanceState) {
if (!firstActivityCreated) {
// if Activity has savedInstanceState then its a warm start
Expand Down
@@ -0,0 +1,95 @@
package io.sentry.android.core.internal.util;

import android.annotation.SuppressLint;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.view.View;
import android.view.ViewTreeObserver;
import androidx.annotation.RequiresApi;
import io.sentry.android.core.BuildInfoProvider;
import java.util.concurrent.atomic.AtomicReference;
import org.jetbrains.annotations.NotNull;

/**
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
* OnDrawListener that unregisters itself and invokes callback when the next draw is done. This API
* 16+ implementation is an approximation of the initial-display-time defined by Android Vitals.
*/
@SuppressLint("ObsoleteSdkInt")
@RequiresApi(api = Build.VERSION_CODES.JELLY_BEAN)
public class FirstDrawDoneListener implements ViewTreeObserver.OnDrawListener {
private final @NotNull Handler mainThreadHandler = new Handler(Looper.getMainLooper());
private final @NotNull AtomicReference<View> viewReference;
private final @NotNull Runnable callback;

/** Registers a post-draw callback for the next draw of a view. */
public static void registerForNextDraw(
final @NotNull View view,
final @NotNull Runnable drawDoneCallback,
final @NotNull BuildInfoProvider buildInfoProvider) {
final FirstDrawDoneListener listener = new FirstDrawDoneListener(view, drawDoneCallback);
// Handle bug prior to API 26 where OnDrawListener from the floating ViewTreeObserver is not
// merged into the real ViewTreeObserver.
// https://android.googlesource.com/platform/frameworks/base/+/9f8ec54244a5e0343b9748db3329733f259604f3
if (buildInfoProvider.getSdkInfoVersion() < 26
&& !isAliveAndAttached(view, buildInfoProvider)) {
view.addOnAttachStateChangeListener(
new View.OnAttachStateChangeListener() {
@Override
public void onViewAttachedToWindow(View view) {
view.getViewTreeObserver().addOnDrawListener(listener);
view.removeOnAttachStateChangeListener(this);
}

@Override
public void onViewDetachedFromWindow(View view) {
view.removeOnAttachStateChangeListener(this);
}
});
} else {
view.getViewTreeObserver().addOnDrawListener(listener);
}
}

private FirstDrawDoneListener(final @NotNull View view, final @NotNull Runnable callback) {
this.viewReference = new AtomicReference<>(view);
this.callback = callback;
}

@Override
public void onDraw() {
// Set viewReference to null so any onDraw past the first is a no-op
final View view = viewReference.getAndSet(null);
if (view == null) {
return;
}
// OnDrawListeners cannot be removed within onDraw, so we remove it with a
// GlobalLayoutListener
view.getViewTreeObserver()
.addOnGlobalLayoutListener(() -> view.getViewTreeObserver().removeOnDrawListener(this));
mainThreadHandler.postAtFrontOfQueue(callback);
}

/**
* Helper to avoid <a
* href="https://android.googlesource.com/platform/frameworks/base/+/9f8ec54244a5e0343b9748db3329733f259604f3">bug
* prior to API 26</a>, where the floating ViewTreeObserver's OnDrawListeners are not merged into
* the real ViewTreeObserver during attach.
*
* @return true if the View is already attached and the ViewTreeObserver is not a floating
* placeholder.
*/
private static boolean isAliveAndAttached(
final @NotNull View view, final @NotNull BuildInfoProvider buildInfoProvider) {
return view.getViewTreeObserver().isAlive() && isAttachedToWindow(view, buildInfoProvider);
}

@SuppressLint("NewApi")
private static boolean isAttachedToWindow(
final @NotNull View view, final @NotNull BuildInfoProvider buildInfoProvider) {
if (buildInfoProvider.getSdkInfoVersion() >= 19) {
return view.isAttachedToWindow();
}
return view.getWindowToken() != null;
}
}
Expand Up @@ -498,6 +498,39 @@ class ActivityLifecycleIntegrationTest {
assertNull(sut.appStartSpan)
}

@Test
fun `When Activity is destroyed, sets ttidSpan status to cancelled and finish it`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
sut.register(fixture.hub, fixture.options)

setAppStartTime()

val activity = mock<Activity>()
sut.onActivityCreated(activity, fixture.bundle)
sut.onActivityDestroyed(activity)

val span = fixture.transaction.children.first { it.description?.endsWith(".ttid") == true }
assertEquals(span.status, SpanStatus.CANCELLED)
assertTrue(span.isFinished)
}

@Test
fun `When Activity is destroyed, sets ttidSpan to null`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
sut.register(fixture.hub, fixture.options)

setAppStartTime()

val activity = mock<Activity>()
sut.onActivityCreated(activity, fixture.bundle)
assertNotNull(sut.ttidSpanMap[activity])

sut.onActivityDestroyed(activity)
assertNull(sut.ttidSpanMap[activity])
}

@Test
fun `When new Activity and transaction is created, finish previous ones`() {
val sut = fixture.getSut()
Expand Down