Skip to content

Commit

Permalink
Remove unityCrashId, as we don't use it anymore (#1702)
Browse files Browse the repository at this point in the history
  • Loading branch information
priettt authored Nov 21, 2024
1 parent e634ee0 commit 341006b
Show file tree
Hide file tree
Showing 11 changed files with 7 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import io.embrace.android.embracesdk.internal.payload.ThreadInfo
import io.embrace.android.embracesdk.internal.prefs.PreferencesService
import io.embrace.android.embracesdk.internal.serialization.PlatformSerializer
import io.embrace.android.embracesdk.internal.spans.toOtelSeverity
import io.embrace.android.embracesdk.internal.utils.Uuid.getEmbUuid
import io.embrace.android.embracesdk.internal.utils.Uuid
import io.embrace.android.embracesdk.internal.utils.toUTF8String
import io.opentelemetry.semconv.ExceptionAttributes
import io.opentelemetry.semconv.incubating.LogIncubatingAttributes
Expand All @@ -30,7 +30,6 @@ import java.util.concurrent.CopyOnWriteArrayList
*/
internal class CrashDataSourceImpl(
private val sessionPropertiesService: SessionPropertiesService,
private val unityCrashIdProvider: () -> String?,
private val preferencesService: PreferencesService,
private val logWriter: LogWriter,
private val configService: ConfigService,
Expand Down Expand Up @@ -64,10 +63,7 @@ internal class CrashDataSourceImpl(
if (!mainCrashHandled) {
mainCrashHandled = true

// Check if the unity crash id exists. If so, means that the native crash capture
// is enabled for an Unity build. When a native crash occurs and the NDK sends an
// uncaught exception the SDK assign the unity crash id as the java crash id.
val crashId = unityCrashIdProvider() ?: getEmbUuid()
val crashId = Uuid.getEmbUuid()
val crashNumber = preferencesService.incrementAndGetCrashNumber()
val crashAttributes = TelemetryAttributes(
configService = configService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ internal class CrashModuleImpl(
storageModule: StorageModule,
essentialServiceModule: EssentialServiceModule,
configModule: ConfigModule,
androidServicesModule: AndroidServicesModule,
private val unityCrashIdProvider: () -> String?,
androidServicesModule: AndroidServicesModule
) : CrashModule {

private val crashMarker: CrashFileMarker by singleton {
Expand All @@ -25,7 +24,6 @@ internal class CrashModuleImpl(
override val crashDataSource: CrashDataSource by singleton {
CrashDataSourceImpl(
essentialServiceModule.sessionPropertiesService,
unityCrashIdProvider,
androidServicesModule.preferencesService,
essentialServiceModule.logWriter,
configModule.configService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ typealias CrashModuleSupplier = (
essentialServiceModule: EssentialServiceModule,
configModule: ConfigModule,
androidServicesModule: AndroidServicesModule,
unityCrashIdProvider: () -> String?,
) -> CrashModule

fun createCrashModule(
Expand All @@ -18,14 +17,12 @@ fun createCrashModule(
essentialServiceModule: EssentialServiceModule,
configModule: ConfigModule,
androidServicesModule: AndroidServicesModule,
unityCrashIdProvider: () -> String?,
): CrashModule {
return CrashModuleImpl(
initModule,
storageModule,
essentialServiceModule,
configModule,
androidServicesModule,
unityCrashIdProvider,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import io.embrace.android.embracesdk.internal.crash.CrashFileMarkerImpl
import io.embrace.android.embracesdk.internal.logging.EmbLogger
import io.embrace.android.embracesdk.internal.logging.InternalErrorType
import io.embrace.android.embracesdk.internal.ndk.jni.JniDelegate
import io.embrace.android.embracesdk.internal.payload.AppFramework
import io.embrace.android.embracesdk.internal.payload.NativeCrashData
import io.embrace.android.embracesdk.internal.payload.NativeCrashDataError
import io.embrace.android.embracesdk.internal.payload.NativeCrashMetadata
Expand Down Expand Up @@ -55,7 +54,6 @@ internal class EmbraceNdkService(
private val handler: Handler = Handler(checkNotNull(Looper.getMainLooper())),
) : NdkService, ProcessStateListener {

override var unityCrashId: String? = null
override val symbolsForCurrentArch by lazy {
val nativeSymbols = getNativeSymbols()
if (nativeSymbols != null) {
Expand All @@ -72,9 +70,6 @@ internal class EmbraceNdkService(
userService.addUserInfoListener(::onUserInfoUpdate)
sessionIdTracker.addListener { updateSessionId(it ?: "") }
sessionPropertiesService.addChangeListener(::onSessionPropertiesUpdate)
if (configService.appFramework == AppFramework.UNITY) {
unityCrashId = Uuid.getEmbUuid()
}
repository.cleanOldCrashFiles()
}
}
Expand Down Expand Up @@ -163,9 +158,7 @@ internal class EmbraceNdkService(
CrashFileMarkerImpl.CRASH_MARKER_FILE_NAME
).absolutePath

// Assign the native crash id to the unity crash id. Then when a unity crash occurs, the
// Embrace crash service will set the unity crash id to the java crash.
val nativeCrashId: String = unityCrashId ?: Uuid.getEmbUuid()
val nativeCrashId: String = Uuid.getEmbUuid()
val is32bit = deviceArchitecture.is32BitDevice
Systrace.traceSynchronous("native-install-handlers") {
delegate.installSignalHandlers(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ interface NdkService {

fun onUserInfoUpdate()

val unityCrashId: String?

/**
* Get the latest stored [NativeCrashData] instance and purge all existing native crash data files.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ internal class CrashDataSourceImplTest {
)
crashDataSource = CrashDataSourceImpl(
sessionPropertiesService,
ndkService::unityCrashId,
preferencesService,
logWriter,
configService,
Expand Down Expand Up @@ -129,22 +128,6 @@ internal class CrashDataSourceImplTest {
assertSame(lastSentCrash, logWriter.logEvents.single())
}

@Test
fun `test LogWriter and SessionOrchestrator are called when handleCrash is called with unityId`() {
setupForHandleCrash()
ndkService.lastUnityCrashId = "Unity123"

crashDataSource.handleCrash(testException)

assertEquals(1, anrService.crashCount)
assertEquals(1, logWriter.logEvents.size)
assertEquals(
logWriter.logEvents.single().schemaType.attributes()[LogIncubatingAttributes.LOG_RECORD_UID.key],
sessionOrchestrator.crashId
)
assertEquals(ndkService.lastUnityCrashId, sessionOrchestrator.crashId)
}

@Test
fun `test handleCrash calls mark() method when capture_last_run config is enabled`() {
setupForHandleCrash()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package io.embrace.android.embracesdk.internal.injection

import io.embrace.android.embracesdk.fakes.FakeConfigModule
import io.embrace.android.embracesdk.fakes.FakeConfigService
import io.embrace.android.embracesdk.fakes.FakeNativeFeatureModule
import io.embrace.android.embracesdk.fakes.behavior.FakeAutoDataCaptureBehavior
import io.embrace.android.embracesdk.fakes.injection.FakeAndroidServicesModule
import io.embrace.android.embracesdk.fakes.injection.FakeEssentialServiceModule
Expand All @@ -17,22 +16,19 @@ internal class CrashModuleImplTest {

@Test
fun testDefaultImplementations() {
val nativeFeatureModule = FakeNativeFeatureModule()
val module = createCrashModule(
FakeInitModule(),
FakeStorageModule(),
FakeEssentialServiceModule(),
FakeConfigModule(),
FakeAndroidServicesModule(),
nativeFeatureModule.ndkService::unityCrashId,
)
assertNotNull(module.lastRunCrashVerifier)
assertNotNull(module.crashDataSource)
}

@Test
fun `default config turns on v2 native crash service`() {
val nativeFeatureModule = FakeNativeFeatureModule()
val module = createCrashModule(
FakeInitModule(),
FakeStorageModule(),
Expand All @@ -43,7 +39,6 @@ internal class CrashModuleImplTest {
)
),
FakeAndroidServicesModule(),
nativeFeatureModule.ndkService::unityCrashId,
)
assertNotNull(module.lastRunCrashVerifier)
assertNotNull(module.crashDataSource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,7 @@ internal class EmbraceNdkServiceTest {
}

@Test
fun `test initialization with unity id and ndk enabled runs installSignals and updateDeviceMetaData`() {
val unityId = "unityId"
every { Uuid.getEmbUuid() } returns unityId

fun `test initialization with ndk enabled runs installSignals and updateDeviceMetaData`() {
configService.appFramework = AppFramework.UNITY
initializeService()
assertEquals(1, processStateService.listeners.size)
Expand All @@ -222,7 +219,7 @@ internal class EmbraceNdkServiceTest {
markerFilePath,
"null",
"foreground",
unityId,
any(),
Build.VERSION.SDK_INT,
deviceArchitecture.is32BitDevice,
false
Expand All @@ -239,7 +236,6 @@ internal class EmbraceNdkServiceTest {
)

verify(exactly = 1) { delegate.updateMetaData(newDeviceMetaData) }
assertEquals(embraceNdkService.unityCrashId, Uuid.getEmbUuid())
}

@Test
Expand Down Expand Up @@ -268,15 +264,6 @@ internal class EmbraceNdkServiceTest {
}
}

@Test
fun `test getUnityCrashId`() {
configService.appFramework = AppFramework.UNITY
every { Uuid.getEmbUuid() } returns "unityId"
initializeService()
val uuid = embraceNdkService.unityCrashId
assertEquals(uuid, "unityId")
}

@Test
fun `test onUserInfoUpdate where _updateMetaData was executed and isInstalled true`() {
initializeService()
Expand Down Expand Up @@ -330,7 +317,6 @@ internal class EmbraceNdkServiceTest {
@Test
fun `test getLatestNativeCrash catches an exception if _getCrashReport returns invalid json syntax`() {
repository.addCrashFiles(File.createTempFile("test", "test"))
every { Uuid.getEmbUuid() } returns "unityId"

val json = "{\n" +
" \"sid\": [\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,6 @@ internal class ModuleInitBootstrapper(
essentialServiceModule,
configModule,
androidServicesModule,
nativeFeatureModule.ndkService::unityCrashId
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ internal fun fakeModuleInitBootstrapper(
nativeCoreModuleSupplier: NativeCoreModuleSupplier = { _ -> FakeNativeCoreModule() },
sessionOrchestrationModuleSupplier: SessionOrchestrationModuleSupplier =
{ _, _, _, _, _, _, _, _, _, _ -> FakeSessionOrchestrationModule() },
crashModuleSupplier: CrashModuleSupplier = { _, _, _, _, _, _ -> FakeCrashModule() },
crashModuleSupplier: CrashModuleSupplier = { _, _, _, _, _ -> FakeCrashModule() },
payloadSourceModuleSupplier: PayloadSourceModuleSupplier =
{ _, _, _, _, _, _, _, _, _, _, _ -> FakePayloadSourceModule() },
) = ModuleInitBootstrapper(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ class FakeNdkService : NdkService {
val propUpdates: MutableList<Map<String, String>> = mutableListOf()
var sessionId: String? = null
var userUpdateCount: Int = 0
var lastUnityCrashId: String? = null
private val nativeCrashDataBlobs = mutableListOf<NativeCrashData>()

override fun initializeService(sessionIdTracker: SessionIdTracker) {
Expand All @@ -30,11 +29,6 @@ class FakeNdkService : NdkService {
userUpdateCount++
}

override val unityCrashId: String?
get() {
return lastUnityCrashId
}

override fun getLatestNativeCrash(): NativeCrashData? = nativeCrashDataBlobs.lastOrNull()

override fun getNativeCrashes(): List<NativeCrashData> = nativeCrashDataBlobs
Expand Down

0 comments on commit 341006b

Please sign in to comment.