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

Remove unityCrashId, as we don't use it anymore #1702

Merged
merged 1 commit into from
Nov 21, 2024
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
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