Skip to content

Commit

Permalink
refactor: inline error service action
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed May 10, 2024
1 parent a1ef581 commit 657137c
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 258 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import android.os.Build
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.embrace.android.embracesdk.IntegrationTestRule
import io.embrace.android.embracesdk.Severity
import io.embrace.android.embracesdk.fakes.FakeInternalErrorService
import io.embrace.android.embracesdk.fakes.FakeLogRecordExporter
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.recordSession
import org.junit.Assert
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.annotation.Config
import java.lang.Thread.sleep
import org.junit.Assert.*

@Config(sdk = [Build.VERSION_CODES.TIRAMISU])
@RunWith(AndroidJUnit4::class)
Expand All @@ -39,17 +40,19 @@ internal class LogRecordExporterTest {

sleep(3000)
}
Assert.assertTrue((fakeLogRecordExporter.exportedLogs?.size ?: 0) > 0)
Assert.assertEquals("test message", fakeLogRecordExporter.exportedLogs?.first()?.body?.asString())
assertTrue((fakeLogRecordExporter.exportedLogs?.size ?: 0) > 0)
assertEquals("test message", fakeLogRecordExporter.exportedLogs?.first()?.body?.asString())
}
}

@Test
fun `a LogRecordExporter added after initialization won't be used`() {
with(testRule) {

val fake = FakeLogAction()
harness.overriddenInitModule.logger.apply { addLoggerAction(fake) }
val fake = FakeInternalErrorService()
harness.overriddenInitModule.logger.apply {
internalErrorService = fake
}

val fakeLogRecordExporter = FakeLogRecordExporter()
embrace.start(harness.overriddenCoreModule.context)
Expand All @@ -60,10 +63,7 @@ internal class LogRecordExporterTest {

sleep(3000)
}
Assert.assertTrue((fakeLogRecordExporter.exportedLogs?.size ?: 0) == 0)
Assert.assertTrue(fake.msgQueue.any {
it.msg == "A LogRecordExporter can only be added before the SDK is started."
})
assertTrue((fakeLogRecordExporter.exportedLogs?.size ?: 0) == 0)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package io.embrace.android.embracesdk.testcases
import android.os.Build
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.embrace.android.embracesdk.IntegrationTestRule
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.fakes.FakeInternalErrorService
import io.embrace.android.embracesdk.fakes.FakeSpanExporter
import io.embrace.android.embracesdk.opentelemetry.assertExpectedAttributes
import io.embrace.android.embracesdk.opentelemetry.assertHasEmbraceAttribute
Expand Down Expand Up @@ -71,8 +71,10 @@ internal class SpanTest {
fun `a SpanExporter added after initialization won't be used`() {
with(testRule) {

val fake = FakeLogAction()
harness.overriddenInitModule.logger.apply { addLoggerAction(fake) }
val fake = FakeInternalErrorService()
harness.overriddenInitModule.logger.apply {
internalErrorService = fake
}

val fakeSpanExporter = FakeSpanExporter()
embrace.start(harness.overriddenCoreModule.context)
Expand All @@ -84,9 +86,6 @@ internal class SpanTest {
Thread.sleep(3000)
}
assertTrue(fakeSpanExporter.exportedSpans.size == 0)
assertTrue(fake.msgQueue.any {
it.msg == "A SpanExporter can only be added before the SDK is started."
})
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import io.embrace.android.embracesdk.internal.utils.ThrowableUtilsKt;
import io.embrace.android.embracesdk.logging.EmbLogger;
import io.embrace.android.embracesdk.logging.InternalErrorService;
import io.embrace.android.embracesdk.logging.InternalErrorServiceAction;
import io.embrace.android.embracesdk.ndk.NativeModule;
import io.embrace.android.embracesdk.ndk.NdkService;
import io.embrace.android.embracesdk.network.EmbraceNetworkRequest;
Expand Down Expand Up @@ -855,7 +854,7 @@ void logInternalError(@Nullable String message, @Nullable String details) {
} else {
messageWithDetails = message;
}
internalErrorService.handleInternalError(new InternalErrorServiceAction.InternalError(messageWithDetails));
internalErrorService.handleInternalError(new RuntimeException(messageWithDetails));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ internal class ModuleInitBootstrapper(

postInit(SdkObservabilityModule::class) {
serviceRegistry.registerService(sdkObservabilityModule.internalErrorService)
initModule.logger.addLoggerAction(sdkObservabilityModule.reportingLoggerAction)
initModule.logger.internalErrorService = sdkObservabilityModule.internalErrorService
}

nativeModule = init(NativeModule::class) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ package io.embrace.android.embracesdk.injection

import io.embrace.android.embracesdk.logging.EmbraceInternalErrorService
import io.embrace.android.embracesdk.logging.InternalErrorService
import io.embrace.android.embracesdk.logging.InternalErrorServiceAction

/**
* Contains dependencies that are used to gain internal observability into how the SDK
* is performing.
*/
internal interface SdkObservabilityModule {
val internalErrorService: InternalErrorService
val reportingLoggerAction: InternalErrorServiceAction
}

internal class SdkObservabilityModuleImpl(
Expand All @@ -24,8 +22,4 @@ internal class SdkObservabilityModuleImpl(
initModule.clock
)
}

override val reportingLoggerAction: InternalErrorServiceAction by singleton {
InternalErrorServiceAction(internalErrorService)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ package io.embrace.android.embracesdk.logging
*/
internal interface EmbLogger {

fun addLoggerAction(action: EmbLoggerImpl.LogAction)
var internalErrorService: InternalErrorService?

/**
* Logs a debug message with an optional throwable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package io.embrace.android.embracesdk.logging

import android.util.Log
import io.embrace.android.embracesdk.internal.ApkToolsConfig
import java.util.concurrent.CopyOnWriteArrayList

internal const val EMBRACE_TAG = "[Embrace]"

Expand All @@ -11,18 +10,9 @@ internal const val EMBRACE_TAG = "[Embrace]"
* Can only be used internally, it's not part of the public API.
*/

// Suppressing "Nothing to inline". These functions are used all around the codebase, pretty often, so we want them to
// perform as fast as possible.
internal class EmbLoggerImpl : EmbLogger {
private val logActions = CopyOnWriteArrayList<LogAction>(listOf())

internal fun interface LogAction {
fun log(msg: String, severity: Severity, throwable: Throwable?, logStacktrace: Boolean)
}

override fun addLoggerAction(action: LogAction) {
logActions.add(action)
}
override var internalErrorService: InternalErrorService? = null

override fun logDebug(msg: String, throwable: Throwable?) {
log(msg, Severity.DEBUG, throwable, true)
Expand Down Expand Up @@ -58,8 +48,14 @@ internal class EmbLoggerImpl : EmbLogger {
if (severity >= Severity.INFO || ApkToolsConfig.IS_DEVELOPER_LOGGING_ENABLED) {
logcatImpl(throwable, logStacktrace, severity, msg)

logActions.forEach {
it.log(msg, severity, throwable, logStacktrace)
// report to internal error service if necessary
if (throwable != null) {
try {
internalErrorService?.handleInternalError(throwable)
} catch (exc: Throwable) {
// don't cause a crash loop!
Log.w(EMBRACE_TAG, msg, exc)
}
}
}
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ import io.embrace.android.embracesdk.comms.delivery.EmbraceCacheService.Companio
import io.embrace.android.embracesdk.comms.delivery.EmbraceCacheService.Companion.TEMP_COPY_SUFFIX
import io.embrace.android.embracesdk.comms.delivery.PendingApiCall
import io.embrace.android.embracesdk.comms.delivery.PendingApiCalls
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.fakes.FakeEmbLogger
import io.embrace.android.embracesdk.fakes.FakeStorageService
import io.embrace.android.embracesdk.fakes.TestPlatformSerializer
import io.embrace.android.embracesdk.fakes.fakeSession
import io.embrace.android.embracesdk.fixtures.testSessionMessage
import io.embrace.android.embracesdk.fixtures.testSessionMessage2
import io.embrace.android.embracesdk.fixtures.testSessionMessageOneMinuteLater
import io.embrace.android.embracesdk.logging.EmbLoggerImpl
import io.embrace.android.embracesdk.network.http.HttpMethod
import io.embrace.android.embracesdk.payload.Session
import io.embrace.android.embracesdk.payload.SessionMessage
Expand All @@ -38,15 +37,13 @@ internal class EmbraceCacheServiceTest {

private lateinit var service: CacheService
private lateinit var storageManager: FakeStorageService
private lateinit var loggerAction: FakeLogAction
private lateinit var logger: EmbLoggerImpl
private lateinit var logger: FakeEmbLogger
private val serializer = TestPlatformSerializer()

@Before
fun setUp() {
storageManager = FakeStorageService()
loggerAction = FakeLogAction()
logger = EmbLoggerImpl().apply { addLoggerAction(loggerAction) }
logger = FakeEmbLogger()
service = EmbraceCacheService(
storageManager,
serializer,
Expand Down Expand Up @@ -359,8 +356,8 @@ internal class EmbraceCacheServiceTest {
assertEquals(1, filesAgain.size)
assertEquals(files[0], filesAgain[0])

val errors = loggerAction.msgQueue.filter { it.severity == EmbLoggerImpl.Severity.ERROR }
assertEquals("The following errors were logged: $errors", 0, errors.size)
val errors = logger.errorMessages
assertEquals("The following errors were logged: $errors", 0, logger.errorMessages.size)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import io.embrace.android.embracesdk.comms.delivery.PendingApiCall
import io.embrace.android.embracesdk.comms.delivery.PendingApiCalls
import io.embrace.android.embracesdk.comms.delivery.SessionPurgeException
import io.embrace.android.embracesdk.fakes.FakeClock
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.fakes.FakeInternalErrorService
import io.embrace.android.embracesdk.fakes.FakeStorageService
import io.embrace.android.embracesdk.fakes.fakeSession
import io.embrace.android.embracesdk.fixtures.testSessionMessage
Expand Down Expand Up @@ -50,7 +50,7 @@ internal class EmbraceDeliveryCacheManagerTest {
private lateinit var deliveryCacheManager: EmbraceDeliveryCacheManager
private lateinit var storageService: StorageService
private lateinit var cacheService: EmbraceCacheService
private lateinit var loggerAction: FakeLogAction
private lateinit var fakeService: FakeInternalErrorService
private lateinit var logger: EmbLoggerImpl
private lateinit var fakeClock: FakeClock

Expand All @@ -61,8 +61,8 @@ internal class EmbraceDeliveryCacheManagerTest {
@Before
fun before() {
fakeClock = FakeClock(clockInit)
loggerAction = FakeLogAction()
logger = EmbLoggerImpl().apply { addLoggerAction(loggerAction) }
fakeService = FakeInternalErrorService()
logger = EmbLoggerImpl().apply { internalErrorService = fakeService }
storageService = FakeStorageService()
cacheService = spyk(
EmbraceCacheService(
Expand Down Expand Up @@ -198,7 +198,7 @@ internal class EmbraceDeliveryCacheManagerTest {

assertEquals(
100 - EmbraceDeliveryCacheManager.MAX_SESSIONS_CACHED,
loggerAction.msgQueue.filter { it.throwable is SessionPurgeException }.size
fakeService.throwables.filterIsInstance<SessionPurgeException>().size
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
package io.embrace.android.embracesdk.anr.detection

import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.logging.EmbLoggerImpl
import io.embrace.android.embracesdk.fakes.FakeEmbLogger
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test

internal class UnbalancedCallDetectorTest {

private lateinit var logger: EmbLoggerImpl
private lateinit var logger: FakeEmbLogger
private lateinit var detector: UnbalancedCallDetector
private lateinit var action: FakeLogAction
private val thread = Thread.currentThread()

@Before
fun setUp() {
action = FakeLogAction()
logger = EmbLoggerImpl().apply { addLoggerAction(action) }
logger = FakeEmbLogger()
detector = UnbalancedCallDetector(logger)
}

Expand Down Expand Up @@ -75,9 +72,7 @@ internal class UnbalancedCallDetectorTest {
}

private fun verifyInternalErrorLogs(expectedCount: Int) {
val messages = action.msgQueue.filter { msg ->
msg.severity == EmbLoggerImpl.Severity.ERROR && msg.logStacktrace
}
val messages = logger.errorMessages.filter { msg -> msg.logStacktrace }
assertEquals(expectedCount, messages.size)
}
}

0 comments on commit 657137c

Please sign in to comment.