Skip to content

Commit

Permalink
refrain from starting a new trace in some cases
Browse files Browse the repository at this point in the history
  • Loading branch information
@adinauer
adinauer committed Jun 29, 2023
1 parent 2eb2140 commit 23dcd1a
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 105 deletions.
207 changes: 105 additions & 102 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 @@ -20,6 +20,7 @@
import io.sentry.ITransaction;
import io.sentry.Instrumenter;
import io.sentry.Integration;
import io.sentry.NoOpTransaction;
import io.sentry.Scope;
import io.sentry.SentryDate;
import io.sentry.SentryLevel;
Expand Down Expand Up @@ -175,106 +176,110 @@ private void stopPreviousTransactions() {

private void startTracing(final @NotNull Activity activity) {
WeakReference<Activity> weakActivity = new WeakReference<>(activity);
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();

final String activityName = getActivityName(activity);

final SentryDate appStartTime =
foregroundImportance ? AppStartState.getInstance().getAppStartTime() : null;
final Boolean coldStart = AppStartState.getInstance().isColdStart();

final TransactionOptions transactionOptions = new TransactionOptions();
if (options.isEnableActivityLifecycleTracingAutoFinish()) {
transactionOptions.setIdleTimeout(options.getIdleTimeout());
transactionOptions.setTrimEnd(true);
}
transactionOptions.setWaitForChildren(true);
transactionOptions.setTransactionFinishedCallback(
(finishingTransaction) -> {
@Nullable Activity unwrappedActivity = weakActivity.get();
if (unwrappedActivity != null) {
activityFramesTracker.setMetrics(
unwrappedActivity, finishingTransaction.getEventId());
} else {
if (options != null) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Unable to track activity frames as the Activity %s has been destroyed.",
activityName);
if (hub != null && !isRunningTransactionOrTrace(activity)) {
if (!performanceEnabled) {
activitiesWithOngoingTransactions.put(activity, NoOpTransaction.getInstance());
TracingUtils.startNewTrace(hub);
} else if (performanceEnabled) {
// as we allow a single transaction running on the bound Scope, we finish the previous ones
stopPreviousTransactions();

final String activityName = getActivityName(activity);

final SentryDate appStartTime =
foregroundImportance ? AppStartState.getInstance().getAppStartTime() : null;
final Boolean coldStart = AppStartState.getInstance().isColdStart();

final TransactionOptions transactionOptions = new TransactionOptions();
if (options.isEnableActivityLifecycleTracingAutoFinish()) {
transactionOptions.setIdleTimeout(options.getIdleTimeout());
transactionOptions.setTrimEnd(true);
}
transactionOptions.setWaitForChildren(true);
transactionOptions.setTransactionFinishedCallback(
(finishingTransaction) -> {
@Nullable Activity unwrappedActivity = weakActivity.get();
if (unwrappedActivity != null) {
activityFramesTracker.setMetrics(
unwrappedActivity, finishingTransaction.getEventId());
} else {
if (options != null) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Unable to track activity frames as the Activity %s has been destroyed.",
activityName);
}
}
}
});

// This will be the start timestamp of the transaction, as well as the ttid/ttfd spans
final @NotNull SentryDate ttidStartTime;
});

if (!(firstActivityCreated || appStartTime == null || coldStart == null)) {
// The first activity ttid/ttfd spans should start at the app start time
ttidStartTime = appStartTime;
} else {
// The ttid/ttfd spans should start when the previous activity called its onPause method
ttidStartTime = lastPausedTime;
}
transactionOptions.setStartTimestamp(ttidStartTime);

// we can only bind to the scope if there's no running transaction
ITransaction transaction =
hub.startTransaction(
new TransactionContext(activityName, TransactionNameSource.COMPONENT, UI_LOAD_OP),
transactionOptions);

// in case appStartTime isn't available, we don't create a span for it.
if (!(firstActivityCreated || appStartTime == null || coldStart == null)) {
// start specific span for app start
appStartSpan =
transaction.startChild(
getAppStartOp(coldStart),
getAppStartDesc(coldStart),
appStartTime,
Instrumenter.SENTRY);

// in case there's already an end time (e.g. due to deferred SDK init)
// we can finish the app-start span
finishAppStartSpan();
}
final @NotNull ISpan ttidSpan =
transaction.startChild(
TTID_OP, getTtidDesc(activityName), ttidStartTime, Instrumenter.SENTRY);
ttidSpanMap.put(activity, ttidSpan);
// This will be the start timestamp of the transaction, as well as the ttid/ttfd spans
final @NotNull SentryDate ttidStartTime;

if (timeToFullDisplaySpanEnabled && fullyDisplayedReporter != null && options != null) {
final @NotNull ISpan ttfdSpan =
if (!(firstActivityCreated || appStartTime == null || coldStart == null)) {
// The first activity ttid/ttfd spans should start at the app start time
ttidStartTime = appStartTime;
} else {
// The ttid/ttfd spans should start when the previous activity called its onPause method
ttidStartTime = lastPausedTime;
}
transactionOptions.setStartTimestamp(ttidStartTime);

// we can only bind to the scope if there's no running transaction
ITransaction transaction =
hub.startTransaction(
new TransactionContext(activityName, TransactionNameSource.COMPONENT, UI_LOAD_OP),
transactionOptions);

// in case appStartTime isn't available, we don't create a span for it.
if (!(firstActivityCreated || appStartTime == null || coldStart == null)) {
// start specific span for app start
appStartSpan =
transaction.startChild(
getAppStartOp(coldStart),
getAppStartDesc(coldStart),
appStartTime,
Instrumenter.SENTRY);

// in case there's already an end time (e.g. due to deferred SDK init)
// we can finish the app-start span
finishAppStartSpan();
}
final @NotNull ISpan ttidSpan =
transaction.startChild(
TTFD_OP, getTtfdDesc(activityName), ttidStartTime, Instrumenter.SENTRY);
try {
ttfdSpanMap.put(activity, ttfdSpan);
ttfdAutoCloseFuture =
options
.getExecutorService()
.schedule(() -> finishExceededTtfdSpan(ttfdSpan, ttidSpan), TTFD_TIMEOUT_MILLIS);
} catch (RejectedExecutionException e) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?",
e);
TTID_OP, getTtidDesc(activityName), ttidStartTime, Instrumenter.SENTRY);
ttidSpanMap.put(activity, ttidSpan);

