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

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

Merged
merged 1 commit into from
Mar 18, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions features/dd-sdk-android-rum/api/apiSurface
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,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
Expand Down
1 change: 1 addition & 0 deletions features/dd-sdk-android-rum/api/dd-sdk-android-rum.api
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -177,7 +177,9 @@ internal class RumFeature(

initializeVitalMonitors(configuration.vitalsMonitorUpdateFrequency)

initializeANRDetector()
if (configuration.trackNonFatalAnrs) {
initializeANRDetector()
}

registerTrackingStrategies(appContext)

Expand Down Expand Up @@ -217,8 +219,8 @@ internal class RumFeature(
frameRateVitalMonitor = NoOpVitalMonitor()

vitalExecutorService.shutdownNow()
anrDetectorExecutorService.shutdownNow()
anrDetectorRunnable.stop()
anrDetectorExecutorService?.shutdownNow()
anrDetectorRunnable?.stop()
vitalExecutorService = NoOpScheduledExecutorService()
sessionListener = NoOpRumSessionListener()

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -627,6 +629,7 @@ internal class RumFeature(
telemetryConfigurationMapper = NoOpEventMapper(),
backgroundEventTracking = false,
trackFrustrations = true,
trackNonFatalAnrs = isTrackNonFatalAnrsEnabledByDefault(),
vitalsMonitorUpdateFrequency = VitalsUpdateFrequency.AVERAGE,
sessionListener = NoOpRumSessionListener(),
additionalConfig = emptyMap()
Expand Down Expand Up @@ -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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down