From b830c7681a3157518344f4dc539ee29e945e7781 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Tue, 19 Nov 2024 17:11:43 +0000 Subject: [PATCH] refactor: improve aei code quality --- .../config/behavior/AppExitInfoBehavior.kt | 6 - .../prefs/EmbracePreferencesService.kt | 4 +- .../internal/prefs/PreferencesService.kt | 2 +- .../internal/capture/aei/AeiDataSourceImpl.kt | 196 ++---------------- .../capture/aei/ApplicationExitInfoExt.kt | 89 ++++++++ .../internal/capture/aei/TraceResult.kt | 6 + .../internal/injection/FeatureModuleImpl.kt | 5 +- .../capture/aei/AeiDataSourceImplTest.kt | 2 +- .../embracesdk/fakes/FakePreferenceService.kt | 2 +- 9 files changed, 123 insertions(+), 189 deletions(-) create mode 100644 embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/ApplicationExitInfoExt.kt create mode 100644 embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/TraceResult.kt diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/config/behavior/AppExitInfoBehavior.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/config/behavior/AppExitInfoBehavior.kt index 4dd2ba5f6a..b7d661c3fb 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/config/behavior/AppExitInfoBehavior.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/config/behavior/AppExitInfoBehavior.kt @@ -13,10 +13,4 @@ interface AppExitInfoBehavior : ConfigBehavior? - get() = prefs.getStringSet(AEI_HASH_CODES, null) + override var deliveredAeiIds: Set + get() = prefs.getStringSet(AEI_HASH_CODES, null) ?: emptySet() set(value) = prefs.setArrayPreference(AEI_HASH_CODES, value) override fun isUsersFirstDay(): Boolean { diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/prefs/PreferencesService.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/prefs/PreferencesService.kt index 9f29f26de5..4bdb138dce 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/prefs/PreferencesService.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/prefs/PreferencesService.kt @@ -148,7 +148,7 @@ interface PreferencesService { /** * Set of hashcodes derived from ApplicationExitInfo objects */ - var applicationExitInfoHistory: Set? + var deliveredAeiIds: Set /** * Whether or not the app was installed within the last 24 hours. diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImpl.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImpl.kt index c5070bda03..7b7c520a54 100644 --- a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImpl.kt +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImpl.kt @@ -4,35 +4,30 @@ import android.app.ActivityManager import android.app.ApplicationExitInfo import android.os.Build.VERSION_CODES import androidx.annotation.RequiresApi -import io.embrace.android.embracesdk.Severity +import io.embrace.android.embracesdk.Severity.INFO import io.embrace.android.embracesdk.internal.arch.datasource.LogDataSourceImpl import io.embrace.android.embracesdk.internal.arch.datasource.NoInputValidation import io.embrace.android.embracesdk.internal.arch.destination.LogWriter import io.embrace.android.embracesdk.internal.arch.limits.UpToLimitStrategy -import io.embrace.android.embracesdk.internal.arch.schema.SchemaType +import io.embrace.android.embracesdk.internal.arch.schema.SchemaType.AeiLog import io.embrace.android.embracesdk.internal.config.behavior.AppExitInfoBehavior import io.embrace.android.embracesdk.internal.logging.EmbLogger import io.embrace.android.embracesdk.internal.logging.InternalErrorType -import io.embrace.android.embracesdk.internal.payload.AppExitInfoData import io.embrace.android.embracesdk.internal.prefs.PreferencesService import io.embrace.android.embracesdk.internal.spans.toOtelSeverity import io.embrace.android.embracesdk.internal.utils.BuildVersionChecker import io.embrace.android.embracesdk.internal.utils.VersionChecker -import io.embrace.android.embracesdk.internal.utils.toUTF8String import io.embrace.android.embracesdk.internal.worker.BackgroundWorker -import java.io.IOException -import java.util.concurrent.Future -import java.util.concurrent.atomic.AtomicBoolean @RequiresApi(VERSION_CODES.R) internal class AeiDataSourceImpl( private val backgroundWorker: BackgroundWorker, private val appExitInfoBehavior: AppExitInfoBehavior, - private val activityManager: ActivityManager?, + private val activityManager: ActivityManager, private val preferencesService: PreferencesService, logWriter: LogWriter, private val logger: EmbLogger, - private val buildVersionChecker: VersionChecker = BuildVersionChecker, + private val versionChecker: VersionChecker = BuildVersionChecker, ) : AeiDataSource, LogDataSourceImpl( logWriter, logger, @@ -43,193 +38,42 @@ internal class AeiDataSourceImpl( private const val SDK_AEI_SEND_LIMIT = 32 } - @Volatile - private var backgroundExecution: Future<*>? = null - private val sessionApplicationExitInfoData: MutableList = mutableListOf() - private val isSessionApplicationExitInfoDataReady = AtomicBoolean(false) - override fun enableDataCapture() { - if (backgroundExecution != null) { - return - } - backgroundExecution = backgroundWorker.submit { + backgroundWorker.submit { try { - processApplicationExitInfo() + processAeiRecords() } catch (exc: Throwable) { logger.trackInternalError(InternalErrorType.ENABLE_DATA_CAPTURE, exc) } } } - override fun disableDataCapture() { - try { - backgroundExecution?.cancel(true) - backgroundExecution = null - } catch (t: Throwable) { - logger.trackInternalError(InternalErrorType.DISABLE_DATA_CAPTURE, t) - } - } - - private fun processApplicationExitInfo() { - val historicalProcessExitReasons = getHistoricalProcessExitReasons() - val unsentExitReasons = getUnsentExitReasons(historicalProcessExitReasons) - - unsentExitReasons.forEach { - sessionApplicationExitInfoData.add(buildSessionAppExitInfoData(it, null, null)) - } - - isSessionApplicationExitInfoDataReady.set(true) - processApplicationExitInfoBlobs(unsentExitReasons) - } - - private fun processApplicationExitInfoBlobs(unsentExitReasons: List) { - unsentExitReasons.forEach { aei: ApplicationExitInfo -> - val traceResult = collectExitInfoTrace(aei) - if (traceResult != null) { - val payload = buildSessionAppExitInfoData( - aei, - getTrace(traceResult), - getTraceStatus(traceResult) - ) - sendApplicationExitInfoWithTraces(listOf(payload)) - } - } - } - - private fun getHistoricalProcessExitReasons(): List { - // A process ID that used to belong to this package but died later; - // a value of 0 means to ignore this parameter and return all matching records. - val pid = 0 - - // number of results to be returned; a value of 0 means to ignore this parameter and return - // all matching records with a maximum of 16 entries + private fun processAeiRecords() { val maxNum = appExitInfoBehavior.appExitInfoMaxNum() + val records = activityManager.getHistoricalProcessExitReasons(null, 0, maxNum).take(SDK_AEI_SEND_LIMIT) + val unsentRecords = getUnsentRecords(records) - var historicalProcessExitReasons: List = - activityManager?.getHistoricalProcessExitReasons(null, pid, maxNum) - ?: return emptyList() - - if (historicalProcessExitReasons.size > SDK_AEI_SEND_LIMIT) { - historicalProcessExitReasons = historicalProcessExitReasons.take(SDK_AEI_SEND_LIMIT) - } - - return historicalProcessExitReasons - } - - private fun getUnsentExitReasons(historicalProcessExitReasons: List): List { - // Generates the set of current aei captured - val allAeiHashCodes = historicalProcessExitReasons.map(::generateUniqueHash).toSet() - - // Get hash codes that were previously delivered - val deliveredHashCodes = preferencesService.applicationExitInfoHistory ?: emptySet() - - // Subtracts aei hashcodes of already sent information to get new entries - val unsentHashCodes = allAeiHashCodes.subtract(deliveredHashCodes) - - // Updates preferences with the new set of hashcodes - preferencesService.applicationExitInfoHistory = allAeiHashCodes - - // Get AEI objects that were not sent - val unsentAeiObjects = historicalProcessExitReasons.filter { - unsentHashCodes.contains(generateUniqueHash(it)) - } - - return unsentAeiObjects - } - - private fun buildSessionAppExitInfoData( - appExitInfo: ApplicationExitInfo, - trace: String?, - traceStatus: String?, - ): AppExitInfoData { - val sessionId = String(appExitInfo.processStateSummary ?: ByteArray(0)) - - return AppExitInfoData( - sessionId = sessionId, - sessionIdError = getSessionIdValidationError(sessionId), - importance = appExitInfo.importance, - pss = appExitInfo.pss, - reason = appExitInfo.reason, - rss = appExitInfo.rss, - status = appExitInfo.status, - timestamp = appExitInfo.timestamp, - trace = trace, - description = appExitInfo.description, - traceStatus = traceStatus - ) - } - - private fun getTrace(traceResult: AppExitInfoBehavior.CollectTracesResult): String? = - when (traceResult) { - is AppExitInfoBehavior.CollectTracesResult.Success -> traceResult.result - is AppExitInfoBehavior.CollectTracesResult.TooLarge -> traceResult.result - else -> null - } - - private fun getTraceStatus(traceResult: AppExitInfoBehavior.CollectTracesResult): String? = - when (traceResult) { - is AppExitInfoBehavior.CollectTracesResult.Success -> null - is AppExitInfoBehavior.CollectTracesResult.TooLarge -> "Trace was too large, sending truncated trace" - else -> traceResult.result - } - - private fun sendApplicationExitInfoWithTraces(appExitInfoWithTraces: List) { - appExitInfoWithTraces.forEach { data -> + unsentRecords.forEach { + val obj = it.constructAeiObject(versionChecker, appExitInfoBehavior.getTraceMaxLimit()) ?: return@forEach captureData( inputValidation = NoInputValidation, captureAction = { - val schemaType = SchemaType.AeiLog(data) - addLog(schemaType, Severity.INFO.toOtelSeverity(), data.trace ?: "") + val schemaType = AeiLog(obj) + addLog(schemaType, INFO.toOtelSeverity(), obj.trace ?: "") } ) } } - private fun collectExitInfoTrace(appExitInfo: ApplicationExitInfo): AppExitInfoBehavior.CollectTracesResult? { - try { - val trace = readTraceAsString(appExitInfo) ?: return null - - val traceMaxLimit = appExitInfoBehavior.getTraceMaxLimit() - if (trace.length > traceMaxLimit) { - return AppExitInfoBehavior.CollectTracesResult.TooLarge(trace.take(traceMaxLimit)) - } - - return AppExitInfoBehavior.CollectTracesResult.Success(trace) - } catch (e: IOException) { - return AppExitInfoBehavior.CollectTracesResult.TraceException(("ioexception: ${e.message}")) - } catch (e: OutOfMemoryError) { - return AppExitInfoBehavior.CollectTracesResult.TraceException(("oom: ${e.message}")) - } catch (tr: Throwable) { - return AppExitInfoBehavior.CollectTracesResult.TraceException(("error: ${tr.message}")) - } - } - - private fun readTraceAsString(appExitInfo: ApplicationExitInfo): String? { - if (appExitInfo.isNdkProtobufFile()) { - val bytes = appExitInfo.traceInputStream?.readBytes() ?: return null - return bytes.toUTF8String() - } else { - return appExitInfo.traceInputStream?.bufferedReader()?.readText() - } - } - /** - * NDK protobuf files are only available on Android 12 and above for AEI with - * the REASON_CRASH_NATIVE reason. + * Calculates what AEI records have been sent by subtracting a collection of IDs that have been previously + * sent from the return value of getHistoricalProcessExitReasons. */ - private fun ApplicationExitInfo.isNdkProtobufFile(): Boolean { - return buildVersionChecker.isAtLeast(VERSION_CODES.S) && reason == ApplicationExitInfo.REASON_CRASH_NATIVE - } - - private fun getSessionIdValidationError(sid: String): String { - return if (sid.isEmpty() || sid.matches(Regex("^[0-9a-fA-F]{32}\$"))) { - "" - } else { - "invalid session ID: $sid" - } + private fun getUnsentRecords(records: List): List { + val deliveredIds = preferencesService.deliveredAeiIds + preferencesService.deliveredAeiIds = records.map { it.getAeiId() }.toSet() + return records.filter { !deliveredIds.contains(it.getAeiId()) } } - private fun generateUniqueHash(appExitInfo: ApplicationExitInfo): String { - return "${appExitInfo.timestamp}_${appExitInfo.pid}" - } + private fun ApplicationExitInfo.getAeiId(): String = "${timestamp}_$pid" } diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/ApplicationExitInfoExt.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/ApplicationExitInfoExt.kt new file mode 100644 index 0000000000..d46e2f8de8 --- /dev/null +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/ApplicationExitInfoExt.kt @@ -0,0 +1,89 @@ +package io.embrace.android.embracesdk.internal.capture.aei + +import android.app.ApplicationExitInfo +import android.os.Build.VERSION_CODES +import androidx.annotation.RequiresApi +import io.embrace.android.embracesdk.internal.capture.aei.TraceResult.Failure +import io.embrace.android.embracesdk.internal.capture.aei.TraceResult.Success +import io.embrace.android.embracesdk.internal.payload.AppExitInfoData +import io.embrace.android.embracesdk.internal.utils.VersionChecker +import io.embrace.android.embracesdk.internal.utils.toUTF8String +import java.io.IOException +import java.util.regex.Pattern + +private val SESSION_ID_PATTERN by lazy { Pattern.compile("^[0-9a-fA-F]{32}\$").toRegex() } + +/** + * Constructs an [AppExitInfoData] object from an [ApplicationExitInfo] object. The trace + * will be truncated if it is above a certain character limit. If no trace is present, this function + * will return null as we don't wish to capture this data right now. + */ +@RequiresApi(VERSION_CODES.R) +internal fun ApplicationExitInfo.constructAeiObject( + versionChecker: VersionChecker, + charLimit: Int, +): AppExitInfoData? { + val result = readAeiTrace(versionChecker, charLimit) ?: return null + val sessionId = String(processStateSummary ?: ByteArray(0)) + + return AppExitInfoData( + sessionId = sessionId, + sessionIdError = getSessionIdValidationError(sessionId), + importance = importance, + pss = pss, + reason = reason, + rss = rss, + status = status, + timestamp = timestamp, + trace = result.trace, + description = description, + traceStatus = result.errMsg, + ) +} + +/** + * Reads an AEI trace as a string and attempts to sanitize/handle exceptions + edge cases. + */ +@RequiresApi(VERSION_CODES.R) +private fun ApplicationExitInfo.readAeiTrace( + versionChecker: VersionChecker, + charLimit: Int, +): TraceResult? { + return try { + val trace = readTraceAsString(versionChecker) ?: return null + + when { + trace.length > charLimit -> Success(trace.take(charLimit), "Trace was too large, sending truncated trace") + else -> Success(trace) + } + } catch (e: IOException) { + Failure("ioexception: ${e.message}") + } catch (e: OutOfMemoryError) { + Failure("oom: ${e.message}") + } catch (tr: Throwable) { + Failure("error: ${tr.message}") + } +} + +@RequiresApi(VERSION_CODES.R) +private fun ApplicationExitInfo.readTraceAsString(versionChecker: VersionChecker): String? { + return if (isNdkProtobufFile(versionChecker)) { + val bytes = traceInputStream?.buffered()?.readBytes() ?: return null + bytes.toUTF8String() + } else { + traceInputStream?.bufferedReader()?.readText() + } +} + +/** + * NDK protobuf files are only available on Android 12 and above for AEI with + * the REASON_CRASH_NATIVE reason. + */ +private fun ApplicationExitInfo.isNdkProtobufFile(versionChecker: VersionChecker): Boolean { + return versionChecker.isAtLeast(VERSION_CODES.S) && reason == ApplicationExitInfo.REASON_CRASH_NATIVE +} + +private fun getSessionIdValidationError(sid: String): String = when { + sid.isEmpty() || sid.matches(SESSION_ID_PATTERN) -> "" + else -> "invalid session ID: $sid" +} diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/TraceResult.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/TraceResult.kt new file mode 100644 index 0000000000..609a67a886 --- /dev/null +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/capture/aei/TraceResult.kt @@ -0,0 +1,6 @@ +package io.embrace.android.embracesdk.internal.capture.aei + +internal sealed class TraceResult(val trace: String?, val errMsg: String?) { + class Success(result: String?, errMsg: String? = null) : TraceResult(result, errMsg) + class Failure(message: String?) : TraceResult(null, message) +} diff --git a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/FeatureModuleImpl.kt b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/FeatureModuleImpl.kt index f208561ece..c76fef262a 100644 --- a/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/FeatureModuleImpl.kt +++ b/embrace-android-features/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/FeatureModuleImpl.kt @@ -181,11 +181,12 @@ internal class FeatureModuleImpl( } private val aeiService: AeiDataSourceImpl? by singleton { - if (BuildVersionChecker.isAtLeast(Build.VERSION_CODES.R)) { + val activityManager = systemServiceModule.activityManager + if (BuildVersionChecker.isAtLeast(Build.VERSION_CODES.R) && activityManager != null) { AeiDataSourceImpl( workerThreadModule.backgroundWorker(Worker.Background.NonIoRegWorker), configService.appExitInfoBehavior, - systemServiceModule.activityManager, + activityManager, androidServicesModule.preferencesService, logWriter, initModule.logger diff --git a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImplTest.kt b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImplTest.kt index 719e777cbd..0160c4b6ea 100644 --- a/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImplTest.kt +++ b/embrace-android-features/src/test/java/io/embrace/android/embracesdk/internal/capture/aei/AeiDataSourceImplTest.kt @@ -175,7 +175,7 @@ internal class AeiDataSourceImplTest { every { mockActivityManager.getHistoricalProcessExitReasons(any(), any(), any()) } returns listOf(appExitInfo1, appExitInfo2, appExitInfo3) - preferenceService.applicationExitInfoHistory = setOf( + preferenceService.deliveredAeiIds = setOf( appExitInfo1Hash, appExitInfo2Hash ) diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePreferenceService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePreferenceService.kt index 27460d72c0..bd6c151148 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePreferenceService.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePreferenceService.kt @@ -27,7 +27,7 @@ class FakePreferenceService( override var javaScriptPatchNumber: String? = null, override var embraceFlutterSdkVersion: String? = null, override var jailbroken: Boolean? = null, - override var applicationExitInfoHistory: Set? = null, + override var deliveredAeiIds: Set = emptySet(), val sessionNumber: () -> Int = { 0 }, val bgActivityNumber: () -> Int = { 5 }, ) : PreferencesService {