Skip to content

Commit

Permalink
refactor: avoid log stacktrace concept
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed May 10, 2024
1 parent 98ea20b commit f74d08b
Show file tree
Hide file tree
Showing 23 changed files with 73 additions and 87 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void startInternal(@NonNull Context context,
Systrace.endSynchronous();
} catch (Throwable t) {
logger.logError(
"Error occurred while initializing the Embrace SDK. Instrumentation may be disabled.", t, true);
"Error occurred while initializing the Embrace SDK. Instrumentation may be disabled.", t);
}
}

Expand All @@ -255,7 +255,7 @@ private void startImpl(@NonNull Context context,
@NonNull Function0<ConfigService> configServiceProvider) {
if (application != null) {
// We don't hard fail if the SDK has been already initialized.
logger.logWarning("Embrace SDK has already been initialized", null, false);
logger.logWarning("Embrace SDK has already been initialized", null);
return;
}

Expand Down Expand Up @@ -414,7 +414,7 @@ private void registerComposeActivityListener(@NonNull CoreModule coreModule) {
composeActivityListenerInstance = composeActivityListener.newInstance();
coreModule.getApplication().registerActivityLifecycleCallbacks((Application.ActivityLifecycleCallbacks) composeActivityListenerInstance);
} catch (Throwable e) {
logger.logError("registerComposeActivityListener error", e, false);
logger.logError("registerComposeActivityListener error", e);

Check warning on line 417 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L417

Added line #L417 was not covered by tests
}
}

Expand All @@ -427,7 +427,7 @@ private void unregisterComposeActivityListener(@NonNull Application app) {
try {
app.unregisterActivityLifecycleCallbacks((Application.ActivityLifecycleCallbacks) composeActivityListenerInstance);
} catch (Throwable e) {
logger.logError("Instantiation error for ComposeActivityListener", e, false);
logger.logError("Instantiation error for ComposeActivityListener", e);

Check warning on line 430 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L430

Added line #L430 was not covered by tests
}
}

Expand All @@ -449,16 +449,16 @@ boolean isStarted() {
*/
boolean setAppId(@NonNull String appId) {
if (isStarted()) {
logger.logError("You must set the custom app ID before the SDK is started.", null, false);
logger.logError("You must set the custom app ID before the SDK is started.", null);
return false;
}
if (appId.isEmpty()) {
logger.logError("App ID cannot be null or empty.", null, false);
logger.logError("App ID cannot be null or empty.", null);
return false;
}
if (!appIdPattern.matcher(appId).find()) {
logger.logError("Invalid app ID. Must be a 5-character string with " +
"characters from the set [A-Za-z0-9], but it was \"" + appId + "\".", null, false);
"characters from the set [A-Za-z0-9], but it was \"" + appId + "\".", null);
return false;
}

Expand All @@ -480,7 +480,7 @@ void stop() {
application = null;
moduleInitBootstrapper.stopServices();
} catch (Exception ex) {
logger.logError("Error while shutting down Embrace SDK", ex, false);
logger.logError("Error while shutting down Embrace SDK", ex);

Check warning on line 483 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L483

Added line #L483 was not covered by tests
}
}
}
Expand Down Expand Up @@ -1103,7 +1103,7 @@ void logRnAction(@NonNull String name, long startTime, long endTime,
*/
void logRnView(@NonNull String screen) {
if (appFramework != Embrace.AppFramework.REACT_NATIVE) {
logger.logWarning("[Embrace] logRnView is only available on React Native", null, false);
logger.logWarning("[Embrace] logRnView is only available on React Native", null);

Check warning on line 1106 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L1106

Added line #L1106 was not covered by tests
return;
}

Expand Down Expand Up @@ -1154,10 +1154,10 @@ private void sampleCurrentThreadDuringAnrs() {
service
);
} else {
logger.logWarning("nativeThreadSamplerInstaller not started, cannot sample current thread", null, false);
logger.logWarning("nativeThreadSamplerInstaller not started, cannot sample current thread", null);

Check warning on line 1157 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L1157

Added line #L1157 was not covered by tests
}
} catch (Exception exc) {
logger.logError("Failed to sample current thread during ANRs", exc, false);
logger.logError("Failed to sample current thread during ANRs", exc);

Check warning on line 1160 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L1160

Added line #L1160 was not covered by tests
}
}

