Skip to content

Commit

Permalink
Create a new wrapper for SpanBuilder and persist all attributes in a …
Browse files Browse the repository at this point in the history
…snapshot there (#611)

## Goal

The SpanBuilder and Span OTel API interfaces are a one-way, write-only street. The previous snapshotting saved span data in EmbraceSpan, but there is data only set in the SpanBuilder that we also have to include on the snapshot, like the attributes that were set on the builder alone.

As as result, we have to wrap THAT too, and ensure what is set on the wrapper builder eventually is set on the span that is created.

With this, we're basically storing everything in the wrapper and only feeding the OTel span/builder what is necessary when it's necessary.

I also took this opportunity to refactor some of the extensions to make them less complex and easier for use going forward.

## Testing

Added appropriate tests but largely leveraging existing tests to validate the refactoring is a-ok.
  • Loading branch information
bidetofevil authored Mar 21, 2024
2 parents 49e98b0 + 88257cf commit 147afe6
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 108 deletions.
Original file line number Diff line number Diff line change
@@ -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

/**
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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

/**
Expand Down Expand Up @@ -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
}

/**
Expand Down Expand Up @@ -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
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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<EmbraceAttribute>(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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<io.opentelemetry.api.trace.Span?> = AtomicReference(null)
private var spanStartTimeMs: Long? = null
private var spanEndTimeMs: Long? = null
private var status = Span.Status.UNSET
private val events = ConcurrentLinkedQueue<EmbraceSpanEvent>()
private val schemaAttributes = spanBuilder.embraceAttributes.associate { it.toOTelKeyValuePair() }.toMutableMap()
private val attributes = ConcurrentHashMap<String, String>()

// size for ConcurrentLinkedQueues is not a constant operation, so it could be subject to race conditions
Expand All @@ -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) {
Expand All @@ -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)
}

Expand Down Expand Up @@ -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<String, String> = attributes + schemaAttributes

private fun canSnapshot(): Boolean = spanId != null && spanStartTimeMs != null

internal fun wrappedSpan(): io.opentelemetry.api.trace.Span? = startedSpan.get()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
*/
Expand Down
Loading

0 comments on commit 147afe6

Please sign in to comment.