Skip to content

Commit

Permalink
Merge pull request #823 from embrace-io/remove-strict-mode
Browse files Browse the repository at this point in the history
Remove strict mode config
  • Loading branch information
fractalwrench committed May 10, 2024
2 parents 461946e + e504e3f commit 307d284
Show file tree
Hide file tree
Showing 43 changed files with 229 additions and 373 deletions.
6 changes: 2 additions & 4 deletions embrace-android-sdk/config/detekt/baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
<SmellBaseline>
<ManuallySuppressedIssues></ManuallySuppressedIssues>
<CurrentIssues>
<ID>ArgumentListWrapping:LegacyExceptionInfoTest.kt$LegacyExceptionInfoTest$( "io.embrace.android.embracesdk.LegacyExceptionInfoTest.testOfThrowable(LegacyExceptionInfoTest.kt:45)", info.lines.first())</ID>
<ID>ArgumentListWrapping:LegacyExceptionInfoTest.kt$LegacyExceptionInfoTest$("io.embrace.android.embracesdk.LegacyExceptionInfoTest.testOfThrowable(LegacyExceptionInfoTest.kt:45)", info.lines.first())</ID>
<ID>DataClassContainsFunctions:LegacyExceptionError.kt$LegacyExceptionError$fun addException(ex: Throwable?, appState: String?, clock: Clock)</ID>
<ID>DataClassContainsFunctions:LegacyExceptionError.kt$LegacyExceptionError$private fun getExceptionInfo(ex: Throwable?): List&lt;LegacyExceptionInfo&gt;</ID>
<ID>DataClassShouldBeImmutable:LegacyExceptionError.kt$LegacyExceptionError$@Json(name = "c") var occurrences = 0</ID>
<ID>DataClassShouldBeImmutable:LegacyExceptionError.kt$LegacyExceptionError$@Json(name = "rep") var exceptionErrors = mutableListOf&lt;LegacyExceptionErrorInfo&gt;()</ID>
<ID>DataClassShouldBeImmutable:LegacyExceptionError.kt$LegacyExceptionError$@Json(name = "c") var occurrences: Int = 0</ID>
<ID>DataClassShouldBeImmutable:LegacyExceptionError.kt$LegacyExceptionError$@Json(name = "rep") var exceptionErrors: MutableList&lt;LegacyExceptionErrorInfo&gt; = mutableListOf()</ID>
</CurrentIssues>
</SmellBaseline>
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ 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.FakeLogRecordExporter
import io.embrace.android.embracesdk.fakes.FakeLoggerAction
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.recordSession
import org.junit.Assert
import org.junit.Rule
Expand Down Expand Up @@ -49,7 +48,7 @@ internal class LogRecordExporterTest {
fun `a LogRecordExporter added after initialization won't be used`() {
with(testRule) {

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

val fakeLogRecordExporter = FakeLogRecordExporter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,13 @@ 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.Severity
import io.embrace.android.embracesdk.fakes.FakeLogRecordExporter
import io.embrace.android.embracesdk.fakes.FakeLoggerAction
import io.embrace.android.embracesdk.fakes.FakeLogAction
import io.embrace.android.embracesdk.fakes.FakeSpanExporter
import io.embrace.android.embracesdk.opentelemetry.assertExpectedAttributes
import io.embrace.android.embracesdk.opentelemetry.assertHasEmbraceAttribute
import io.embrace.android.embracesdk.opentelemetry.embProcessIdentifier
import io.embrace.android.embracesdk.opentelemetry.embSequenceId
import io.embrace.android.embracesdk.recordSession
import org.junit.Assert
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Rule
Expand Down Expand Up @@ -74,7 +71,7 @@ internal class SpanTest {
fun `a SpanExporter added after initialization won't be used`() {
with(testRule) {

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

val fakeSpanExporter = FakeSpanExporter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
import io.embrace.android.embracesdk.internal.utils.ThrowableUtilsKt;
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger;
import io.embrace.android.embracesdk.logging.InternalErrorService;
import io.embrace.android.embracesdk.logging.ReportingLoggerAction;
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 @@ -858,7 +858,7 @@ void logInternalError(@Nullable String message, @Nullable String details) {
} else {
messageWithDetails = message;
}
internalErrorService.handleInternalError(new ReportingLoggerAction.InternalError(messageWithDetails));
internalErrorService.handleInternalError(new InternalErrorServiceAction.InternalError(messageWithDetails));
}
}

Expand Down Expand Up @@ -1193,7 +1193,7 @@ private boolean checkSdkStartedAndLogPublicApiUsage(@NonNull String action) {
private boolean checkSdkStarted(@NonNull String action, boolean logPublicApiUsage) {
boolean isStarted = isStarted();
if (!isStarted) {
internalEmbraceLogger.logSDKNotInitialized(action);
internalEmbraceLogger.logSdkNotInitialized(action);
}
if (telemetryService != null && logPublicApiUsage) {
telemetryService.onPublicApiCalled(action);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal class FlutterInternalInterfaceImpl(
hostedSdkVersionInfo.hostedSdkVersion = version
}
} else {
logger.logSDKNotInitialized("setEmbraceFlutterSdkVersion")
logger.logSdkNotInitialized("setEmbraceFlutterSdkVersion")
}
}

Expand All @@ -27,7 +27,7 @@ internal class FlutterInternalInterfaceImpl(
hostedSdkVersionInfo.hostedPlatformVersion = version
}
} else {
logger.logSDKNotInitialized("setDartVersion")
logger.logSdkNotInitialized("setDartVersion")
}
}

Expand Down Expand Up @@ -73,7 +73,7 @@ internal class FlutterInternalInterfaceImpl(
message
)
} else {
logger.logSDKNotInitialized("logDartError")
logger.logSdkNotInitialized("logDartError")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal class ReactNativeInternalInterfaceImpl(
val exception = JsException(name, message, type, stacktrace)
crashService.logUnhandledJsException(exception)
} else {
logger.logSDKNotInitialized("log JS exception")
logger.logSdkNotInitialized("log JS exception")
}
}

Expand All @@ -51,7 +51,7 @@ internal class ReactNativeInternalInterfaceImpl(
null
)
} else {
logger.logSDKNotInitialized("log JS exception")
logger.logSdkNotInitialized("log JS exception")
}
}