Expand All @@ -1168,7 +1168,7 @@ private Map<String, Object> normalizeProperties(@Nullable Map<String, ?> propert
try {
normalizedProperties = PropertyUtils.sanitizeProperties(properties, logger);
} catch (Exception e) {
this.logger.logError("Exception occurred while normalizing the properties.", e, false);
this.logger.logError("Exception occurred while normalizing the properties.", e);

Check warning on line 1171 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.java#L1171

Added line #L1171 was not covered by tests
}
return normalizedProperties;
} else {
Expand Down Expand Up @@ -1199,15 +1199,15 @@ private boolean checkSdkStarted(@NonNull String action, boolean logPublicApiUsag

public void addSpanExporter(@NonNull SpanExporter spanExporter) {
if (isStarted()) {
logger.logError("A SpanExporter can only be added before the SDK is started.", null, false);
logger.logError("A SpanExporter can only be added before the SDK is started.", null);
return;
}
moduleInitBootstrapper.getOpenTelemetryModule().getOpenTelemetryConfiguration().addSpanExporter(spanExporter);
}

public void addLogRecordExporter(@NonNull LogRecordExporter logRecordExporter) {
if (isStarted()) {
logger.logError("A LogRecordExporter can only be added before the SDK is started.", null, false);
logger.logError("A LogRecordExporter can only be added before the SDK is started.", null);
return;
}
moduleInitBootstrapper.getOpenTelemetryModule().getOpenTelemetryConfiguration().addLogExporter(logRecordExporter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ internal class EmbraceAnrService(
}
anrMonitorWorker.submit(callable).get(MAX_DATA_WAIT_MS, TimeUnit.MILLISECONDS)
} catch (exc: Exception) {
logger.logWarning("Failed to getAnrIntervals()", exc, true)
logger.logWarning("Failed to getAnrIntervals()", exc)
logger.trackInternalError(InternalErrorType.ANR_DATA_FETCH, exc)
emptyList()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ internal class LivenessCheckScheduler internal constructor(
} catch (exc: Exception) {
// ignore any RejectedExecution - ScheduledExecutorService only throws when shutting down.
val message = "ANR capture initialization failed"
logger.logWarning(message, exc, true)
logger.logWarning(message, exc)
}
}

Expand Down Expand Up @@ -150,7 +150,7 @@ internal class LivenessCheckScheduler internal constructor(
blockedThreadDetector.updateAnrTracking(now)
}
} catch (exc: Exception) {
logger.logError("Failed to process ANR monitor thread heartbeat", exc, true)
logger.logError("Failed to process ANR monitor thread heartbeat", exc)
logger.trackInternalError(InternalErrorType.ANR_HEARTBEAT_CHECK_FAIL, exc)

Check warning on line 154 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/anr/detection/LivenessCheckScheduler.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/anr/detection/LivenessCheckScheduler.kt#L153-L154

Added lines #L153 - L154 were not covered by tests
}
}
Expand All @@ -160,8 +160,7 @@ internal class LivenessCheckScheduler internal constructor(
if (!targetThreadHandler.sendMessage(heartbeatMessage)) {
logger.logWarning(
"Failed to send message to targetHandler, main thread likely shutting down.",
IllegalStateException("Failed to send message to targetHandler"),
true
IllegalStateException("Failed to send message to targetHandler")
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal class TargetThreadHandler(
}
}
} catch (ex: Exception) {
logger.logError("ANR healthcheck failed in main (monitored) thread", ex, true)
logger.logError("ANR healthcheck failed in main (monitored) thread", ex)

Check warning on line 69 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/anr/detection/TargetThreadHandler.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/anr/detection/TargetThreadHandler.kt#L69

Added line #L69 was not covered by tests
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ internal class UnbalancedCallDetector(
private fun checkTimeTravel(name: String, timestamp: Long) {
if (lastTimestamp > timestamp) {
val msg = "Time travel in $name. $lastTimestamp to $timestamp"
logger.logWarning(msg, IllegalStateException(msg), true)
logger.logWarning(msg)
logger.trackInternalError(InternalErrorType.TIME_TRAVEL, IllegalStateException("Time Travel"))
}
lastTimestamp = timestamp
Expand All @@ -44,7 +44,7 @@ internal class UnbalancedCallDetector(
if (blocked != expected) {
val threadName = Thread.currentThread().name
val msg = "Unbalanced call to $name in ANR detection. Thread=$threadName"
logger.logWarning(msg, IllegalStateException(msg), true)
logger.logWarning(msg)
logger.trackInternalError(InternalErrorType.UNBALANCED_CALL, IllegalStateException("Unbalanced call"))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ internal class AeiDataSourceImpl(
} catch (exc: Throwable) {
logger.logWarning(
"AEI - Failed to process AEIs due to unexpected error",
exc,
true
exc
)
logger.trackInternalError(InternalErrorType.ENABLE_DATA_CAPTURE, exc)
}
Expand Down Expand Up @@ -234,13 +233,13 @@ internal class AeiDataSourceImpl(

return AppExitInfoBehavior.CollectTracesResult.Success(trace)
} catch (e: IOException) {
logger.logWarning("AEI - IOException: ${e.message}", e, true)
logger.logWarning("AEI - IOException", e)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("ioexception: ${e.message}"))
} catch (e: OutOfMemoryError) {
logger.logWarning("AEI - Out of Memory: ${e.message}", e, true)
logger.logWarning("AEI - Out of Memory", e)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("oom: ${e.message}"))
} catch (tr: Throwable) {
logger.logWarning("AEI - An error occurred: ${tr.message}", tr, true)
logger.logWarning("AEI - An error occurred", tr)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("error: ${tr.message}"))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ internal class EmbraceCpuInfoDelegate(
try {
getNativeCpuName()
} catch (exception: LinkageError) {
logger.logWarning("Could not get the CPU name. Exception: $exception", exception)
logger.logWarning("Could not get the CPU name.", exception)

Check warning on line 16 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/cpu/EmbraceCpuInfoDelegate.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/cpu/EmbraceCpuInfoDelegate.kt#L16

Added line #L16 was not covered by tests
null
}
} else {
Expand All @@ -26,7 +26,7 @@ internal class EmbraceCpuInfoDelegate(
try {
getNativeEgl()
} catch (exception: LinkageError) {
logger.logWarning("Could not get the EGL name. Exception: $exception", exception)
logger.logWarning("Could not get the EGL name.", exception)

Check warning on line 29 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/cpu/EmbraceCpuInfoDelegate.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/cpu/EmbraceCpuInfoDelegate.kt#L29

Added line #L29 was not covered by tests
null
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal class EmbraceUncaughtExceptionHandler(
logger.logError("Error occurred in the uncaught exception handler", ex)
logger.trackInternalError(InternalErrorType.UNCAUGHT_EXC_HANDLER, ex)
} finally {
logger.logDebug("Finished handling exception. Delegating to default handler.", exception)
logger.logDebug("Finished handling exception. Delegating to default handler.")
defaultHandler?.uncaughtException(thread, exception)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal class EmbraceUserService(
return try {
ofStored(preferencesService)
} catch (ex: Exception) {
logger.logError("Failed to load user info from persistent storage.", ex, true)
logger.logError("Failed to load user info from persistent storage.", ex)
logger.trackInternalError(InternalErrorType.USER_LOAD_FAIL, ex)

Check warning on line 27 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/user/EmbraceUserService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/capture/user/EmbraceUserService.kt#L26-L27

Added lines #L26 - L27 were not covered by tests
null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal class EmbraceDeliveryCacheManager(
}
}
} catch (exc: Throwable) {
logger.logError("Save session failed", exc, true)
logger.logError("Save session failed", exc)

Check warning on line 65 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryCacheManager.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryCacheManager.kt#L65

Added line #L65 was not covered by tests
throw exc
}
}
Expand Down Expand Up @@ -243,7 +243,7 @@ internal class EmbraceDeliveryCacheManager(
}
}
} catch (ex: Throwable) {
logger.logError("Failed to cache current active session", ex, true)
logger.logError("Failed to cache current active session", ex)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ internal class EmbraceDeliveryService(
logger.logError("Session $sessionId not found")
}
} catch (ex: Throwable) {
logger.logError("Could not send cached session", ex, true)
logger.logError("Could not send cached session", ex)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ internal class EmbraceEventService(
} catch (ex: Exception) {
logger.logError(
"Cannot start event with name: $name, identifier: $identifier due to an exception",
ex,
false
ex

Check warning on line 168 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt#L168

Added line #L168 was not covered by tests
)
logger.trackInternalError(InternalErrorType.START_EVENT_FAIL, ex)

Check warning on line 170 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt#L170

Added line #L170 was not covered by tests
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ internal interface EmbLogger {
/**
* Logs a warning message with an optional throwable.
*/
fun logWarning(msg: String, throwable: Throwable? = null, logStacktrace: Boolean = false)
fun logWarning(msg: String, throwable: Throwable? = null)

/**
* Logs a warning message with an optional error.
*/
fun logError(msg: String, throwable: Throwable? = null, logStacktrace: Boolean = false)
fun logError(msg: String, throwable: Throwable? = null)

/**
* Logs a warning message that the SDK is not yet initialized for the given action.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@ internal class EmbLoggerImpl : EmbLogger {
override var internalErrorService: InternalErrorService? = null

override fun logDebug(msg: String, throwable: Throwable?) {
log(msg, Severity.DEBUG, throwable, true)
log(msg, Severity.DEBUG, throwable)
}

override fun logInfo(msg: String, throwable: Throwable?) {
log(msg, Severity.INFO, throwable, false)
log(msg, Severity.INFO, throwable)
}

override fun logWarning(msg: String, throwable: Throwable?, logStacktrace: Boolean) {
log(msg, Severity.WARNING, throwable, logStacktrace)
override fun logWarning(msg: String, throwable: Throwable?) {
log(msg, Severity.WARNING, throwable)
}

override fun logError(msg: String, throwable: Throwable?, logStacktrace: Boolean) {
log(msg, Severity.ERROR, throwable, logStacktrace)
override fun logError(msg: String, throwable: Throwable?) {
log(msg, Severity.ERROR, throwable)
}

override fun logSdkNotInitialized(action: String) {
val msg = "Embrace SDK is not initialized yet, cannot $action."
log(msg, Severity.WARNING, Throwable(msg), true)
log(msg, Severity.WARNING, Throwable(msg))

Check warning on line 35 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/logging/EmbLoggerImpl.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/logging/EmbLoggerImpl.kt#L35

Added line #L35 was not covered by tests
}

override fun trackInternalError(type: InternalErrorType, throwable: Throwable) {
Expand All @@ -50,12 +50,11 @@ internal class EmbLoggerImpl : EmbLogger {
* @param msg the message to log.
* @param severity how severe the log is. If it's lower than the threshold, the message will not be logged.
* @param throwable exception, if any.
* @param logStacktrace should add the throwable to the logging
*/
@Suppress("NOTHING_TO_INLINE") // hot path - optimize by inlining
private inline fun log(msg: String, severity: Severity, throwable: Throwable?, logStacktrace: Boolean) {
private inline fun log(msg: String, severity: Severity, throwable: Throwable?) {
if (severity >= Severity.INFO || ApkToolsConfig.IS_DEVELOPER_LOGGING_ENABLED) {
logcatImpl(throwable, logStacktrace, severity, msg)
logcatImpl(throwable, severity, msg)
}
}

Expand All @@ -65,16 +64,14 @@ internal class EmbLoggerImpl : EmbLogger {
@Suppress("NOTHING_TO_INLINE") // hot path - optimize by inlining
private inline fun logcatImpl(
throwable: Throwable?,
logStacktrace: Boolean,
severity: Severity,
msg: String
) {
val exception = throwable?.takeIf { logStacktrace }
when (severity) {
Severity.DEBUG -> Log.d(EMBRACE_TAG, msg, exception)
Severity.INFO -> Log.i(EMBRACE_TAG, msg, exception)
Severity.WARNING -> Log.w(EMBRACE_TAG, msg, exception)
Severity.ERROR -> Log.e(EMBRACE_TAG, msg, exception)
Severity.DEBUG -> Log.d(EMBRACE_TAG, msg, throwable)
Severity.INFO -> Log.i(EMBRACE_TAG, msg, throwable)
Severity.WARNING -> Log.w(EMBRACE_TAG, msg, throwable)
Severity.ERROR -> Log.e(EMBRACE_TAG, msg, throwable)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ internal class EmbraceNdkService(
""".trimIndent()
val exc = RuntimeException(errMsg)
exc.stackTrace = arrayOfNulls(0)
logger.logWarning(errMsg, exc, false)
logger.logWarning(errMsg, exc)

Check warning on line 182 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/ndk/EmbraceNdkService.kt#L182

Added line #L182 was not covered by tests
delegate._reinstallSignalHandlers()
}
}
Expand Down Expand Up @@ -334,8 +334,7 @@ internal class EmbraceNdkService(
crashFile.delete()
logger.logError(
"Failed to read native crash file {crashFilePath=" + crashFile.absolutePath + "}.",
ex,
true
ex
)
logger.trackInternalError(InternalErrorType.NATIVE_CRASH_LOAD_FAIL, ex)
}
Expand Down

0 comments on commit f74d08b

Please sign in to comment.