Skip to content

Commit

Permalink
Merge 1db34ad into 703d523
Browse files Browse the repository at this point in the history
  • Loading branch information
stefanosiano committed Dec 9, 2022
2 parents 703d523 + 1db34ad commit 89f6386
Show file tree
Hide file tree
Showing 9 changed files with 366 additions and 18 deletions.
12 changes: 8 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Add ttid span to ActivityLifecycleIntegration ([#2369](https://github.com/getsentry/sentry-java/pull/2369))

### Dependencies

- Bump Native SDK from v0.5.2 to v0.5.3 ([#2423](https://github.com/getsentry/sentry-java/pull/2423))
Expand Down Expand Up @@ -55,15 +59,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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()) {
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
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;

/**
* 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.
*
* <p>Adapted from <a
* href="https://github.com/firebase/firebase-android-sdk/blob/master/firebase-perf/src/main/java/com/google/firebase/perf/util/FirstDrawDoneListener.java">Firebase</a>
* under the Apache License, Version 2.0.
*/
@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;
}
}
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit 89f6386

Please sign in to comment.