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

Start a new automatic transaction on every click #2891

Merged
merged 2 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Breaking changes:
- Add deadline timeout for automatic transactions ([#2865](https://github.com/getsentry/sentry-java/pull/2865))
- This affects all automatically generated transactions on Android (UI, clicks), the default timeout is 30s
- Apollo v2 BeforeSpanCallback now allows returning null ([#2890](https://github.com/getsentry/sentry-java/pull/2890))
- Automatic user interaction tracking: every click now starts a new automatic transaction ([#2891](https://github.com/getsentry/sentry-java/pull/2891))
- Previously performing a click on the same UI widget twice would keep the existing transaction running, the new behavior now better aligns with other SDKs

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@
@ApiStatus.Internal
public final class SentryGestureListener implements GestureDetector.OnGestureListener {

private enum GestureType {
Click,
Scroll,
Swipe,
Unknown
}

static final String UI_ACTION = "ui.action";
private static final String TRACE_ORIGIN = "auto.ui.gesture_listener";

Expand All @@ -41,7 +48,7 @@ public final class SentryGestureListener implements GestureDetector.OnGestureLis

private @Nullable UiElement activeUiElement = null;
private @Nullable ITransaction activeTransaction = null;
private @Nullable String activeEventType = null;
private @NotNull GestureType activeEventType = GestureType.Unknown;

private final ScrollState scrollState = new ScrollState();

Expand All @@ -61,7 +68,7 @@ public void onUp(final @NotNull MotionEvent motionEvent) {
return;
}

if (scrollState.type == null) {
if (scrollState.type == GestureType.Unknown) {
options
.getLogger()
.log(SentryLevel.DEBUG, "Unable to define scroll type. No breadcrumb captured.");
Expand Down Expand Up @@ -107,8 +114,8 @@ public boolean onSingleTapUp(final @Nullable MotionEvent motionEvent) {
return false;
}

addBreadcrumb(target, "click", Collections.emptyMap(), motionEvent);
startTracing(target, "click");
addBreadcrumb(target, GestureType.Click, Collections.emptyMap(), motionEvent);
startTracing(target, GestureType.Click);
return false;
}

Expand All @@ -123,7 +130,7 @@ public boolean onScroll(
return false;
}

if (scrollState.type == null) {
if (scrollState.type == GestureType.Unknown) {
final @Nullable UiElement target =
ViewUtils.findTarget(
options, decorView, firstEvent.getX(), firstEvent.getY(), UiElement.Type.SCROLLABLE);
Expand All @@ -140,7 +147,7 @@ public boolean onScroll(
}

scrollState.setTarget(target);
scrollState.type = "scroll";
scrollState.type = GestureType.Scroll;
}
return false;
}
Expand All @@ -151,7 +158,7 @@ public boolean onFling(
final @Nullable MotionEvent motionEvent1,
final float v,
final float v1) {
scrollState.type = "swipe";
scrollState.type = GestureType.Swipe;
return false;
}

Expand All @@ -164,32 +171,37 @@ public void onLongPress(MotionEvent motionEvent) {}
// region utils
private void addBreadcrumb(
final @NotNull UiElement target,
final @NotNull String eventType,
final @NotNull GestureType eventType,
final @NotNull Map<String, Object> additionalData,
final @NotNull MotionEvent motionEvent) {

if (!options.isEnableUserInteractionBreadcrumbs()) {
return;
}

final String type = getGestureType(eventType);

final Hint hint = new Hint();
hint.set(ANDROID_MOTION_EVENT, motionEvent);
hint.set(ANDROID_VIEW, target.getView());

hub.addBreadcrumb(
Breadcrumb.userInteraction(
eventType,
target.getResourceName(),
target.getClassName(),
target.getTag(),
additionalData),
type, target.getResourceName(), target.getClassName(), target.getTag(), additionalData),
hint);
}

private void startTracing(final @NotNull UiElement target, final @NotNull String eventType) {
final UiElement uiElement = activeUiElement;
private void startTracing(final @NotNull UiElement target, final @NotNull GestureType eventType) {

final boolean isNewGestureSameAsActive =
(eventType == activeEventType && target.equals(activeUiElement));
final boolean isClickGesture = eventType == GestureType.Click;
// we always want to start new transaction/traces for clicks, for swipe/scroll only if the
// target changed
final boolean isNewInteraction = isClickGesture || !isNewGestureSameAsActive;

if (!(options.isTracingEnabled() && options.isEnableUserInteractionTracing())) {
if (!(target.equals(uiElement) && eventType.equals(activeEventType))) {
if (isNewInteraction) {
TracingUtils.startNewTrace(hub);
activeUiElement = target;
activeEventType = eventType;
Expand All @@ -206,9 +218,7 @@ private void startTracing(final @NotNull UiElement target, final @NotNull String
final @Nullable String viewIdentifier = target.getIdentifier();

if (activeTransaction != null) {
if (target.equals(uiElement)
&& eventType.equals(activeEventType)
&& !activeTransaction.isFinished()) {
if (!isNewInteraction && !activeTransaction.isFinished()) {
options
.getLogger()
.log(
Expand All @@ -233,7 +243,7 @@ private void startTracing(final @NotNull UiElement target, final @NotNull String

// we can only bind to the scope if there's no running transaction
final String name = getActivityName(activity) + "." + viewIdentifier;
final String op = UI_ACTION + "." + eventType;
final String op = UI_ACTION + "." + getGestureType(eventType);

final TransactionOptions transactionOptions = new TransactionOptions();
transactionOptions.setWaitForChildren(true);
Expand Down Expand Up @@ -270,13 +280,15 @@ void stopTracing(final @NotNull SpanStatus status) {
}
hub.configureScope(
scope -> {
// avoid method refs on Android due to some issues with older AGP setups
// noinspection Convert2MethodRef
clearScope(scope);
});
activeTransaction = null;
if (activeUiElement != null) {
activeUiElement = null;
}
activeEventType = null;
activeEventType = GestureType.Unknown;
}

@VisibleForTesting
Expand Down Expand Up @@ -337,11 +349,32 @@ void applyScope(final @NotNull Scope scope, final @NotNull ITransaction transact
}
return decorView;
}

@NotNull
private static String getGestureType(final @NotNull GestureType eventType) {
final @NotNull String type;
switch (eventType) {
case Click:
type = "click";
break;
case Scroll:
type = "scroll";
break;
case Swipe:
type = "swipe";
break;
default:
case Unknown:
type = "unknown";
break;
}
return type;
}
// endregion

// region scroll logic
private static final class ScrollState {
private @Nullable String type = null;
private @NotNull GestureType type = GestureType.Unknown;
private @Nullable UiElement target;
private float startX = 0f;
private float startY = 0f;
Expand Down Expand Up @@ -378,7 +411,7 @@ private void setTarget(final @NotNull UiElement target) {

private void reset() {
target = null;
type = null;
type = GestureType.Unknown;
startX = 0f;
startY = 0f;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import org.mockito.kotlin.clearInvocations
import org.mockito.kotlin.doAnswer
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import kotlin.test.Test
Expand Down Expand Up @@ -333,20 +334,18 @@ class SentryGestureListenerTracingTest {
SpanContext(SentryId.EMPTY_ID, SpanId.EMPTY_ID, "op", null, null)
)

// when the same button is clicked twice
sut.onSingleTapUp(fixture.event)
sut.onSingleTapUp(fixture.event)

verify(fixture.hub).startTransaction(
// then two transaction should be captured
verify(fixture.hub, times(2)).startTransaction(
check {
assertEquals("Activity.test_button", it.name)
assertEquals(TransactionNameSource.COMPONENT, it.transactionNameSource)
},
any<TransactionOptions>()
)

// second view interaction
sut.onSingleTapUp(fixture.event)

verify(fixture.transaction).scheduleFinish()
}

@Test
Expand Down