Expand All @@ -67,15 +67,15 @@ internal class ReactNativeInternalInterfaceImpl(
}
hostedSdkVersionInfo.javaScriptPatchNumber = number
} else {
logger.logSDKNotInitialized("set JavaScript patch number")
logger.logSdkNotInitialized("set JavaScript patch number")
}
}

override fun setReactNativeSdkVersion(version: String?) {
if (embrace.isStarted) {
hostedSdkVersionInfo.hostedSdkVersion = version
} else {
logger.logSDKNotInitialized("set React Native SDK version")
logger.logSdkNotInitialized("set React Native SDK version")
}
}

Expand All @@ -91,7 +91,7 @@ internal class ReactNativeInternalInterfaceImpl(
}
hostedSdkVersionInfo.hostedPlatformVersion = version
} else {
logger.logSDKNotInitialized("set React Native version number")
logger.logSdkNotInitialized("set React Native version number")
}
}

Expand All @@ -114,7 +114,7 @@ internal class ReactNativeInternalInterfaceImpl(
}
metadataService.setReactNativeBundleId(context, url, didUpdate)
} else {
logger.logSDKNotInitialized("set JavaScript bundle URL")
logger.logSdkNotInitialized("set JavaScript bundle URL")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ internal class UnityInternalInterfaceImpl(
hostedSdkVersionInfo.unityBuildIdNumber = buildGuid
}
} else {
logger.logSDKNotInitialized("set Unity metadata")
logger.logSdkNotInitialized("set Unity metadata")
}
}

