From 88257cf2816f740142517657b8ace16518c9434b Mon Sep 17 00:00:00 2001 From: bidetofevil Date: Thu, 21 Mar 2024 00:24:20 -0700 Subject: [PATCH] Create a new wrapper for SpanBuilder and persist all attributes in a snapshot there --- .../arch/schema/EmbraceAttribute.kt | 6 ++ .../internal/spans/CurrentSessionSpanImpl.kt | 19 ++--- .../internal/spans/EmbraceExtensions.kt | 76 +++---------------- .../internal/spans/EmbraceSpanBuilder.kt | 68 +++++++++++++++++ .../internal/spans/EmbraceSpanImpl.kt | 22 ++---- .../internal/spans/SpanServiceImpl.kt | 9 +-- .../arch/EmbraceAttributeExtensions.kt | 36 ++++++++- .../internal/spans/EmbraceSpanImplTest.kt | 31 +++++--- 8 files changed, 159 insertions(+), 108 deletions(-) create mode 100644 embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilder.kt diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/EmbraceAttribute.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/EmbraceAttribute.kt index 569e6a1fcf..1daffa8ab3 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/EmbraceAttribute.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/arch/schema/EmbraceAttribute.kt @@ -1,5 +1,6 @@ package io.embrace.android.embracesdk.arch.schema +import io.embrace.android.embracesdk.internal.payload.Attribute import io.embrace.android.embracesdk.internal.spans.toEmbraceAttributeName /** @@ -25,4 +26,9 @@ internal interface EmbraceAttribute { * Return attribute as a key-value pair appropriate to use as an OpenTelemetry attribute */ fun toOTelKeyValuePair() = Pair(otelAttributeName(), attributeValue) + + /** + * Return attribute as [Attribute] + */ + fun toAttributePayload() = Attribute(otelAttributeName(), attributeValue) } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt index e5be3b780e..5167356504 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImpl.kt @@ -12,7 +12,6 @@ import io.embrace.android.embracesdk.spans.EmbraceSpan import io.embrace.android.embracesdk.telemetry.TelemetryService import io.opentelemetry.api.trace.Tracer import io.opentelemetry.sdk.common.Clock -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference @@ -108,21 +107,17 @@ internal class CurrentSessionSpanImpl( traceCount.set(0) val spanName = "session".toEmbraceSpanName() - val spanBuilder = createEmbraceSpanBuilder( - tracer = tracerSupplier(), - name = spanName, - type = EmbType.Ux.Session - ) - .setNoParent() - .setStartTimestamp(startTimeMs, TimeUnit.MILLISECONDS) - return EmbraceSpanImpl( spanName = spanName, openTelemetryClock = openTelemetryClock, - spanBuilder = spanBuilder, - sessionSpan = true + spanBuilder = tracerSupplier().embraceSpanBuilder( + name = spanName, + type = EmbType.Ux.Session, + internal = true, + parent = null + ) ).apply { - start() + start(startTimeMs = startTimeMs) } } } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt index e70517d6c0..f441bb1dd7 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceExtensions.kt @@ -1,8 +1,6 @@ package io.embrace.android.embracesdk.internal.spans import io.embrace.android.embracesdk.arch.schema.EmbraceAttribute -import io.embrace.android.embracesdk.arch.schema.KeySpan -import io.embrace.android.embracesdk.arch.schema.PrivateSpan import io.embrace.android.embracesdk.arch.schema.TelemetryType import io.embrace.android.embracesdk.spans.EmbraceSpan import io.embrace.android.embracesdk.spans.EmbraceSpanEvent @@ -13,7 +11,6 @@ import io.opentelemetry.api.trace.Span import io.opentelemetry.api.trace.SpanBuilder import io.opentelemetry.api.trace.StatusCode import io.opentelemetry.api.trace.Tracer -import io.opentelemetry.context.Context import java.util.concurrent.TimeUnit /** @@ -45,70 +42,18 @@ private const val SEQUENCE_ID_ATTRIBUTE_NAME = EMBRACE_ATTRIBUTE_NAME_PREFIX + " /** * Creates a new [SpanBuilder] that marks the resulting span as private if [internal] is true */ -internal fun Tracer.embraceSpanBuilder(name: String, internal: Boolean): SpanBuilder { - return if (internal) { - spanBuilder(name).makePrivate() - } else { - spanBuilder(name) - } -} - -/** - * Create a [SpanBuilder] that will create a [Span] with the appropriate custom Embrace attributes - */ -internal fun createEmbraceSpanBuilder( - tracer: Tracer, - name: String, - type: TelemetryType, - internal: Boolean = true -): SpanBuilder = tracer.embraceSpanBuilder(name, internal).setEmbraceAttribute(type) - -/** - * Create a [SpanBuilder] that will create a root [Span] with the appropriate custom Embrace attributes - */ -internal fun createRootSpanBuilder( - tracer: Tracer, +internal fun Tracer.embraceSpanBuilder( name: String, type: TelemetryType, - internal: Boolean -): SpanBuilder = createEmbraceSpanBuilder(tracer = tracer, name = name, type = type, internal = internal).setNoParent() - -/** - * Sets an [EmbraceAttribute] on the given [SpanBuilder] and return it - */ -internal fun SpanBuilder.setEmbraceAttribute(embraceAttribute: EmbraceAttribute): SpanBuilder { - setAttribute(embraceAttribute.otelAttributeName(), embraceAttribute.attributeValue) - return this -} - -/** - * Mark the span generated by this builder as a [KeySpan] - */ -internal fun SpanBuilder.makeKey(): SpanBuilder { - setEmbraceAttribute(KeySpan) - return this -} - -/** - * Mark the span generated by this builder as [PrivateSpan] - */ -internal fun SpanBuilder.makePrivate(): SpanBuilder { - setEmbraceAttribute(PrivateSpan) - return this -} - -/** - * Extract the parent span from an [EmbraceSpan] and set it as the parent. It's a no-op if the parent has not been started yet. - */ -internal fun SpanBuilder.updateParent(parent: EmbraceSpan?): SpanBuilder { - if (parent == null) { - makeKey() - } else if (parent is EmbraceSpanImpl) { - parent.wrappedSpan()?.let { - setParent(Context.current().with(it)) - } + internal: Boolean, + parent: EmbraceSpan? = null +): EmbraceSpanBuilder { + val builder = EmbraceSpanBuilder(spanBuilder(name), type, parent) + return if (internal) { + builder.makePrivate() + } else { + builder } - return this } /** @@ -136,8 +81,9 @@ internal fun Span.setSequenceId(id: Long): Span { return this } -internal fun Span.setEmbraceAttribute(embraceAttribute: EmbraceAttribute) { +internal fun Span.setEmbraceAttribute(embraceAttribute: EmbraceAttribute): Span { setAttribute(embraceAttribute.otelAttributeName(), embraceAttribute.attributeValue) + return this } /** diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilder.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilder.kt new file mode 100644 index 0000000000..eada287076 --- /dev/null +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanBuilder.kt @@ -0,0 +1,68 @@ +package io.embrace.android.embracesdk.internal.spans + +import io.embrace.android.embracesdk.arch.schema.EmbType +import io.embrace.android.embracesdk.arch.schema.EmbraceAttribute +import io.embrace.android.embracesdk.arch.schema.KeySpan +import io.embrace.android.embracesdk.arch.schema.PrivateSpan +import io.embrace.android.embracesdk.arch.schema.TelemetryType +import io.embrace.android.embracesdk.spans.EmbraceSpan +import io.opentelemetry.api.trace.Span +import io.opentelemetry.api.trace.SpanBuilder +import io.opentelemetry.context.Context +import java.util.concurrent.TimeUnit + +internal class EmbraceSpanBuilder( + private val otelSpanBuilder: SpanBuilder, + telemetryType: TelemetryType, + parent: EmbraceSpan? +) { + val embraceAttributes = mutableListOf(telemetryType) + + /** + * Extract the parent span from an [EmbraceSpan] and set it as the parent + */ + init { + if (parent == null) { + otelSpanBuilder.setNoParent() + if (telemetryType == EmbType.Performance.Default) { + makeKey() + } + } else if (parent is EmbraceSpanImpl) { + parent.wrappedSpan()?.let { + otelSpanBuilder.setParent(Context.current().with(it)) + } + } + } + + fun startSpan(startTimeMs: Long): Span { + val startedSpan = otelSpanBuilder.setStartTimestamp(startTimeMs, TimeUnit.MILLISECONDS).startSpan() + embraceAttributes.forEach { embraceAttribute -> + startedSpan.setEmbraceAttribute(embraceAttribute) + } + return startedSpan + } + + /** + * Mark the span generated by this builder as [PrivateSpan] + */ + fun makePrivate(): EmbraceSpanBuilder { + setEmbraceAttribute(PrivateSpan) + return this + } + + /** + * Mark the span generated by this builder as a [KeySpan] + */ + private fun makeKey(): EmbraceSpanBuilder { + setEmbraceAttribute(KeySpan) + return this + } + + /** + * Sets an [EmbraceAttribute] on the given [SpanBuilder] and return it + */ + private fun setEmbraceAttribute(embraceAttribute: EmbraceAttribute): EmbraceSpanBuilder { + embraceAttributes.add(embraceAttribute) + return this + } +} diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt index d6395b1c2c..40515d3d0a 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt @@ -12,7 +12,6 @@ import io.embrace.android.embracesdk.spans.EmbraceSpanEvent.Companion.inputsVali import io.embrace.android.embracesdk.spans.ErrorCode import io.embrace.android.embracesdk.spans.PersistableEmbraceSpan import io.opentelemetry.api.common.Attributes -import io.opentelemetry.api.trace.SpanBuilder import io.opentelemetry.sdk.common.Clock import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.ConcurrentLinkedQueue @@ -23,23 +22,17 @@ import java.util.concurrent.atomic.AtomicReference internal class EmbraceSpanImpl( private val spanName: String, private val openTelemetryClock: Clock, - private val spanBuilder: SpanBuilder, + private val spanBuilder: EmbraceSpanBuilder, override val parent: EmbraceSpan? = null, - private val spanRepository: SpanRepository? = null, - sessionSpan: Boolean = false + private val spanRepository: SpanRepository? = null ) : PersistableEmbraceSpan { - init { - if (!sessionSpan) { - spanBuilder.updateParent(parent) - } - } - private val startedSpan: AtomicReference = AtomicReference(null) private var spanStartTimeMs: Long? = null private var spanEndTimeMs: Long? = null private var status = Span.Status.UNSET private val events = ConcurrentLinkedQueue() + private val schemaAttributes = spanBuilder.embraceAttributes.associate { it.toOTelKeyValuePair() }.toMutableMap() private val attributes = ConcurrentHashMap() // size for ConcurrentLinkedQueues is not a constant operation, so it could be subject to race conditions @@ -61,9 +54,8 @@ internal class EmbraceSpanImpl( } else { var successful: Boolean val attemptedStartTimeMs = startTimeMs ?: openTelemetryClock.now().nanosToMillis() - spanBuilder.setStartTimestamp(attemptedStartTimeMs, TimeUnit.MILLISECONDS) synchronized(startedSpan) { - startedSpan.set(spanBuilder.startSpan()) + startedSpan.set(spanBuilder.startSpan(attemptedStartTimeMs)) successful = startedSpan.get() != null } if (successful) { @@ -83,7 +75,7 @@ internal class EmbraceSpanImpl( synchronized(startedSpan) { startedSpan.get()?.let { spanToStop -> - attributes.forEach { attribute -> + allAttributes().forEach { attribute -> spanToStop.setAttribute(attribute.key, attribute.value) } @@ -161,13 +153,15 @@ internal class EmbraceSpanImpl( endTimeUnixNano = spanEndTimeMs?.millisToNanos(), status = status, events = events.map(EmbraceSpanEvent::toNewPayload), - attributes = attributes.toNewPayload() + attributes = allAttributes().toNewPayload() ) } else { null } } + private fun allAttributes(): Map = attributes + schemaAttributes + private fun canSnapshot(): Boolean = spanId != null && spanStartTimeMs != null internal fun wrappedSpan(): io.opentelemetry.api.trace.Span? = startedSpan.get() diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/SpanServiceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/SpanServiceImpl.kt index 91c58d47c9..8bfe8a8f4d 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/SpanServiceImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/SpanServiceImpl.kt @@ -9,7 +9,6 @@ import io.embrace.android.embracesdk.spans.PersistableEmbraceSpan import io.opentelemetry.api.common.Attributes import io.opentelemetry.api.trace.Tracer import io.opentelemetry.sdk.common.Clock -import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean /** @@ -38,7 +37,7 @@ internal class SpanServiceImpl( EmbraceSpanImpl( spanName = spanName, openTelemetryClock = openTelemetryClock, - spanBuilder = createRootSpanBuilder(tracer = tracer, name = spanName, type = type, internal = internal), + spanBuilder = tracer.embraceSpanBuilder(name = spanName, type = type, internal = internal, parent = parent), parent = parent, spanRepository = spanRepository ) @@ -98,10 +97,8 @@ internal class SpanServiceImpl( } return if (EmbraceSpanImpl.inputsValid(name, events, attributes) && currentSessionSpan.canStartNewSpan(parent, internal)) { - createRootSpanBuilder(tracer = tracer, name = getSpanName(name, internal), type = type, internal = internal) - .updateParent(parent) - .setStartTimestamp(startTimeMs, TimeUnit.MILLISECONDS) - .startSpan() + tracer.embraceSpanBuilder(name = getSpanName(name, internal), type = type, internal = internal, parent = parent) + .startSpan(startTimeMs) .setAllAttributes(Attributes.builder().fromMap(attributes).build()) .addEvents(events) .endSpan(errorCode, endTimeMs) diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt index 71309e604a..8182d7ee9f 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/arch/EmbraceAttributeExtensions.kt @@ -8,12 +8,14 @@ import io.embrace.android.embracesdk.arch.schema.ErrorCodeAttribute import io.embrace.android.embracesdk.arch.schema.KeySpan import io.embrace.android.embracesdk.arch.schema.PrivateSpan import io.embrace.android.embracesdk.arch.schema.TelemetryType +import io.embrace.android.embracesdk.internal.payload.Span import io.embrace.android.embracesdk.internal.spans.EmbraceSpanData import io.embrace.android.embracesdk.spans.ErrorCode import io.opentelemetry.api.trace.StatusCode -import junit.framework.TestCase.assertEquals -import junit.framework.TestCase.assertNull +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue /** * Assert [EmbraceSpanData] is of type [EmbType.Performance.Default] @@ -60,6 +62,36 @@ internal fun EmbraceSpanData.assertSuccessful() { assertNull(attributes[ErrorCodeAttribute.Failure.otelAttributeName()]) } +internal fun Span.assertIsTypePerformance() = assertIsType(EmbType.Performance.Default) + +internal fun Span.assertIsType(telemetryType: TelemetryType) = assertHasEmbraceAttribute(telemetryType) + +internal fun Span.assertIsKeySpan() = assertHasEmbraceAttribute(KeySpan) + +internal fun Span.assertNotKeySpan() = assertDoesNotHaveEmbraceAttribute(KeySpan) + +internal fun Span.assertIsPrivateSpan() = assertHasEmbraceAttribute(PrivateSpan) + +internal fun Span.assertNotPrivateSpan() = assertDoesNotHaveEmbraceAttribute(PrivateSpan) + +internal fun Span.assertHasEmbraceAttribute(embraceAttribute: EmbraceAttribute) { + assertTrue(checkNotNull(attributes).contains(embraceAttribute.toAttributePayload())) +} + +internal fun Span.assertDoesNotHaveEmbraceAttribute(embraceAttribute: EmbraceAttribute) { + assertFalse(checkNotNull(attributes).contains(embraceAttribute.toAttributePayload())) +} + +internal fun Span.assertError(errorCode: ErrorCode) { + assertEquals(StatusCode.ERROR, status) + assertHasEmbraceAttribute(errorCode.fromErrorCode()) +} + +internal fun Span.assertSuccessful() { + assertEquals(StatusCode.OK, status) + assertEquals(0, checkNotNull(attributes).filter { it.key == ErrorCodeAttribute.Failure.otelAttributeName() }.size) +} + /** * Assert [StartSpanData] is of type [telemetryType] */ diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImplTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImplTest.kt index d1c29a6a9c..fd6c09f74d 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImplTest.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImplTest.kt @@ -1,5 +1,6 @@ package io.embrace.android.embracesdk.internal.spans +import io.embrace.android.embracesdk.arch.schema.EmbType import io.embrace.android.embracesdk.fakes.FakeClock import io.embrace.android.embracesdk.fakes.injection.FakeInitModule import io.embrace.android.embracesdk.fixtures.MAX_LENGTH_ATTRIBUTE_KEY @@ -44,7 +45,11 @@ internal class EmbraceSpanImplTest { embraceSpan = EmbraceSpanImpl( spanName = EXPECTED_SPAN_NAME, openTelemetryClock = fakeInitModule.openTelemetryClock, - spanBuilder = tracer.spanBuilder(EXPECTED_SPAN_NAME), + spanBuilder = tracer.embraceSpanBuilder( + name = EXPECTED_SPAN_NAME, + type = EmbType.Performance.Default, + internal = false + ), spanRepository = spanRepository ) fakeClock.tick(100) @@ -76,7 +81,7 @@ internal class EmbraceSpanImplTest { assertSnapshot(expectedStartTimeMs = expectedStartTimeMs) assertTrue(addEvent("eventName")) assertTrue(addAttribute("first", "value")) - assertSnapshot(expectedStartTimeMs = expectedStartTimeMs, eventCount = 1, attributeCount = 1) + assertSnapshot(expectedStartTimeMs = expectedStartTimeMs, eventCount = 1, expectedCustomAttributeCount = 1) assertEquals(1, spanRepository.getActiveSpans().size) assertEquals(0, spanRepository.getCompletedSpans().size) } @@ -117,12 +122,15 @@ internal class EmbraceSpanImplTest { @Test fun `validate usage without SpanRepository`() { - val spanName = "test-span" with( EmbraceSpanImpl( - spanName = spanName, + spanName = EXPECTED_SPAN_NAME, openTelemetryClock = openTelemetryClock, - spanBuilder = tracer.spanBuilder(spanName) + spanBuilder = tracer.embraceSpanBuilder( + name = EXPECTED_SPAN_NAME, + type = EmbType.Performance.Default, + internal = false + ), ) ) { assertTrue(start()) @@ -243,11 +251,11 @@ internal class EmbraceSpanImplTest { assertEquals(EXPECTED_EVENT_NAME, snapshotEvent.name) assertEquals(expectedEventTime.millisToNanos(), snapshotEvent.timeUnixNano) - val eventAttributes = checkNotNull(snapshotEvent.attributes?.single()) + val eventAttributes = checkNotNull(snapshotEvent.attributes).single { !checkNotNull(it.key).startsWith("emb.") } assertEquals(EXPECTED_ATTRIBUTE_NAME, eventAttributes.key) assertEquals(EXPECTED_ATTRIBUTE_VALUE, eventAttributes.data) - val snapshotAttributes = checkNotNull(snapshot.attributes?.single()) + val snapshotAttributes = checkNotNull(snapshot.attributes).single { !checkNotNull(it.key).startsWith("emb.") } assertEquals(EXPECTED_ATTRIBUTE_NAME, snapshotAttributes.key) assertEquals(EXPECTED_ATTRIBUTE_VALUE, snapshotAttributes.data) } @@ -258,14 +266,19 @@ internal class EmbraceSpanImplTest { expectedEndTimeMs: Long? = null, expectedStatus: Span.Status = Span.Status.UNSET, eventCount: Int = 0, - attributeCount: Int = 0 + expectedEmbraceAttributes: Int = 2, + expectedCustomAttributeCount: Int = 0 ) { with(checkNotNull(snapshot())) { assertEquals(expectedStartTimeMs, startTimeUnixNano?.nanosToMillis()) assertEquals(expectedEndTimeMs, endTimeUnixNano?.nanosToMillis()) assertEquals(expectedStatus, status) assertEquals(eventCount, events?.size) - assertEquals(attributeCount, attributes?.size) + checkNotNull(attributes) + val embraceAttributeCount = attributes.filter { checkNotNull(it.key).startsWith("emb.") }.size + val customAttributeCount = attributes.size - embraceAttributeCount + assertEquals(expectedEmbraceAttributes, embraceAttributeCount) + assertEquals(expectedCustomAttributeCount, customAttributeCount) } }