Skip to content

Commit

Permalink
Refactor how Embrace attributes are defined (#526)
Browse files Browse the repository at this point in the history
## Goal

Create an abstraction for an Embrace Attribute and define a sealed class for each attribute and an implementation of that class for each valid value. 

I converted two attributes just to get some feedback as to whether this is a good idea/implementation. If it looks good, I can convert EmbType/TelemetryType to this format as well.

The goal is to be able to use these class when validating values in tests, and also when we write out the property to OTel objects, instead of having to hardcode rules like putting `emb.` in front of attribute names, etc.

If this looks good, I'll add convenience methods to internal objects and tests to simply the code that words with Embrace attributes, so you can do something like span.setEmbraceAttribute(ErrorCode.Failure)

## Testing

Existing tests cover the usage, but unit tests should be added to test the abstract classes if this is deemed a good way to go.
  • Loading branch information
bidetofevil committed Mar 11, 2024
2 parents 1a4f08b + 61848e4 commit 11625ab
Show file tree
Hide file tree
Showing 16 changed files with 122 additions and 68 deletions.
4 changes: 1 addition & 3 deletions embrace-android-sdk/api/embrace-android-sdk.api
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,10 @@ public final class io/embrace/android/embracesdk/spans/EmbraceSpanEventJsonAdapt
public fun toString ()Ljava/lang/String;
}

