fixup! [ECO-4998] feat: add logger abstraction

ably · Oct 23, 2024 · 33bfe05 · 33bfe05
1 parent 2de465e
commit 33bfe05
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 54 deletions.
diff --git a/chat-android/src/main/java/com/ably/chat/ChatApi.kt b/chat-android/src/main/java/com/ably/chat/ChatApi.kt
@@ -140,13 +140,11 @@ internal class ChatApi(
                 override fun onError(reason: ErrorInfo?) {
                     logger.error(
                         "ChatApi.makeAuthorizedRequest(); failed to make request",
-                        context = LogContext(
-                            contextMap = mapOf(
-                                "url" to url,
-                                "statusCode" to reason?.statusCode.toString(),
-                                "errorCode" to reason?.code.toString(),
-                                "errorMessage" to reason?.message.toString(),
-                            ),
+                        contextMap = mapOf(
+                            "url" to url,
+                            "statusCode" to reason?.statusCode.toString(),
+                            "errorCode" to reason?.code.toString(),
+                            "errorMessage" to reason?.message.toString(),
                         ),
                     )
                     // (CHA-M3e)
@@ -176,13 +174,11 @@ internal class ChatApi(
                 override fun onError(reason: ErrorInfo?) {
                     logger.error(
                         "ChatApi.makeAuthorizedPaginatedRequest(); failed to make request",
-                        context = LogContext(
-                            contextMap = mapOf(
-                                "url" to url,
-                                "statusCode" to reason?.statusCode.toString(),
-                                "errorCode" to reason?.code.toString(),
-                                "errorMessage" to reason?.message.toString(),
-                            ),
+                        contextMap = mapOf(
+                            "url" to url,
+                            "statusCode" to reason?.statusCode.toString(),
+                            "errorCode" to reason?.code.toString(),
+                            "errorMessage" to reason?.message.toString(),
                         ),
                     )
                     continuation.resumeWithException(AblyException.fromErrorInfo(reason))

diff --git a/chat-android/src/main/java/com/ably/chat/ChatClient.kt b/chat-android/src/main/java/com/ably/chat/ChatClient.kt
@@ -46,12 +46,16 @@ internal class DefaultChatClient(
 ) : ChatClient {
 
     private val logger: Logger = if (clientOptions.logHandler != null) {
-        CustomLogger(clientOptions.logHandler, clientOptions.logLevel)
+        CustomLogger(
+            clientOptions.logHandler,
+            clientOptions.logLevel,
+            LogContext(tag = "ChatClient", instanceId = generateUUID(), clientId = clientId),
+        )
     } else {
-        AndroidLogger(clientOptions.logLevel)
+        AndroidLogger(clientOptions.logLevel, LogContext(tag = "ChatClient", instanceId = generateUUID(), clientId = clientId))
     }
 
-    private val chatApi = ChatApi(realtime, clientId, logger.withContext(LogContext(tag = "AblyChatAPI")))
+    private val chatApi = ChatApi(realtime, clientId, logger.withContext(tag = "AblyChatAPI"))
 
     override val rooms: Rooms = DefaultRooms(
         realtimeClient = realtime,

diff --git a/chat-android/src/main/java/com/ably/chat/Logger.kt b/chat-android/src/main/java/com/ably/chat/Logger.kt
@@ -7,59 +7,73 @@ fun interface LogHandler {
     fun log(message: String, level: LogLevel, throwable: Throwable?, context: LogContext)
 }
 
-data class LogContext(val tag: String? = null, val contextMap: Map<String, String> = mapOf())
+data class LogContext(
+    val tag: String,
+    val clientId: String,
+    val instanceId: String,
+    val contextMap: Map<String, String> = mapOf(),
+)
 
 internal interface Logger {
-    val defaultContext: LogContext
-    fun withContext(additionalContext: LogContext): Logger
-    fun log(message: String, level: LogLevel, throwable: Throwable? = null, context: LogContext? = null)
+    val context: LogContext
+    fun withContext(tag: String? = null, contextMap: Map<String, String> = mapOf()): Logger
+    fun log(
+        message: String,
+        level: LogLevel,
+        throwable: Throwable? = null,
+        newTag: String? = null,
+        newContextMap: Map<String, String> = mapOf(),
+    )
 }
 
-internal fun Logger.trace(message: String, throwable: Throwable? = null, context: LogContext? = null) {
-    log(message, LogLevel.Trace, throwable, context)
+internal fun Logger.trace(message: String, throwable: Throwable? = null, tag: String? = null, contextMap: Map<String, String> = mapOf()) {
+    log(message, LogLevel.Trace, throwable, tag, contextMap)
 }
 
-internal fun Logger.debug(message: String, throwable: Throwable? = null, context: LogContext? = null) {
-    log(message, LogLevel.Debug, throwable, context)
+internal fun Logger.debug(message: String, throwable: Throwable? = null, tag: String? = null, contextMap: Map<String, String> = mapOf()) {
+    log(message, LogLevel.Debug, throwable, tag, contextMap)
 }
 
-internal fun Logger.info(message: String, throwable: Throwable? = null, context: LogContext? = null) {
-    log(message, LogLevel.Info, throwable, context)
+internal fun Logger.info(message: String, throwable: Throwable? = null, tag: String? = null, contextMap: Map<String, String> = mapOf()) {
+    log(message, LogLevel.Info, throwable, tag, contextMap)
 }
 
-internal fun Logger.warn(message: String, throwable: Throwable? = null, context: LogContext? = null) {
-    log(message, LogLevel.Warn, throwable, context)
+internal fun Logger.warn(message: String, throwable: Throwable? = null, tag: String? = null, contextMap: Map<String, String> = mapOf()) {
+    log(message, LogLevel.Warn, throwable, tag, contextMap)
 }
 
-internal fun Logger.error(message: String, throwable: Throwable? = null, context: LogContext? = null) {
-    log(message, LogLevel.Error, throwable, context)
+internal fun Logger.error(message: String, throwable: Throwable? = null, tag: String? = null, contextMap: Map<String, String> = mapOf()) {
+    log(message, LogLevel.Error, throwable, tag, contextMap)
 }
 
-internal fun LogContext.mergeWith(other: LogContext): LogContext {
+internal fun LogContext.mergeWith(tag: String? = null, contextMap: Map<String, String> = mapOf()): LogContext {
     return LogContext(
-        tag = other.tag ?: tag,
-        contextMap = contextMap + other.contextMap,
+        tag = tag ?: this.tag,
+        instanceId = instanceId,
+        clientId = clientId,
+        contextMap = this.contextMap + contextMap,
     )
 }
 
 internal class AndroidLogger(
     private val minimalVisibleLogLevel: LogLevel,
-    override val defaultContext: LogContext = LogContext(),
+    override val context: LogContext,
 ) : Logger {
 
-    override fun withContext(additionalContext: LogContext): Logger {
+    override fun withContext(newTag: String?, newContextMap: Map<String, String>): Logger {
         return AndroidLogger(
             minimalVisibleLogLevel = minimalVisibleLogLevel,
-            defaultContext = defaultContext.mergeWith(additionalContext),
+            context = context.mergeWith(newTag, newContextMap),
         )
     }
 
-    override fun log(message: String, level: LogLevel, throwable: Throwable?, context: LogContext?) {
+    override fun log(message: String, level: LogLevel, throwable: Throwable?, newTag: String?, newContextMap: Map<String, String>) {
         if (level.logLevelValue <= minimalVisibleLogLevel.logLevelValue) return
-        val finalContext = context?.let { defaultContext.mergeWith(it) } ?: this.defaultContext
-        val tag = finalContext.tag ?: "AblyChatSDK"
+        val finalContext = context.mergeWith(newTag, newContextMap)
+        val tag = finalContext.tag
+        val contextMap = finalContext.contextMap + ("instanceId" to finalContext.instanceId)
 
-        val contextString = if (this.defaultContext.contextMap.isEmpty()) "" else ", context: $finalContext"
+        val contextString = ", context: $contextMap"
         val formattedMessage = "[${LocalDateTime.now()}] ${level.name} ably-chat: ${message}$contextString"
         when (level) {
             // We use Logcat's info level for Trace and Debug
@@ -76,20 +90,20 @@ internal class AndroidLogger(
 internal class CustomLogger(
     private val logHandler: LogHandler,
     private val minimalVisibleLogLevel: LogLevel,
-    override val defaultContext: LogContext = LogContext(),
+    override val context: LogContext,
 ) : Logger {
 
-    override fun withContext(additionalContext: LogContext): Logger {
+    override fun withContext(tag: String?, contextMap: Map<String, String>): Logger {
         return CustomLogger(
             logHandler = logHandler,
             minimalVisibleLogLevel = minimalVisibleLogLevel,
-            defaultContext = defaultContext.mergeWith(additionalContext),
+            context = context.mergeWith(tag, contextMap),
         )
     }
 
-    override fun log(message: String, level: LogLevel, throwable: Throwable?, context: LogContext?) {
+    override fun log(message: String, level: LogLevel, throwable: Throwable?, newTag: String?, newContextMap: Map<String, String>) {
         if (level.logLevelValue <= minimalVisibleLogLevel.logLevelValue) return
-        val finalContext = context?.let { defaultContext.mergeWith(it) } ?: this.defaultContext
+        val finalContext = context.mergeWith(newTag, newContextMap)
         logHandler.log(
             message = message,
             level = level,
@@ -99,10 +113,7 @@ internal class CustomLogger(
     }
 }
 
-internal object EmptyLogger : Logger {
-    override val defaultContext: LogContext = LogContext()
-
-    override fun withContext(additionalContext: LogContext): Logger = this
-
-    override fun log(message: String, level: LogLevel, throwable: Throwable?, context: LogContext?) = Unit
+internal class EmptyLogger(override val context: LogContext) : Logger {
+    override fun withContext(newTag: String?, newContextMap: Map<String, String>): Logger = this
+    override fun log(message: String, level: LogLevel, throwable: Throwable?, newTag: String?, newContextMap: Map<String, String>) = Unit
 }
diff --git a/chat-android/src/main/java/com/ably/chat/Utils.kt b/chat-android/src/main/java/com/ably/chat/Utils.kt
@@ -5,6 +5,7 @@ import io.ably.lib.realtime.CompletionListener
 import io.ably.lib.types.AblyException
 import io.ably.lib.types.ChannelOptions
 import io.ably.lib.types.ErrorInfo
+import java.util.UUID
 import kotlin.coroutines.resume
 import kotlin.coroutines.resumeWithException
 import kotlin.coroutines.suspendCoroutine
@@ -47,6 +48,8 @@ fun ChatChannelOptions(init: (ChannelOptions.() -> Unit)? = null): ChannelOption
     return options
 }
 
+fun generateUUID() = UUID.randomUUID().toString()
+
 /**
  * A value that can be evaluated at a later time, similar to `kotlinx.coroutines.Deferred` or a JavaScript Promise.
  *

diff --git a/chat-android/src/test/java/com/ably/chat/ChatApiTest.kt b/chat-android/src/test/java/com/ably/chat/ChatApiTest.kt
@@ -13,7 +13,8 @@ import org.junit.Test
 class ChatApiTest {
 
     private val realtime = mockk<RealtimeClient>(relaxed = true)
-    private val chatApi = ChatApi(realtime, "clientId", logger = EmptyLogger)
+    private val chatApi =
+        ChatApi(realtime, "clientId", logger = EmptyLogger(LogContext(tag = "TEST", instanceId = generateUUID(), clientId = "clientId")))
 
     /**
      * @nospec

diff --git a/chat-android/src/test/java/com/ably/chat/MessagesTest.kt b/chat-android/src/test/java/com/ably/chat/MessagesTest.kt
@@ -28,7 +28,8 @@ class MessagesTest {
     private val realtimeClient = mockk<RealtimeClient>(relaxed = true)
     private val realtimeChannels = mockk<Channels>(relaxed = true)
     private val realtimeChannel = spyk<Channel>(buildRealtimeChannel())
-    private val chatApi = spyk(ChatApi(realtimeClient, "clientId", EmptyLogger))
+    private val chatApi =
+        spyk(ChatApi(realtimeClient, "clientId", EmptyLogger(LogContext(tag = "TEST", instanceId = generateUUID(), clientId = "clientId"))))
     private lateinit var messages: DefaultMessages
 
     private val channelStateListenerSlot = slot<ChannelStateListener>()