Skip to content

Commit

Permalink
Merge 3cb799c into 9246ed4
Browse files Browse the repository at this point in the history
  • Loading branch information
@adinauer
adinauer authored Jun 20, 2023
2 parents 9246ed4 + 3cb799c commit 1d85b7f
Show file tree
Hide file tree
Showing 50 changed files with 1,332 additions and 534 deletions.
130 changes: 69 additions & 61 deletions sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import io.sentry.protocol.MeasurementValue;
import io.sentry.protocol.TransactionNameSource;
import io.sentry.util.Objects;
import io.sentry.util.TracingUtils;
import java.io.Closeable;
import java.io.IOException;
import java.lang.ref.WeakReference;
Expand Down Expand Up @@ -122,11 +123,9 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio
fullyDisplayedReporter = this.options.getFullyDisplayedReporter();
timeToFullDisplaySpanEnabled = this.options.isEnableTimeToFullDisplayTracing();

if (this.options.isEnableActivityLifecycleBreadcrumbs() || performanceEnabled) {
application.registerActivityLifecycleCallbacks(this);
this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed.");
addIntegrationToSdkVersion();
}
application.registerActivityLifecycleCallbacks(this);
this.options.getLogger().log(SentryLevel.DEBUG, "ActivityLifecycleIntegration installed.");
addIntegrationToSdkVersion();
}