public final class io/embrace/android/embracesdk/spans/ErrorCode : java/lang/Enum, io/embrace/android/embracesdk/internal/spans/EmbraceAttributes$Attribute {
public final class io/embrace/android/embracesdk/spans/ErrorCode : java/lang/Enum {
public static final field FAILURE Lio/embrace/android/embracesdk/spans/ErrorCode;
public static final field UNKNOWN Lio/embrace/android/embracesdk/spans/ErrorCode;
public static final field USER_ABANDON Lio/embrace/android/embracesdk/spans/ErrorCode;
public fun getCanonicalName ()Ljava/lang/String;
public fun keyName ()Ljava/lang/String;
public static fun valueOf (Ljava/lang/String;)Lio/embrace/android/embracesdk/spans/ErrorCode;
public static fun values ()[Lio/embrace/android/embracesdk/spans/ErrorCode;
}
Expand Down
1 change: 1 addition & 0 deletions embrace-android-sdk/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ dependencies {
testImplementation "com.squareup.okhttp3:mockwebserver:4.9.3"
testImplementation("com.google.protobuf:protobuf-java:3.24.0")
testImplementation("com.google.protobuf:protobuf-java-util:3.24.0")
testImplementation "org.jetbrains.kotlin:kotlin-reflect:1.4.32"

dokkaHtmlPlugin("org.jetbrains.dokka:kotlin-as-java-plugin:${Versions.dokka}")
dokkaHtmlPlugin("org.jetbrains.dokka:android-documentation-plugin:${Versions.dokka}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ internal fun assertEmbraceSpanData(
assertEquals(32, traceId.length)
}
assertEquals(expectedStatus, status)
assertEquals(errorCode?.name, attributes[errorCode?.keyName()])
errorCode?.run {
val errorCodeAttribute = fromErrorCode()
assertEquals(
errorCodeAttribute.attributeValue,
attributes[errorCodeAttribute.otelAttributeName()]
)
}
expectedCustomAttributes.forEach { entry ->
assertEquals(entry.value, attributes[entry.key])
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package io.embrace.android.embracesdk.arch.schema

/**
* Attribute that stores the reason an app instance terminated
*/
internal sealed class AppTerminationCause(
override val attributeValue: String
) : EmbraceAttribute {
override val attributeName: String = "termination_cause"

internal object Crash : AppTerminationCause("crash")

internal object UserTermination : AppTerminationCause("user_termination")

internal object Unknown : AppTerminationCause("unknown")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.embrace.android.embracesdk.arch.schema

import io.embrace.android.embracesdk.internal.spans.toEmbraceAttributeName

/**
* Instance of a valid value for an attribute that has special meaning in the Embrace platform.
*/
internal interface EmbraceAttribute {
/**
* The unique name given to the attribute
*/
val attributeName: String

/**
* The value of the particular instance of the attribute
*/
val attributeValue: String

fun otelAttributeName(): String = attributeName.toEmbraceAttributeName()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.embrace.android.embracesdk.arch.schema

import io.embrace.android.embracesdk.spans.ErrorCode
import java.util.Locale

/**
* Attribute that stores the [ErrorCode] in an OpenTelemetry span
*/
internal sealed class ErrorCodeAttribute(
errorCode: ErrorCode
) : EmbraceAttribute {
override val attributeName: String = "error_code"
override val attributeValue: String = errorCode.name.toLowerCase(Locale.ENGLISH)

internal object Failure : ErrorCodeAttribute(ErrorCode.FAILURE)

internal object UserAbandon : ErrorCodeAttribute(ErrorCode.USER_ABANDON)

internal object Unknown : ErrorCodeAttribute(ErrorCode.UNKNOWN)
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.embrace.android.embracesdk.internal.spans

import io.embrace.android.embracesdk.arch.destination.SessionSpanWriter
import io.embrace.android.embracesdk.arch.schema.AppTerminationCause
import io.embrace.android.embracesdk.internal.Initializable
import io.embrace.android.embracesdk.spans.EmbraceSpan

Expand All @@ -11,7 +12,7 @@ internal interface CurrentSessionSpan : Initializable, SessionSpanWriter {
/**
* End the current session span and start a new one if the app is not terminating
*/
fun endSession(appTerminationCause: EmbraceAttributes.AppTerminationCause? = null): List<EmbraceSpanData>
fun endSession(appTerminationCause: AppTerminationCause? = null): List<EmbraceSpanData>

/**
* Returns true if a span with the given parameters can be started in the current session
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.embrace.android.embracesdk.internal.spans
import io.embrace.android.embracesdk.arch.destination.SessionSpanWriter
import io.embrace.android.embracesdk.arch.destination.SpanAttributeData
import io.embrace.android.embracesdk.arch.destination.SpanEventData
import io.embrace.android.embracesdk.arch.schema.AppTerminationCause
import io.embrace.android.embracesdk.arch.schema.EmbType
import io.embrace.android.embracesdk.internal.clock.nanosToMillis
import io.embrace.android.embracesdk.internal.utils.Provider
Expand Down Expand Up @@ -65,7 +66,7 @@ internal class CurrentSessionSpanImpl(
}
}

override fun endSession(appTerminationCause: EmbraceAttributes.AppTerminationCause?): List<EmbraceSpanData> {
override fun endSession(appTerminationCause: AppTerminationCause?): List<EmbraceSpanData> {
val endingSessionSpan = sessionSpan.get() ?: return emptyList()
synchronized(sessionSpan) {
// Right now, session spans don't survive native crashes and sudden process terminations,
Expand All @@ -82,8 +83,8 @@ internal class CurrentSessionSpanImpl(
sessionSpan.set(startSessionSpan(clock.now().nanosToMillis()))
} else {
endingSessionSpan.addAttribute(
appTerminationCause.keyName(),
appTerminationCause.name
appTerminationCause.otelAttributeName(),
appTerminationCause.attributeValue
)
endingSessionSpan.stop()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.embrace.android.embracesdk.internal.spans

import io.embrace.android.embracesdk.arch.schema.TelemetryType
import io.embrace.android.embracesdk.internal.spans.EmbraceAttributes.Attribute
import io.embrace.android.embracesdk.internal.utils.Provider
import io.embrace.android.embracesdk.spans.EmbraceSpan
import io.embrace.android.embracesdk.spans.EmbraceSpanEvent
Expand Down Expand Up @@ -84,7 +83,7 @@ internal fun createRootSpanBuilder(
): SpanBuilder = createEmbraceSpanBuilder(tracer = tracer, name = name, type = type, internal = internal).setNoParent()

/**
* Sets and returns the [EmbraceAttributes.Type] attribute for the given [SpanBuilder]
* Sets and returns the [TelemetryType] attribute for the given [SpanBuilder]
*/
internal fun SpanBuilder.setType(value: TelemetryType): SpanBuilder {
setAttribute(value.attributeName(), value.description)
Expand Down Expand Up @@ -182,7 +181,8 @@ internal fun Span.endSpan(errorCode: ErrorCode? = null, endTimeMs: Long? = null)
setStatus(StatusCode.OK)
} else {
setStatus(StatusCode.ERROR)
setAttribute(errorCode.keyName(), errorCode.toString())
val errorCodeAttribute = errorCode.fromErrorCode()
setAttribute(errorCodeAttribute.otelAttributeName(), errorCodeAttribute.attributeValue)
}

if (endTimeMs != null) {
Expand Down Expand Up @@ -228,36 +228,3 @@ internal fun String.toEmbraceAttributeName(): String = EMBRACE_ATTRIBUTE_NAME_PR
* Return the appropriate internal Embrace attribute usage name given the current string
*/
internal fun String.toEmbraceUsageAttributeName(): String = EMBRACE_USAGE_ATTRIBUTE_NAME_PREFIX + this

/**
* Contains the set of attributes (i.e. implementers of the [Attribute] interface) set on a [Span] by the SDK that has special meaning
* in the Embrace world. Each enum defines the attribute name used in the [Span] and specifies the set of valid values it can be set to.
*/
internal object EmbraceAttributes {
/**
* The reason for the termination of a process span
*/
internal enum class AppTerminationCause : Attribute {
CRASH,
USER_TERMINATION,
UNKNOWN;

override val canonicalName: String = "termination_cause"
}

/**
* Denotes an attribute added by the SDK with a restricted set of valid values
*/
internal interface Attribute {

/**
* The name used to identify this [Attribute]
*/
val canonicalName: String

/**
* The name used as the key for the [Attribute] in the attributes map
*/
fun keyName(): String = EMBRACE_ATTRIBUTE_NAME_PREFIX + canonicalName
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.embrace.android.embracesdk.session.message

import io.embrace.android.embracesdk.anr.ndk.NativeThreadSamplerService
import io.embrace.android.embracesdk.arch.schema.AppTerminationCause
import io.embrace.android.embracesdk.capture.PerformanceInfoService
import io.embrace.android.embracesdk.capture.crumbs.BreadcrumbService
import io.embrace.android.embracesdk.capture.metadata.MetadataService
Expand All @@ -12,7 +13,6 @@ import io.embrace.android.embracesdk.config.ConfigService
import io.embrace.android.embracesdk.event.EventService
import io.embrace.android.embracesdk.event.LogMessageService
import io.embrace.android.embracesdk.internal.spans.CurrentSessionSpan
import io.embrace.android.embracesdk.internal.spans.EmbraceAttributes
import io.embrace.android.embracesdk.internal.spans.EmbraceSpanData
import io.embrace.android.embracesdk.internal.spans.SpanSink
import io.embrace.android.embracesdk.internal.utils.Uuid
Expand Down Expand Up @@ -160,11 +160,12 @@ internal class PayloadMessageCollator(
when {
!params.isCacheAttempt -> {
val appTerminationCause = when {
finalPayload.crashReportId != null -> EmbraceAttributes.AppTerminationCause.CRASH
finalPayload.crashReportId != null -> AppTerminationCause.Crash
else -> null
}
currentSessionSpan.endSession(appTerminationCause)
}

else -> spanSink.completedSpans()
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package io.embrace.android.embracesdk.spans

import io.embrace.android.embracesdk.annotation.BetaApi
import io.embrace.android.embracesdk.internal.spans.EmbraceAttributes
import io.embrace.android.embracesdk.arch.schema.ErrorCodeAttribute

/**
* Attribute to categorize the broad reason a Span completed unsuccessfully.
* Categorize the broad reason a Span completed unsuccessfully.
*/
@BetaApi
public enum class ErrorCode : EmbraceAttributes.Attribute {

public enum class ErrorCode {
/**
* An application failure caused the Span to terminate
*/
Expand All @@ -24,5 +23,9 @@ public enum class ErrorCode : EmbraceAttributes.Attribute {
*/
UNKNOWN;

override val canonicalName: String = "error_code"
internal fun fromErrorCode(): ErrorCodeAttribute = when (this) {
FAILURE -> ErrorCodeAttribute.Failure
USER_ABANDON -> ErrorCodeAttribute.UserAbandon
UNKNOWN -> ErrorCodeAttribute.Unknown
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package io.embrace.android.embracesdk.fakes

import io.embrace.android.embracesdk.arch.destination.SpanAttributeData
import io.embrace.android.embracesdk.arch.destination.SpanEventData
import io.embrace.android.embracesdk.arch.schema.AppTerminationCause
import io.embrace.android.embracesdk.internal.spans.CurrentSessionSpan
import io.embrace.android.embracesdk.internal.spans.EmbraceAttributes
import io.embrace.android.embracesdk.internal.spans.EmbraceSpanData
import io.embrace.android.embracesdk.spans.EmbraceSpan

Expand Down Expand Up @@ -31,7 +31,7 @@ internal class FakeCurrentSessionSpan : CurrentSessionSpan {
return true
}

override fun endSession(appTerminationCause: EmbraceAttributes.AppTerminationCause?): List<EmbraceSpanData> {
override fun endSession(appTerminationCause: AppTerminationCause?): List<EmbraceSpanData> {
return emptyList()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.embrace.android.embracesdk.internal.spans

import io.embrace.android.embracesdk.arch.destination.SpanAttributeData
import io.embrace.android.embracesdk.arch.destination.SpanEventData
import io.embrace.android.embracesdk.arch.schema.AppTerminationCause
import io.embrace.android.embracesdk.arch.schema.EmbType
import io.embrace.android.embracesdk.arch.schema.SchemaType
import io.embrace.android.embracesdk.fakes.FakeClock
Expand Down Expand Up @@ -131,11 +132,12 @@ internal class CurrentSessionSpanImplTests {

@Test
fun `flushing with app termination and termination reason flushes session span with right termination type`() {
EmbraceAttributes.AppTerminationCause.values().forEach {
AppTerminationCause::class.sealedSubclasses.forEach {
val cause = checkNotNull(it.objectInstance)
val module = FakeInitModule(clock = clock)
val sessionSpan = module.openTelemetryModule.currentSessionSpan
module.openTelemetryModule.spanService.initializeService(clock.now())
val flushedSpans = sessionSpan.endSession(it)
val flushedSpans = sessionSpan.endSession(cause)
assertEquals(1, flushedSpans.size)

val lastFlushedSpan = flushedSpans[0]
Expand All @@ -144,7 +146,7 @@ internal class CurrentSessionSpanImplTests {
assertEquals(EmbType.Ux.Session.description, attributes[EmbType.Ux.Session.attributeName()])
assertEquals(StatusCode.OK, status)
assertFalse(isKey())
assertEquals(it.name, attributes[it.keyName()])
assertEquals(cause.attributeValue, attributes[cause.otelAttributeName()])
}

assertEquals(0, module.openTelemetryModule.spanSink.completedSpans().size)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,13 @@ internal class EmbraceTracerTest {
currentSpan.attributes[EmbType.Performance.attributeName()]
)
assertEquals(if (traceRoot) "true" else null, currentSpan.attributes["emb.key"])
assertEquals(errorCode?.name, currentSpan.attributes[errorCode?.keyName()])
errorCode?.run {
val errorCodeAttribute = fromErrorCode()
assertEquals(
errorCodeAttribute.attributeValue,
currentSpan.attributes[errorCodeAttribute.otelAttributeName()]
)
}
assertFalse(currentSpan.isPrivate())
return currentSpan
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,13 @@ internal class InternalTracerTest {
currentSpan.attributes[EmbType.Performance.attributeName()]
)
assertEquals(if (traceRoot) "true" else null, currentSpan.attributes["emb.key"])
assertEquals(errorCode?.name, currentSpan.attributes[errorCode?.keyName()])
errorCode?.run {
val errorCodeAttribute = fromErrorCode()
assertEquals(
errorCodeAttribute.attributeValue,
currentSpan.attributes[errorCodeAttribute.otelAttributeName()]
)
}
assertFalse(currentSpan.isPrivate())
return currentSpan
}
Expand Down
Loading

0 comments on commit 11625ab

Please sign in to comment.