Merge pull request #1732 from embrace-io/remove-key-spans

Remove concept of key spans

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilder.kt

@@ -1,8 +1,6 @@
 package io.embrace.android.embracesdk.internal.spans
 
-import io.embrace.android.embracesdk.internal.arch.schema.EmbType
 import io.embrace.android.embracesdk.internal.arch.schema.FixedAttribute
-import io.embrace.android.embracesdk.internal.arch.schema.KeySpan
 import io.embrace.android.embracesdk.internal.arch.schema.PrivateSpan
 import io.embrace.android.embracesdk.internal.arch.schema.TelemetryType
 import io.embrace.android.embracesdk.spans.EmbraceSpan
@@ -71,26 +69,14 @@ class EmbraceSpanBuilder(
     fun setParentContext(context: Context) {
         parentContext = context
         sdkSpanBuilder.setParent(parentContext)
-        updateKeySpan()
     }
 
     fun setNoParent() {
         parentContext = Context.root()
         sdkSpanBuilder.setNoParent()
-        updateKeySpan()
     }
 
     fun setSpanKind(spanKind: SpanKind) {
         sdkSpanBuilder.setSpanKind(spanKind)
     }
-
-    private fun updateKeySpan() {
-        if (fixedAttributes.contains(EmbType.Performance.Default) || fixedAttributes.contains(EmbType.Performance.UiLoad)) {
-            if (getParentSpan() == null) {
-                fixedAttributes.add(KeySpan)
-            } else {
-                fixedAttributes.remove(KeySpan)
-            }
-        }
-    }
 }

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/payload/SpanMapperTest.kt

@@ -1,7 +1,6 @@
 package io.embrace.android.embracesdk.internal.payload
 
 import io.embrace.android.embracesdk.arch.assertError
-import io.embrace.android.embracesdk.arch.assertIsKeySpan
 import io.embrace.android.embracesdk.arch.assertIsTypePerformance
 import io.embrace.android.embracesdk.arch.assertNotPrivateSpan
 import io.embrace.android.embracesdk.arch.assertSuccessful
