Merge pull request #2645 from DataDog/nogorodnikov/rum-9940/return-datadog-context-on-context-thread

RUM-9940: Make `getDatadogContext` read on the context thread

Author: 0xnm

dd-sdk-android-core/api/apiSurface

@@ -310,7 +310,6 @@ fun Collection<ByteArray>.join(ByteArray, ByteArray = ByteArray(0), ByteArray =
 fun java.util.concurrent.Executor.executeSafe(String, com.datadog.android.api.InternalLogger, Runnable)
 fun java.util.concurrent.ScheduledExecutorService.scheduleSafe(String, Long, java.util.concurrent.TimeUnit, com.datadog.android.api.InternalLogger, Runnable): java.util.concurrent.ScheduledFuture<*>?
 fun java.util.concurrent.ExecutorService.submitSafe(String, com.datadog.android.api.InternalLogger, Runnable): java.util.concurrent.Future<*>?
-fun <T> java.util.concurrent.ExecutorService.submitSafe(String, com.datadog.android.api.InternalLogger, java.util.concurrent.Callable<T>): java.util.concurrent.Future<T>?
 val NULL_MAP_VALUE: Object
 object com.datadog.android.core.internal.utils.JsonSerializer
   fun toJsonElement(Any?): com.google.gson.JsonElement

dd-sdk-android-core/api/dd-sdk-android-core.api

@@ -813,7 +813,6 @@ public final class com/datadog/android/core/internal/utils/ConcurrencyExtKt {
 	public static final fun executeSafe (Ljava/util/concurrent/Executor;Ljava/lang/String;Lcom/datadog/android/api/InternalLogger;Ljava/lang/Runnable;)V
 	public static final fun scheduleSafe (Ljava/util/concurrent/ScheduledExecutorService;Ljava/lang/String;JLjava/util/concurrent/TimeUnit;Lcom/datadog/android/api/InternalLogger;Ljava/lang/Runnable;)Ljava/util/concurrent/ScheduledFuture;
 	public static final fun submitSafe (Ljava/util/concurrent/ExecutorService;Ljava/lang/String;Lcom/datadog/android/api/InternalLogger;Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
-	public static final fun submitSafe (Ljava/util/concurrent/ExecutorService;Ljava/lang/String;Lcom/datadog/android/api/InternalLogger;Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
 }
 
 public final class com/datadog/android/core/internal/utils/JsonSerializer {

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

@@ -35,6 +35,7 @@ import com.datadog.android.core.internal.net.FirstPartyHostHeaderTypeResolver
 import com.datadog.android.core.internal.system.BuildSdkVersionProvider
 import com.datadog.android.core.internal.time.DefaultAppStartTimeProvider
 import com.datadog.android.core.internal.utils.executeSafe
+import com.datadog.android.core.internal.utils.getSafe
 import com.datadog.android.core.internal.utils.scheduleSafe
 import com.datadog.android.core.internal.utils.submitSafe
 import com.datadog.android.core.thread.FlushableExecutorService
@@ -47,9 +48,7 @@ import java.io.File
 import java.util.Locale
 import java.util.UUID
 import java.util.concurrent.Callable
-import java.util.concurrent.CancellationException
 import java.util.concurrent.ConcurrentHashMap
-import java.util.concurrent.ExecutionException
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.ScheduledExecutorService
 import java.util.concurrent.TimeUnit
@@ -300,25 +299,13 @@ internal class DatadogCore(
 
     override val trackingConsent: TrackingConsent
         get() {
-            val future = coreFeature.contextExecutorService.submitSafe(
+            return coreFeature.contextExecutorService.submitSafe(
                 "getTrackingConsent",
                 internalLogger,
                 Callable<TrackingConsent> {
                     coreFeature.trackingConsentProvider.getConsent()
                 }
-            )
-            return try {
-                future?.get() ?: TrackingConsent.NOT_GRANTED
-            } catch (e: InterruptedException) {
-                logTrackingConsentGetFailure(internalLogger, e)
-                TrackingConsent.NOT_GRANTED
-            } catch (e: CancellationException) {
-                logTrackingConsentGetFailure(internalLogger, e)
-                TrackingConsent.NOT_GRANTED
-            } catch (e: ExecutionException) {
-                logTrackingConsentGetFailure(internalLogger, e)
-                TrackingConsent.NOT_GRANTED
-            }
+            ).getSafe("getTrackingConsent", internalLogger) ?: TrackingConsent.NOT_GRANTED
         }
 
     override val rootStorageDir: File
@@ -371,7 +358,9 @@ internal class DatadogCore(
     }
 
     override fun getDatadogContext(): DatadogContext? {
-        return contextProvider?.context
+        return coreFeature.contextExecutorService
+            .submitSafe("getDatadogContext", internalLogger, Callable { contextProvider?.context })
+            .getSafe("getDatadogContext", internalLogger)
     }
 
     // endregion
@@ -569,15 +558,6 @@ internal class DatadogCore(
         )
     }
 
-    private fun logTrackingConsentGetFailure(internalLogger: InternalLogger, throwable: Throwable) {
-        internalLogger.log(
-            level = InternalLogger.Level.ERROR,
-            targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
-            messageBuilder = { UNABLE_TO_GET_TRACKING_CONSENT },
-            throwable = throwable
-        )
-    }
-
     /**
      * Stops all process for this instance of the Datadog SDK.
      */
@@ -633,8 +613,6 @@ internal class DatadogCore(
             "No need to write last RUM view event: NDK" +
                 " crash reports feature is not enabled and API is below 30."
 
-        internal const val UNABLE_TO_GET_TRACKING_CONSENT = "Unable to get tracking consent."
-
         internal val CONFIGURATION_TELEMETRY_DELAY_MS = TimeUnit.SECONDS.toMillis(5)
 
         // fallback for APIs below Android N, see [DefaultAppStartTimeProvider].

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt

@@ -53,6 +53,7 @@ import com.datadog.android.core.internal.persistence.file.advanced.FeatureFileOr
 import com.datadog.android.core.internal.persistence.file.batch.BatchFileReaderWriter
 import com.datadog.android.core.internal.persistence.tlvformat.TLVBlockFileReader
 import com.datadog.android.core.internal.utils.executeSafe
+import com.datadog.android.core.internal.utils.getSafe
 import com.datadog.android.core.internal.utils.submitSafe
 import com.datadog.android.core.persistence.PersistenceStrategy
 import com.datadog.android.internal.profiler.BenchmarkSdkUploads
@@ -62,9 +63,7 @@ import com.datadog.android.security.Encryption
 import java.util.Collections
 import java.util.Locale
 import java.util.concurrent.Callable
-import java.util.concurrent.CancellationException
 import java.util.concurrent.ConcurrentHashMap
-import java.util.concurrent.ExecutionException
 import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicReference
 
@@ -187,9 +186,10 @@ internal class SdkFeature(
     }
 
     override fun getWriteContextSync(): Pair<DatadogContext, EventWriteScope>? {
-        val future = coreFeature.contextExecutorService
+        val operationName = "getWriteContextSync-${wrappedFeature.name}"
+        return coreFeature.contextExecutorService
             .submitSafe(
-                "getWriteContextSync-${wrappedFeature.name}",
+                operationName,
                 internalLogger,
                 Callable {
                     val contextProvider = coreFeature.contextProvider
@@ -199,18 +199,7 @@ internal class SdkFeature(
                     context to eventBatchWriteScope
                 }
             )
-        return try {
-            future?.get()
-        } catch (e: CancellationException) {
-            logGetWriteContextSyncError(e)
-            null
-        } catch (e: ExecutionException) {
-            logGetWriteContextSyncError(e)
-            null
-        } catch (e: InterruptedException) {
-            logGetWriteContextSyncError(e)
-            null
-        }
+            .getSafe(operationName, internalLogger)
     }
 
     override fun sendEvent(event: Any) {
@@ -461,15 +450,6 @@ internal class SdkFeature(
         )
     }
 
-    private fun logGetWriteContextSyncError(e: Exception) {
-        internalLogger.log(
-            level = InternalLogger.Level.ERROR,
-            target = InternalLogger.Target.USER,
-            { FAILED_TO_GET_WRITE_CONTEXT_SYNC },
-            e
-        )
-    }
-
     // endregion
 
     // Used for nightly tests only
@@ -493,7 +473,6 @@ internal class SdkFeature(
             "Feature \"%s\" already has this listener registered."
         const val NO_EVENT_RECEIVER =
             "Feature \"%s\" has no event receiver registered, ignoring event."
-        internal const val FAILED_TO_GET_WRITE_CONTEXT_SYNC = "Failed to get write context in a sync manner."
         internal const val TRACK_NAME = "track"
         internal const val METER_NAME = "dd-sdk-android"
         internal const val BATCH_COUNT_METRIC_NAME = "android.benchmark.batch_count"

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/user/DatadogUserInfoProvider.kt

@@ -13,6 +13,7 @@ internal class DatadogUserInfoProvider(
     internal val dataWriter: DataWriter<UserInfo>
 ) : MutableUserInfoProvider {
 
+    @Volatile
     private var internalUserInfo = UserInfo()
         set(value) {
             field = value

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/utils/ConcurrencyExt.kt

@@ -11,6 +11,8 @@ import com.datadog.android.api.InternalLogger
 import com.datadog.android.lint.InternalApi
 import java.util.Locale
 import java.util.concurrent.Callable
+import java.util.concurrent.CancellationException
+import java.util.concurrent.ExecutionException
 import java.util.concurrent.Executor
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Future
@@ -20,6 +22,7 @@ import java.util.concurrent.ScheduledFuture
 import java.util.concurrent.TimeUnit
 
 internal const val ERROR_TASK_REJECTED = "Unable to schedule %s task on the executor"
+internal const val ERROR_FUTURE_GET_FAILED = "Unable to get result of the %s task"
 
 /**
  * Executes [Runnable] without throwing [RejectedExecutionException] if it cannot be accepted
@@ -118,8 +121,7 @@ fun ExecutorService.submitSafe(
  * @param internalLogger Internal logger.
  * @param callable Task to run.
  */
-@InternalApi
-fun <T> ExecutorService.submitSafe(
+internal fun <T> ExecutorService.submitSafe(
     operationName: String,
     internalLogger: InternalLogger,
     callable: Callable<T>
@@ -137,3 +139,36 @@ fun <T> ExecutorService.submitSafe(
         null
     }
 }
+
+internal fun <T> Future<T>?.getSafe(
+    operationName: String,
+    internalLogger: InternalLogger
+): T? {
+    return try {
+        this?.get()
+    } catch (e: InterruptedException) {
+        internalLogger.log(
+            InternalLogger.Level.ERROR,
+            listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
+            { ERROR_FUTURE_GET_FAILED.format(Locale.US, operationName) },
+            e
+        )
+        null
+    } catch (e: CancellationException) {
+        internalLogger.log(
+            InternalLogger.Level.ERROR,
+            listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
+            { ERROR_FUTURE_GET_FAILED.format(Locale.US, operationName) },
+            e
+        )
+        null
+    } catch (e: ExecutionException) {
+        internalLogger.log(
+            InternalLogger.Level.ERROR,
+            listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
+            { ERROR_FUTURE_GET_FAILED.format(Locale.US, operationName) },
+            e
+        )
+        null
+    }
+}

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/DatadogCoreTest.kt

@@ -679,12 +679,6 @@ internal class DatadogCoreTest {
 
         // When + Then
         assertThat(trackingConsent).isEqualTo(TrackingConsent.NOT_GRANTED)
-        mockInternalLogger.verifyLog(
-            InternalLogger.Level.ERROR,
-            listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
-            DatadogCore.UNABLE_TO_GET_TRACKING_CONSENT,
-            fakeThrowable
-        )
     }
 
     @Test

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/SdkFeatureTest.kt

@@ -627,12 +627,6 @@ internal class SdkFeatureTest {
 
         // Then
         assertThat(writeContext).isNull()
-        mockInternalLogger.verifyLog(
-            InternalLogger.Level.ERROR,
-            InternalLogger.Target.USER,
-            SdkFeature.FAILED_TO_GET_WRITE_CONTEXT_SYNC,
-            throwable
-        )
     }
 
     @Test

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/utils/ConcurrencyExtTest.kt

@@ -9,6 +9,8 @@ package com.datadog.android.core.internal.utils
 import com.datadog.android.api.InternalLogger
 import com.datadog.android.utils.forge.Configurator
 import com.datadog.android.utils.verifyLog
+import com.datadog.tools.unit.forge.aThrowable
+import fr.xgouchet.elmyr.Forge
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.annotation.LongForgery
 import fr.xgouchet.elmyr.annotation.StringForgery
@@ -26,9 +28,12 @@ import org.mockito.kotlin.doReturn
 import org.mockito.kotlin.doThrow
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.verify
+import org.mockito.kotlin.verifyNoInteractions
 import org.mockito.kotlin.whenever
 import org.mockito.quality.Strictness
 import java.util.concurrent.Callable
+import java.util.concurrent.CancellationException
+import java.util.concurrent.ExecutionException
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Future
 import java.util.concurrent.RejectedExecutionException
@@ -219,4 +224,48 @@ internal class ConcurrencyExtTest {
             exception
         )
     }
+
+    @Test
+    fun `M return result W getSafe()`(
+        @StringForgery operationName: String
+    ) {
+        // Given
+        val future = mock<Future<Any>>()
+        val fakeResult = Any()
+        whenever(future.getSafe(operationName, mockInternalLogger)) doReturn fakeResult
+
+        // When
+        val result = future.getSafe(operationName, mockInternalLogger)
+
+        // Then
+        assertThat(result).isSameAs(fakeResult)
+        verifyNoInteractions(mockInternalLogger)
+    }
+
+    @Test
+    fun `M log error W getSafe() { exception thrown }`(
+        @StringForgery operationName: String,
+        forge: Forge
+    ) {
+        // Given
+        val future = mock<Future<Any>>()
+        val fakeException = forge.anElementFrom(
+            ExecutionException(forge.aThrowable()),
+            CancellationException(),
+            InterruptedException()
+        )
+        whenever(future.getSafe(operationName, mockInternalLogger)) doThrow fakeException
+
+        // When
+        val result = future.getSafe(operationName, mockInternalLogger)
+
+        // Then
+        assertThat(result).isNull()
+        mockInternalLogger.verifyLog(
+            level = InternalLogger.Level.ERROR,
+            targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY),
+            message = "Unable to get result of the $operationName task",
+            throwable = fakeException
+        )
+    }
 }