if (timeToFullDisplaySpanEnabled && fullyDisplayedReporter != null && options != null) {
final @NotNull ISpan ttfdSpan =
transaction.startChild(
TTFD_OP, getTtfdDesc(activityName), ttidStartTime, Instrumenter.SENTRY);
try {
ttfdSpanMap.put(activity, ttfdSpan);
ttfdAutoCloseFuture =
options
.getExecutorService()
.schedule(
() -> finishExceededTtfdSpan(ttfdSpan, ttidSpan), TTFD_TIMEOUT_MILLIS);
} catch (RejectedExecutionException e) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?",
e);
}
}
}

// lets bind to the scope so other integrations can pick it up
hub.configureScope(
scope -> {
applyScope(scope, transaction);
});
// lets bind to the scope so other integrations can pick it up
hub.configureScope(
scope -> {
applyScope(scope, transaction);
});

activitiesWithOngoingTransactions.put(activity, transaction);
activitiesWithOngoingTransactions.put(activity, transaction);
}
}
}

Expand Down Expand Up @@ -307,7 +312,7 @@ void clearScope(final @NotNull Scope scope, final @NotNull ITransaction transact
});
}

private boolean isRunningTransaction(final @NotNull Activity activity) {
private boolean isRunningTransactionOrTrace(final @NotNull Activity activity) {
return activitiesWithOngoingTransactions.containsKey(activity);
}

Expand Down Expand Up @@ -480,15 +485,13 @@ public synchronized void onActivityDestroyed(final @NotNull Activity activity) {
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);
}
}

// 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.
activitiesWithOngoingTransactions.remove(activity);
}

