Merge branch 'feat/7.0.0' into feat/get-span-root-span

Author: markushi

CHANGELOG.md

@@ -12,7 +12,10 @@ Breaking changes:
 - Reduce timeout of AsyncHttpTransport to avoid ANR ([#2879](https://github.com/getsentry/sentry-java/pull/2879))
 - 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
-- If global hub mode is enabled (default on Android), Sentry.getSpan() returns the root span instead of the latest span ([#2855](https://github.com/getsentry/sentry-java/pull/2855))
+- 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
+- Android: If global hub mode is enabled (default on Android), Sentry.getSpan() returns the root span instead of the latest span ([#2855](https://github.com/getsentry/sentry-java/pull/2855))
 
 ### Fixes
 

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

@@ -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";
 
@@ -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();
 
@@ -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.");
@@ -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;
   }
 
@@ -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);
@@ -140,7 +147,7 @@ public boolean onScroll(
       }
 
       scrollState.setTarget(target);
-      scrollState.type = "scroll";
+      scrollState.type = GestureType.Scroll;
     }
     return false;
   }
@@ -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;
   }
 
@@ -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;
@@ -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(
@@ -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);
@@ -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
@@ -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;
@@ -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;
     }

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

@@ -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
@@ -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

sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt

@@ -142,15 +142,20 @@ class SentryApolloInterceptor(
     }
 
     private fun finish(span: ISpan, request: InterceptorRequest, response: InterceptorResponse? = null) {
-        var newSpan: ISpan = span
+        var newSpan: ISpan? = span
         if (beforeSpan != null) {
             try {
                 newSpan = beforeSpan.execute(span, request, response)
             } catch (e: Exception) {
                 hub.options.logger.log(SentryLevel.ERROR, "An error occurred while executing beforeSpan on ApolloInterceptor", e)
             }
         }
-        newSpan.finish()
+        if (newSpan == null) {
+            // span is dropped
+            span.spanContext.sampled = false
+        } else {
+            span.finish()
+        }
 
         response?.let {
             if (it.httpResponse.isPresent) {
@@ -166,9 +171,9 @@ class SentryApolloInterceptor(
                     breadcrumb.setData("response_body_size", contentLength)
                 }
 
-                val hint = Hint().also {
-                    it.set(APOLLO_REQUEST, httpRequest)
-                    it.set(APOLLO_RESPONSE, httpResponse)
+                val hint = Hint().apply {
+                    set(APOLLO_REQUEST, httpRequest)
+                    set(APOLLO_RESPONSE, httpResponse)
                 }
                 hub.addBreadcrumb(breadcrumb, hint)
             }
@@ -192,6 +197,6 @@ class SentryApolloInterceptor(
          * @param request the HTTP request executed by okHttp
          * @param response the HTTP response received by okHttp
          */
-        fun execute(span: ISpan, request: InterceptorRequest, response: InterceptorResponse?): ISpan
+        fun execute(span: ISpan, request: InterceptorRequest, response: InterceptorResponse?): ISpan?
     }
 }

sentry-apollo/src/test/java/io/sentry/apollo/SentryApolloInterceptorTest.kt

@@ -35,6 +35,7 @@ import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
+import kotlin.test.assertTrue
 
 class SentryApolloInterceptorTest {
 
@@ -177,6 +178,24 @@ class SentryApolloInterceptorTest {
         )
     }
 
+    @Test
+    fun `when beforeSpan callback returns null, span is dropped`() {
+        executeQuery(
+            fixture.getSut { _, _, _ ->
+                null
+            }
+        )
+
+        verify(fixture.hub).captureTransaction(
+            check {
+                assertTrue(it.spans.isEmpty())
+            },
+            anyOrNull<TraceContext>(),
+            anyOrNull(),
+            anyOrNull()
+        )
+    }
+
     @Test
     fun `when customizer throws, exception is handled`() {
         executeQuery(