Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow all activities to ge traced by default #1789

Merged
merged 2 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion embrace-android-api/api/embrace-android-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ public final class io/embrace/android/embracesdk/Severity : java/lang/Enum {
public abstract interface annotation class io/embrace/android/embracesdk/annotation/InternalApi : java/lang/annotation/Annotation {
}

public abstract interface annotation class io/embrace/android/embracesdk/annotation/ObservedActivity : java/lang/annotation/Annotation {
public abstract interface annotation class io/embrace/android/embracesdk/annotation/NotTracedActivity : java/lang/annotation/Annotation {
}

public abstract interface annotation class io/embrace/android/embracesdk/annotation/StartupActivity : java/lang/annotation/Annotation {
}

public abstract interface annotation class io/embrace/android/embracesdk/annotation/TracedActivity : java/lang/annotation/Annotation {
}

public abstract interface class io/embrace/android/embracesdk/internal/api/BreadcrumbApi {
public abstract fun addBreadcrumb (Ljava/lang/String;)V
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.embrace.android.embracesdk.annotation

/**
* The loading of Activities annotated with this class will not generate traces if the feature is enabled, irrespective of
* the configured defaults.
*/
@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.RUNTIME)
public annotation class NotTracedActivity

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.embrace.android.embracesdk.annotation

/**
* The loading of Activities annotated with this class will generate traces if the feature is enabled, irrespective of
* the configured defaults.
*/
@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.RUNTIME)
public annotation class TracedActivity
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ interface AutoDataCaptureBehavior : ConfigBehavior<EnabledFeatureConfig, RemoteC
*/
fun isUiLoadPerfCaptureEnabled(): Boolean

/**
* Whether the SDK is configured to capture traces for the performance of the opening of all Activities by default.
*/
fun isUiLoadPerfAutoCaptureEnabled(): Boolean

/**
* Gates whether the SDK should use OkHttp or fallback to UrlConnection.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class AutoDataCaptureBehaviorImpl(
override fun isNativeCrashCaptureEnabled(): Boolean = local.isNativeCrashCaptureEnabled()
override fun isDiskUsageCaptureEnabled(): Boolean = local.isDiskUsageCaptureEnabled()
override fun isUiLoadPerfCaptureEnabled(): Boolean = local.isUiLoadPerfCaptureEnabled()
override fun isUiLoadPerfAutoCaptureEnabled(): Boolean = local.isUiLoadPerfAutoCaptureEnabled()

private val v2StorageImpl by lazy {
thresholdCheck.isBehaviorEnabled(remote?.killSwitchConfig?.v2StoragePct) ?: V2_STORAGE_ENABLED_DEFAULT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal class AutoDataCaptureBehaviorImplTest {
assertTrue(isDiskUsageCaptureEnabled())
assertTrue(isThermalStatusCaptureEnabled())
assertFalse(isUiLoadPerfCaptureEnabled())
assertFalse(isUiLoadPerfAutoCaptureEnabled())
bidetofevil marked this conversation as resolved.
Show resolved Hide resolved
assertTrue(isThermalStatusCaptureEnabled())
assertTrue(isV2StorageEnabled())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import android.app.Activity
import android.os.Build.VERSION_CODES
import android.os.Bundle
import androidx.annotation.RequiresApi
import io.embrace.android.embracesdk.annotation.ObservedActivity
import io.embrace.android.embracesdk.annotation.NotTracedActivity
import io.embrace.android.embracesdk.annotation.TracedActivity
import io.embrace.android.embracesdk.internal.clock.nanosToMillis
import io.embrace.android.embracesdk.internal.session.lifecycle.ActivityLifecycleListener
import io.embrace.android.embracesdk.internal.utils.VersionChecker
Expand All @@ -18,11 +19,13 @@ import io.opentelemetry.sdk.common.Clock
*/
fun createActivityLoadEventEmitter(
uiLoadEventListener: UiLoadEventListener,
autoTraceEnabled: Boolean,
clock: Clock,
versionChecker: VersionChecker,
): ActivityLifecycleListener {
val lifecycleEventEmitter = LifecycleEventEmitter(
uiLoadEventListener = uiLoadEventListener,
autoTraceEnabled = autoTraceEnabled,
clock = clock,
)
return if (versionChecker.isAtLeast(VERSION_CODES.Q)) {
Expand All @@ -32,50 +35,36 @@ fun createActivityLoadEventEmitter(
}
}

fun Activity.observeOpening() = javaClass.isAnnotationPresent(ObservedActivity::class.java)

/**
* Implementation that works with Android 10+ APIs
*/
@RequiresApi(VERSION_CODES.Q)
private class ActivityLoadEventEmitter(
private val lifecycleEventEmitter: LifecycleEventEmitter
private val lifecycleEventEmitter: LifecycleEventEmitter,
) : ActivityLifecycleListener {

override fun onActivityPreCreated(activity: Activity, savedInstanceState: Bundle?) {
if (activity.observeOpening()) {
lifecycleEventEmitter.create(activity)
}
lifecycleEventEmitter.create(activity)
}

override fun onActivityPostCreated(activity: Activity, savedInstanceState: Bundle?) {
if (activity.observeOpening()) {
lifecycleEventEmitter.createEnd(activity)
}
lifecycleEventEmitter.createEnd(activity)
}

override fun onActivityPreStarted(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.start(activity)
}
lifecycleEventEmitter.start(activity)
}

override fun onActivityPostStarted(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.startEnd(activity)
}
lifecycleEventEmitter.startEnd(activity)
}

override fun onActivityPreResumed(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.resume(activity)
}
lifecycleEventEmitter.resume(activity)
}

