Merge pull request #980 from DataDog/nogorodnikov/rumm-2322/improve-i…

…nteraction-tracking-reporting

RUMM-2322: Report back interaction as back type, prevent reporting empty target name

dd-sdk-android/apiSurface

@@ -308,6 +308,7 @@ enum com.datadog.android.rum.RumActionType
   - SCROLL
   - SWIPE
   - CLICK
+  - BACK
   - CUSTOM
 object com.datadog.android.rum.RumAttributes
   const val APPLICATION_VERSION: String

dd-sdk-android/src/main/kotlin/com/datadog/android/rum/RumActionType.kt

@@ -23,6 +23,9 @@ enum class RumActionType {
     /** User clicked on a widget (not used on Mobile). */
     CLICK,
 
+    /** User navigated back. */
+    BACK,
+
     /** A custom action. */
     CUSTOM
 }

dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumEventExt.kt

@@ -114,6 +114,7 @@ internal fun RumActionType.toSchemaType(): ActionEvent.ActionType {
         RumActionType.SCROLL -> ActionEvent.ActionType.SCROLL
         RumActionType.SWIPE -> ActionEvent.ActionType.SWIPE
         RumActionType.CLICK -> ActionEvent.ActionType.CLICK
+        RumActionType.BACK -> ActionEvent.ActionType.BACK
         RumActionType.CUSTOM -> ActionEvent.ActionType.CUSTOM
     }
 }

dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesListener.kt

