Various fixes to instrumentations running on the main thread

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+### Fixes
+
+- Do not instrument File I/O operations if tracing is disabled ([#4039](https://github.com/getsentry/sentry-java/pull/4039))
+- Do not instrument User Interaction multiple times ([#4039](https://github.com/getsentry/sentry-java/pull/4039))
+- Speed up view traversal to find touched target in `UserInteractionIntegration` ([#4039](https://github.com/getsentry/sentry-java/pull/4039))
+
 ### Internal
 
 - Make `SentryClient` constructor public ([#4045](https://github.com/getsentry/sentry-java/pull/4045))

sentry-android-core/src/main/java/io/sentry/android/core/UserInteractionIntegration.java

@@ -50,6 +50,11 @@ private void startTracking(final @NotNull Activity activity) {
         delegate = new NoOpWindowCallback();
       }
 
+      if (delegate instanceof SentryWindowCallback) {
+        // already instrumented
+        return;
+      }
+
       final SentryGestureListener gestureListener =
           new SentryGestureListener(activity, scopes, options);
       window.setCallback(new SentryWindowCallback(delegate, activity, gestureListener, options));

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

@@ -18,26 +18,23 @@ public final class AndroidViewGestureTargetLocator implements GestureTargetLocat
   private static final String ORIGIN = "old_view_system";
 
   private final boolean isAndroidXAvailable;
-  private final int[] coordinates = new int[2];
 
   public AndroidViewGestureTargetLocator(final boolean isAndroidXAvailable) {
     this.isAndroidXAvailable = isAndroidXAvailable;
   }
 
   @Override
   public @Nullable UiElement locate(
-      @NotNull Object root, float x, float y, UiElement.Type targetType) {
+      @Nullable Object root, float x, float y, UiElement.Type targetType) {
     if (!(root instanceof View)) {
       return null;
     }
     final View view = (View) root;
-    if (touchWithinBounds(view, x, y)) {
-      if (targetType == UiElement.Type.CLICKABLE && isViewTappable(view)) {
-        return createUiElement(view);
-      } else if (targetType == UiElement.Type.SCROLLABLE
-          && isViewScrollable(view, isAndroidXAvailable)) {
-        return createUiElement(view);
-      }
+    if (targetType == UiElement.Type.CLICKABLE && isViewTappable(view)) {
+      return createUiElement(view);
+    } else if (targetType == UiElement.Type.SCROLLABLE
+        && isViewScrollable(view, isAndroidXAvailable)) {
+      return createUiElement(view);
     }
     return null;
   }
@@ -52,17 +49,6 @@ private UiElement createUiElement(final @NotNull View targetView) {
     }
   }
 
-  private boolean touchWithinBounds(final @NotNull View view, final float x, final float y) {
-    view.getLocationOnScreen(coordinates);
-    int vx = coordinates[0];
-    int vy = coordinates[1];
-
-    int w = view.getWidth();
-    int h = view.getHeight();
-
-    return !(x < vx || x > vx + w || y < vy || y > vy + h);
-  }
-
   private static boolean isViewTappable(final @NotNull View view) {
     return view.isClickable() && view.getVisibility() == View.VISIBLE;
   }

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

@@ -7,7 +7,6 @@
 import io.sentry.android.core.SentryAndroidOptions;
 import io.sentry.internal.gestures.GestureTargetLocator;
 import io.sentry.internal.gestures.UiElement;
-import io.sentry.util.Objects;
 import java.util.LinkedList;
 import java.util.Queue;
 import org.jetbrains.annotations.ApiStatus;
@@ -17,6 +16,32 @@
 @ApiStatus.Internal
 public final class ViewUtils {
 
+  private static final int[] coordinates = new int[2];
+
+  /**
+   * Verifies if the given touch coordinates are within the bounds of the given view.
+   *
+   * @param view the view to check if the touch coordinates are within its bounds
+   * @param x - the x coordinate of a {@link MotionEvent}
+   * @param y - the y coordinate of {@link MotionEvent}
+   * @return true if the touch coordinates are within the bounds of the view, false otherwise
+   */
+  private static boolean touchWithinBounds(
+      final @Nullable View view, final float x, final float y) {
+    if (view == null) {
+      return false;
+    }
+
+    view.getLocationOnScreen(coordinates);
+    int vx = coordinates[0];
+    int vy = coordinates[1];
+
+    int w = view.getWidth();
+    int h = view.getHeight();
+
+    return !(x < vx || x > vx + w || y < vy || y > vy + h);
+  }
+
   /**
    * Finds a target view, that has been selected/clicked by the given coordinates x and y and the
    * given {@code viewTargetSelector}.
@@ -40,7 +65,12 @@ public final class ViewUtils {
 
     @Nullable UiElement target = null;
     while (queue.size() > 0) {
-      final View view = Objects.requireNonNull(queue.poll(), "view is required");
+      final View view = queue.poll();
+
+      if (!touchWithinBounds(view, x, y)) {
+        // if the touch is not hitting the view, skip traversal of its children
+        continue;
+      }
 
       if (view instanceof ViewGroup) {
         final ViewGroup viewGroup = (ViewGroup) view;
@@ -54,7 +84,7 @@ public final class ViewUtils {
         if (newTarget != null) {
           if (targetType == UiElement.Type.CLICKABLE) {
             target = newTarget;
-          } else {
+          } else if (targetType == UiElement.Type.SCROLLABLE) {
             return newTarget;
           }
         }

sentry-android-core/src/test/java/io/sentry/android/core/UserInteractionIntegrationTest.kt

@@ -140,6 +140,7 @@ class UserInteractionIntegrationTest {
             )
         )
 
+        sut.register(fixture.scopes, fixture.options)
         sut.onActivityPaused(fixture.activity)
 
         verify(fixture.window).callback = null
@@ -160,6 +161,7 @@ class UserInteractionIntegrationTest {
             )
         )
 
+        sut.register(fixture.scopes, fixture.options)
         sut.onActivityPaused(fixture.activity)
 
         verify(fixture.window).callback = delegate
@@ -170,8 +172,30 @@ class UserInteractionIntegrationTest {
         val callback = mock<SentryWindowCallback>()
         val sut = fixture.getSut(callback)
 
+        sut.register(fixture.scopes, fixture.options)
         sut.onActivityPaused(fixture.activity)
 
         verify(callback).stopTracking()
     }
+
+    @Test
+    fun `does not instrument if the callback is already ours`() {
+        val delegate = mock<Window.Callback>()
+        val context = mock<Context>()
+        val resources = Fixture.mockResources()
+        whenever(context.resources).thenReturn(resources)
+        val existingCallback = SentryWindowCallback(
+            delegate,
+            context,
+            mock(),
+            mock()
+        )
+        val sut = fixture.getSut(existingCallback)
+
+        sut.register(fixture.scopes, fixture.options)
+        sut.onActivityResumed(fixture.activity)
+
+        val argumentCaptor = argumentCaptor<Window.Callback>()
+        verify(fixture.window, never()).callback = argumentCaptor.capture()
+    }
 }

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

@@ -185,7 +185,7 @@ class SentryGestureListenerClickTest {
 
         sut.onSingleTapUp(event)
 
-        verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>())
+        verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>(), anyOrNull())
     }
 
     @Test
@@ -214,7 +214,7 @@ class SentryGestureListenerClickTest {
 
         sut.onSingleTapUp(event)
 
-        verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>())
+        verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>(), anyOrNull())
     }
 
     @Test
@@ -223,7 +223,7 @@ class SentryGestureListenerClickTest {
 
         val event = mock<MotionEvent>()
         val sut = fixture.getSut<LocalView>(event, attachViewsToRoot = false)
-        fixture.window.mockDecorView<ViewGroup>(event = event, touchWithinBounds = false) {
+        fixture.window.mockDecorView<ViewGroup>(event = event, touchWithinBounds = true) {
             whenever(it.childCount).thenReturn(1)
             whenever(it.getChildAt(0)).thenReturn(fixture.target)
         }
@@ -244,7 +244,7 @@ class SentryGestureListenerClickTest {
 
         val event = mock<MotionEvent>()
         val sut = fixture.getSut<LocalView>(event, attachViewsToRoot = false)
-        fixture.window.mockDecorView<ViewGroup>(event = event, touchWithinBounds = false) {
+        fixture.window.mockDecorView<ViewGroup>(event = event, touchWithinBounds = true) {
             whenever(it.childCount).thenReturn(1)
             whenever(it.getChildAt(0)).thenReturn(fixture.target)
         }
@@ -253,4 +253,18 @@ class SentryGestureListenerClickTest {
 
         verify(fixture.scope).propagationContext = any()
     }
+
+    @Test
+    fun `if touch is not within view group bounds does not traverse its children`() {
+        val event = mock<MotionEvent>()
+        val sut = fixture.getSut<View>(event, attachViewsToRoot = false)
+        fixture.window.mockDecorView<ViewGroup>(event = event, touchWithinBounds = false) {
+            whenever(it.childCount).thenReturn(1)
+            whenever(it.getChildAt(0)).thenReturn(fixture.target)
+        }
+
+        sut.onSingleTapUp(event)
+
+        verify(fixture.scopes, never()).addBreadcrumb(any<Breadcrumb>(), anyOrNull())
+    }
 }

sentry-compose-helper/src/jvmMain/java/io/sentry/compose/gestures/ComposeGestureTargetLocator.java

@@ -42,7 +42,7 @@ public ComposeGestureTargetLocator(final @NotNull ILogger logger) {
 
   @Override
   public @Nullable UiElement locate(
-      @NotNull Object root, float x, float y, UiElement.Type targetType) {
+      @Nullable Object root, float x, float y, UiElement.Type targetType) {
 
     // lazy init composeHelper as it's using some reflection under the hood
     if (composeHelper == null) {

sentry/api/sentry.api

@@ -4147,6 +4147,7 @@ public final class io/sentry/instrumentation/file/SentryFileOutputStream : java/
 public final class io/sentry/instrumentation/file/SentryFileOutputStream$Factory {
 	public fun <init> ()V
 	public static fun create (Ljava/io/FileOutputStream;Ljava/io/File;)Ljava/io/FileOutputStream;
+	public static fun create (Ljava/io/FileOutputStream;Ljava/io/File;Lio/sentry/IScopes;)Ljava/io/FileOutputStream;
 	public static fun create (Ljava/io/FileOutputStream;Ljava/io/File;Z)Ljava/io/FileOutputStream;
 	public static fun create (Ljava/io/FileOutputStream;Ljava/io/FileDescriptor;)Ljava/io/FileOutputStream;
 	public static fun create (Ljava/io/FileOutputStream;Ljava/lang/String;)Ljava/io/FileOutputStream;

sentry/src/main/java/io/sentry/instrumentation/file/SentryFileInputStream.java

@@ -3,6 +3,7 @@
 import io.sentry.IScopes;
 import io.sentry.ISpan;
 import io.sentry.ScopesAdapter;
+import io.sentry.SentryOptions;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileInputStream;
@@ -127,28 +128,42 @@ public static final class Factory {
     public static FileInputStream create(
         final @NotNull FileInputStream delegate, final @Nullable String name)
         throws FileNotFoundException {
-      return new SentryFileInputStream(
-          init(name != null ? new File(name) : null, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileInputStream(init(name != null ? new File(name) : null, delegate, scopes))
+          : delegate;
     }
 
     public static FileInputStream create(
         final @NotNull FileInputStream delegate, final @Nullable File file)
         throws FileNotFoundException {
-      return new SentryFileInputStream(init(file, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileInputStream(init(file, delegate, scopes))
+          : delegate;
     }
 
     public static FileInputStream create(
         final @NotNull FileInputStream delegate, final @NotNull FileDescriptor descriptor) {
-      return new SentryFileInputStream(
-          init(descriptor, delegate, ScopesAdapter.getInstance()), descriptor);
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileInputStream(init(descriptor, delegate, scopes), descriptor)
+          : delegate;
     }
 
     static FileInputStream create(
         final @NotNull FileInputStream delegate,
         final @Nullable File file,
         final @NotNull IScopes scopes)
         throws FileNotFoundException {
-      return new SentryFileInputStream(init(file, delegate, scopes));
+      return isTracingEnabled(scopes)
+          ? new SentryFileInputStream(init(file, delegate, scopes))
+          : delegate;
+    }
+
+    private static boolean isTracingEnabled(final @NotNull IScopes scopes) {
+      final @NotNull SentryOptions options = scopes.getOptions();
+      return options.isTracingEnabled();
     }
   }
 }

sentry/src/main/java/io/sentry/instrumentation/file/SentryFileOutputStream.java

@@ -3,6 +3,7 @@
 import io.sentry.IScopes;
 import io.sentry.ISpan;
 import io.sentry.ScopesAdapter;
+import io.sentry.SentryOptions;
 import java.io.File;
 import java.io.FileDescriptor;
 import java.io.FileNotFoundException;
@@ -134,33 +135,62 @@ public static final class Factory {
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @Nullable String name)
         throws FileNotFoundException {
-      return new SentryFileOutputStream(
-          init(name != null ? new File(name) : null, false, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(
+              init(name != null ? new File(name) : null, false, delegate, scopes))
+          : delegate;
     }
 
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @Nullable String name, final boolean append)
         throws FileNotFoundException {
-      return new SentryFileOutputStream(
-          init(
-              name != null ? new File(name) : null, append, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(
+              init(name != null ? new File(name) : null, append, delegate, scopes))
+          : delegate;
     }
 
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @Nullable File file)
         throws FileNotFoundException {
-      return new SentryFileOutputStream(init(file, false, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(init(file, false, delegate, scopes))
+          : delegate;
     }
 
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @Nullable File file, final boolean append)
         throws FileNotFoundException {
-      return new SentryFileOutputStream(init(file, append, delegate, ScopesAdapter.getInstance()));
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(init(file, append, delegate, scopes))
+          : delegate;
     }
 
     public static FileOutputStream create(
         final @NotNull FileOutputStream delegate, final @NotNull FileDescriptor fdObj) {
-      return new SentryFileOutputStream(init(fdObj, delegate, ScopesAdapter.getInstance()), fdObj);
+      final @NotNull IScopes scopes = ScopesAdapter.getInstance();
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(init(fdObj, delegate, scopes), fdObj)
+          : delegate;
+    }
+
+    public static FileOutputStream create(
+        final @NotNull FileOutputStream delegate,
+        final @Nullable File file,
+        final @NotNull IScopes scopes)
+        throws FileNotFoundException {
+      return isTracingEnabled(scopes)
+          ? new SentryFileOutputStream(init(file, false, delegate, scopes))
+          : delegate;
+    }
+
+    private static boolean isTracingEnabled(final @NotNull IScopes scopes) {
+      final @NotNull SentryOptions options = scopes.getOptions();
+      return options.isTracingEnabled();
     }
   }
 }