Merge develop branch

CHANGELOG.md

@@ -1,3 +1,7 @@
+# 2.5.1 / 2024-01-24
+
+* [BUGFIX] RUM: Prevent crash due to concurrent modification of custom attributes. See [#1838](https://github.com/DataDog/dd-sdk-android/pull/1838)
+
 # 2.5.0 / 2024-01-15
 
 * [FEATURE] Add accessor for current session id. See [#1810](https://github.com/DataDog/dd-sdk-android/pull/1810)

detekt_custom.yml

@@ -325,6 +325,7 @@ datadog:
       - "android.content.res.Resources.Theme.resolveAttribute(kotlin.Int, android.util.TypedValue?, kotlin.Boolean)"
       - "android.database.DatabaseErrorHandler.onCorruption(android.database.sqlite.SQLiteDatabase?)"
       - "android.database.DefaultDatabaseErrorHandler.constructor()"
+      - "android.hardware.display.DisplayManager.getDisplay(kotlin.Int)"
       - "android.net.ConnectivityManager.NetworkCallback.onCapabilitiesChanged(android.net.Network, android.net.NetworkCapabilities)"
       - "android.net.ConnectivityManager.NetworkCallback.onLost(android.net.Network)"
       - "android.net.ConnectivityManager.getNetworkCapabilities(android.net.Network?)"
@@ -352,6 +353,7 @@ datadog:
       - "android.util.TypedValue.constructor()"
       - "android.view.Choreographer.postFrameCallback(android.view.Choreographer.FrameCallback)"
       - "android.view.Display.getSize(android.graphics.Point?)"
+      - "android.view.FrameMetrics.getMetric(kotlin.Int)"
       - "android.view.inspector.WindowInspector.getGlobalWindowViews()"
       - "android.view.MotionEvent.recycle()"
       - "android.view.MotionEvent.getPointerId(kotlin.Int)"
@@ -370,9 +372,11 @@ datadog:
       - "android.view.ViewTreeObserver.removeOnDrawListener(android.view.ViewTreeObserver.OnDrawListener?)"
       - "android.view.ViewTreeObserver.removeOnGlobalLayoutListener()"
       - "android.view.ViewTreeObserver.removeOnGlobalLayoutListener(android.view.ViewTreeObserver.OnGlobalLayoutListener)"
+      - "android.view.Window.addOnFrameMetricsAvailableListener(android.view.Window.OnFrameMetricsAvailableListener, android.os.Handler?)"
       - "android.view.Window.Callback.dispatchKeyEvent(android.view.KeyEvent?)"
       - "android.view.Window.Callback.dispatchTouchEvent(android.view.MotionEvent?)"
       - "android.view.Window.Callback.onMenuItemSelected(kotlin.Int, android.view.MenuItem)"
+      - "android.view.Window.removeOnFrameMetricsAvailableListener(android.view.Window.OnFrameMetricsAvailableListener?)"
       - "android.view.View.getChildAt(kotlin.Int)"
       - "android.view.View.hashCode()"
       - "androidx.collection.LruCache.size()"
@@ -931,6 +935,7 @@ datadog:
       - "kotlin.collections.MutableMap.remove(kotlin.String)"
       - "kotlin.collections.MutableMap.safeMapValuesToJson(com.datadog.android.api.InternalLogger)"
       - "kotlin.collections.MutableMap.toMap()"
+      - "kotlin.collections.MutableMap.toMutableMap()"
       - "kotlin.collections.MutableMap?.forEach(kotlin.Function1)"
       - "kotlin.collections.MutableSet.add(com.datadog.android.api.feature.FeatureContextUpdateReceiver?)"
       - "kotlin.collections.MutableSet.add(com.datadog.android.core.internal.persistence.ConsentAwareStorage.Batch)"
@@ -1010,13 +1015,15 @@ datadog:
       - "kotlin.Char.isLowerCase()"
       - "kotlin.Char.titlecase(java.util.Locale)"
       - "kotlin.CharArray.constructor(kotlin.Int, kotlin.Function1)"
+      - "kotlin.Double.coerceAtMost(kotlin.Double)"
       - "kotlin.Double.isNaN()"
       - "kotlin.Double.pow(kotlin.Int)"
       - "kotlin.Double.rangeTo(kotlin.Double)"
       - "kotlin.Double.toFloat()"
       - "kotlin.Double.toInt()"
       - "kotlin.Double.toLong()"
       - "kotlin.Float.percent()"
+      - "kotlin.Float.toDouble()"
       - "kotlin.Float.toFloat()"
       - "kotlin.Float.toInt()"
       - "kotlin.Float.toLong()"

features/dd-sdk-android-rum/src/main/json/rum/_common-schema.json

@@ -69,7 +69,7 @@
     "source": {
       "type": "string",
       "description": "The source of this event",
-      "enum": ["android", "ios", "browser", "flutter", "react-native", "roku"],
+      "enum": ["android", "ios", "browser", "flutter", "react-native", "roku", "unity"],
       "readOnly": true
     },
     "view": {
@@ -127,7 +127,7 @@
     "connectivity": {
       "type": "object",
       "description": "Device connectivity properties",
-      "required": ["status", "interfaces"],
+      "required": ["status"],
       "properties": {
         "status": {
           "type": "string",
@@ -144,6 +144,12 @@
           },
           "readOnly": true
         },
+        "effective_type": {
+          "type": "string",
+          "description": "Cellular connection type reflecting the measured network performance",
+          "enum": ["slow_2g", "2g", "3g", "4g"],
+          "readOnly": true
+        },
         "cellular": {
           "type": "object",
           "description": "Cellular connectivity properties",

features/dd-sdk-android-rum/src/main/json/rum/_view-container-schema.json

@@ -0,0 +1,37 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema",
+  "$id": "rum/_view-container-schema.json",
+  "title": "ViewContainerSchema",
+  "type": "object",
+  "description": "View Container schema for views that are nested (webviews in mobile)",
+  "properties": {
+    "container": {
+      "type": "object",
+      "description": "View Container properties (view wrapping the current view)",
+      "required": ["view", "source"],
+      "properties": {
+        "view": {
+          "type": "object",
+          "description": "Attributes of the view's container",
+          "required": ["id"],
+          "properties": {
+            "id": {
+              "type": "string",
+              "description": "ID of the parent view",
+              "pattern": "^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$",
+              "readOnly": true
+            }
+          },
+          "readOnly": true
+        },
+        "source": {
+          "type": "string",
+          "description": "Source of the parent view",
+          "enum": ["android", "ios", "browser", "flutter", "react-native", "roku", "unity"],
+          "readOnly": true
+        }
+      },
+      "readOnly": true
+    }
+  }
+}

features/dd-sdk-android-rum/src/main/json/rum/action-schema.json

@@ -9,7 +9,7 @@
       "$ref": "_common-schema.json"
     },
     {
-      "$ref": "_parent-view-schema.json"
+      "$ref": "_view-container-schema.json"
     },
     {
       "required": ["type", "action"],

features/dd-sdk-android-rum/src/main/json/rum/error-schema.json

@@ -12,7 +12,7 @@
       "$ref": "_action-child-schema.json"
     },
     {
-      "$ref": "_parent-view-schema.json"
+      "$ref": "_view-container-schema.json"
     },
     {
       "required": ["type", "error"],
@@ -123,7 +123,7 @@
                 "method": {
                   "type": "string",
                   "description": "HTTP method of the resource",
-                  "enum": ["POST", "GET", "HEAD", "PUT", "DELETE", "PATCH"],
+                  "enum": ["POST", "GET", "HEAD", "PUT", "DELETE", "PATCH", "TRACE", "OPTIONS", "CONNECT"],
                   "readOnly": true
                 },
                 "status_code": {

features/dd-sdk-android-rum/src/main/json/rum/long_task-schema.json

@@ -12,7 +12,7 @@
       "$ref": "_action-child-schema.json"
     },
     {
-      "$ref": "_parent-view-schema.json"
+      "$ref": "_view-container-schema.json"
     },
     {
       "required": ["type", "long_task"],

features/dd-sdk-android-rum/src/main/json/rum/resource-schema.json

@@ -12,7 +12,7 @@
       "$ref": "_action-child-schema.json"
     },
     {
-      "$ref": "_parent-view-schema.json"
+      "$ref": "_view-container-schema.json"
     },
     {
       "required": ["type", "resource"],
@@ -43,7 +43,7 @@
             "method": {
               "type": "string",
               "description": "HTTP method of the resource",
-              "enum": ["POST", "GET", "HEAD", "PUT", "DELETE", "PATCH"],
+              "enum": ["POST", "GET", "HEAD", "PUT", "DELETE", "PATCH", "TRACE", "OPTIONS", "CONNECT"],
               "readOnly": true
             },
             "url": {

features/dd-sdk-android-rum/src/main/json/rum/view-schema.json

@@ -9,7 +9,7 @@
       "$ref": "_common-schema.json"
     },
     {
-      "$ref": "_parent-view-schema.json"
+      "$ref": "_view-container-schema.json"
     },
     {
       "required": ["type", "view", "_dd"],

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

@@ -11,6 +11,7 @@ import android.os.Looper
 import com.datadog.android.Datadog
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.SdkCore
+import com.datadog.android.api.feature.Feature
 import com.datadog.android.api.feature.FeatureSdkCore
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.sampling.RateBasedSampler
@@ -30,6 +31,7 @@ object Rum {
      * @param sdkCore SDK instance to register feature in. If not provided, default SDK instance
      * will be used.
      */
+    @Suppress("ReturnCount")
     @JvmOverloads
     @JvmStatic
     fun enable(rumConfiguration: RumConfiguration, sdkCore: SdkCore = Datadog.getInstance()) {
@@ -52,18 +54,35 @@ object Rum {
             return
         }
 
+        if (sdkCore.getFeature(Feature.RUM_FEATURE_NAME) != null) {
+            sdkCore.internalLogger.log(
+                InternalLogger.Level.WARN,
+                InternalLogger.Target.USER,
+                { RUM_FEATURE_ALREADY_ENABLED }
+            )
+            return
+        }
+
         val rumFeature = RumFeature(
-            sdkCore = sdkCore as FeatureSdkCore,
+            sdkCore = sdkCore,
             applicationId = rumConfiguration.applicationId,
             configuration = rumConfiguration.featureConfiguration
         )
 
         sdkCore.registerFeature(rumFeature)
 
+        val rumMonitor = createMonitor(sdkCore, rumFeature)
         GlobalRumMonitor.registerIfAbsent(
-            monitor = createMonitor(sdkCore, rumFeature),
+            monitor = rumMonitor,
             sdkCore
         )
+
+        // TODO RUM-0000 there is a small chance of application crashing between RUM monitor
+        //  registration and the moment SDK init is processed, in this case we will miss this crash
+        //  (it won't activate new session). Ideally we should start session when monitor is created
+        //  and before it is registered, but with current code (internal RUM scopes using the
+        //  `GlobalRumMonitor`) it is impossible to break cycle dependency.
+        rumMonitor.start()
     }
 
     // region private
@@ -104,5 +123,8 @@ object Rum {
         "You're trying to create a RumMonitor instance, " +
             "but the RUM application id was empty. No RUM data will be sent."
 
+    internal const val RUM_FEATURE_ALREADY_ENABLED =
+        "RUM Feature is already enabled in this SDK core, ignoring the call to enable it."
+
     // endregion
 }

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

@@ -39,5 +39,20 @@ enum class RumResourceMethod {
     /**
      * PATCH Method.
      */
-    PATCH
+    PATCH,
+
+    /**
+     * TRACE Method.
+     */
+    TRACE,
+
+    /**
+     * OPTIONS Method.
+     */
+    OPTIONS,
+
+    /**
+     * CONNECT Method.
+     */
+    CONNECT
 }

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

@@ -194,6 +194,7 @@ internal class RumActionScope(
 
         val actualType = type
         attributes.putAll(GlobalRumMonitor.get(sdkCore).getAttributes())
+        val eventAttributes = attributes.toMutableMap()
         val rumContext = getRumContext()
 
         // make a copy so that closure captures at the state as of now
@@ -250,7 +251,7 @@ internal class RumActionScope(
                         null
                     }
                 ),
-                view = ActionEvent.View(
+                view = ActionEvent.ActionEventView(
                     id = rumContext.viewId.orEmpty(),
                     name = rumContext.viewName,
                     url = rumContext.viewUrl.orEmpty()
@@ -288,7 +289,7 @@ internal class RumActionScope(
                     brand = datadogContext.deviceInfo.deviceBrand,
                     architecture = datadogContext.deviceInfo.architecture
                 ),
-                context = ActionEvent.Context(additionalProperties = attributes),
+                context = ActionEvent.Context(additionalProperties = eventAttributes),
                 dd = ActionEvent.Dd(
                     session = ActionEvent.DdSession(
                         plan = ActionEvent.Plan.PLAN_1,

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

@@ -6,21 +6,15 @@
 
 package com.datadog.android.rum.internal.domain.scope
 
-import android.app.ActivityManager
 import androidx.annotation.WorkerThread
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.api.feature.Feature
 import com.datadog.android.api.storage.DataWriter
 import com.datadog.android.core.InternalSdkCore
 import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
-import com.datadog.android.rum.DdRumContentProvider
 import com.datadog.android.rum.RumSessionListener
-import com.datadog.android.rum.internal.AppStartTimeProvider
-import com.datadog.android.rum.internal.DefaultAppStartTimeProvider
 import com.datadog.android.rum.internal.domain.RumContext
-import com.datadog.android.rum.internal.domain.Time
 import com.datadog.android.rum.internal.vitals.VitalMonitor
-import java.util.concurrent.TimeUnit
 
 @Suppress("LongParameterList")
 internal class RumApplicationScope(
@@ -33,8 +27,7 @@ internal class RumApplicationScope(
     private val cpuVitalMonitor: VitalMonitor,
     private val memoryVitalMonitor: VitalMonitor,
     private val frameRateVitalMonitor: VitalMonitor,
-    private val sessionListener: RumSessionListener?,
-    private val appStartTimeProvider: AppStartTimeProvider = DefaultAppStartTimeProvider()
+    private val sessionListener: RumSessionListener?
 ) : RumScope, RumViewChangedListener {
 
     private var rumContext = RumContext(applicationId = applicationId)
@@ -62,7 +55,6 @@ internal class RumApplicationScope(
         }
 
     private var lastActiveViewInfo: RumViewInfo? = null
-    private var isSentAppStartedEvent = false
 
     // region RumScope
 
@@ -87,10 +79,6 @@ internal class RumApplicationScope(
             }
         }
 
-        if (!isSentAppStartedEvent) {
-            sendApplicationStartEvent(event.eventTime, writer)
-        }
-
         delegateToChildren(event, writer)
 
         return this
@@ -166,37 +154,9 @@ internal class RumApplicationScope(
         }
     }
 
-    @WorkerThread
-    private fun sendApplicationStartEvent(eventTime: Time, writer: DataWriter<Any>) {
-        val processImportance = DdRumContentProvider.processImportance
-        val isForegroundProcess = processImportance ==
-                ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND
-        if (isForegroundProcess) {
-            val processStartTimeNs = appStartTimeProvider.appStartTimeNs
-            // processStartTime is the time in nanoseconds since VM start. To get a timestamp, we want
-            // to convert it to milliseconds since epoch provided by System.currentTimeMillis.
-            // To do so, we take the offset of those times in the event time, which should be consistent,
-            // then add that to our processStartTime to get the correct value.
-            val timestampNs = (
-                    TimeUnit.MILLISECONDS.toNanos(eventTime.timestamp) - eventTime.nanoTime
-                    ) + processStartTimeNs
-            val applicationLaunchViewTime = Time(
-                timestamp = TimeUnit.NANOSECONDS.toMillis(timestampNs),
-                nanoTime = processStartTimeNs
-            )
-            val startupTime = eventTime.nanoTime - processStartTimeNs
-            val appStartedEvent =
-                RumRawEvent.ApplicationStarted(applicationLaunchViewTime, startupTime)
-            delegateToChildren(appStartedEvent, writer)
-            isSentAppStartedEvent = true
-        }
-    }
-
     // endregion
 
     companion object {
-        internal const val LAST_ACTIVE_VIEW_GONE_WARNING_MESSAGE = "Attempting to start a new " +
-                "session on the last known view (%s) failed because that view has been disposed. "
         internal const val MULTIPLE_ACTIVE_SESSIONS_ERROR = "Application has multiple active " +
                 "sessions when starting a new session"
     }