private boolean isPerformanceEnabled(final @NotNull SentryAndroidOptions options) {
Expand Down Expand Up @@ -176,7 +175,9 @@ private void stopPreviousTransactions() {

private void startTracing(final @NotNull Activity activity) {
WeakReference<Activity> weakActivity = new WeakReference<>(activity);
if (performanceEnabled && !isRunningTransaction(activity) && hub != null) {
if (!performanceEnabled && hub != null) {
TracingUtils.startNewTrace(hub);
} else if (performanceEnabled && !isRunningTransaction(activity) && hub != null) {
// as we allow a single transaction running on the bound Scope, we finish the previous ones
stopPreviousTransactions();

Expand Down Expand Up @@ -367,44 +368,48 @@ public synchronized void onActivityCreated(

@Override
public synchronized void onActivityStarted(final @NotNull Activity activity) {
// The docs on the screen rendering performance tracing
// (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition),
// state that the tracing starts for every Activity class when the app calls .onActivityStarted.
// Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not
// working. Moving this to onActivityStarted fixes the problem.
activityFramesTracker.addActivity(activity);
if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) {
// The docs on the screen rendering performance tracing
// (https://firebase.google.com/docs/perf-mon/screen-traces?platform=android#definition),
// state that the tracing starts for every Activity class when the app calls
// .onActivityStarted.
// Adding an Activity in onActivityCreated leads to Window.FEATURE_NO_TITLE not
// working. Moving this to onActivityStarted fixes the problem.
activityFramesTracker.addActivity(activity);
}

addBreadcrumb(activity, "started");
}

@SuppressLint("NewApi")
@Override
public synchronized void onActivityResumed(final @NotNull Activity activity) {

// app start span
@Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime();
@Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime();
// in case the SentryPerformanceProvider is disabled it does not set the app start times,
// and we need to set the end time manually here,
// the start time gets set manually in SentryAndroid.init()
if (appStartStartTime != null && appStartEndTime == null) {
AppStartState.getInstance().setAppStartEnd();
}
finishAppStartSpan();

final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity);
final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity);
final View rootView = activity.findViewById(android.R.id.content);
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN
&& rootView != null) {
FirstDrawDoneListener.registerForNextDraw(
rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), 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(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan));
if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) {
// app start span
@Nullable final SentryDate appStartStartTime = AppStartState.getInstance().getAppStartTime();
@Nullable final SentryDate appStartEndTime = AppStartState.getInstance().getAppStartEndTime();
// in case the SentryPerformanceProvider is disabled it does not set the app start times,
// and we need to set the end time manually here,
// the start time gets set manually in SentryAndroid.init()
if (appStartStartTime != null && appStartEndTime == null) {
AppStartState.getInstance().setAppStartEnd();
}
finishAppStartSpan();

final @Nullable ISpan ttidSpan = ttidSpanMap.get(activity);
final @Nullable ISpan ttfdSpan = ttfdSpanMap.get(activity);
final View rootView = activity.findViewById(android.R.id.content);
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.JELLY_BEAN
&& rootView != null) {
FirstDrawDoneListener.registerForNextDraw(
rootView, () -> onFirstFrameDrawn(ttfdSpan, ttidSpan), 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(() -> onFirstFrameDrawn(ttfdSpan, ttidSpan));
}
addBreadcrumb(activity, "resumed");
}
addBreadcrumb(activity, "resumed");
}

@Override
Expand Down Expand Up @@ -450,35 +455,38 @@ public synchronized void onActivitySaveInstanceState(

@Override
public synchronized void onActivityDestroyed(final @NotNull Activity activity) {
addBreadcrumb(activity, "destroyed");

// in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid
// memory leak
finishSpan(appStartSpan, SpanStatus.CANCELLED);
if (performanceEnabled || options.isEnableActivityLifecycleBreadcrumbs()) {
addBreadcrumb(activity, "destroyed");

// we finish the ttidSpan as cancelled in case it isn't completed yet
final ISpan ttidSpan = ttidSpanMap.get(activity);
final ISpan ttfdSpan = ttfdSpanMap.get(activity);
finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED);

// we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet
finishExceededTtfdSpan(ttfdSpan, ttidSpan);
cancelTtfdAutoClose();
// in case the appStartSpan isn't completed yet, we finish it as cancelled to avoid
// memory leak
finishSpan(appStartSpan, SpanStatus.CANCELLED);

// in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it,
// we make sure to finish it when the activity gets destroyed.
stopTracing(activity, true);
// we finish the ttidSpan as cancelled in case it isn't completed yet
final ISpan ttidSpan = ttidSpanMap.get(activity);
final ISpan ttfdSpan = ttfdSpanMap.get(activity);
finishSpan(ttidSpan, SpanStatus.DEADLINE_EXCEEDED);

// set it to null in case its been just finished as cancelled
appStartSpan = null;
ttidSpanMap.remove(activity);
ttfdSpanMap.remove(activity);
// we finish the ttfdSpan as deadline_exceeded in case it isn't completed yet
finishExceededTtfdSpan(ttfdSpan, ttidSpan);
cancelTtfdAutoClose();

// clear it up, so we don't start again for the same activity if the activity is in the activity
// stack still.
// if the activity is opened again and not in memory, transactions will be created normally.
if (performanceEnabled) {
activitiesWithOngoingTransactions.remove(activity);
// in case people opt-out enableActivityLifecycleTracingAutoFinish and forgot to finish it,
// we make sure to finish it when the activity gets destroyed.
stopTracing(activity, true);

// set it to null in case its been just finished as cancelled
appStartSpan = null;
ttidSpanMap.remove(activity);
ttfdSpanMap.remove(activity);

// clear it up, so we don't start again for the same activity if the activity is in the
// activity
// stack still.
// if the activity is opened again and not in memory, transactions will be created normally.
if (performanceEnabled) {
activitiesWithOngoingTransactions.remove(activity);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions ...id-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ private void addBreadcrumb(
hint);
}

// TODO should we start a new trace here if performance is disabled?
private void startTracing(final @NotNull UiElement target, final @NotNull String eventType) {
if (!(options.isTracingEnabled() && options.isEnableUserInteractionTracing())) {
return;
Expand Down
35 changes: 29 additions & 6 deletions sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import io.sentry.FullyDisplayedReporter
import io.sentry.Hub
import io.sentry.ISentryExecutorService
import io.sentry.Scope
import io.sentry.ScopeCallback
import io.sentry.Sentry
import io.sentry.SentryDate
import io.sentry.SentryLevel
Expand All @@ -32,6 +33,7 @@ import io.sentry.protocol.MeasurementValue
import io.sentry.protocol.TransactionNameSource
import io.sentry.test.getProperty
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.argumentCaptor
Expand All @@ -54,6 +56,7 @@ import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNotSame
import kotlin.test.assertNull
import kotlin.test.assertSame
import kotlin.test.assertTrue
Expand Down Expand Up @@ -141,13 +144,13 @@ class ActivityLifecycleIntegrationTest {
}

@Test
fun `When activity lifecycle breadcrumb and tracing are disabled, it doesn't register callback`() {
fun `When activity lifecycle breadcrumb and tracing are disabled, it still registers callback`() {
val sut = fixture.getSut()
fixture.options.isEnableActivityLifecycleBreadcrumbs = false

sut.register(fixture.hub, fixture.options)

verify(fixture.application, never()).registerActivityLifecycleCallbacks(any())
verify(fixture.application).registerActivityLifecycleCallbacks(any())
}

@Test
Expand All @@ -162,15 +165,15 @@ class ActivityLifecycleIntegrationTest {
}

@Test
fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it does not register callback`() {
fun `When activity lifecycle breadcrumb is disabled and tracesSampleRate is set but tracing is disabled, it still registers callback`() {
val sut = fixture.getSut()
fixture.options.isEnableActivityLifecycleBreadcrumbs = false
fixture.options.tracesSampleRate = 1.0
fixture.options.enableTracing = false

sut.register(fixture.hub, fixture.options)

verify(fixture.application, never()).registerActivityLifecycleCallbacks(any())
verify(fixture.application).registerActivityLifecycleCallbacks(any())
}

@Test
Expand All @@ -196,7 +199,7 @@ class ActivityLifecycleIntegrationTest {
}

@Test
fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, it doesn't register callback`() {
fun `When activity lifecycle breadcrumb and tracing activity flag are disabled, its still registers callback`() {
val sut = fixture.getSut()
fixture.options.isEnableActivityLifecycleBreadcrumbs = false
fixture.options.tracesSampleRate = 1.0
Expand All @@ -205,7 +208,7 @@ class ActivityLifecycleIntegrationTest {

sut.register(fixture.hub, fixture.options)

verify(fixture.application, never()).registerActivityLifecycleCallbacks(any())
verify(fixture.application).registerActivityLifecycleCallbacks(any())
}

@Test
Expand Down Expand Up @@ -1384,6 +1387,26 @@ class ActivityLifecycleIntegrationTest {
assertFalse(ttfdSpan2.isFinished)
}

@Test
fun `starts new trace if performance is disabled`() {
val sut = fixture.getSut()
val activity = mock<Activity>()
fixture.options.enableTracing = false

val argumentCaptor: ArgumentCaptor<ScopeCallback> = ArgumentCaptor.forClass(ScopeCallback::class.java)
val scope = Scope(fixture.options)
val propagationContextAtStart = scope.propagationContext
whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer {
argumentCaptor.value.run(scope)
}

sut.register(fixture.hub, fixture.options)
sut.onActivityCreated(activity, fixture.bundle)

verify(fixture.hub).configureScope(any())
assertNotSame(propagationContextAtStart, scope.propagationContext)
}

private fun runFirstDraw(view: View) {
// Removes OnDrawListener in the next OnGlobalLayout after onDraw
view.viewTreeObserver.dispatchOnDraw()
Expand Down
2 changes: 2 additions & 0 deletions ...android-navigation/src/main/java/io/sentry/android/navigation/SentryNavigationListener.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import io.sentry.TransactionContext
import io.sentry.TransactionOptions
import io.sentry.TypeCheckHint
import io.sentry.protocol.TransactionNameSource
import io.sentry.util.TracingUtils
import java.lang.ref.WeakReference

/**
Expand Down Expand Up @@ -96,6 +97,7 @@ class SentryNavigationListener @JvmOverloads constructor(
arguments: Map<String, Any?>
) {
if (!isPerformanceEnabled) {
TracingUtils.startNewTrace(hub)
return
}

Expand Down
35 changes: 27 additions & 8 deletions ...oid-navigation/src/test/java/io/sentry/android/navigation/SentryNavigationListenerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.sentry.TransactionContext
import io.sentry.TransactionOptions
import io.sentry.protocol.TransactionNameSource
import org.junit.runner.RunWith
import org.mockito.ArgumentCaptor
import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.check
Expand All @@ -29,6 +30,7 @@ import org.mockito.kotlin.whenever
import org.robolectric.annotation.Config
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotSame
import kotlin.test.assertNull

@RunWith(AndroidJUnit4::class)
Expand All @@ -43,6 +45,7 @@ class SentryNavigationListenerTest {
val context = mock<Context>()
val resources = mock<Resources>()
val scope = mock<Scope>()
lateinit var options: SentryOptions

lateinit var transaction: SentryTracer

Expand All @@ -56,14 +59,13 @@ class SentryNavigationListenerTest {
hasViewIdInRes: Boolean = true,
transaction: SentryTracer? = null
): SentryNavigationListener {
whenever(hub.options).thenReturn(
SentryOptions().apply {
dsn = "http://key@localhost/proj"
setTracesSampleRate(
tracesSampleRate
)
}
)
options = SentryOptions().apply {
dsn = "http://key@localhost/proj"
setTracesSampleRate(
tracesSampleRate
)
}
whenever(hub.options).thenReturn(options)

this.transaction = transaction ?: SentryTracer(
TransactionContext(
Expand Down Expand Up @@ -347,4 +349,21 @@ class SentryNavigationListenerTest {
captor.thirdValue.accept(fixture.transaction)
verify(fixture.scope).clearTransaction()
}

@Test
fun `starts new trace if performance is disabled`() {
val sut = fixture.getSut(enableTracing = false)

val argumentCaptor: ArgumentCaptor<ScopeCallback> = ArgumentCaptor.forClass(ScopeCallback::class.java)
val scope = Scope(fixture.options)
val propagationContextAtStart = scope.propagationContext
whenever(fixture.hub.configureScope(argumentCaptor.capture())).thenAnswer {
argumentCaptor.value.run(scope)
}

sut.onDestinationChanged(fixture.navController, fixture.destination, null)

verify(fixture.hub).configureScope(any())
assertNotSame(propagationContextAtStart, scope.propagationContext)
}
}
Loading

0 comments on commit 1d85b7f

Please sign in to comment.