RUMM-2834: Tracing feature stores context in the common context storage

DataDog · Jan 4, 2023 · 3eaf25d · 3eaf25d
1 parent 62fa834
commit 3eaf25d
Show file tree

Hide file tree

Showing 7 changed files with 211 additions and 100 deletions.
diff --git a/...dk-android/src/main/kotlin/com/datadog/android/log/internal/domain/DatadogLogGenerator.kt b/...dk-android/src/main/kotlin/com/datadog/android/log/internal/domain/DatadogLogGenerator.kt
@@ -10,10 +10,10 @@ import com.datadog.android.log.LogAttributes
 import com.datadog.android.log.internal.utils.buildLogDateFormat
 import com.datadog.android.log.model.LogEvent
 import com.datadog.android.rum.internal.RumFeature
+import com.datadog.android.tracing.internal.TracingFeature
 import com.datadog.android.v2.api.context.DatadogContext
 import com.datadog.android.v2.api.context.NetworkInfo
 import com.datadog.android.v2.api.context.UserInfo
-import io.opentracing.util.GlobalTracer
 import java.util.Date
 
 @Suppress("TooManyFunctions")
@@ -126,8 +126,13 @@ internal class DatadogLogGenerator(
         networkInfo: NetworkInfo?
     ): LogEvent {
         val resolvedTimestamp = timestamp + datadogContext.time.serverTimeOffsetMs
-        val combinedAttributes =
-            resolveAttributes(datadogContext, attributes, bundleWithTraces, bundleWithRum)
+        val combinedAttributes = resolveAttributes(
+            datadogContext,
+            attributes,
+            bundleWithTraces,
+            threadName,
+            bundleWithRum
+        )
         val formattedDate = synchronized(simpleDateFormat) {
             @Suppress("UnsafeThirdPartyFunctionCall") // NPE cannot happen here
             simpleDateFormat.format(Date(resolvedTimestamp))
@@ -240,15 +245,17 @@ internal class DatadogLogGenerator(
         datadogContext: DatadogContext,
         attributes: Map<String, Any?>,
         bundleWithTraces: Boolean,
+        threadName: String,
         bundleWithRum: Boolean
     ): MutableMap<String, Any?> {
         val combinedAttributes = mutableMapOf<String, Any?>().apply { putAll(attributes) }
-        if (bundleWithTraces && GlobalTracer.isRegistered()) {
-            val tracer = GlobalTracer.get()
-            val activeContext = tracer.activeSpan()?.context()
-            if (activeContext != null) {
-                combinedAttributes[LogAttributes.DD_TRACE_ID] = activeContext.toTraceId()
-                combinedAttributes[LogAttributes.DD_SPAN_ID] = activeContext.toSpanId()
+        if (bundleWithTraces) {
+            datadogContext.featuresContext[TracingFeature.TRACING_FEATURE_NAME]?.let {
+                val threadLocalContext = it["context@$threadName"] as? Map<*, *>
+                if (threadLocalContext != null) {
+                    combinedAttributes[LogAttributes.DD_TRACE_ID] = threadLocalContext["trace_id"]
+                    combinedAttributes[LogAttributes.DD_SPAN_ID] = threadLocalContext["span_id"]
+                }
             }
         }
         if (bundleWithRum) {

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/AndroidTracer.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/tracing/AndroidTracer.kt
@@ -10,6 +10,7 @@ import com.datadog.android.Datadog
 import com.datadog.android.core.internal.utils.internalLogger
 import com.datadog.android.log.LogAttributes
 import com.datadog.android.rum.internal.RumFeature
+import com.datadog.android.tracing.internal.TracingFeature
 import com.datadog.android.tracing.internal.data.NoOpWriter
 import com.datadog.android.tracing.internal.handlers.AndroidSpanLogsHandler
 import com.datadog.android.v2.api.InternalLogger
@@ -20,6 +21,7 @@ import com.datadog.opentracing.DDTracer
 import com.datadog.opentracing.LogHandler
 import com.datadog.trace.api.Config
 import com.datadog.trace.common.writer.Writer
+import com.datadog.trace.context.ScopeListener
 import io.opentracing.Span
 import io.opentracing.log.Fields
 import java.security.SecureRandom
@@ -43,6 +45,34 @@ class AndroidTracer internal constructor(
     private val bundleWithRum: Boolean
 ) : DDTracer(config, writer, random) {
 
+    init {
+        addScopeListener(object : ScopeListener {
+            override fun afterScopeActivated() {
+                // scope is thread-local and at the given time for the particular thread it can
+                // be only one active scope.
+                val threadName = Thread.currentThread().name
+                val activeContext = activeSpan()?.context()
+                if (activeContext != null) {
+                    val activeSpanId = activeContext.toSpanId()
+                    val activeTraceId = activeContext.toTraceId()
+                    sdkCore.updateFeatureContext(TracingFeature.TRACING_FEATURE_NAME) {
+                        it["context@$threadName"] = mapOf(
+                            "span_id" to activeSpanId,
+                            "trace_id" to activeTraceId
+                        )
+                    }
+                }
+            }
+
+            override fun afterScopeClosed() {
+                val threadName = Thread.currentThread().name
+                sdkCore.updateFeatureContext(TracingFeature.TRACING_FEATURE_NAME) {
+                    it.remove("context@$threadName")
+                }
+            }
+        })
+    }
+
     // region Tracer
 
     override fun buildSpan(operationName: String): DDSpanBuilder {

diff --git a/dd-sdk-android/src/main/kotlin/com/datadog/android/v2/api/FeatureScope.kt b/dd-sdk-android/src/main/kotlin/com/datadog/android/v2/api/FeatureScope.kt
@@ -17,7 +17,9 @@ interface FeatureScope {
      * @param forceNewBatch if `true` forces the [EventBatchWriter] to write in a new file and
      * not reuse the already existing pending data persistence file. By default it is `false`.
      * @param callback an operation called with an up-to-date [DatadogContext]
-     * and an [EventBatchWriter]. Callback will be executed on a worker thread from I/O pool
+     * and an [EventBatchWriter]. Callback will be executed on a worker thread from I/O pool.
+     * [DatadogContext] will have a state created at the moment this method is called, before the
+     * thread switch for the callback invocation.
      */
     fun withWriteContext(
         forceNewBatch: Boolean = false,

diff --git a/dd-sdk-android/src/test/kotlin/com/datadog/android/log/internal/LogsFeatureTest.kt b/dd-sdk-android/src/test/kotlin/com/datadog/android/log/internal/LogsFeatureTest.kt
@@ -13,6 +13,7 @@ import com.datadog.android.log.internal.domain.event.LogEventMapperWrapper
 import com.datadog.android.log.model.LogEvent
 import com.datadog.android.rum.internal.RumFeature
 import com.datadog.android.rum.internal.domain.RumContext
+import com.datadog.android.tracing.internal.TracingFeature
 import com.datadog.android.utils.config.InternalLoggerTestConfiguration
 import com.datadog.android.utils.extension.toIsoFormattedTimestamp
 import com.datadog.android.utils.forge.Configurator
@@ -26,12 +27,10 @@ import com.datadog.android.v2.api.context.NetworkInfo
 import com.datadog.android.v2.api.context.UserInfo
 import com.datadog.android.v2.core.internal.storage.DataWriter
 import com.datadog.android.v2.log.internal.storage.LogsDataWriter
-import com.datadog.opentracing.DDSpanContext
 import com.datadog.tools.unit.annotations.TestConfigurationsProvider
 import com.datadog.tools.unit.extensions.TestConfigurationExtension
 import com.datadog.tools.unit.extensions.config.TestConfiguration
 import com.datadog.tools.unit.forge.aThrowable
-import com.datadog.tools.unit.setStaticValue
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.argumentCaptor
 import com.nhaarman.mockitokotlin2.doAnswer
@@ -47,11 +46,7 @@ import fr.xgouchet.elmyr.annotation.StringForgery
 import fr.xgouchet.elmyr.annotation.StringForgeryType
 import fr.xgouchet.elmyr.junit5.ForgeConfiguration
 import fr.xgouchet.elmyr.junit5.ForgeExtension
-import io.opentracing.Span
-import io.opentracing.Tracer
-import io.opentracing.util.GlobalTracer
 import org.assertj.core.api.Assertions.assertThat
-import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
@@ -92,15 +87,6 @@ internal class LogsFeatureTest {
     @Mock
     lateinit var mockDataWriter: DataWriter<LogEvent>
 
-    @Mock
-    lateinit var mockTracer: Tracer
-
-    @Mock
-    lateinit var mockSpanContext: DDSpanContext
-
-    @Mock
-    lateinit var mockSpan: Span
-
     @Forgery
     lateinit var fakeDatadogContext: DatadogContext
 
@@ -113,6 +99,9 @@ internal class LogsFeatureTest {
     @StringForgery(StringForgeryType.HEXADECIMAL)
     lateinit var fakeTraceId: String
 
+    @StringForgery(StringForgeryType.ALPHABETICAL)
+    lateinit var fakeThreadName: String
+
     private var fakeServerTimeOffset: Long = 0L
 
     @BeforeEach
@@ -131,30 +120,27 @@ internal class LogsFeatureTest {
             callback.invoke(fakeDatadogContext, mockEventBatchWriter)
         }
 
-        whenever(mockTracer.activeSpan()).thenReturn(mockSpan)
-        whenever(mockSpan.context()) doReturn mockSpanContext
-        whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId
-        whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId
-
         fakeDatadogContext = fakeDatadogContext.copy(
             time = fakeDatadogContext.time.copy(
                 serverTimeOffsetMs = fakeServerTimeOffset
             ),
             featuresContext = fakeDatadogContext.featuresContext.toMutableMap().apply {
                 put(RumFeature.RUM_FEATURE_NAME, fakeRumContext.toMap())
+                put(
+                    TracingFeature.TRACING_FEATURE_NAME,
+                    mapOf(
+                        "context@$fakeThreadName" to mapOf(
+                            "span_id" to fakeSpanId,
+                            "trace_id" to fakeTraceId
+                        )
+                    )
+                )
             }
         )
 
-        GlobalTracer.registerIfAbsent(mockTracer)
-
         testedFeature = LogsFeature(mockSdkCore)
     }
 
-    @AfterEach
-    fun `tear down`() {
-        GlobalTracer::class.java.setStaticValue("isRegistered", false)
-    }
-
     @Test
     fun `𝕄 initialize data writer 𝕎 initialize()`() {
         // When
@@ -245,7 +231,6 @@ internal class LogsFeatureTest {
     @EnumSource
     fun `𝕄 log warning and do nothing 𝕎 onReceive() { corrupted mandatory fields, JVM crash }`(
         missingType: ValueMissingType,
-        @StringForgery fakeThreadName: String,
         @LongForgery fakeTimestamp: Long,
         @StringForgery fakeMessage: String,
         @StringForgery fakeLoggerName: String,
@@ -294,7 +279,6 @@ internal class LogsFeatureTest {
 
     @Test
     fun `𝕄 write crash log event 𝕎 onReceive() { JVM crash }`(
-        @StringForgery fakeThreadName: String,
         @LongForgery fakeTimestamp: Long,
         @StringForgery fakeMessage: String,
         @StringForgery fakeLoggerName: String,
@@ -359,7 +343,6 @@ internal class LogsFeatureTest {
 
     @Test
     fun `𝕄 write crash log event and wait 𝕎 onReceive() { JVM crash }`(
-        @StringForgery fakeThreadName: String,
         @LongForgery fakeTimestamp: Long,
         @StringForgery fakeMessage: String,
         @StringForgery fakeLoggerName: String,
@@ -425,7 +408,6 @@ internal class LogsFeatureTest {
 
     @Test
     fun `𝕄 not wait forever for crash log write 𝕎 onReceive() { JVM crash, timeout }`(
-        @StringForgery fakeThreadName: String,
         @LongForgery fakeTimestamp: Long,
         @StringForgery fakeMessage: String,
         @StringForgery fakeLoggerName: String,

diff --git a/...ndroid/src/test/kotlin/com/datadog/android/log/internal/domain/DatadogLogGeneratorTest.kt b/...ndroid/src/test/kotlin/com/datadog/android/log/internal/domain/DatadogLogGeneratorTest.kt
@@ -11,59 +11,39 @@ import com.datadog.android.log.assertj.LogEventAssert.Companion.assertThat
 import com.datadog.android.log.model.LogEvent
 import com.datadog.android.rum.internal.RumFeature
 import com.datadog.android.rum.internal.domain.RumContext
+import com.datadog.android.tracing.internal.TracingFeature
 import com.datadog.android.utils.extension.asLogStatus
 import com.datadog.android.utils.extension.toIsoFormattedTimestamp
 import com.datadog.android.utils.forge.Configurator
 import com.datadog.android.v2.api.context.DatadogContext
 import com.datadog.android.v2.api.context.NetworkInfo
 import com.datadog.android.v2.api.context.UserInfo
-import com.datadog.opentracing.DDSpanContext
-import com.datadog.tools.unit.extensions.TestConfigurationExtension
 import com.datadog.tools.unit.forge.aThrowable
-import com.datadog.tools.unit.setStaticValue
-import com.datadog.trace.api.interceptor.MutableSpan
-import com.nhaarman.mockitokotlin2.doReturn
-import com.nhaarman.mockitokotlin2.whenever
 import fr.xgouchet.elmyr.Forge
 import fr.xgouchet.elmyr.annotation.Forgery
 import fr.xgouchet.elmyr.annotation.StringForgery
 import fr.xgouchet.elmyr.annotation.StringForgeryType
 import fr.xgouchet.elmyr.junit5.ForgeConfiguration
 import fr.xgouchet.elmyr.junit5.ForgeExtension
-import io.opentracing.Span
-import io.opentracing.Tracer
-import io.opentracing.util.GlobalTracer
 import org.assertj.core.api.Assertions
-import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.junit.jupiter.api.extension.Extensions
-import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.junit.jupiter.MockitoSettings
 import org.mockito.quality.Strictness
 
 @Extensions(
     ExtendWith(MockitoExtension::class),
-    ExtendWith(ForgeExtension::class),
-    ExtendWith(TestConfigurationExtension::class)
+    ExtendWith(ForgeExtension::class)
 )
 @MockitoSettings(strictness = Strictness.LENIENT)
 @ForgeConfiguration(Configurator::class)
 internal class DatadogLogGeneratorTest {
 
     lateinit var testedLogGenerator: DatadogLogGenerator
 
-    @Mock
-    lateinit var mockTracer: Tracer
-
-    @Mock
-    lateinit var mockSpanContext: DDSpanContext
-
-    @Mock(extraInterfaces = [MutableSpan::class])
-    lateinit var mockSpan: Span
-
     lateinit var fakeServiceName: String
     lateinit var fakeLoggerName: String
     lateinit var fakeAttributes: Map<String, Any?>
@@ -118,24 +98,23 @@ internal class DatadogLogGeneratorTest {
                         "action_id" to fakeRumContext.actionId
                     )
                 )
+                put(
+                    TracingFeature.TRACING_FEATURE_NAME,
+                    mapOf(
+                        "context@$fakeThreadName" to mapOf(
+                            "span_id" to fakeSpanId,
+                            "trace_id" to fakeTraceId
+                        )
+                    )
+                )
             }
         )
 
-        whenever(mockTracer.activeSpan()).thenReturn(mockSpan)
-        whenever(mockSpan.context()) doReturn mockSpanContext
-        whenever(mockSpanContext.toSpanId()) doReturn fakeSpanId
-        whenever(mockSpanContext.toTraceId()) doReturn fakeTraceId
-        GlobalTracer.registerIfAbsent(mockTracer)
         testedLogGenerator = DatadogLogGenerator(
             fakeServiceName
         )
     }
 
-    @AfterEach
-    fun `tear down`() {
-        GlobalTracer::class.java.setStaticValue("isRegistered", false)
-    }
-
     @Test
     fun `M add log message W creating the Log`() {
         // WHEN
@@ -662,9 +641,25 @@ internal class DatadogLogGeneratorTest {
     }
 
     @Test
-    fun `M do nothing W required to bundle the trace information {no active Span}`() {
+    fun `M do nothing W required to bundle the trace information {no active Span for given thread}`(
+        @StringForgery fakeOtherThreadName: String,
+        @StringForgery(StringForgeryType.HEXADECIMAL) fakeOtherThreadSpanId: String,
+        @StringForgery(StringForgeryType.HEXADECIMAL) fakeOtherThreadTraceId: String
+    ) {
         // GIVEN
-        whenever(mockTracer.activeSpan()).doReturn(null)
+        fakeDatadogContext = fakeDatadogContext.copy(
+            featuresContext = fakeDatadogContext.featuresContext.toMutableMap().apply {
+                put(
+                    TracingFeature.TRACING_FEATURE_NAME,
+                    mapOf(
+                        "context@$fakeOtherThreadName" to mapOf(
+                            "span_id" to fakeOtherThreadSpanId,
+                            "trace_id" to fakeOtherThreadTraceId
+                        )
+                    )
+                )
+            }
+        )
 
         // WHEN
         val log = testedLogGenerator.generateLog(
@@ -691,9 +686,13 @@ internal class DatadogLogGeneratorTest {
     }
 
     @Test
-    fun `M do nothing W required to bundle the trace information {AndroidTracer not registered}`() {
+    fun `M do nothing W required to bundle the trace information {no tracing feature context}`() {
         // GIVEN
-        GlobalTracer::class.java.setStaticValue("isRegistered", false)
+        fakeDatadogContext = fakeDatadogContext.copy(
+            featuresContext = fakeDatadogContext.featuresContext.toMutableMap().apply {
+                remove(TracingFeature.TRACING_FEATURE_NAME)
+            }
+        )
 
         // WHEN
         val log = testedLogGenerator.generateLog(
@@ -721,9 +720,6 @@ internal class DatadogLogGeneratorTest {
 
     @Test
     fun `M do nothing W not required to bundle the trace information`() {
-        // GIVEN
-        GlobalTracer::class.java.setStaticValue("isRegistered", false)
-
         // WHEN
         val log = testedLogGenerator.generateLog(
             fakeLevel,