Expand Down Expand Up @@ -67,7 +67,7 @@ internal class UnityInternalInterfaceImpl(
message
)
} else {
logger.logSDKNotInitialized("log Unity exception")
logger.logSdkNotInitialized("log Unity exception")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ internal class AeiDataSourceImpl(
try {
processApplicationExitInfo()
} catch (exc: Throwable) {
logger.logWarningWithException(
logger.logWarning(
"AEI - Failed to process AEIs due to unexpected error",
exc,
true
Expand All @@ -77,7 +77,7 @@ internal class AeiDataSourceImpl(
backgroundExecution?.cancel(true)
backgroundExecution = null
} catch (t: Throwable) {
logger.logWarningWithException(
logger.logWarning(
"AEI - Failed to disable EmbraceApplicationExitInfoService work",
t
)
Expand Down Expand Up @@ -124,7 +124,6 @@ internal class AeiDataSourceImpl(
?: return emptyList()

if (historicalProcessExitReasons.size > SDK_AEI_SEND_LIMIT) {
logger.logInfoWithException("AEI - size greater than $SDK_AEI_SEND_LIMIT")
historicalProcessExitReasons = historicalProcessExitReasons.take(SDK_AEI_SEND_LIMIT)
}

Expand Down Expand Up @@ -227,19 +226,18 @@ internal class AeiDataSourceImpl(

val traceMaxLimit = appExitInfoBehavior.getTraceMaxLimit()
if (trace.length > traceMaxLimit) {
logger.logInfoWithException("AEI - Blob size was reduced. Current size is ${trace.length} and the limit is $traceMaxLimit")
return AppExitInfoBehavior.CollectTracesResult.TooLarge(trace.take(traceMaxLimit))
}

return AppExitInfoBehavior.CollectTracesResult.Success(trace)
} catch (e: IOException) {
logger.logWarningWithException("AEI - IOException: ${e.message}", e, true)
logger.logWarning("AEI - IOException: ${e.message}", e, true)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("ioexception: ${e.message}"))
} catch (e: OutOfMemoryError) {
logger.logWarningWithException("AEI - Out of Memory: ${e.message}", e, true)
logger.logWarning("AEI - Out of Memory: ${e.message}", e, true)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("oom: ${e.message}"))
} catch (tr: Throwable) {
logger.logWarningWithException("AEI - An error occurred: ${tr.message}", tr, true)
logger.logWarning("AEI - An error occurred: ${tr.message}", tr, true)
return AppExitInfoBehavior.CollectTracesResult.TraceException(("error: ${tr.message}"))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,9 @@ internal class SessionBehavior(
) {

companion object {

/**
* By default, prevents to capture internal error logs as part of session payload
*/
const val ERROR_LOG_STRICT_MODE_DEFAULT = false

const val SESSION_PROPERTY_LIMIT = 10
}

/**
* Whether the limit on the number of internal exceptions in the payload should be increased
* for strict mode.
*/
fun isSessionErrorLogStrictModeEnabled(): Boolean =
local?.sessionEnableErrorLogStrictMode ?: ERROR_LOG_STRICT_MODE_DEFAULT

/**
* The whitelist of events (crashes, errors) that should send a full session payload even
* if the gating feature is enabled.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,5 @@ internal class SessionLocalConfig(
* gating feature is enabled.
*/
@Json(name = "send_full_for")
val fullSessionEvents: Set<String>? = null,

/**
* Local/Internal logs with ERROR severity are going to be captured as part of our session payload tp monitor potential issues
*/
@Json(name = "error_log_strict_mode")
val sessionEnableErrorLogStrictMode: Boolean? = null
val fullSessionEvents: Set<String>? = null
)
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,30 @@ 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.ReportingLoggerAction
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: ReportingLoggerAction
val reportingLoggerAction: InternalErrorServiceAction
}

internal class SdkObservabilityModuleImpl(
initModule: InitModule,
essentialServiceModule: EssentialServiceModule
) : SdkObservabilityModule {

private val logStrictMode by lazy {
val configService = essentialServiceModule.configService
configService.sessionBehavior.isSessionErrorLogStrictModeEnabled()
}

override val internalErrorService: InternalErrorService by singleton {
EmbraceInternalErrorService(essentialServiceModule.processStateService, initModule.clock, logStrictMode)
EmbraceInternalErrorService(
essentialServiceModule.processStateService,
initModule.clock
)
}

override val reportingLoggerAction: ReportingLoggerAction by singleton {
ReportingLoggerAction(internalErrorService, logStrictMode)
override val reportingLoggerAction: InternalErrorServiceAction by singleton {
InternalErrorServiceAction(internalErrorService)
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import java.net.UnknownServiceException
*/
internal class EmbraceInternalErrorService(
private val processStateService: ProcessStateService,
private val clock: Clock,
private val logStrictMode: Boolean
private val clock: Clock
) : InternalErrorService {
private var configService: ConfigService? = null
private var err: LegacyExceptionError? = null
Expand Down Expand Up @@ -95,7 +94,7 @@ internal class EmbraceInternalErrorService(
return
}
if (err == null) {
err = LegacyExceptionError(logStrictMode)
err = LegacyExceptionError()
}

// if the config service has not been set yet, capture the exception
Expand Down
Loading

0 comments on commit 307d284

Please sign in to comment.