Merge pull request #657 from DataDog/xgouchet/RUMM-1504/rum_crash_rep…

…orts RUMM-1504 handle Rum crash reports
DataDog · Jul 19, 2021 · 5456b0e · 5456b0e
2 parents f7fddad + 9fa31cf
commit 5456b0e
Show file tree

Hide file tree

Showing 10 changed files with 214 additions and 127 deletions.
diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt
@@ -42,9 +42,7 @@ internal class RumDataWriter(
             is ActionEvent -> notifyEventSent(event.view.id, EventType.ACTION)
             is ResourceEvent -> notifyEventSent(event.view.id, EventType.RESOURCE)
             is ErrorEvent -> {
-                if (event.error.isCrash == true) {
-                    notifyEventSent(event.view.id, EventType.CRASH)
-                } else {
+                if (event.error.isCrash != true) {
                     notifyEventSent(event.view.id, EventType.ERROR)
                 }
             }

diff --git a/...k-android/src/main/kotlin/com/datadog/android/rum/internal/domain/event/RumEventMapper.kt b/...k-android/src/main/kotlin/com/datadog/android/rum/internal/domain/event/RumEventMapper.kt
@@ -42,7 +42,13 @@ internal data class RumEventMapper(
         return when (val bundledEvent = event.event) {
             is ViewEvent -> viewEventMapper.map(bundledEvent)
             is ActionEvent -> actionEventMapper.map(bundledEvent)
-            is ErrorEvent -> errorEventMapper.map(bundledEvent)
+            is ErrorEvent -> {
+                if (bundledEvent.error.isCrash != true) {
+                    errorEventMapper.map(bundledEvent)
+                } else {
+                    bundledEvent
+                }
+            }
             is ResourceEvent -> resourceEventMapper.map(bundledEvent)
             is LongTaskEvent -> longTaskEventMapper.map(bundledEvent)
             else -> {

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEvent.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumRawEvent.kt
@@ -113,7 +113,6 @@ internal sealed class RumRawEvent {
 
     internal data class ErrorSent(
         val viewId: String,
-        val isCrash: Boolean,
         override val eventTime: Time = Time()
     ) : RumRawEvent()
 

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumViewScope.kt
@@ -298,7 +298,13 @@ internal open class RumViewScope(
             userExtraAttributes = user.additionalProperties
         )
         writer.write(rumEvent)
-        pendingErrorCount++
+        if (event.isFatal) {
+            errorCount++
+            crashCount++
+            sendViewUpdate(event, writer)
+        } else {
+            pendingErrorCount++
+        }
     }
 
     private fun onAddCustomTiming(
@@ -383,7 +389,6 @@ internal open class RumViewScope(
         if (event.viewId == viewId) {
             pendingErrorCount--
             errorCount++
-            if (event.isCrash) crashCount++
             sendViewUpdate(event, writer)
         }
     }

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/monitor/DatadogRumMonitor.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/monitor/DatadogRumMonitor.kt
@@ -255,8 +255,7 @@ internal class DatadogRumMonitor(
         when (type) {
             EventType.ACTION -> handleEvent(RumRawEvent.ActionSent(viewId))
             EventType.RESOURCE -> handleEvent(RumRawEvent.ResourceSent(viewId))
-            EventType.ERROR -> handleEvent(RumRawEvent.ErrorSent(viewId, false))
-            EventType.CRASH -> handleEvent(RumRawEvent.ErrorSent(viewId, true))
+            EventType.ERROR -> handleEvent(RumRawEvent.ErrorSent(viewId))
             EventType.LONG_TASK -> handleEvent(RumRawEvent.LongTaskSent(viewId))
             EventType.VIEW -> {
                 // Nothing to do
@@ -268,7 +267,7 @@ internal class DatadogRumMonitor(
         when (type) {
             EventType.ACTION -> handleEvent(RumRawEvent.ActionDropped(viewId))
             EventType.RESOURCE -> handleEvent(RumRawEvent.ResourceDropped(viewId))
-            EventType.ERROR, EventType.CRASH -> handleEvent(RumRawEvent.ErrorDropped(viewId))
+            EventType.ERROR -> handleEvent(RumRawEvent.ErrorDropped(viewId))
             EventType.LONG_TASK -> handleEvent(RumRawEvent.LongTaskDropped(viewId))
             EventType.VIEW -> {
                 // Nothing to do
@@ -293,14 +292,18 @@ internal class DatadogRumMonitor(
     }
 
     internal fun handleEvent(event: RumRawEvent) {
-        handler.removeCallbacks(keepAliveRunnable)
-        // avoid trowing a RejectedExecutionException
-        if (!executorService.isShutdown) {
-            executorService.submit {
-                synchronized(rootScope) {
-                    rootScope.handleEvent(event, writer)
+        if (event is RumRawEvent.AddError && event.isFatal) {
+            rootScope.handleEvent(event, writer)
+        } else {
+            handler.removeCallbacks(keepAliveRunnable)
+            // avoid trowing a RejectedExecutionException
+            if (!executorService.isShutdown) {
+                executorService.submit {
+                    synchronized(rootScope) {
+                        rootScope.handleEvent(event, writer)
+                    }
+                    handler.postDelayed(keepAliveRunnable, KEEP_ALIVE_MS)
                 }
-                handler.postDelayed(keepAliveRunnable, KEEP_ALIVE_MS)
             }
         }
     }

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/monitor/EventType.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/rum/internal/monitor/EventType.kt
@@ -11,6 +11,5 @@ internal enum class EventType {
     ACTION,
     RESOURCE,
     ERROR,
-    CRASH,
     LONG_TASK
 }
diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt
@@ -22,6 +22,9 @@ import com.datadog.android.utils.forge.Configurator
 import com.datadog.tools.unit.annotations.TestConfigurationsProvider
 import com.datadog.tools.unit.extensions.TestConfigurationExtension
 import com.datadog.tools.unit.extensions.config.TestConfiguration
+import com.nhaarman.mockitokotlin2.any
+import com.nhaarman.mockitokotlin2.eq
+import com.nhaarman.mockitokotlin2.never
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import fr.xgouchet.elmyr.annotation.Forgery
@@ -210,7 +213,7 @@ internal class RumDataWriterTest {
     }
 
     @Test
-    fun `𝕄 notify the RumMonitor 𝕎 onDataWritten() { ErrorEvent isCrash=true }`(
+    fun `𝕄 not notify the RumMonitor 𝕎 onDataWritten() { ErrorEvent isCrash=true }`(
         @Forgery fakeModel: RumEvent,
         @Forgery errorEvent: ErrorEvent
     ) {
@@ -225,7 +228,7 @@ internal class RumDataWriterTest {
         testedWriter.onDataWritten(rumEvent, fakeSerializedData)
 
         // Then
-        verify(rumMonitor.mockInstance).eventSent(errorEvent.view.id, EventType.CRASH)
+        verify(rumMonitor.mockInstance, never()).eventSent(eq(errorEvent.view.id), any())
         verifyZeroInteractions(mockFileHandler)
     }
 

diff --git a/...droid/src/test/kotlin/com/datadog/android/rum/internal/domain/event/RumEventMapperTest.kt b/...droid/src/test/kotlin/com/datadog/android/rum/internal/domain/event/RumEventMapperTest.kt
@@ -248,7 +248,10 @@ internal class RumEventMapperTest {
     @Test
     fun `M return null event W map returns null object { ErrorEvent }`(forge: Forge) {
         // GIVEN
-        val fakeBundledEvent = forge.getForgery<ErrorEvent>()
+        val fakeErrorEvent = forge.getForgery<ErrorEvent>()
+        val fakeBundledEvent = fakeErrorEvent.copy(
+            error = fakeErrorEvent.error.copy(isCrash = false)
+        )
         fakeRumEvent = fakeRumEvent.copy(event = fakeBundledEvent)
         whenever(mockErrorEventMapper.map(fakeBundledEvent))
             .thenReturn(null)
@@ -265,6 +268,26 @@ internal class RumEventMapperTest {
         )
     }
 
+    @Test
+    fun `M return event W map returns null object { fatal ErrorEvent }`(forge: Forge) {
+        // GIVEN
+        val fakeErrorEvent = forge.getForgery<ErrorEvent>()
+        val fakeBundledEvent = fakeErrorEvent.copy(
+            error = fakeErrorEvent.error.copy(isCrash = true)
+        )
+        fakeRumEvent = fakeRumEvent.copy(event = fakeBundledEvent)
+        whenever(mockErrorEventMapper.map(fakeBundledEvent))
+            .thenReturn(null)
+
+        // WHEN
+        val mappedRumEvent = testedRumEventMapper.map(fakeRumEvent)
+
+        // THEN
+        assertThat(mappedRumEvent)
+            .isSameAs(fakeRumEvent)
+            .isEqualTo(fakeRumEvent)
+    }
+
     @Test
     fun `M return null event W map returns null object { ActionEvent }`(forge: Forge) {
         // GIVEN
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,6 +11,5 @@ internal enum class EventType { @@
         ACTION,
         RESOURCE,
         ERROR,
-        CRASH,
         LONG_TASK
     }