Remove the population of emb.type from schema attributes (#931)

## Goal Remove the population `emb.type` via an object's schema attributes. Instead, do it when the schema is being processed, which is already being done for spans so that it can't be overwritten by users who have a reference to the span object. Correspondingly, changed where it's populated for logs and span events on the session span as well, which are the two other places that reads schema attributes. ## Testing Fixed unit tests looking at DataSource *Data objects' attributes for the emb.type, and instead look at the schema itself to check right value. Integration test correctly check the attributes and therefore are still passing.
embrace-io · Jun 7, 2024 · ca3da94 · ca3da94
2 parents 020205a + 137875c
commit ca3da94
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 31 deletions.
diff --git a/...android-sdk/src/main/java/io/embrace/android/embracesdk/arch/destination/LogWriterImpl.kt b/...android-sdk/src/main/java/io/embrace/android/embracesdk/arch/destination/LogWriterImpl.kt
@@ -46,8 +46,11 @@ internal class LogWriterImpl(
             builder.setFixedAttribute(PrivateSpan)
         }
 
-        logEventData.schemaType.attributes().forEach {
-            builder.setAttribute(AttributeKey.stringKey(it.key), it.value)
+        with(logEventData.schemaType) {
+            builder.setFixedAttribute(telemetryType)
+            attributes().forEach {
+                builder.setAttribute(AttributeKey.stringKey(it.key), it.value)
+            }
         }
 
         builder.emit()

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/SchemaType.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/SchemaType.kt
@@ -20,7 +20,7 @@ internal sealed class SchemaType(
 ) {
     protected abstract val schemaAttributes: Map<String, String>
 
-    private val commonAttributes: Map<String, String> = mutableMapOf(telemetryType.toEmbraceKeyValuePair()).apply {
+    private val commonAttributes: Map<String, String> = mutableMapOf<String, String>().apply {
         if (telemetryType.sendImmediately) {
             plusAssign(SendImmediately.toEmbraceKeyValuePair())
         }

diff --git a/...-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt b/...-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt
@@ -103,7 +103,7 @@ internal class CurrentSessionSpanImpl(
         return currentSession.addEvent(
             event.schemaType.fixedObjectName.toEmbraceObjectName(),
             event.spanStartTimeMs,
-            event.schemaType.attributes()
+            event.schemaType.attributes() + event.schemaType.telemetryType.toEmbraceKeyValuePair()
         )
     }
 

diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/AeiDataSourceImplTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/AeiDataSourceImplTest.kt
@@ -3,6 +3,7 @@ package io.embrace.android.embracesdk
 import android.app.ActivityManager
 import android.app.ApplicationExitInfo
 import com.google.common.util.concurrent.MoreExecutors
+import io.embrace.android.embracesdk.arch.schema.EmbType
 import io.embrace.android.embracesdk.capture.aei.AeiDataSourceImpl
 import io.embrace.android.embracesdk.config.remote.AppExitInfoConfig
 import io.embrace.android.embracesdk.config.remote.RemoteConfig
@@ -360,11 +361,10 @@ internal class AeiDataSourceImplTest {
 
     private fun getAeiLogAttrs(): Map<String, String> {
         val logEventData = logWriter.logEvents.single()
+        assertEquals(EmbType.System.Exit, logEventData.schemaType.telemetryType)
         assertEquals("", logEventData.schemaType.fixedObjectName)
         assertEquals(Severity.INFO, logEventData.severity)
 
-        return logEventData.schemaType.attributes().apply {
-            assertEquals("sys.exit", this["emb.type"])
-        }
+        return logEventData.schemaType.attributes()
     }
 }
diff --git a/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt b/...ndroid-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt
@@ -1,6 +1,7 @@
 package io.embrace.android.embracesdk.arch
 
 import io.embrace.android.embracesdk.arch.destination.LogEventData
+import io.embrace.android.embracesdk.arch.destination.SpanEventData
 import io.embrace.android.embracesdk.arch.destination.StartSpanData
 import io.embrace.android.embracesdk.arch.schema.EmbType
 import io.embrace.android.embracesdk.arch.schema.ErrorCodeAttribute
@@ -94,25 +95,16 @@ internal fun Span.assertSuccessful() {
 }
 
 /**
- * Assert [StartSpanData] is of type [telemetryType]
+ * Assert [SpanEventData] is of type [telemetryType]
  */
-internal fun StartSpanData.assertIsType(telemetryType: TelemetryType) = assertHasEmbraceAttribute(telemetryType)
+internal fun SpanEventData.assertIsType(telemetryType: TelemetryType) = assertEquals(telemetryType, schemaType.telemetryType)
 
 /**
- * Assert [StartSpanData] has the [FixedAttribute] defined by [fixedAttribute]
+ * Assert [StartSpanData] is of type [telemetryType]
  */
-internal fun StartSpanData.assertHasEmbraceAttribute(fixedAttribute: FixedAttribute) {
-    assertEquals(fixedAttribute.value, schemaType.attributes()[fixedAttribute.key.name])
-}
+internal fun StartSpanData.assertIsType(telemetryType: TelemetryType) = assertEquals(telemetryType, schemaType.telemetryType)
 
 /**
  * Assert [LogEventData] is of type [telemetryType]
  */
-internal fun LogEventData.assertIsType(telemetryType: TelemetryType) = assertHasEmbraceAttribute(telemetryType)
-
-/**
- * Assert [LogEventData] has the [FixedAttribute] defined by [fixedAttribute]
- */
-internal fun LogEventData.assertHasEmbraceAttribute(fixedAttribute: FixedAttribute) {
-    assertEquals(fixedAttribute.value, schemaType.attributes()[fixedAttribute.key.name])
-}
+internal fun LogEventData.assertIsType(telemetryType: TelemetryType) = assertEquals(telemetryType, schemaType.telemetryType)
diff --git a/...-sdk/src/test/java/io/embrace/android/embracesdk/capture/crash/CrashDataSourceImplTest.kt b/...-sdk/src/test/java/io/embrace/android/embracesdk/capture/crash/CrashDataSourceImplTest.kt
@@ -1,6 +1,8 @@
 package io.embrace.android.embracesdk.capture.crash
 
 import io.embrace.android.embracesdk.FakeNdkService
+import io.embrace.android.embracesdk.arch.assertIsType
+import io.embrace.android.embracesdk.arch.schema.EmbType
 import io.embrace.android.embracesdk.fakes.FakeAnrService
 import io.embrace.android.embracesdk.fakes.FakeConfigService
 import io.embrace.android.embracesdk.fakes.FakeCrashFileMarker
@@ -136,11 +138,12 @@ internal class CrashDataSourceImplTest {
         crashDataSource.logUnhandledJsException(localJsException)
         crashDataSource.handleCrash(testException)
 
-        val lastSentCrashAttributes = logWriter.logEvents.single().schemaType.attributes()
+        val logEvent = logWriter.logEvents.single()
+        logEvent.assertIsType(EmbType.System.ReactNativeCrash)
+        val lastSentCrashAttributes = logEvent.schemaType.attributes()
         assertEquals(1, anrService.forceAnrTrackingStopOnCrashCount)
         assertEquals(1, logWriter.logEvents.size)
         assertEquals(lastSentCrashAttributes["log.record.uid"], sessionOrchestrator.crashId)
-        assertEquals(lastSentCrashAttributes["emb.type"], "sys.android.react_native_crash")
         assertEquals(
             "{\"n\":\"NullPointerException\",\"" +
                 "m\":\"Null pointer exception occurred\",\"" +

diff --git a/...dk/src/test/java/io/embrace/android/embracesdk/capture/crumbs/BreadcrumbDataSourceTest.kt b/...dk/src/test/java/io/embrace/android/embracesdk/capture/crumbs/BreadcrumbDataSourceTest.kt
@@ -1,5 +1,6 @@
 package io.embrace.android.embracesdk.capture.crumbs
 
+import io.embrace.android.embracesdk.arch.assertIsType
 import io.embrace.android.embracesdk.arch.schema.EmbType
 import io.embrace.android.embracesdk.fakes.FakeConfigService
 import io.embrace.android.embracesdk.fakes.FakeCurrentSessionSpan
@@ -34,13 +35,10 @@ internal class BreadcrumbDataSourceTest {
     fun `add breadcrumb`() {
         source.logCustom("Hello, world!", 15000000000)
         with(writer.addedEvents.single()) {
-            assertEquals(EmbType.System.Breadcrumb, schemaType.telemetryType)
+            assertIsType(EmbType.System.Breadcrumb)
             assertEquals(15000000000.millisToNanos(), spanStartTimeMs)
             assertEquals(
-                mapOf(
-                    EmbType.System.Breadcrumb.toEmbraceKeyValuePair(),
-                    "message" to "Hello, world!"
-                ),
+                mapOf("message" to "Hello, world!"),
                 schemaType.attributes()
             )
         }

diff --git a/...src/test/java/io/embrace/android/embracesdk/capture/crumbs/TapBreadcrumbDataSourceTest.kt b/...src/test/java/io/embrace/android/embracesdk/capture/crumbs/TapBreadcrumbDataSourceTest.kt
@@ -40,7 +40,6 @@ internal class TapBreadcrumbDataSourceTest {
             assertEquals(15000000000.millisToNanos(), spanStartTimeMs)
             assertEquals(
                 mapOf(
-                    EmbType.Ux.Tap.toEmbraceKeyValuePair(),
                     "view.name" to "my-button-id",
                     "tap.type" to "tap",
                     "tap.coords" to "126,309"

diff --git a/...dk/src/test/java/io/embrace/android/embracesdk/capture/crumbs/WebViewUrlDataSourceTest.kt b/...dk/src/test/java/io/embrace/android/embracesdk/capture/crumbs/WebViewUrlDataSourceTest.kt
@@ -52,7 +52,6 @@ internal class WebViewUrlDataSourceTest {
             Assert.assertEquals(15000000000.millisToNanos(), spanStartTimeMs)
             Assert.assertEquals(
                 mapOf(
-                    EmbType.Ux.WebView.toEmbraceKeyValuePair(),
                     "webview.url" to "http://www.google.com?query=123"
                 ),
                 schemaType.attributes()
@@ -88,7 +87,6 @@ internal class WebViewUrlDataSourceTest {
             Assert.assertEquals(15000000000.millisToNanos(), spanStartTimeMs)
             Assert.assertEquals(
                 mapOf(
-                    EmbType.Ux.WebView.toEmbraceKeyValuePair(),
                     "webview.url" to "http://www.google.com"
                 ),
                 schemaType.attributes()