fix: add additional logging to detect network issues (#193)

* fix: add additional logging to detect network issues * fix: log on error level
spotify · Feb 18, 2025 · 780feb1 · 780feb1
1 parent aee8b09
commit 780feb1
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 24 deletions.
diff --git a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt
@@ -362,7 +362,8 @@ object ConfidenceFactory {
                 .callTimeout(timeoutMillis, TimeUnit.MILLISECONDS)
                 .build(),
             dispatcher = dispatcher,
-            sdkMetadata = SdkMetadata(SDK_ID, BuildConfig.SDK_VERSION)
+            sdkMetadata = SdkMetadata(SDK_ID, BuildConfig.SDK_VERSION),
+            debugLogger = debugLogger
         )
         val visitorId = ConfidenceValue.String(VisitorUtil.getId(context))
         val initContext = initialContext.toMutableMap()

diff --git a/Confidence/src/main/java/com/spotify/confidence/DebugLogger.kt b/Confidence/src/main/java/com/spotify/confidence/DebugLogger.kt
@@ -10,6 +10,7 @@ internal interface DebugLogger {
     fun logFlag(action: String, details: String? = null)
     fun logContext(action: String, context: Map<String, ConfidenceValue>)
     fun logResolve(flag: String, context: JsonElement)
+    fun logError(message: String, throwable: Throwable? = null)
     companion object {
         const val TAG = "Confidence"
     }
@@ -51,12 +52,19 @@ internal class DebugLoggerImpl(private val filterLevel: LoggingLevel, private va
         )
     }
 
+    override fun logError(message: String, throwable: Throwable?) {
+        error(message, throwable)
+    }
+
     private fun verbose(message: String) = log(LoggingLevel.VERBOSE, message)
     private fun debug(message: String) = log(LoggingLevel.DEBUG, message)
     private fun warn(message: String, throwable: Throwable?) =
         log(LoggingLevel.WARN, throwable?.let { "$message: ${throwable.message}" } ?: message)
 
-    private fun error(message: String, throwable: Throwable) = log(LoggingLevel.ERROR, "$message: ${throwable.message}")
+    private fun error(message: String, throwable: Throwable?) = log(
+        LoggingLevel.ERROR,
+        throwable?.let { "$message: ${throwable.message}" } ?: message
+    )
 
     private fun log(messageLevel: LoggingLevel, message: String) {
         if (messageLevel >= filterLevel) {

diff --git a/Confidence/src/main/java/com/spotify/confidence/EventSenderEngine.kt b/Confidence/src/main/java/com/spotify/confidence/EventSenderEngine.kt
@@ -151,7 +151,7 @@ internal class EventSenderEngineImpl(
                 EventSenderEngineImpl(
                     EventStorageImpl(context),
                     clientSecret,
-                    uploader = EventSenderUploaderImpl(OkHttpClient(), dispatcher),
+                    uploader = EventSenderUploaderImpl(OkHttpClient(), dispatcher, debugLogger),
                     flushPolicies = flushPolicies.toMutableList(),
                     dispatcher = dispatcher,
                     sdkMetadata = sdkMetadata,

diff --git a/Confidence/src/main/java/com/spotify/confidence/EventSenderUploader.kt b/Confidence/src/main/java/com/spotify/confidence/EventSenderUploader.kt
@@ -40,7 +40,8 @@ internal data class EngineEvent(
 
 internal class EventSenderUploaderImpl(
     private val httpClient: OkHttpClient,
-    private val dispatcher: CoroutineDispatcher
+    private val dispatcher: CoroutineDispatcher,
+    private val debugLogger: DebugLogger? = null
 ) : EventSenderUploader {
     private val headers by lazy {
         Headers.headersOf(
@@ -65,6 +66,9 @@ internal class EventSenderUploaderImpl(
             .build()
 
         val response = httpClient.newCall(httpRequest).await()
+        if (!response.isSuccessful) {
+            debugLogger?.logError(message = "Failed to upload events. http code ${response.code}")
+        }
         when (response.code) {
             // clean up in case of success
             200 -> true

diff --git a/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt b/Confidence/src/main/java/com/spotify/confidence/RemoteFlagResolver.kt
@@ -28,7 +28,8 @@ internal class RemoteFlagResolver(
     private val httpClient: OkHttpClient,
     private val sdkMetadata: SdkMetadata,
     private val dispatcher: CoroutineDispatcher = Dispatchers.IO,
-    private val baseUrl: HttpUrl? = null
+    private val baseUrl: HttpUrl? = null,
+    private val debugLogger: DebugLogger? = null
 ) : FlagResolver {
     private val headers = Headers.headersOf(
         "Content-Type",
@@ -68,6 +69,27 @@ internal class RemoteFlagResolver(
         ConfidenceRegion.EUROPE -> "https://resolver.eu.confidence.dev"
         ConfidenceRegion.USA -> "https://resolver.us.confidence.dev"
     }
+
+    private fun Response.toResolveFlags(): ResolveResponse {
+        if (!isSuccessful) {
+            debugLogger?.logError("Failed to resolve flags. Http code: $code")
+        }
+        body?.let { body ->
+            val bodyString = body.string()
+
+            // building the json class responsible for serializing the object
+            val networkJson = Json {
+                serializersModule = SerializersModule {
+                    ignoreUnknownKeys = true
+                }
+            }
+            try {
+                return ResolveResponse.Resolved(networkJson.decodeFromString(bodyString))
+            } finally {
+                body.close()
+            }
+        } ?: throw ConfidenceError.ParseError("Response body is null", listOf())
+    }
 }
 
 @Serializable
@@ -77,22 +99,4 @@ private data class ResolveFlagsRequest(
     val clientSecret: String,
     val apply: Boolean,
     val sdk: Sdk
-)
-
-private fun Response.toResolveFlags(): ResolveResponse {
-    body?.let { body ->
-        val bodyString = body.string()
-
-        // building the json class responsible for serializing the object
-        val networkJson = Json {
-            serializersModule = SerializersModule {
-                ignoreUnknownKeys = true
-            }
-        }
-        try {
-            return ResolveResponse.Resolved(networkJson.decodeFromString(bodyString))
-        } finally {
-            body.close()
-        }
-    } ?: throw ConfidenceError.ParseError("Response body is null", listOf())
-}
+)
diff --git a/Confidence/src/test/java/com/spotify/confidence/DebugLoggerFake.kt b/Confidence/src/test/java/com/spotify/confidence/DebugLoggerFake.kt
@@ -25,5 +25,9 @@ internal open class DebugLoggerFake : DebugLogger {
         // not important enough to test right now
     }
 
+    override fun logError(message: String, throwable: Throwable?) {
+        messagesLogged.add(Msg(message, true, throwable))
+    }
+
     data class Msg(val message: String, val isWarning: Boolean, val throwable: Throwable?)
 }
diff --git a/Confidence/src/test/java/com/spotify/confidence/DebugLoggerImplTest.kt b/Confidence/src/test/java/com/spotify/confidence/DebugLoggerImplTest.kt
@@ -109,6 +109,9 @@ class DebugLoggerImplTest {
 
         errorLogger.logMessage("Error msg", throwable = Error("my error"))
         verify { Log.e("Confidence", "Error msg: my error") }
+
+        errorLogger.logError("Error msg")
+        verify { Log.e("Confidence", "Error msg") }
     }
 
     @Test