@@ -35,7 +34,6 @@ internal class SpanMapperTest {
         // test attributes
         output.assertSuccessful()
         output.assertIsTypePerformance()
-        output.assertIsKeySpan()
         output.assertNotPrivateSpan()
         checkNotNull(output.attributes).forEach {
             assertEquals(input.attributes[it.key], it.data)
@@ -58,7 +56,6 @@ internal class SpanMapperTest {
         assertEquals(snapshot.events?.single(), failedSpan.events?.single())
         failedSpan.assertError(ErrorCode.FAILURE)
         failedSpan.assertIsTypePerformance()
-        failedSpan.assertIsKeySpan()
         failedSpan.assertNotPrivateSpan()
         val attributesOfFailedSpan = failedSpan.attributes?.associate { it.key to it.data } ?: emptyMap()
         checkNotNull(snapshot.attributes).forEach {

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImplTests.kt

@@ -3,7 +3,6 @@ package io.embrace.android.embracesdk.internal.spans
 import io.embrace.android.embracesdk.arch.assertError
 import io.embrace.android.embracesdk.arch.assertHasEmbraceAttribute
 import io.embrace.android.embracesdk.arch.assertIsType
-import io.embrace.android.embracesdk.arch.assertNotKeySpan
 import io.embrace.android.embracesdk.assertions.assertEmbraceSpanData
 import io.embrace.android.embracesdk.assertions.findEventOfType
 import io.embrace.android.embracesdk.fakes.FakeClock
@@ -235,7 +234,6 @@ internal class CurrentSessionSpanImplTests {
                 assertEquals("emb-session", name)
                 assertIsType(EmbType.Ux.Session)
                 assertError(ErrorCode.FAILURE)
-                assertNotKeySpan()
                 assertHasEmbraceAttribute(cause)
             }
 
@@ -278,8 +276,7 @@ internal class CurrentSessionSpanImplTests {
             expectedErrorCode = ErrorCode.FAILURE,
             expectedCustomAttributes = mapOf(
                 EmbType.Performance.Default.toEmbraceKeyValuePair()
-            ),
-            key = true
+            )
         )
 
         assertEquals(0, spanSink.completedSpans().size)

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilderTest.kt

@@ -6,15 +6,11 @@ import io.embrace.android.embracesdk.fakes.FakeSpan
 import io.embrace.android.embracesdk.fakes.FakeTracer
 import io.embrace.android.embracesdk.fixtures.fakeContextKey
 import io.embrace.android.embracesdk.internal.arch.schema.EmbType
-import io.embrace.android.embracesdk.internal.arch.schema.KeySpan
 import io.embrace.android.embracesdk.internal.arch.schema.PrivateSpan
 import io.opentelemetry.api.trace.Span
 import io.opentelemetry.api.trace.SpanKind
 import io.opentelemetry.context.Context
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertNotNull
-import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -44,7 +40,6 @@ internal class EmbraceSpanBuilderTest {
         with(spanBuilder.getFixedAttributes().toSet()) {
             assertTrue(contains(PrivateSpan))
             assertTrue(contains(EmbType.Performance.Default))
-            assertTrue(contains(KeySpan))
         }
         assertEquals("emb-test", spanBuilder.spanName)
         spanBuilder.startSpan(startTime).assertFakeSpanBuilder(
@@ -67,9 +62,6 @@ internal class EmbraceSpanBuilderTest {
         val parent = FakePersistableEmbraceSpan.started()
         val parentContext = checkNotNull(parent.asNewContext()?.with(fakeContextKey, "value"))
         spanBuilder.setParentContext(parentContext)
-        with(spanBuilder.getFixedAttributes().toSet()) {
-            assertFalse(contains(KeySpan))
-        }
         val startTime = clock.now()
         spanBuilder.startSpan(startTime).assertFakeSpanBuilder(
             expectedName = "test",
@@ -92,9 +84,7 @@ internal class EmbraceSpanBuilderTest {
             parentSpan = parent,
         )
 
-        assertNull(spanBuilder.getFixedAttributes().find { it == KeySpan })
         spanBuilder.setNoParent()
-        assertNotNull(spanBuilder.getFixedAttributes().find { it == KeySpan })
         with(spanBuilder.startSpan(startTime)) {
             assertFakeSpanBuilder(
                 expectedName = "test",
@@ -111,9 +101,7 @@ internal class EmbraceSpanBuilderTest {
             parentSpan = parent,
         )
 
-        assertNull(uxSpanBuilder.getFixedAttributes().find { it == KeySpan })
         uxSpanBuilder.setNoParent()
-        assertNull(uxSpanBuilder.getFixedAttributes().find { it == KeySpan })
         with(uxSpanBuilder.startSpan(startTime)) {
             assertFakeSpanBuilder(
                 expectedName = "ux-test",
@@ -155,31 +143,6 @@ internal class EmbraceSpanBuilderTest {
         assertEquals("test-value", spanBuilder.getCustomAttributes()["test-key"])
     }
 
-    @Test
-    fun `perf and ui_load spans are key spans if parent is null`() {
-        val perfSpanBuilder = EmbraceSpanBuilder(
-            tracer = tracer,
-            name = "test",
-            telemetryType = EmbType.Performance.Default,
-            internal = false,
-            private = false,
-            parentSpan = null,
-        )
-
-        assertTrue(perfSpanBuilder.getFixedAttributes().toSet().contains(KeySpan))
-
-        val uiLoadSpanBuilder = EmbraceSpanBuilder(
-            tracer = tracer,
-            name = "test",
-            telemetryType = EmbType.Performance.UiLoad,
-            internal = false,
-            private = false,
-            parentSpan = null,
-        )
-
-        assertTrue(uiLoadSpanBuilder.getFixedAttributes().toSet().contains(KeySpan))
-    }
-
     @Test
     fun `context value propagated even if it does not context a span`() {
         val fakeRootContext = Context.root().with(fakeContextKey, "fake-value")

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImplTest.kt

@@ -508,7 +508,7 @@ internal class EmbraceSpanImplTest {
         expectedType: EmbType = EmbType.Performance.Default,
         expectedStatus: Span.Status = Span.Status.UNSET,
         eventCount: Int = 0,
-        expectedEmbraceAttributes: Int = 2,
+        expectedEmbraceAttributes: Int = 1,
         expectedCustomAttributeCount: Int = 0,
         isPrivate: Boolean = false,
     ) {

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceTracerTest.kt

@@ -1,9 +1,7 @@
 package io.embrace.android.embracesdk.internal.spans
 
 import io.embrace.android.embracesdk.arch.assertError
-import io.embrace.android.embracesdk.arch.assertIsKeySpan
 import io.embrace.android.embracesdk.arch.assertIsTypePerformance
-import io.embrace.android.embracesdk.arch.assertNotKeySpan
 import io.embrace.android.embracesdk.arch.assertNotPrivateSpan
 import io.embrace.android.embracesdk.arch.assertSuccessful
 import io.embrace.android.embracesdk.fakes.FakeClock
@@ -13,6 +11,7 @@ import io.embrace.android.embracesdk.internal.clock.millisToNanos
 import io.embrace.android.embracesdk.internal.clock.nanosToMillis
 import io.embrace.android.embracesdk.spans.EmbraceSpanEvent
 import io.embrace.android.embracesdk.spans.ErrorCode
+import io.opentelemetry.api.trace.SpanId
 import io.opentelemetry.api.trace.StatusCode
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
@@ -356,9 +355,9 @@ internal class EmbraceTracerTest {
         assertEquals(name, currentSpan.name)
         currentSpan.assertIsTypePerformance()
         if (traceRoot) {
-            currentSpan.assertIsKeySpan()
+            assertEquals(SpanId.getInvalid(), currentSpan.parentSpanId)
         } else {
-            currentSpan.assertNotKeySpan()
+            assertNotNull(currentSpan.parentSpanId)
         }
         if (errorCode == null) {
             currentSpan.assertSuccessful()

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/InternalTracerTest.kt

@@ -1,9 +1,7 @@
 package io.embrace.android.embracesdk.internal.spans
 
 import io.embrace.android.embracesdk.arch.assertError
-import io.embrace.android.embracesdk.arch.assertIsKeySpan
 import io.embrace.android.embracesdk.arch.assertIsTypePerformance
-import io.embrace.android.embracesdk.arch.assertNotKeySpan
 import io.embrace.android.embracesdk.arch.assertNotPrivateSpan
 import io.embrace.android.embracesdk.arch.assertSuccessful
 import io.embrace.android.embracesdk.fakes.FakeClock
@@ -61,7 +59,7 @@ internal class InternalTracerTest {
         val childEndTimeMs = clock.now() - 1L
         assertTrue(internalTracer.stopSpan(spanId = spanId, endTimeMs = childEndTimeMs))
         assertFalse(internalTracer.addSpanAttribute(spanId = spanId, key = "fail", value = "value"))
-        with(verifyPublicSpan(name = "test-span", traceRoot = false)) {
+        with(verifyPublicSpan(name = "test-span")) {
             assertEquals("valuez", attributes["keyz"])
             assertEquals(childStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(childEndTimeMs, endTimeNanos.nanosToMillis())
@@ -211,7 +209,7 @@ internal class InternalTracerTest {
             )
         )
 
-        with(verifyPublicSpan(expectedName, true, ErrorCode.FAILURE)) {
+        with(verifyPublicSpan(expectedName, ErrorCode.FAILURE)) {
             assertEquals(expectedStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(expectedEndTimeMs, endTimeNanos.nanosToMillis())
             assertEquals(StatusCode.ERROR, status)
@@ -234,7 +232,7 @@ internal class InternalTracerTest {
             )
         )
 
-        with(verifyPublicSpan(expectedName, false)) {
+        with(verifyPublicSpan(expectedName)) {
             assertEquals(expectedStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(expectedEndTimeMs, endTimeNanos.nanosToMillis())
         }
@@ -269,7 +267,6 @@ internal class InternalTracerTest {
             assertEquals(expectedStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(expectedEndTimeMs, endTimeNanos.nanosToMillis())
             assertIsTypePerformance()
-            assertIsKeySpan()
             expectedAttributes.forEach {
                 assertEquals(it.value, attributes[it.key])
             }
@@ -305,19 +302,13 @@ internal class InternalTracerTest {
 
     private fun verifyPublicSpan(
         name: String,
-        traceRoot: Boolean = true,
         errorCode: ErrorCode? = null,
     ): EmbraceSpanData {
         val currentSpans = spanSink.completedSpans()
         assertEquals(1, currentSpans.size)
         val currentSpan = currentSpans[0]
         assertEquals(name, currentSpan.name)
         currentSpan.assertIsTypePerformance()
-        if (traceRoot) {
-            currentSpan.assertIsKeySpan()
-        } else {
-            currentSpan.assertNotKeySpan()
-        }
         if (errorCode == null) {
             currentSpan.assertSuccessful()
         } else {

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/SpanServiceImplTest.kt

@@ -2,11 +2,9 @@ package io.embrace.android.embracesdk.internal.spans
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.embrace.android.embracesdk.arch.assertError
-import io.embrace.android.embracesdk.arch.assertIsKeySpan
 import io.embrace.android.embracesdk.arch.assertIsPrivateSpan
 import io.embrace.android.embracesdk.arch.assertIsType
 import io.embrace.android.embracesdk.arch.assertIsTypePerformance
-import io.embrace.android.embracesdk.arch.assertNotKeySpan
 import io.embrace.android.embracesdk.arch.assertNotPrivateSpan
 import io.embrace.android.embracesdk.fakes.FakeClock
 import io.embrace.android.embracesdk.fakes.injection.FakeInitModule
@@ -69,7 +67,6 @@ internal class SpanServiceImplTest {
         with(verifyAndReturnSoleCompletedSpan("emb-test-span")) {
             assertEquals(SpanId.getInvalid(), parentSpanId)
             assertIsTypePerformance()
-            assertIsKeySpan()
             assertNotPrivateSpan()
         }
     }
@@ -118,7 +115,6 @@ internal class SpanServiceImplTest {
         with(verifyAndReturnSoleCompletedSpan("emb-test-span")) {
             assertEquals(SpanId.getInvalid(), parentSpanId)
             assertIsTypePerformance()
-            assertIsKeySpan()
         }
     }
 
@@ -141,7 +137,6 @@ internal class SpanServiceImplTest {
             assertEquals("emb-child-span", name)
             assertEquals(childSpan.spanId, spanId)
             assertEquals(childSpan.traceId, traceId)
-            assertNotKeySpan()
             assertNotPrivateSpan()
         }
 
@@ -150,7 +145,6 @@ internal class SpanServiceImplTest {
             assertEquals(SpanId.getInvalid(), parentSpanId)
             assertEquals(parentSpan.spanId, spanId)
             assertEquals(parentSpan.traceId, traceId)
-            assertIsKeySpan()
             assertNotPrivateSpan()
         }
     }
@@ -211,7 +205,6 @@ internal class SpanServiceImplTest {
             assertEquals(childStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(childSpanEndTimeMs, endTimeNanos.nanosToMillis())
             assertNotPrivateSpan()
-            assertNotKeySpan()
             assertIsType(EmbType.Ux.View)
         }
         clock.tick(10)
@@ -222,7 +215,6 @@ internal class SpanServiceImplTest {
             assertEquals(parentStartTime, startTimeNanos.nanosToMillis())
             assertEquals(parentEndTime, endTimeNanos.nanosToMillis())
             assertNotPrivateSpan()
-            assertIsKeySpan()
         }
     }
 
@@ -256,7 +248,6 @@ internal class SpanServiceImplTest {
             assertEquals(expectedEndTimeMs, endTimeNanos.nanosToMillis())
             assertIsTypePerformance()
             assertEquals(SpanId.getInvalid(), parentSpanId)
-            assertIsKeySpan()
             assertNotPrivateSpan()
             expectedAttributes.forEach {
                 assertEquals(it.value, attributes[it.key])
@@ -284,7 +275,6 @@ internal class SpanServiceImplTest {
         with(verifyAndReturnSoleCompletedSpan("emb-$expectedName")) {
             assertEquals(expectedStartTimeMs, startTimeNanos.nanosToMillis())
             assertEquals(expectedEndTimeMs, endTimeNanos.nanosToMillis())
-            assertNotKeySpan()
             assertNotPrivateSpan()
         }
         assertTrue(parentSpan.stop())
@@ -383,7 +373,6 @@ internal class SpanServiceImplTest {
         with(verifyAndReturnSoleCompletedSpan("emb-test-span")) {
             assertEquals(SpanId.getInvalid(), parentSpanId)
             assertIsTypePerformance()
-            assertIsKeySpan()
             assertNotPrivateSpan()
         }
     }
@@ -405,7 +394,6 @@ internal class SpanServiceImplTest {
 
         with(currentSpans[0]) {
             assertEquals("emb-child-span", name)
-            assertNotKeySpan()
             assertNotPrivateSpan()
         }
     }

embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/activity/UiLoadTraceEmitterTest.kt

@@ -145,7 +145,6 @@ internal class UiLoadTraceEmitterTest {
                 expectedStartTimeMs = timestamps.first,
                 expectedEndTimeMs = timestamps.second,
                 expectedParentId = SpanId.getInvalid(),
-                key = true,
             )
 
             val events = timestamps.third