Add session properties to native crashes from the associated session

embrace-io · Dec 4, 2024 · caf4088 · caf4088
1 parent a748086
commit caf4088
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 77 deletions.
diff --git a/...src/main/kotlin/io/embrace/android/embracesdk/internal/arch/schema/EmbraceAttributeExt.kt b/...src/main/kotlin/io/embrace/android/embracesdk/internal/arch/schema/EmbraceAttributeExt.kt
@@ -28,3 +28,5 @@ internal fun String.toEmbraceAttributeName(isPrivate: Boolean = false): String {
 }
 
 internal fun String.toSessionPropertyAttributeName(): String = EMBRACE_SESSION_PROPERTY_NAME_PREFIX + this
+
+internal fun String.isSessionPropertyAttributeName(): Boolean = startsWith(EMBRACE_SESSION_PROPERTY_NAME_PREFIX)
diff --git a/...oid-core/src/main/kotlin/io/embrace/android/embracesdk/internal/ndk/NativeCrashService.kt b/...oid-core/src/main/kotlin/io/embrace/android/embracesdk/internal/ndk/NativeCrashService.kt
@@ -1,6 +1,7 @@
 package io.embrace.android.embracesdk.internal.ndk
 
 import io.embrace.android.embracesdk.internal.payload.NativeCrashData
+import io.opentelemetry.api.common.AttributeKey
 
 /**
  * Service to retrieve and delivery native crash data
@@ -21,7 +22,11 @@ interface NativeCrashService {
     /**
      * Send the given native crash
      */
-    fun sendNativeCrash(nativeCrash: NativeCrashData)
+    fun sendNativeCrash(
+        nativeCrash: NativeCrashData,
+        sessionProperties: Map<String, String>,
+        metadata: Map<AttributeKey<String>, String> = emptyMap(),
+    )
 
     /**
      * Delete the data files associated with all the native crashes that have been recorded by the SDK

diff --git a/...ndroid-core/src/main/kotlin/io/embrace/android/embracesdk/internal/payload/EnvelopeExt.kt b/...ndroid-core/src/main/kotlin/io/embrace/android/embracesdk/internal/payload/EnvelopeExt.kt
@@ -2,6 +2,7 @@ package io.embrace.android.embracesdk.internal.payload
 
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType
 import io.embrace.android.embracesdk.internal.spans.findAttributeValue
+import io.embrace.android.embracesdk.internal.spans.getSessionProperties
 import io.embrace.android.embracesdk.internal.spans.hasFixedAttribute
 import io.opentelemetry.semconv.incubating.SessionIncubatingAttributes
 
@@ -13,3 +14,7 @@ fun Envelope<SessionPayload>.getSessionSpan(): Span? {
 fun Envelope<SessionPayload>.getSessionId(): String? {
     return getSessionSpan()?.attributes?.findAttributeValue(SessionIncubatingAttributes.SESSION_ID.key)
 }
+
+fun Envelope<SessionPayload>.getSessionProperties(): Map<String, String> {
+    return getSessionSpan()?.getSessionProperties() ?: emptyMap()
+}
diff --git a/...lin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt b/...lin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt
@@ -17,6 +17,7 @@ import io.embrace.android.embracesdk.internal.payload.NativeCrashData
 import io.embrace.android.embracesdk.internal.payload.SessionPayload
 import io.embrace.android.embracesdk.internal.payload.Span
 import io.embrace.android.embracesdk.internal.payload.getSessionId
+import io.embrace.android.embracesdk.internal.payload.getSessionProperties
 import io.embrace.android.embracesdk.internal.payload.getSessionSpan
 import io.embrace.android.embracesdk.internal.payload.toFailedSpan
 import io.embrace.android.embracesdk.internal.serialization.PlatformSerializer
@@ -36,68 +37,82 @@ internal class PayloadResurrectionServiceImpl(
 
     override fun resurrectOldPayloads(nativeCrashServiceProvider: Provider<NativeCrashService?>) {
         val nativeCrashService = nativeCrashServiceProvider()
-        val nativeCrashes = nativeCrashService
-            ?.getNativeCrashes()
-            ?.associateBy { it.sessionId }
-            ?.apply {
-                values.forEach { nativeCrash ->
-                    nativeCrashService.sendNativeCrash(nativeCrash)
-                }
-            } ?: emptyMap()
+        val undeliveredPayloads = cacheStorageService.getUndeliveredPayloads()
+        val nativeCrashes = nativeCrashService?.getNativeCrashes()?.associateBy { it.sessionId } ?: emptyMap()
 
-        cacheStorageService
-            .getUndeliveredPayloads()
-            .forEach { deadProcessPayloadMetadata ->
-                deadProcessPayloadMetadata.processUndeliveredPayload(nativeCrashes::get)
-            }
+        processUndeliveredPayload(undeliveredPayloads, nativeCrashes, nativeCrashService)
         nativeCrashService?.deleteAllNativeCrashes()
     }
 
     /**
      * Load and modify the given incomplete payload envelope and send the result to the [IntakeService] for delivery.
      * Resurrected payloads sent to the [IntakeService] will be deleted.
      */