@@ -87,10 +87,14 @@ internal class GesturesListener(
             val scrollTarget = findTargetForScroll(decorView, startDownEvent.x, startDownEvent.y)
             if (scrollTarget != null) {
                 scrollTargetReference = WeakReference(scrollTarget)
+                val targetId: String = resourceIdName(scrollTarget.id)
+                val attributes = resolveAttributes(scrollTarget, targetId, null)
+                // although we report scroll here, while it can be swipe in the end, it is fine,
+                // because the final type is taken from stopUserAction call anyway
                 rumMonitor.startUserAction(
-                    RumActionType.CUSTOM,
-                    "",
-                    emptyMap()
+                    RumActionType.SCROLL,
+                    resolveTargetName(interactionPredicate, scrollTarget),
+                    attributes
                 )
             } else {
                 return false
@@ -132,14 +136,16 @@ internal class GesturesListener(
     private fun resolveAttributes(
         scrollTarget: View,
         targetId: String,
-        onUpEvent: MotionEvent
+        onUpEvent: MotionEvent?
     ): MutableMap<String, Any?> {
         val attributes = mutableMapOf<String, Any?>(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollTarget.targetClassName(),
             RumAttributes.ACTION_TARGET_RESOURCE_ID to targetId
         )
-        gestureDirection = resolveGestureDirection(onUpEvent)
-        attributes.put(RumAttributes.ACTION_GESTURE_DIRECTION, gestureDirection)
+        if (onUpEvent != null) {
+            gestureDirection = resolveGestureDirection(onUpEvent)
+            attributes.put(RumAttributes.ACTION_GESTURE_DIRECTION, gestureDirection)
+        }
 
         attributesProviders.forEach {
             it.extractAttributes(scrollTarget, attributes)

dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/WindowCallbackWrapper.kt

@@ -130,7 +130,7 @@ internal class WindowCallbackWrapper(
         } else {
             customTargetName
         }
-        GlobalRum.get().addUserAction(RumActionType.CUSTOM, targetName, emptyMap())
+        GlobalRum.get().addUserAction(RumActionType.BACK, targetName, emptyMap())
     }
 
     // endregion

dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/instrumentation/gestures/GesturesListenerScrollSwipeTest.kt

@@ -20,7 +20,6 @@ import com.datadog.android.rum.tracking.InteractionPredicate
 import com.datadog.android.utils.forge.Configurator
 import com.datadog.tools.unit.extensions.TestConfigurationExtension
 import com.nhaarman.mockitokotlin2.any
-import com.nhaarman.mockitokotlin2.argumentCaptor
 import com.nhaarman.mockitokotlin2.eq
 import com.nhaarman.mockitokotlin2.inOrder
 import com.nhaarman.mockitokotlin2.mock
@@ -34,7 +33,6 @@ import fr.xgouchet.elmyr.junit5.ForgeConfiguration
 import fr.xgouchet.elmyr.junit5.ForgeExtension
 import java.lang.ref.WeakReference
 import java.util.stream.Stream
-import org.assertj.core.api.Assertions.assertThat
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.junit.jupiter.api.extension.Extensions
@@ -97,11 +95,12 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes = expectedStartAttributes +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -116,14 +115,10 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SCROLL),
-                eq(""),
-                argumentCaptor.capture()
-            )
-            assertThat(argumentCaptor.firstValue).isEqualTo(expectedAttributes)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SCROLL, "", expectedStopAttributes)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }
@@ -168,11 +163,12 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes = expectedStartAttributes +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -187,14 +183,10 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SCROLL),
-                eq(""),
-                argumentCaptor.capture()
-            )
-            assertThat(argumentCaptor.firstValue).isEqualTo(expectedAttributes)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SCROLL, "", expectedStopAttributes)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }
@@ -232,16 +224,19 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes1: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes1 = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection1
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
-        val expectedAttributes2: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes2 = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection2
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes1 = expectedStartAttributes1 +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection1)
+
+        val expectedStopAttributes2 = expectedStartAttributes2 +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection2)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -264,22 +259,14 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor1 = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SCROLL),
-                eq(""),
-                argumentCaptor1.capture()
-            )
-            assertThat(argumentCaptor1.firstValue).isEqualTo(expectedAttributes1)
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor2 = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SCROLL),
-                eq(""),
-                argumentCaptor2.capture()
-            )
-            assertThat(argumentCaptor2.firstValue).isEqualTo(expectedAttributes2)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes1)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SCROLL, "", expectedStopAttributes1)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes2)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SCROLL, "", expectedStopAttributes2)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }
@@ -410,11 +397,12 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes = expectedStartAttributes +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -430,14 +418,10 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SWIPE),
-                eq(""),
-                argumentCaptor.capture()
-            )
-            assertThat(argumentCaptor.firstValue).isEqualTo(expectedAttributes)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SWIPE, "", expectedStopAttributes)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }
@@ -488,11 +472,12 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.simpleName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes = expectedStartAttributes +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -508,14 +493,10 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SWIPE),
-                eq(""),
-                argumentCaptor.capture()
-            )
-            assertThat(argumentCaptor.firstValue).isEqualTo(expectedAttributes)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SWIPE, "", expectedStopAttributes)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }
@@ -567,7 +548,11 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
 
         // Then
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
+            verify(rumMonitor.mockInstance).startUserAction(
+                eq(RumActionType.SCROLL),
+                eq(fakeCustomTargetName),
+                any()
+            )
             verify(rumMonitor.mockInstance).stopUserAction(
                 eq(RumActionType.SCROLL),
                 eq(fakeCustomTargetName),
@@ -623,7 +608,11 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
 
         // Then
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
+            verify(rumMonitor.mockInstance).startUserAction(
+                eq(RumActionType.SCROLL),
+                eq(""),
+                any()
+            )
             verify(rumMonitor.mockInstance).stopUserAction(
                 eq(RumActionType.SCROLL),
                 eq(""),
@@ -679,7 +668,11 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
 
         // Then
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
+            verify(rumMonitor.mockInstance).startUserAction(
+                eq(RumActionType.SCROLL),
+                eq(""),
+                any()
+            )
             verify(rumMonitor.mockInstance).stopUserAction(
                 eq(RumActionType.SCROLL),
                 eq(""),
@@ -724,16 +717,18 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         }
         val expectedResourceName = forge.anAlphabeticalString()
         mockResourcesForTarget(scrollingTarget, expectedResourceName)
-        val expectedAttributes1: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes1 = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection1
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
-        val expectedAttributes2: MutableMap<String, Any?> = mutableMapOf(
+        val expectedStartAttributes2 = mutableMapOf(
             RumAttributes.ACTION_TARGET_CLASS_NAME to scrollingTarget.javaClass.canonicalName,
-            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName,
-            RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection2
+            RumAttributes.ACTION_TARGET_RESOURCE_ID to expectedResourceName
         )
+        val expectedStopAttributes1 = expectedStartAttributes1 +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection1)
+        val expectedStopAttributes2 = expectedStartAttributes2 +
+            (RumAttributes.ACTION_GESTURE_DIRECTION to expectedDirection2)
         testedListener = GesturesListener(
             WeakReference(mockWindow)
         )
@@ -758,22 +753,14 @@ internal class GesturesListenerScrollSwipeTest : AbstractGesturesListenerTest()
         // Then
 
         inOrder(rumMonitor.mockInstance) {
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor1 = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SWIPE),
-                eq(""),
-                argumentCaptor1.capture()
-            )
-            assertThat(argumentCaptor1.firstValue).isEqualTo(expectedAttributes1)
-            verify(rumMonitor.mockInstance).startUserAction(RumActionType.CUSTOM, "", emptyMap())
-            val argumentCaptor2 = argumentCaptor<Map<String, Any?>>()
-            verify(rumMonitor.mockInstance).stopUserAction(
-                eq(RumActionType.SWIPE),
-                eq(""),
-                argumentCaptor2.capture()
-            )
-            assertThat(argumentCaptor2.firstValue).isEqualTo(expectedAttributes2)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes1)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SWIPE, "", expectedStopAttributes1)
+            verify(rumMonitor.mockInstance)
+                .startUserAction(RumActionType.SCROLL, "", expectedStartAttributes2)
+            verify(rumMonitor.mockInstance)
+                .stopUserAction(RumActionType.SWIPE, "", expectedStopAttributes2)
         }
         verifyNoMoreInteractions(rumMonitor.mockInstance)
     }