Merge pull request #1914 from DataDog/nogorodnikov/rum-2879/opt-in-no…

…n-fatal-anr-reporting

RUM-2879: Disable non-fatal ANR reporting by default

features/dd-sdk-android-rum/api/apiSurface

@@ -70,6 +70,7 @@ data class com.datadog.android.rum.RumConfiguration
     fun disableUserInteractionTracking(): Builder
     fun useViewTrackingStrategy(com.datadog.android.rum.tracking.ViewTrackingStrategy?): Builder
     fun trackLongTasks(Long = RumFeature.DEFAULT_LONG_TASK_THRESHOLD_MS): Builder
+    fun trackNonFatalAnrs(Boolean): Builder
     fun setViewEventMapper(com.datadog.android.rum.event.ViewEventMapper): Builder
     fun setResourceEventMapper(com.datadog.android.event.EventMapper<com.datadog.android.rum.model.ResourceEvent>): Builder
     fun setActionEventMapper(com.datadog.android.event.EventMapper<com.datadog.android.rum.model.ActionEvent>): Builder

features/dd-sdk-android-rum/api/dd-sdk-android-rum.api

@@ -112,6 +112,7 @@ public final class com/datadog/android/rum/RumConfiguration$Builder {
 	public final fun trackLongTasks ()Lcom/datadog/android/rum/RumConfiguration$Builder;
 	public final fun trackLongTasks (J)Lcom/datadog/android/rum/RumConfiguration$Builder;
 	public static synthetic fun trackLongTasks$default (Lcom/datadog/android/rum/RumConfiguration$Builder;JILjava/lang/Object;)Lcom/datadog/android/rum/RumConfiguration$Builder;
+	public final fun trackNonFatalAnrs (Z)Lcom/datadog/android/rum/RumConfiguration$Builder;
 	public final fun trackUserInteractions ()Lcom/datadog/android/rum/RumConfiguration$Builder;
 	public final fun trackUserInteractions ([Lcom/datadog/android/rum/tracking/ViewAttributesProvider;)Lcom/datadog/android/rum/RumConfiguration$Builder;
 	public final fun trackUserInteractions ([Lcom/datadog/android/rum/tracking/ViewAttributesProvider;Lcom/datadog/android/rum/tracking/InteractionPredicate;)Lcom/datadog/android/rum/RumConfiguration$Builder;

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

@@ -133,6 +133,21 @@ data class RumConfiguration internal constructor(
             return this
         }
 
+        /**
+         * Enable tracking of non-fatal ANRs. This is enabled by default on Android API 29 and
+         * below, and disabled by default on Android API 30 and above. Android API 30+ has a
+         * capability to report fatal ANRs (always enabled). Please note, that tracking non-fatal
+         * ANRs is using Watchdog thread approach, which can be noisy, and also leads to ANR
+         * duplication on Android 30+ if fatal ANR happened, because Watchdog thread approach cannot
+         * categorize ANR as fatal or non-fatal.
+         *
+         * @param enabled whether tracking of non-fatal ANRs is enabled or not.
+         */
+        fun trackNonFatalAnrs(enabled: Boolean): Builder {
+            rumConfig = rumConfig.copy(trackNonFatalAnrs = enabled)
+            return this
+        }
+
         /**
          * Sets the [ViewEventMapper] for the RUM [ViewEvent]. You can use this interface implementation
          * to modify the [ViewEvent] attributes before serialisation.

features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt

@@ -25,6 +25,7 @@ import com.datadog.android.api.storage.DataWriter
 import com.datadog.android.api.storage.FeatureStorageConfiguration
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.feature.event.JvmCrash
+import com.datadog.android.core.internal.system.BuildSdkVersionProvider
 import com.datadog.android.core.internal.thread.LoggingScheduledThreadPoolExecutor
 import com.datadog.android.core.internal.utils.executeSafe
 import com.datadog.android.core.internal.utils.scheduleSafe
@@ -127,9 +128,8 @@ internal class RumFeature(
     internal var sessionListener: RumSessionListener = NoOpRumSessionListener()
 
     internal var vitalExecutorService: ScheduledExecutorService = NoOpScheduledExecutorService()
-    internal lateinit var anrDetectorExecutorService: ExecutorService
-    internal lateinit var anrDetectorRunnable: ANRDetectorRunnable
-    internal lateinit var anrDetectorHandler: Handler
+    private var anrDetectorExecutorService: ExecutorService? = null
+    internal var anrDetectorRunnable: ANRDetectorRunnable? = null
     internal lateinit var appContext: Context
     internal lateinit var telemetry: Telemetry
 
@@ -177,7 +177,9 @@ internal class RumFeature(
 
         initializeVitalMonitors(configuration.vitalsMonitorUpdateFrequency)
 
-        initializeANRDetector()
+        if (configuration.trackNonFatalAnrs) {
+            initializeANRDetector()
+        }
 
         registerTrackingStrategies(appContext)
 
@@ -217,8 +219,8 @@ internal class RumFeature(
         frameRateVitalMonitor = NoOpVitalMonitor()
 
         vitalExecutorService.shutdownNow()
-        anrDetectorExecutorService.shutdownNow()
-        anrDetectorRunnable.stop()
+        anrDetectorExecutorService?.shutdownNow()
+        anrDetectorRunnable?.stop()
         vitalExecutorService = NoOpScheduledExecutorService()
         sessionListener = NoOpRumSessionListener()
 
@@ -435,14 +437,14 @@ internal class RumFeature(
     }
 
     private fun initializeANRDetector() {
-        anrDetectorHandler = Handler(Looper.getMainLooper())
-        anrDetectorRunnable = ANRDetectorRunnable(sdkCore, anrDetectorHandler)
+        val detectorRunnable = ANRDetectorRunnable(sdkCore, Handler(Looper.getMainLooper()))
         anrDetectorExecutorService = Executors.newSingleThreadExecutor()
-        anrDetectorExecutorService.executeSafe(
+        anrDetectorExecutorService?.executeSafe(
             "ANR detection",
             sdkCore.internalLogger,
-            anrDetectorRunnable
+            detectorRunnable
         )
+        anrDetectorRunnable = detectorRunnable
     }
 
     private fun addJvmCrash(crashEvent: JvmCrash.Rum) {
@@ -581,14 +583,14 @@ internal class RumFeature(
         val telemetryConfigurationMapper: EventMapper<TelemetryConfigurationEvent>,
         val backgroundEventTracking: Boolean,
         val trackFrustrations: Boolean,
+        val trackNonFatalAnrs: Boolean,
         val vitalsMonitorUpdateFrequency: VitalsUpdateFrequency,
         val sessionListener: RumSessionListener,
         val additionalConfig: Map<String, Any>
     )
 
     internal companion object {
 
-        internal const val JVM_CRASH_BUS_MESSAGE_TYPE = "jvm_crash"
         internal const val NDK_CRASH_BUS_MESSAGE_TYPE = "ndk_crash"
         internal const val LOGGER_ERROR_BUS_MESSAGE_TYPE = "logger_error"
         internal const val LOGGER_ERROR_WITH_STACK_TRACE_MESSAGE_TYPE = "logger_error_with_stacktrace"
@@ -627,6 +629,7 @@ internal class RumFeature(
             telemetryConfigurationMapper = NoOpEventMapper(),
             backgroundEventTracking = false,
             trackFrustrations = true,
+            trackNonFatalAnrs = isTrackNonFatalAnrsEnabledByDefault(),
             vitalsMonitorUpdateFrequency = VitalsUpdateFrequency.AVERAGE,
             sessionListener = NoOpRumSessionListener(),
             additionalConfig = emptyMap()
@@ -690,5 +693,11 @@ internal class RumFeature(
             val providers = customProviders + defaultProviders
             return DatadogGesturesTracker(providers, interactionPredicate, internalLogger)
         }
+
+        internal fun isTrackNonFatalAnrsEnabledByDefault(
+            buildSdkVersionProvider: BuildSdkVersionProvider = BuildSdkVersionProvider.DEFAULT
+        ): Boolean {
+            return buildSdkVersionProvider.version < Build.VERSION_CODES.R
+        }
     }
 }

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/RumConfigurationBuilderTest.kt

@@ -92,6 +92,11 @@ internal class RumConfigurationBuilderTest {
                 longTaskTrackingStrategy = MainLooperLongTaskStrategy(100L),
                 backgroundEventTracking = false,
                 trackFrustrations = true,
+                // on Android R+ this should be false, but since default value is static property
+                // RumFeature.DEFAULT_RUM_CONFIG, it is evaluated at the static() block during class
+                // loading, so we are not able to set Build API version at this point. We will test
+                // it through a helper method in RumFeature.Companion
+                trackNonFatalAnrs = true,
                 vitalsMonitorUpdateFrequency = VitalsUpdateFrequency.AVERAGE,
                 sessionListener = NoOpRumSessionListener(),
                 additionalConfig = emptyMap()
@@ -319,6 +324,23 @@ internal class RumConfigurationBuilderTest {
         )
     }
 
+    @Test
+    fun `𝕄 build config with track non-fatal ANRs 𝕎 trackNonFatalAnrs() and build()`(
+        @BoolForgery trackNonFatalAnrsEnabled: Boolean
+    ) {
+        // When
+        val rumConfiguration = testedBuilder
+            .trackNonFatalAnrs(trackNonFatalAnrsEnabled)
+            .build()
+
+        // Then
+        assertThat(rumConfiguration.featureConfiguration).isEqualTo(
+            RumFeature.DEFAULT_RUM_CONFIG.copy(
+                trackNonFatalAnrs = trackNonFatalAnrsEnabled
+            )
+        )
+    }
+
     @Test
     fun `𝕄 build config with RUM View eventMapper 𝕎 setViewEventMapper() and build()`() {
         // Given

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/RumFeatureTest.kt

@@ -15,6 +15,7 @@ import com.datadog.android.api.InternalLogger
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.feature.event.JvmCrash
 import com.datadog.android.core.feature.event.ThreadDump
+import com.datadog.android.core.internal.system.BuildSdkVersionProvider
 import com.datadog.android.event.EventMapper
 import com.datadog.android.event.MapperSerializer
 import com.datadog.android.rum.GlobalRumMonitor
@@ -641,6 +642,48 @@ internal class RumFeatureTest {
             .isInstanceOf(NoOpScheduledExecutorService::class.java)
     }
 
+    @Test
+    fun `𝕄 initialize non-fatal ANR tracking  𝕎 initialize { trackNonFatalAnrs = true }()`() {
+        // Given
+        fakeConfiguration = fakeConfiguration.copy(
+            trackNonFatalAnrs = true
+        )
+        testedFeature = RumFeature(
+            mockSdkCore,
+            fakeApplicationId.toString(),
+            fakeConfiguration,
+            lateCrashReporterFactory = { mockLateCrashReporter }
+        )
+
+        // When
+        testedFeature.onInitialize(appContext.mockInstance)
+
+        // Then
+        assertThat(testedFeature.anrDetectorRunnable)
+            .isNotNull()
+    }
+
+    @Test
+    fun `𝕄 not initialize non-fatal ANR tracking  𝕎 initialize { trackNonFatalAnrs = false }()`() {
+        // Given
+        fakeConfiguration = fakeConfiguration.copy(
+            trackNonFatalAnrs = false
+        )
+        testedFeature = RumFeature(
+            mockSdkCore,
+            fakeApplicationId.toString(),
+            fakeConfiguration,
+            lateCrashReporterFactory = { mockLateCrashReporter }
+        )
+
+        // When
+        testedFeature.onInitialize(appContext.mockInstance)
+
+        // Then
+        assertThat(testedFeature.anrDetectorRunnable)
+            .isNull()
+    }
+
     @Test
     fun `𝕄 shut down vital executor 𝕎 onStop()`() {
         // Given
@@ -967,6 +1010,36 @@ internal class RumFeatureTest {
         )
     }
 
+    @Test
+    fun `𝕄 return true 𝕎 isTrackNonFatalAnrsEnabledByDefault() { Android Q and below }`(
+        @IntForgery(min = 1, max = Build.VERSION_CODES.R) fakeBuildSdkVersion: Int
+    ) {
+        // Given
+        val mockBuildSdkVersionProvider = mock<BuildSdkVersionProvider>()
+        whenever(mockBuildSdkVersionProvider.version) doReturn fakeBuildSdkVersion
+
+        // When
+        val isEnabled = RumFeature.isTrackNonFatalAnrsEnabledByDefault(mockBuildSdkVersionProvider)
+
+        // Then
+        assertThat(isEnabled).isTrue()
+    }
+
+    @Test
+    fun `𝕄 return false 𝕎 isTrackNonFatalAnrsEnabledByDefault() { Android R and above }`(
+        @IntForgery(min = Build.VERSION_CODES.R) fakeBuildSdkVersion: Int
+    ) {
+        // Given
+        val mockBuildSdkVersionProvider = mock<BuildSdkVersionProvider>()
+        whenever(mockBuildSdkVersionProvider.version) doReturn fakeBuildSdkVersion
+
+        // When
+        val isEnabled = RumFeature.isTrackNonFatalAnrsEnabledByDefault(mockBuildSdkVersionProvider)
+
+        // Then
+        assertThat(isEnabled).isFalse()
+    }
+
     // region FeatureEventReceiver#onReceive + logger error
 
     @Test

features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/utils/forge/ConfigurationRumForgeryFactory.kt

@@ -44,6 +44,7 @@ internal class ConfigurationRumForgeryFactory :
             longTaskTrackingStrategy = mock(),
             backgroundEventTracking = forge.aBool(),
             trackFrustrations = forge.aBool(),
+            trackNonFatalAnrs = forge.aBool(),
             vitalsMonitorUpdateFrequency = forge.aValueFrom(VitalsUpdateFrequency::class.java),
             sessionListener = mock(),
             additionalConfig = forge.aMap { aString() to aString() }

sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt

@@ -202,6 +202,7 @@ class SampleApplication : Application() {
             .setTelemetrySampleRate(100f)
             .trackUserInteractions()
             .trackLongTasks(250L)
+            .trackNonFatalAnrs(true)
             .setViewEventMapper(object : ViewEventMapper {
                 override fun map(event: ViewEvent): ViewEvent {
                     event.context?.additionalProperties?.put(ATTR_IS_MAPPED, true)