private void finishSpan(final @Nullable ISpan span) {
Expand Down
8 changes: 6 additions & 2 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 @@ -186,8 +186,13 @@ private void addBreadcrumb(
}

private void startTracing(final @NotNull UiElement target, final @NotNull String eventType) {
final UiElement uiElement = activeUiElement;
if (!(options.isTracingEnabled() && options.isEnableUserInteractionTracing())) {
TracingUtils.startNewTrace(hub);
if (!(target.equals(uiElement) && eventType.equals(activeEventType))) {
TracingUtils.startNewTrace(hub);
activeUiElement = target;
activeEventType = eventType;
}
return;
}

Expand All @@ -198,7 +203,6 @@ private void startTracing(final @NotNull UiElement target, final @NotNull String
}

final @Nullable String viewIdentifier = target.getIdentifier();
final UiElement uiElement = activeUiElement;

if (activeTransaction != null) {
if (target.equals(uiElement)
Expand Down
35 changes: 35 additions & 0 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 @@ -38,6 +38,7 @@ import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.check
import org.mockito.kotlin.clearInvocations
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
Expand Down Expand Up @@ -1407,6 +1408,40 @@ class ActivityLifecycleIntegrationTest {
assertNotSame(propagationContextAtStart, scope.propagationContext)
}

@Test
fun `does not start another new trace if one has already been started but does after activity was destroyed`() {
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())
val propagationContextAfterNewTrace = scope.propagationContext
assertNotSame(propagationContextAtStart, propagationContextAfterNewTrace)

clearInvocations(fixture.hub)
sut.onActivityCreated(activity, fixture.bundle)

verify(fixture.hub, never()).configureScope(any())
assertSame(propagationContextAfterNewTrace, scope.propagationContext)

sut.onActivityDestroyed(activity)

clearInvocations(fixture.hub)
sut.onActivityCreated(activity, fixture.bundle)
verify(fixture.hub).configureScope(any())
assertNotSame(propagationContextAfterNewTrace, scope.propagationContext)
}

private fun runFirstDraw(view: View) {
// Removes OnDrawListener in the next OnGlobalLayout after onDraw
view.viewTreeObserver.dispatchOnDraw()
Expand Down
33 changes: 32 additions & 1 deletion ...src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.sentry.android.core.SentryAndroidOptions
import org.mockito.kotlin.any
import org.mockito.kotlin.anyOrNull
import org.mockito.kotlin.check
import org.mockito.kotlin.clearInvocations
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.inOrder
import org.mockito.kotlin.mock
Expand Down Expand Up @@ -166,7 +167,6 @@ class SentryGestureListenerScrollTest {
},
anyOrNull()
)
verify(fixture.hub).configureScope(anyOrNull())
}
verifyNoMoreInteractions(fixture.hub)
}
Expand Down Expand Up @@ -206,6 +206,37 @@ class SentryGestureListenerScrollTest {
verify(fixture.scope).propagationContext = any()
}

@Test
fun `does not start a new trace on repeated scroll but does for a different event`() {
val sut = fixture.getSut<ScrollableListView>(direction = "left")

sut.onDown(fixture.firstEvent)
fixture.eventsInBetween.forEach {
sut.onScroll(fixture.firstEvent, it, 10.0f, 0f)
}
sut.onUp(fixture.endEvent)

verify(fixture.scope).propagationContext = any()

clearInvocations(fixture.scope)

sut.onDown(fixture.firstEvent)
fixture.eventsInBetween.forEach {
sut.onScroll(fixture.firstEvent, it, 10.0f, 0f)
}
sut.onUp(fixture.endEvent)
verify(fixture.scope, never()).propagationContext = any()

clearInvocations(fixture.scope)

sut.onDown(fixture.firstEvent)
fixture.eventsInBetween.forEach { sut.onScroll(fixture.firstEvent, it, 0f, 30.0f) }
sut.onFling(fixture.firstEvent, fixture.endEvent, 1.0f, 1.0f)
sut.onUp(fixture.endEvent)

verify(fixture.scope).propagationContext = any()
}

internal class ScrollableView : View(mock()), ScrollingView {
override fun computeVerticalScrollOffset(): Int = 0
override fun computeVerticalScrollExtent(): Int = 0
Expand Down

0 comments on commit 23dcd1a

Please sign in to comment.