-    private fun StoredTelemetryMetadata.processUndeliveredPayload(nativeCrashProvider: (String) -> NativeCrashData?) {
-        val result = runCatching {
-            val resurrectedPayload = when (envelopeType) {
-                SupportedEnvelopeType.SESSION -> {
-                    val deadSession = serializer.fromJson<Envelope<SessionPayload>>(
-                        inputStream = GZIPInputStream(cacheStorageService.loadPayloadAsStream(this)),
-                        type = envelopeType.serializedType
-                    )
+    private fun processUndeliveredPayload(
+        payloadMetadata: List<StoredTelemetryMetadata>,
+        nativeCrashes: Map<String, NativeCrashData>,
+        nativeCrashService: NativeCrashService?
+    ) {
+        val processedCrashes = mutableSetOf<NativeCrashData>()
+        payloadMetadata.forEach { payload ->
+            val result = runCatching {
+                with(payload) {
+                    val resurrectedPayload = when (envelopeType) {
+                        SupportedEnvelopeType.SESSION -> {
+                            val deadSession = serializer.fromJson<Envelope<SessionPayload>>(
+                                inputStream = GZIPInputStream(cacheStorageService.loadPayloadAsStream(this)),
+                                type = envelopeType.serializedType
+                            )
+
+                            val sessionId = deadSession.getSessionId()
+                            val nativeCrash = if (sessionId != null) {
+                                nativeCrashes[sessionId]?.apply {
+                                    processedCrashes.add(this)
+                                    nativeCrashService?.sendNativeCrash(
+                                        nativeCrash = this,
+                                        sessionProperties = deadSession.getSessionProperties()
+                                    )
+                                }
+                            } else {
+                                null
+                            }
 
-                    val nativeCrash = deadSession.getSessionId()?.run {
-                        nativeCrashProvider(this)
+                            deadSession.resurrectSession(nativeCrash)
+                                ?: throw IllegalArgumentException(
+                                    "Session resurrection failed. Payload does not contain exactly one session span."
+                                )
+                        }
+
+                        else -> null
                     }
 
-                    deadSession.resurrectSession(nativeCrash)
-                        ?: throw IllegalArgumentException(
-                            "Session resurrection failed. Payload does not contain exactly one session span."
+                    if (resurrectedPayload != null) {
+                        intakeService.take(
+                            intake = resurrectedPayload,
+                            metadata = copy(complete = true)
                         )
+                    }
                 }
-                else -> null
             }
 
-            if (resurrectedPayload != null) {
-                intakeService.take(
-                    intake = resurrectedPayload,
-                    metadata = copy(complete = true)
+            if (result.isSuccess) {
+                cacheStorageService.delete(payload)
+            } else {
+                val exception = IllegalStateException(
+                    "Resurrecting and sending incomplete payloads from previous app launches failed.",
+                    result.exceptionOrNull()
+                )
+
+                logger.trackInternalError(
+                    type = InternalErrorType.PAYLOAD_RESURRECTION_FAIL,
+                    throwable = exception
                 )
             }
         }
-
-        if (result.isSuccess) {
-            cacheStorageService.delete(this)
-        } else {
-            val exception = IllegalStateException(
-                "Resurrecting and sending incomplete payloads from previous app launches failed.",
-                result.exceptionOrNull()
-            )
-
-            logger.trackInternalError(
-                type = InternalErrorType.PAYLOAD_RESURRECTION_FAIL,
-                throwable = exception
-            )
+        if (nativeCrashService != null) {
+            nativeCrashes.values.filterNot { processedCrashes.contains(it) }.forEach { nativeCrash ->
+                nativeCrashService.sendNativeCrash(nativeCrash, emptyMap())
+            }
         }
     }
 

diff --git a/...id-core/src/main/kotlin/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt b/...id-core/src/main/kotlin/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt
@@ -4,6 +4,7 @@ import io.embrace.android.embracesdk.internal.arch.schema.EmbraceAttributeKey
 import io.embrace.android.embracesdk.internal.arch.schema.FixedAttribute
 import io.embrace.android.embracesdk.internal.arch.schema.toSessionPropertyAttributeName
 import io.embrace.android.embracesdk.internal.payload.Attribute
+import io.embrace.android.embracesdk.internal.payload.Span
 import io.embrace.android.embracesdk.internal.payload.SpanEvent
 import io.embrace.android.embracesdk.internal.spans.EmbraceSpanLimits.isAttributeValid
 import io.embrace.android.embracesdk.internal.utils.isBlankish
@@ -46,7 +47,7 @@ fun AttributesBuilder.fromMap(attributes: Map<String, String>, internal: Boolean
     return this
 }
 
-fun io.embrace.android.embracesdk.internal.payload.Span.hasFixedAttribute(fixedAttribute: FixedAttribute): Boolean {
+fun Span.hasFixedAttribute(fixedAttribute: FixedAttribute): Boolean {
     return fixedAttribute.value == attributes?.singleOrNull { it.key == fixedAttribute.key.name }?.data
 }
 
@@ -62,10 +63,14 @@ fun SpanEvent.hasFixedAttribute(fixedAttribute: FixedAttribute): Boolean =
 
 fun List<Attribute>.findAttributeValue(key: String): String? = singleOrNull { it.key == key }?.data
 
-fun io.embrace.android.embracesdk.internal.payload.Span.getSessionProperty(key: String): String? {
+fun Span.getSessionProperty(key: String): String? {
     return attributes?.findAttributeValue(key.toSessionPropertyAttributeName())
 }
 
+@Suppress("UNCHECKED_CAST")
+fun Span.getSessionProperties(): Map<String, String> =
+    attributes?.filter { it.key != null && it.data != null }?.associate { it.key to it.data } as Map<String, String>
+
 fun Map<String, String>.hasFixedAttribute(fixedAttribute: FixedAttribute): Boolean =
     this[fixedAttribute.key.name] == fixedAttribute.value
 
@@ -84,7 +89,7 @@ private fun String.isValidLongValueAttribute(): Boolean = longValueAttributes.co
 
 private val longValueAttributes: Set<String> = setOf(ExceptionAttributes.EXCEPTION_STACKTRACE.key)
 
-fun StatusCode.toStatus(): io.embrace.android.embracesdk.internal.payload.Span.Status {
+fun StatusCode.toStatus(): Span.Status {
     return when (this) {
         StatusCode.UNSET -> io.embrace.android.embracesdk.internal.payload.Span.Status.UNSET
         StatusCode.OK -> io.embrace.android.embracesdk.internal.payload.Span.Status.OK

diff --git a/...io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImplTest.kt b/...io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImplTest.kt
@@ -249,8 +249,8 @@ class PayloadResurrectionServiceImplTest {
         }
 
         assertEquals(2, nativeCrashService.nativeCrashesSent.size)
-        assertEquals(deadSessionCrashData, nativeCrashService.nativeCrashesSent.first())
-        assertEquals(earlierSessionCrashData, nativeCrashService.nativeCrashesSent.last())
+        assertEquals(deadSessionCrashData, nativeCrashService.nativeCrashesSent.first().first)
+        assertEquals(earlierSessionCrashData, nativeCrashService.nativeCrashesSent.last().first)
     }
 
     private fun Envelope<SessionPayload>.resurrectPayload() {

diff --git a/...c/main/kotlin/io/embrace/android/embracesdk/internal/injection/NativeFeatureModuleImpl.kt b/...c/main/kotlin/io/embrace/android/embracesdk/internal/injection/NativeFeatureModuleImpl.kt
@@ -57,7 +57,6 @@ internal class NativeFeatureModuleImpl(
             null
         } else {
             NativeCrashDataSourceImpl(
-                sessionPropertiesService = essentialServiceModule.sessionPropertiesService,
                 nativeCrashProcessor = nativeCoreModule.processor,
                 preferencesService = androidServicesModule.preferencesService,
                 logWriter = essentialServiceModule.logWriter,

diff --git a/...s/src/main/kotlin/io/embrace/android/embracesdk/internal/ndk/NativeCrashDataSourceImpl.kt b/...s/src/main/kotlin/io/embrace/android/embracesdk/internal/ndk/NativeCrashDataSourceImpl.kt
@@ -7,18 +7,17 @@ import io.embrace.android.embracesdk.internal.arch.limits.NoopLimitStrategy
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType
 import io.embrace.android.embracesdk.internal.arch.schema.SchemaType
 import io.embrace.android.embracesdk.internal.arch.schema.TelemetryAttributes
-import io.embrace.android.embracesdk.internal.capture.session.SessionPropertiesService
 import io.embrace.android.embracesdk.internal.config.ConfigService
 import io.embrace.android.embracesdk.internal.logging.EmbLogger
 import io.embrace.android.embracesdk.internal.opentelemetry.embCrashNumber
 import io.embrace.android.embracesdk.internal.payload.NativeCrashData
 import io.embrace.android.embracesdk.internal.prefs.PreferencesService
 import io.embrace.android.embracesdk.internal.serialization.PlatformSerializer
 import io.embrace.android.embracesdk.internal.spans.toOtelSeverity
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.semconv.incubating.SessionIncubatingAttributes
 
 internal class NativeCrashDataSourceImpl(
-    private val sessionPropertiesService: SessionPropertiesService,
     private val nativeCrashProcessor: NativeCrashProcessor,
     private val preferencesService: PreferencesService,
     private val logWriter: LogWriter,
@@ -32,38 +31,38 @@ internal class NativeCrashDataSourceImpl(
 ) {
     override fun getAndSendNativeCrash(): NativeCrashData? {
         return nativeCrashProcessor.getLatestNativeCrash()?.apply {
-            sendNativeCrash(this)
+            sendNativeCrash(nativeCrash = this, sessionProperties = emptyMap())
         }
     }
 
     override fun getNativeCrashes(): List<NativeCrashData> = nativeCrashProcessor.getNativeCrashes()
 
-    override fun sendNativeCrash(nativeCrash: NativeCrashData) {
+    override fun sendNativeCrash(
+        nativeCrash: NativeCrashData,
+        sessionProperties: Map<String, String>,
+        metadata: Map<AttributeKey<String>, String>,
+    ) {
         val nativeCrashNumber = preferencesService.incrementAndGetNativeCrashNumber()
         val crashAttributes = TelemetryAttributes(
             configService = configService,
-            sessionPropertiesProvider = sessionPropertiesService::getProperties
+            sessionPropertiesProvider = { sessionProperties }
         )
         crashAttributes.setAttribute(
             key = SessionIncubatingAttributes.SESSION_ID,
             value = nativeCrash.sessionId,
             keepBlankishValues = false,
         )
 
+        metadata.forEach { attribute ->
+            crashAttributes.setAttribute(attribute.key, attribute.value)
+        }
+
         crashAttributes.setAttribute(
             key = embCrashNumber,
             value = nativeCrashNumber.toString(),
             keepBlankishValues = false,
         )
 
-//        nativeCrash.appState?.let { appState ->
-//            crashAttributes.setAttribute(
-//                key = embState,
-//                value = appState,
-//                keepBlankishValues = false,
-//            )
-//        }
-
         nativeCrash.crash?.let { crashData ->
             crashAttributes.setAttribute(
                 key = EmbType.System.NativeCrash.embNativeCrashException,

diff --git a/...src/test/java/io/embrace/android/embracesdk/internal/ndk/NativeCrashDataSourceImplTest.kt b/...src/test/java/io/embrace/android/embracesdk/internal/ndk/NativeCrashDataSourceImplTest.kt
@@ -8,14 +8,12 @@ import io.embrace.android.embracesdk.fakes.FakeOpenTelemetryLogger
 import io.embrace.android.embracesdk.fakes.FakePreferenceService
 import io.embrace.android.embracesdk.fakes.FakeProcessStateService
 import io.embrace.android.embracesdk.fakes.FakeSessionIdTracker
-import io.embrace.android.embracesdk.fakes.FakeSessionPropertiesService
 import io.embrace.android.embracesdk.fixtures.testNativeCrashData
 import io.embrace.android.embracesdk.internal.arch.destination.LogWriter
 import io.embrace.android.embracesdk.internal.arch.destination.LogWriterImpl
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType.System.NativeCrash.embNativeCrashException
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType.System.NativeCrash.embNativeCrashSymbols
-import io.embrace.android.embracesdk.internal.capture.session.SessionPropertiesService
 import io.embrace.android.embracesdk.internal.clock.nanosToMillis
 import io.embrace.android.embracesdk.internal.logging.EmbLogger
 import io.embrace.android.embracesdk.internal.logging.EmbLoggerImpl
@@ -38,7 +36,6 @@ import org.junit.Test
 
 internal class NativeCrashDataSourceImplTest {
 
-    private lateinit var sessionPropertiesService: SessionPropertiesService
     private lateinit var crashProcessor: FakeNativeCrashProcessor
     private lateinit var preferencesService: FakePreferenceService
     private lateinit var configService: FakeConfigService
@@ -54,7 +51,6 @@ internal class NativeCrashDataSourceImplTest {
 
     @Before
     fun setUp() {
-        sessionPropertiesService = FakeSessionPropertiesService()
         crashProcessor = FakeNativeCrashProcessor()
         preferencesService = FakePreferenceService()
         logger = EmbLoggerImpl()
@@ -72,7 +68,6 @@ internal class NativeCrashDataSourceImplTest {
         configService = FakeConfigService()
         serializer = EmbraceSerializer()
         nativeCrashDataSource = NativeCrashDataSourceImpl(
-            sessionPropertiesService = sessionPropertiesService,
             nativeCrashProcessor = crashProcessor,
             preferencesService = preferencesService,
             logWriter = logWriter,

diff --git a/...-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeNativeCrashService.kt b/...-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeNativeCrashService.kt
@@ -2,30 +2,35 @@ package io.embrace.android.embracesdk.fakes
 
 import io.embrace.android.embracesdk.internal.ndk.NativeCrashService
 import io.embrace.android.embracesdk.internal.payload.NativeCrashData
+import io.opentelemetry.api.common.AttributeKey
 import java.util.concurrent.ConcurrentLinkedQueue
 
 class FakeNativeCrashService : NativeCrashService {
 
-    val nativeCrashesSent = ConcurrentLinkedQueue<NativeCrashData>()
-    private val nativeCrashDataBlobs = mutableListOf<NativeCrashData>()
+    val nativeCrashesSent = ConcurrentLinkedQueue<Pair<NativeCrashData, Map<String, String>>>()
+    private val nativeCrashDataBlobs = mutableListOf<Pair<NativeCrashData, Map<String, String>>>()
     var checkAndSendNativeCrashInvocation: Int = 0
 
     override fun getAndSendNativeCrash(): NativeCrashData? {
         checkAndSendNativeCrashInvocation++
-        return nativeCrashDataBlobs.lastOrNull()
+        return nativeCrashDataBlobs.lastOrNull()?.first
     }
 
-    override fun getNativeCrashes(): List<NativeCrashData> = nativeCrashDataBlobs
+    override fun getNativeCrashes(): List<NativeCrashData> = nativeCrashDataBlobs.map { it.first }
 
-    override fun sendNativeCrash(nativeCrash: NativeCrashData) {
-        nativeCrashesSent.add(nativeCrash)
+    override fun sendNativeCrash(
+        nativeCrash: NativeCrashData,
+        sessionProperties: Map<String, String>,
+        metadata: Map<AttributeKey<String>, String>,
+    ) {
+        nativeCrashesSent.add(Pair(nativeCrash, metadata.mapKeys { it.value } + sessionProperties))
     }
 
     override fun deleteAllNativeCrashes() {
         nativeCrashDataBlobs.clear()
     }
 
-    fun addNativeCrashData(nativeCrashData: NativeCrashData) {
-        nativeCrashDataBlobs.add(nativeCrashData)
+    fun addNativeCrashData(nativeCrashData: NativeCrashData, metadata: Map<String, String> = emptyMap()) {
+        nativeCrashDataBlobs.add(Pair(nativeCrashData, metadata))
     }
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -28,3 +28,5 @@ internal fun String.toEmbraceAttributeName(isPrivate: Boolean = false): String {
		}

		internal fun String.toSessionPropertyAttributeName(): String = EMBRACE_SESSION_PROPERTY_NAME_PREFIX + this

		internal fun String.isSessionPropertyAttributeName(): Boolean = startsWith(EMBRACE_SESSION_PROPERTY_NAME_PREFIX)
Check warning on line 32 in embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/arch/schema/EmbraceAttributeExt.kt View check run for this annotation Codecov / codecov/patch embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/arch/schema/EmbraceAttributeExt.kt#L32 `Added line #L32 was not covered by tests`