Flesh out synchronization of EmbraceSpan functionality (#938)

## Goal Flesh out synchronization of the wrapper EmbraceSpan given that we know the operations in the `SdkSpan` are synchronized already.
embrace-io · Jun 11, 2024 · 4a7a61e · 4a7a61e
2 parents 269801f + 1f179b3
commit 4a7a61e
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 57 deletions.
diff --git a/...android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt b/...android-sdk/src/main/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanImpl.kt
@@ -66,75 +66,81 @@ internal class EmbraceSpanImpl(
  get() = startedSpan.get()?.isRecording == true
 
  override fun start(startTimeMs: Long?): Boolean {
- return if (spanStarted()) {
- false
- } else {
- var successful: Boolean
- val attemptedStartTimeMs = startTimeMs?.normalizeTimestampAsMillis()
- ?: spanBuilder.startTimeMs
- ?: openTelemetryClock.now().nanosToMillis()
+ if (spanStarted()) {
+ return false
+ }
 
- synchronized(startedSpan) {
- startedSpan.set(spanBuilder.startSpan(attemptedStartTimeMs))
- successful = spanStarted()
+ val attemptedStartTimeMs = startTimeMs?.normalizeTimestampAsMillis()
+ ?: spanBuilder.startTimeMs
+ ?: openTelemetryClock.now().nanosToMillis()
+
+ synchronized(startedSpan) {
+ val newSpan = spanBuilder.startSpan(attemptedStartTimeMs)
+ if (newSpan.isRecording) {
+ startedSpan.set(newSpan)
+ } else {
+ return false
  }
- if (successful) {
- updatedName?.let { newName ->
- startedSpan.get()?.updateName(newName)
- }
 
- spanStartTimeMs = attemptedStartTimeMs
- spanRepository.trackStartedSpan(this)
+ spanRepository.trackStartedSpan(this)
+ updatedName?.let { newName ->
+ newSpan.updateName(newName)
  }
- return successful
+ spanStartTimeMs = attemptedStartTimeMs
  }
+
+ return true
  }
 
  override fun stop(errorCode: ErrorCode?, endTimeMs: Long?): Boolean {
- return if (!isRecording) {
- false
- } else {
-  var successful = false
-  val attemptedEndTimeMs = endTimeMs?.normalizeTimestampAsMillis() ?: openTelemetryClock.now().nanosToMillis()
+ if (!isRecording) {
+ return false
+ }
+ var successful = false
+ val attemptedEndTimeMs = endTimeMs?.normalizeTimestampAsMillis() ?: openTelemetryClock.now().nanosToMillis()
 
- synchronized(startedSpan) {
- startedSpan.get()?.let { spanToStop ->
- systemAttributes.forEach { systemAttribute ->
- spanToStop.setEmbraceAttribute(systemAttribute.key, systemAttribute.value)
- }
- customAttributes.forEach { attribute ->
- spanToStop.setAttribute(attribute.key, attribute.value)
- }
+ synchronized(startedSpan) {
+ if (!isRecording) {
+ return false
+ }
+
+ startedSpan.get()?.let { spanToStop ->
+ systemAttributes.forEach { systemAttribute ->
+ spanToStop.setEmbraceAttribute(systemAttribute.key, systemAttribute.value)
+ }
+ customAttributes.forEach { attribute ->
+ spanToStop.setAttribute(attribute.key, attribute.value)
+ }
 
- events.forEach { event ->
- val eventAttributes = if (event.attributes.isNotEmpty()) {
- Attributes.builder().fromMap(event.attributes).build()
- } else {
- Attributes.empty()
- }
-
- spanToStop.addEvent(
- event.name,
- eventAttributes,
- event.timestampNanos,
- TimeUnit.NANOSECONDS
- )
+ events.forEach { event ->
+ val eventAttributes = if (event.attributes.isNotEmpty()) {
+ Attributes.builder().fromMap(event.attributes).build()
+ } else {
+ Attributes.empty()
  }
- spanToStop.endSpan(errorCode, attemptedEndTimeMs)
- successful = !isRecording
+
+ spanToStop.addEvent(
+ event.name,
+ eventAttributes,
+ event.timestampNanos,
+ TimeUnit.NANOSECONDS
+ )
  }
- }
- if (successful) {
- status = if (errorCode != null) {
- Span.Status.ERROR
- } else {
- Span.Status.OK
+ spanToStop.endSpan(errorCode, attemptedEndTimeMs)
+ successful = !isRecording
+ if (successful) {
+ spanId?.let { spanRepository.trackedSpanStopped(it) }
+ status = if (errorCode != null) {
+ Span.Status.ERROR
+ } else {
+ Span.Status.OK
+ }
+ spanEndTimeMs = attemptedEndTimeMs
  }
- spanEndTimeMs = attemptedEndTimeMs
- spanId?.let { spanRepository.trackedSpanStopped(it) }
  }
- return successful
  }
+
+ return successful
  }
 
  override fun addEvent(name: String, timestampMs: Long?, attributes: Map<String, String>?): Boolean =

diff --git a/.../src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanFactoryImplTest.kt b/.../src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceSpanFactoryImplTest.kt
@@ -4,8 +4,8 @@ import io.embrace.android.embracesdk.arch.schema.EmbType
 import io.embrace.android.embracesdk.arch.schema.PrivateSpan
 import io.embrace.android.embracesdk.fakes.FakeClock
 import io.embrace.android.embracesdk.fakes.FakePersistableEmbraceSpan
+import io.embrace.android.embracesdk.fakes.FakeTracer
 import io.embrace.android.embracesdk.fakes.injection.FakeInitModule
-import io.opentelemetry.api.trace.Tracer
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotNull
@@ -18,13 +18,13 @@ internal class EmbraceSpanFactoryImplTest {
  private val clock = FakeClock()
  private lateinit var embraceSpanFactory: EmbraceSpanFactoryImpl
  private lateinit var spanRepository: SpanRepository
- private lateinit var tracer: Tracer
+ private lateinit var tracer: FakeTracer
 
  @Before
  fun setup() {
  val initModule = FakeInitModule(clock)
  spanRepository = initModule.openTelemetryModule.spanRepository
- tracer = initModule.openTelemetryModule.tracer
+ tracer = FakeTracer()
  embraceSpanFactory = EmbraceSpanFactoryImpl(
  tracer = tracer,
  openTelemetryClock = initModule.openTelemetryClock,