Merge pull request #1806 from embrace-io/refactor-log-impl-2

Additional log implementation refactor

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/LogModuleImpl.kt

@@ -61,8 +61,7 @@ internal class LogModuleImpl(
         EmbraceLogService(
             essentialServiceModule.logWriter,
             configModule.configService,
-            essentialServiceModule.sessionPropertiesService,
-            initModule.jsonSerializer
+            essentialServiceModule.sessionPropertiesService
         )
     }
 

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/EmbraceLogService.kt

@@ -3,8 +3,6 @@ package io.embrace.android.embracesdk.internal.logs
 import io.embrace.android.embracesdk.LogExceptionType
 import io.embrace.android.embracesdk.Severity
 import io.embrace.android.embracesdk.internal.arch.destination.LogWriter
-import io.embrace.android.embracesdk.internal.arch.schema.EmbType.System.FlutterException.embFlutterExceptionContext
-import io.embrace.android.embracesdk.internal.arch.schema.EmbType.System.FlutterException.embFlutterExceptionLibrary
 import io.embrace.android.embracesdk.internal.arch.schema.SchemaType
 import io.embrace.android.embracesdk.internal.arch.schema.SchemaType.Exception
 import io.embrace.android.embracesdk.internal.arch.schema.SchemaType.FlutterException
@@ -15,11 +13,10 @@ import io.embrace.android.embracesdk.internal.config.ConfigService
 import io.embrace.android.embracesdk.internal.config.behavior.REDACTED_LABEL
 import io.embrace.android.embracesdk.internal.opentelemetry.embExceptionHandling
 import io.embrace.android.embracesdk.internal.payload.AppFramework
-import io.embrace.android.embracesdk.internal.serialization.PlatformSerializer
-import io.embrace.android.embracesdk.internal.serialization.truncatedStacktrace
 import io.embrace.android.embracesdk.internal.spans.toOtelSeverity
+import io.embrace.android.embracesdk.internal.utils.PropertyUtils.normalizeProperties
 import io.embrace.android.embracesdk.internal.utils.Uuid
-import io.opentelemetry.semconv.ExceptionAttributes
+import io.opentelemetry.api.common.AttributeKey
 import io.opentelemetry.semconv.incubating.LogIncubatingAttributes
 
 /**
@@ -29,7 +26,6 @@ class EmbraceLogService(
     private val logWriter: LogWriter,
     private val configService: ConfigService,
     private val sessionPropertiesService: SessionPropertiesService,
-    private val serializer: PlatformSerializer,
 ) : LogService {
 
     private val behavior = configService.logMessageBehavior
@@ -44,38 +40,18 @@ class EmbraceLogService(
         severity: Severity,
         logExceptionType: LogExceptionType,
         properties: Map<String, Any>?,
-        stackTraceElements: Array<StackTraceElement>?,
-        customStackTrace: String?,
-        context: String?,
-        library: String?,
-        exceptionName: String?,
-        exceptionMessage: String?,
+        customLogAttrs: Map<AttributeKey<String>, String>,
     ) {
-        val redactedProperties = redactSensitiveProperties(properties)
-        val attrs = createTelemetryAttributes(redactedProperties)
+        val redactedProperties = redactSensitiveProperties(normalizeProperties(properties))
+        val attrs = createTelemetryAttributes(redactedProperties, customLogAttrs)
 
         val schemaProvider: (TelemetryAttributes) -> SchemaType = when {
             logExceptionType == LogExceptionType.NONE -> ::Log
             configService.appFramework == AppFramework.FLUTTER -> ::FlutterException
             else -> ::Exception
         }
-
         if (logExceptionType != LogExceptionType.NONE) {
-            val stacktrace = stackTraceElements?.let(serializer::truncatedStacktrace) ?: customStackTrace
-            populateLogExceptionAttributes(
-                attributes = attrs,
-                logExceptionType = logExceptionType,
-                stackTrace = stacktrace,
-                type = exceptionName,
-                message = exceptionMessage,
-            )
-            if (configService.appFramework == AppFramework.FLUTTER) {
-                populateFlutterExceptionAttrs(
-                    context = context,
-                    library = library,
-                    attrs = attrs
-                )
-            }
+            attrs.setAttribute(embExceptionHandling, logExceptionType.value)
         }
         addLogEventData(
             message = message,
@@ -85,15 +61,6 @@ class EmbraceLogService(
         )
     }
 
-    private fun populateFlutterExceptionAttrs(
-        context: String?,
-        library: String?,
-        attrs: TelemetryAttributes,
-    ) {
-        context?.let { attrs.setAttribute(embFlutterExceptionContext, it) }
-        library?.let { attrs.setAttribute(embFlutterExceptionLibrary, it) }
-    }
-
     override fun getErrorLogsCount(): Int {
         return logCounters.getValue(Severity.ERROR).getCount()
     }
@@ -105,29 +72,22 @@ class EmbraceLogService(
     /**
      * Create [TelemetryAttributes] with the standard log properties
      */