override fun onActivityPostResumed(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.resumeEnd(activity)
}
lifecycleEventEmitter.resumeEnd(activity)
}

override fun onActivityPrePaused(activity: Activity) {
Expand All @@ -87,27 +76,21 @@ private class ActivityLoadEventEmitter(
* Version of [ActivityLoadEventEmitter] that works with all Android version and used for Android 9 or lower
*/
private class LegacyActivityLoadEventEmitter(
private val lifecycleEventEmitter: LifecycleEventEmitter
private val lifecycleEventEmitter: LifecycleEventEmitter,
) : ActivityLifecycleListener {

override fun onActivityCreated(activity: Activity, bundle: Bundle?) {
if (activity.observeOpening()) {
lifecycleEventEmitter.create(activity)
}
lifecycleEventEmitter.create(activity)
}

override fun onActivityStarted(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.createEnd(activity)
lifecycleEventEmitter.start(activity)
}
lifecycleEventEmitter.createEnd(activity)
lifecycleEventEmitter.start(activity)
}

override fun onActivityResumed(activity: Activity) {
if (activity.observeOpening()) {
lifecycleEventEmitter.startEnd(activity)
lifecycleEventEmitter.resume(activity)
}
lifecycleEventEmitter.startEnd(activity)
lifecycleEventEmitter.resume(activity)
}

override fun onActivityPaused(activity: Activity) {
Expand All @@ -120,60 +103,80 @@ private class LegacyActivityLoadEventEmitter(
*/
private class LifecycleEventEmitter(
private val uiLoadEventListener: UiLoadEventListener,
private val autoTraceEnabled: Boolean,
private val clock: Clock,
) {

fun create(activity: Activity) {
uiLoadEventListener.create(
instanceId = traceInstanceId(activity),
activityName = activity.localClassName,
timestampMs = nowMs(),
manualEnd = false,
)
if (activity.observe()) {
uiLoadEventListener.create(
instanceId = traceInstanceId(activity),
activityName = activity.localClassName,
timestampMs = nowMs(),
manualEnd = false,
)
}
}

fun createEnd(activity: Activity) {
uiLoadEventListener.createEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
if (activity.observe()) {
uiLoadEventListener.createEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
}
}

fun start(activity: Activity) {
uiLoadEventListener.start(
instanceId = traceInstanceId(activity),
activityName = activity.localClassName,
timestampMs = nowMs(),
manualEnd = false,
)
if (activity.observe()) {
uiLoadEventListener.start(
instanceId = traceInstanceId(activity),
activityName = activity.localClassName,
timestampMs = nowMs(),
manualEnd = false,
)
}
}

fun startEnd(activity: Activity) {
uiLoadEventListener.startEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
if (activity.observe()) {
uiLoadEventListener.startEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
}
}

fun resume(activity: Activity) {
uiLoadEventListener.resume(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
if (activity.observe()) {
uiLoadEventListener.resume(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
}
}

fun resumeEnd(activity: Activity) {
uiLoadEventListener.resumeEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
if (activity.observe()) {
uiLoadEventListener.resumeEnd(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
}
}

fun pause(activity: Activity) {
uiLoadEventListener.discard(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
if (activity.observe()) {
uiLoadEventListener.discard(
instanceId = traceInstanceId(activity),
timestampMs = nowMs()
)
}
}

private fun Activity.observe(): Boolean {
return javaClass.isAnnotationPresent(TracedActivity::class.java) ||
autoTraceEnabled && !javaClass.isAnnotationPresent(NotTracedActivity::class.java)
Comment on lines +178 to +179
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with R8 or are additional rules required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work given that it's how we check @StartupActivity, but I'll double check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. Looks like this is an issue - which means @startupactivity detection didn't work either at runtime either.

}

private fun traceInstanceId(activity: Activity): Int = activity.hashCode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ internal class DataCaptureServiceModuleImpl @JvmOverloads constructor(
if (configService.autoDataCaptureBehavior.isUiLoadPerfCaptureEnabled()) {
createActivityLoadEventEmitter(
uiLoadEventListener = uiLoadTraceEmitter,
autoTraceEnabled = configService.autoDataCaptureBehavior.isUiLoadPerfAutoCaptureEnabled(),
bidetofevil marked this conversation as resolved.
Show resolved Hide resolved
clock = openTelemetryModule.openTelemetryClock,
versionChecker = versionChecker
)
Expand Down
Loading
Loading