Skip to content

Commit 307edcd

Browse files
authored
refactor: do not start redundant UI event transaction when one is already on Scope (#5658)
SentryGestureListener.startTracing always started a UI transaction and only later, in applyScope, declined to bind it when the Scope already held a manually-bound transaction. The unbound UI transaction then gathered no children and was dropped as an idle transaction. Now we read the Scope's bound transaction first and return early without starting a new one when it is present. Fixes #5491 Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
1 parent 3859a2c commit 307edcd

3 files changed

Lines changed: 29 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
### Fixes
1010

11+
- Don't start a redundant UI interaction transaction when a transaction is already bound to the Scope ([#5491](https://github.com/getsentry/sentry-java/issues/5491))
12+
- Previously, `SentryGestureListener` always started a UI transaction and only afterwards skipped binding it to the Scope when a manually-bound transaction already existed, leaving the new transaction to be dropped as an idle transaction without children.
1113
- Fix potential NPE within `Scope.endSession()` ([#5657](https://github.com/getsentry/sentry-java/pull/5657))
1214

1315
### Performance

sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,21 @@ private void startTracing(final @NotNull UiElement target, final @NotNull Gestur
244244
}
245245
}
246246

247+
// if there's already a transaction bound to the Scope (e.g. started manually by the user), we
248+
// skip starting a new UI transaction: it would never be bound to the Scope in applyScope, would
249+
// gather no children, and would be dropped as an idle transaction without children
250+
final @Nullable ITransaction[] boundTransaction = {null};
251+
scopes.configureScope(scope -> boundTransaction[0] = scope.getTransaction());
252+
if (boundTransaction[0] != null) {
253+
options
254+
.getLogger()
255+
.log(
256+
SentryLevel.DEBUG,
257+
"Transaction won't be created for view with id: %s since there's already a transaction bound to the Scope.",
258+
viewIdentifier);
259+
return;
260+
}
261+
247262
// we can only bind to the scope if there's no running transaction
248263
final String name = getActivityName(activity) + "." + viewIdentifier;
249264
final String op = UI_ACTION + "." + getGestureType(eventType);

sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerTracingTest.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,18 @@ class SentryGestureListenerTracingTest {
160160
sut.onSingleTapUp(fixture.event)
161161
}
162162

163+
@Test
164+
fun `when a transaction is already bound to the Scope, does not start a new UI transaction`() {
165+
val sut = fixture.getSut<View>()
166+
val boundTransaction = SentryTracer(TransactionContext("bound", "op"), fixture.scopes)
167+
whenever(fixture.scope.transaction).thenReturn(boundTransaction)
168+
169+
sut.onSingleTapUp(fixture.event)
170+
171+
verify(fixture.scopes, never()).startTransaction(any(), any<TransactionOptions>())
172+
assertEquals(false, boundTransaction.isFinished)
173+
}
174+
163175
@Test
164176
fun `stopTracing remove transaction from scope`() {
165177
val sut = fixture.getSut<View>()

0 commit comments

Comments
 (0)