-    private fun createTelemetryAttributes(customProperties: Map<String, Any>?): TelemetryAttributes {
+    private fun createTelemetryAttributes(
+        customProperties: Map<String, Any>?,
+        logAttrs: Map<AttributeKey<String>, String>,
+    ): TelemetryAttributes {
         val attributes = TelemetryAttributes(
             configService = configService,
             sessionPropertiesProvider = sessionPropertiesService::getProperties,
             customAttributes = customProperties?.mapValues { it.value.toString() } ?: emptyMap()
         )
         attributes.setAttribute(LogIncubatingAttributes.LOG_RECORD_UID, Uuid.getEmbUuid())
+        logAttrs.forEach {
+            attributes.setAttribute(it.key, it.value)
+        }
         return attributes
     }
 
-    private fun populateLogExceptionAttributes(
-        attributes: TelemetryAttributes,
-        logExceptionType: LogExceptionType,
-        stackTrace: String?,
-        type: String?,
-        message: String?,
-    ) {
-        attributes.setAttribute(embExceptionHandling, logExceptionType.value)
-        type?.let { attributes.setAttribute(ExceptionAttributes.EXCEPTION_TYPE, it) }
-        message?.let { attributes.setAttribute(ExceptionAttributes.EXCEPTION_MESSAGE, it) }
-        stackTrace?.let { attributes.setAttribute(ExceptionAttributes.EXCEPTION_STACKTRACE, it) }
-    }
-
     private fun addLogEventData(
         message: String,
         severity: Severity,

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/LogService.kt

@@ -3,6 +3,7 @@ package io.embrace.android.embracesdk.internal.logs
 import io.embrace.android.embracesdk.LogExceptionType
 import io.embrace.android.embracesdk.Severity
 import io.embrace.android.embracesdk.internal.session.MemoryCleanerListener
+import io.opentelemetry.api.common.AttributeKey
 
 /**
  * Creates log records to be sent using the Open Telemetry Logs data model.
@@ -13,28 +14,16 @@ interface LogService : MemoryCleanerListener {
      * Creates a remote log.
      *
      * @param message            the message to log
-     * @param type               the type of message to log, which must be INFO_LOG, WARNING_LOG, or ERROR_LOG
+     * @param severity           the log severity
      * @param logExceptionType   whether the log is a handled exception, unhandled, or non an exception
      * @param properties         custom properties to send as part of the event
-     * @param stackTraceElements the stacktrace elements of a throwable
-     * @param customStackTrace   stacktrace string for non-JVM exceptions
-     * @param context            the context of the exception
-     * @param library            the library of the exception
-     * @param exceptionName      the exception name of a Throwable is it is present
-     * @param exceptionMessage   the exception message of a Throwable is it is present
      */
-    @Suppress("LongParameterList")
     fun log(
         message: String,
         severity: Severity,
         logExceptionType: LogExceptionType,
         properties: Map<String, Any>? = null,
-        stackTraceElements: Array<StackTraceElement>? = null,
-        customStackTrace: String? = null,
-        context: String? = null,
-        library: String? = null,
-        exceptionName: String? = null,
-        exceptionMessage: String? = null,
+        customLogAttrs: Map<AttributeKey<String>, String> = emptyMap(),
     )
 
     /**

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/EmbraceLogServiceTest.kt

@@ -2,26 +2,20 @@ package io.embrace.android.embracesdk.internal.logs
 
 import io.embrace.android.embracesdk.LogExceptionType
 import io.embrace.android.embracesdk.Severity
-import io.embrace.android.embracesdk.arch.assertIsType
 import io.embrace.android.embracesdk.fakes.FakeConfigService
 import io.embrace.android.embracesdk.fakes.FakeLogWriter
 import io.embrace.android.embracesdk.fakes.FakeSessionPropertiesService
 import io.embrace.android.embracesdk.fakes.behavior.FakeLogMessageBehavior
 import io.embrace.android.embracesdk.fakes.config.FakeInstrumentedConfig
 import io.embrace.android.embracesdk.fakes.config.FakeRedactionConfig
 import io.embrace.android.embracesdk.fakes.createSessionBehavior
-import io.embrace.android.embracesdk.internal.arch.schema.EmbType
 import io.embrace.android.embracesdk.internal.arch.schema.toSessionPropertyAttributeName
 import io.embrace.android.embracesdk.internal.config.behavior.REDACTED_LABEL
 import io.embrace.android.embracesdk.internal.config.behavior.SensitiveKeysBehaviorImpl
 import io.embrace.android.embracesdk.internal.config.remote.RemoteConfig
 import io.embrace.android.embracesdk.internal.config.remote.SessionRemoteConfig
-import io.embrace.android.embracesdk.internal.opentelemetry.embExceptionHandling
 import io.embrace.android.embracesdk.internal.payload.AppFramework
-import io.embrace.android.embracesdk.internal.serialization.EmbraceSerializer
-import io.opentelemetry.semconv.ExceptionAttributes
 import io.opentelemetry.semconv.incubating.LogIncubatingAttributes
-import org.junit.Assert
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotEquals
 import org.junit.Assert.assertTrue
@@ -35,8 +29,6 @@ internal class EmbraceLogServiceTest {
     private lateinit var fakeSessionPropertiesService: FakeSessionPropertiesService
     private lateinit var fakeConfigService: FakeConfigService
 
-    private val embraceSerializer = EmbraceSerializer()
-
     @Before
     fun setUp() {
         fakeConfigService = FakeConfigService(
@@ -54,7 +46,6 @@ internal class EmbraceLogServiceTest {
         logWriter = fakeLogWriter,
         configService = fakeConfigService,
         sessionPropertiesService = fakeSessionPropertiesService,
-        serializer = embraceSerializer,
     )
 
     @Test
@@ -246,85 +237,4 @@ internal class EmbraceLogServiceTest {
         // then the correct number of error logs is returned
         assertEquals(5, logService.getErrorLogsCount())
     }
-
-    @Test
-    fun `stacktrace elements are truncated`() {
-        // given a stacktrace with more than 200 elements
-        val stackTrace = Array(201) { StackTraceElement("TestClass", "testMethod", "testFile", it) }
-
-        // when logging a message with the stacktrace
-        logService.log("message", Severity.INFO, LogExceptionType.HANDLED, stackTraceElements = stackTrace)
-
-        // then the stacktrace is truncated
-        val log = fakeLogWriter.logEvents.single()
-        val truncatedStacktraceString = log.schemaType.attributes()[ExceptionAttributes.EXCEPTION_STACKTRACE.key] ?: ""
-
-        // serialize back to Array<StackTraceElement> to get the size
-        val truncatedStacktrace = embraceSerializer.fromJson(truncatedStacktraceString, List::class.java)
-        assertEquals(200, truncatedStacktrace.size)
-    }
-
-    @Test
-    fun `exception is logged`() {
-        // given an exception
-        val message = "Oh no"
-        val exception = NullPointerException("Oh no!")
-
-        // when logging it
-        logService.log(
-            message = message,
-            severity = Severity.WARNING,
-            logExceptionType = LogExceptionType.HANDLED,
-            stackTraceElements = exception.stackTrace,
-            exceptionName = exception.javaClass.simpleName,
-            exceptionMessage = exception.message,
-        )
-
-        // then the exception is correctly logged
-        val log = fakeLogWriter.logEvents.single()
-        assertEquals(message, log.message)
-        assertEquals(io.opentelemetry.api.logs.Severity.WARN, log.severity)
-        val attributes = log.schemaType.attributes()
-        Assert.assertNotNull(attributes[LogIncubatingAttributes.LOG_RECORD_UID.key])
-        assertEquals(LogExceptionType.HANDLED.value, attributes[embExceptionHandling.name])
-        assertEquals(exception.javaClass.simpleName, attributes[ExceptionAttributes.EXCEPTION_TYPE.key])
-        assertEquals(exception.message, attributes[ExceptionAttributes.EXCEPTION_MESSAGE.key])
-        log.assertIsType(EmbType.System.Exception)
-    }
-
-    @Test
-    fun `flutter exception is logged`() {
-        // given a flutter exception
-        val flutterMessage = "Dart error"
-        val flutterContext = "Flutter context"
-        val flutterLibrary = "Flutter library"
-        val flutterException = NullPointerException("Something broke on Flutter")
-        fakeConfigService = FakeConfigService(appFramework = AppFramework.FLUTTER)
-        logService = createEmbraceLogService()
-
-        // when logging it
-        logService.log(
-            message = flutterMessage,
-            severity = Severity.ERROR,
-            logExceptionType = LogExceptionType.HANDLED,
-            stackTraceElements = flutterException.stackTrace,
-            exceptionName = flutterException.javaClass.simpleName,
-            exceptionMessage = flutterException.message,
-            context = flutterContext,
-            library = flutterLibrary
-        )
-
-        // then the exception is correctly logged
-        val log = fakeLogWriter.logEvents.single()
-        assertEquals(flutterMessage, log.message)
-        assertEquals(io.opentelemetry.api.logs.Severity.ERROR, log.severity)
-        val attributes = log.schemaType.attributes()
-        Assert.assertNotNull(attributes[LogIncubatingAttributes.LOG_RECORD_UID.key])
-        assertEquals(LogExceptionType.HANDLED.value, attributes[embExceptionHandling.name])
-        assertEquals(flutterException.javaClass.simpleName, attributes[ExceptionAttributes.EXCEPTION_TYPE.key])
-        assertEquals(flutterException.message, attributes[ExceptionAttributes.EXCEPTION_MESSAGE.key])
-        assertEquals(flutterContext, attributes[EmbType.System.FlutterException.embFlutterExceptionContext.name])
-        assertEquals(flutterLibrary, attributes[EmbType.System.FlutterException.embFlutterExceptionLibrary.name])
-        log.assertIsType(EmbType.System.FlutterException)
-    }
 }

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

@@ -48,6 +48,7 @@ import io.embrace.android.embracesdk.internal.payload.AppFramework
 import io.embrace.android.embracesdk.internal.worker.TaskPriority
 import io.embrace.android.embracesdk.internal.worker.Worker
 import io.embrace.android.embracesdk.spans.TracingApi
+import io.opentelemetry.api.common.AttributeKey
 import java.util.concurrent.Executors
 import java.util.concurrent.atomic.AtomicBoolean
 
@@ -296,26 +297,24 @@ internal class EmbraceImpl @JvmOverloads constructor(
     fun logMessage(
         severity: Severity,
         message: String,
-        properties: Map<String, Any>?,
-        stackTraceElements: Array<StackTraceElement>?,
-        customStackTrace: String?,
-        logExceptionType: LogExceptionType,
-        context: String?,
-        library: String?,
+        properties: Map<String, Any>? = null,
+        stackTraceElements: Array<StackTraceElement>? = null,
+        customStackTrace: String? = null,
+        logExceptionType: LogExceptionType = LogExceptionType.NONE,
         exceptionName: String? = null,
         exceptionMessage: String? = null,
+        customLogAttrs: Map<AttributeKey<String>, String> = emptyMap(),
     ) {
-        logsApiDelegate.logMessage(
-            severity,
-            message,
-            properties,
-            stackTraceElements,
-            customStackTrace,
-            logExceptionType,
-            context,
-            library,
-            exceptionName,
-            exceptionMessage
+        logsApiDelegate.logMessageImpl(
+            severity = severity,
+            message = message,
+            properties = properties,
+            stackTraceElements = stackTraceElements,
+            customStackTrace = customStackTrace,
+            logExceptionType = logExceptionType,
+            exceptionName = exceptionName,
+            exceptionMessage = exceptionMessage,
+            customLogAttrs = customLogAttrs
         )
     }
 

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/EmbraceInternalInterfaceImpl.kt

@@ -4,7 +4,6 @@ package io.embrace.android.embracesdk.internal.api.delegate
 
 import android.annotation.SuppressLint
 import io.embrace.android.embracesdk.EmbraceImpl
-import io.embrace.android.embracesdk.LogExceptionType
 import io.embrace.android.embracesdk.LogType
 import io.embrace.android.embracesdk.Severity
 import io.embrace.android.embracesdk.internal.EmbraceInternalInterface
@@ -61,14 +60,10 @@ internal class EmbraceInternalInterfaceImpl(
             else -> Severity.INFO
         }
         embraceImpl.logMessage(
-            eventType,
-            throwable.message ?: "",
-            properties,
-            customStackTrace ?: throwable.stackTrace,
-            null,
-            LogExceptionType.NONE,
-            null,
-            null
+            severity = eventType,
+            message = throwable.message ?: "",
+            properties = properties,
+            stackTraceElements = customStackTrace ?: throwable.stackTrace,
         )
     }