From 0d782b220dbca42597f9399934b7417b9ad35fd5 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Fri, 10 Jan 2025 17:44:17 +0000 Subject: [PATCH] poc: continue refactoring log implementation --- .../internal/logs/EmbraceLogService.kt | 35 +++++++----------- .../embracesdk/internal/logs/LogService.kt | 8 ++--- .../internal/logs/EmbraceLogServiceTest.kt | 12 ++++--- .../embrace/android/embracesdk/EmbraceImpl.kt | 9 +++-- .../delegate/EmbraceInternalInterfaceImpl.kt | 4 +-- .../delegate/FlutterInternalInterfaceImpl.kt | 12 +++++-- .../internal/api/delegate/LogsApiDelegate.kt | 36 ++++++------------- .../ReactNativeInternalInterfaceImpl.kt | 4 +-- .../delegate/UnityInternalInterfaceImpl.kt | 2 -- .../api/EmbraceInternalInterfaceImplTest.kt | 4 +-- .../api/FlutterInternalInterfaceImplTest.kt | 4 --- .../api/UnityInternalInterfaceImplTest.kt | 4 --- .../embracesdk/fakes/FakeLogService.kt | 12 ++----- 13 files changed, 53 insertions(+), 93 deletions(-) diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/EmbraceLogService.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/EmbraceLogService.kt index 5b36c8629e..88a3faf4b2 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/EmbraceLogService.kt +++ b/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 @@ -18,7 +16,9 @@ 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.api.common.AttributeKey import io.opentelemetry.semconv.ExceptionAttributes import io.opentelemetry.semconv.incubating.LogIncubatingAttributes @@ -46,13 +46,12 @@ class EmbraceLogService( properties: Map?, stackTraceElements: Array?, customStackTrace: String?, - context: String?, - library: String?, exceptionName: String?, exceptionMessage: String?, + customLogAttrs: Map, String> // TODO: lift up & reduce callers ) { - 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 @@ -69,13 +68,6 @@ class EmbraceLogService( type = exceptionName, message = exceptionMessage, ) - if (configService.appFramework == AppFramework.FLUTTER) { - populateFlutterExceptionAttrs( - context = context, - library = library, - attrs = attrs - ) - } } addLogEventData( message = message, @@ -85,15 +77,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,13 +88,19 @@ class EmbraceLogService( /** * Create [TelemetryAttributes] with the standard log properties */ - private fun createTelemetryAttributes(customProperties: Map?): TelemetryAttributes { + private fun createTelemetryAttributes( + customProperties: Map?, + logAttrs: Map, 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 } diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/LogService.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/LogService.kt index 7289c4eab9..4cd5690e75 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/LogService.kt +++ b/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,13 +14,11 @@ 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 */ @@ -31,10 +30,9 @@ interface LogService : MemoryCleanerListener { properties: Map? = null, stackTraceElements: Array? = null, customStackTrace: String? = null, - context: String? = null, - library: String? = null, exceptionName: String? = null, exceptionMessage: String? = null, + customLogAttrs: Map, String> = emptyMap() ) /** diff --git a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/EmbraceLogServiceTest.kt b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/EmbraceLogServiceTest.kt index 428f8ab5a0..b90e6f4318 100644 --- a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/EmbraceLogServiceTest.kt +++ b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/EmbraceLogServiceTest.kt @@ -11,6 +11,8 @@ 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.EmbType.System.FlutterException.embFlutterExceptionContext +import io.embrace.android.embracesdk.internal.arch.schema.EmbType.System.FlutterException.embFlutterExceptionLibrary 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 @@ -310,8 +312,10 @@ internal class EmbraceLogServiceTest { stackTraceElements = flutterException.stackTrace, exceptionName = flutterException.javaClass.simpleName, exceptionMessage = flutterException.message, - context = flutterContext, - library = flutterLibrary + customLogAttrs = mapOf( + embFlutterExceptionContext.attributeKey to flutterContext, + embFlutterExceptionLibrary.attributeKey to flutterLibrary, + ), ) // then the exception is correctly logged @@ -323,8 +327,8 @@ internal class EmbraceLogServiceTest { 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]) + assertEquals(flutterContext, attributes[embFlutterExceptionContext.name]) + assertEquals(flutterLibrary, attributes[embFlutterExceptionLibrary.name]) log.assertIsType(EmbType.System.FlutterException) } } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.kt index 0ca46f7a5d..e31c64cab3 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/EmbraceImpl.kt +++ b/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 @@ -300,10 +301,9 @@ internal class EmbraceImpl @JvmOverloads constructor( stackTraceElements: Array?, customStackTrace: String?, logExceptionType: LogExceptionType, - context: String?, - library: String?, exceptionName: String? = null, exceptionMessage: String? = null, + customLogAttrs: Map, String> = emptyMap(), ) { logsApiDelegate.logMessage( severity, @@ -312,10 +312,9 @@ internal class EmbraceImpl @JvmOverloads constructor( stackTraceElements, customStackTrace, logExceptionType, - context, - library, exceptionName, - exceptionMessage + exceptionMessage, + customLogAttrs ) } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/EmbraceInternalInterfaceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/EmbraceInternalInterfaceImpl.kt index 1309a37efd..b9dc2480a9 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/EmbraceInternalInterfaceImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/EmbraceInternalInterfaceImpl.kt @@ -66,9 +66,7 @@ internal class EmbraceInternalInterfaceImpl( properties, customStackTrace ?: throwable.stackTrace, null, - LogExceptionType.NONE, - null, - null + LogExceptionType.NONE ) } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/FlutterInternalInterfaceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/FlutterInternalInterfaceImpl.kt index 04b40deaf1..a0b1df8e75 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/FlutterInternalInterfaceImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/FlutterInternalInterfaceImpl.kt @@ -5,8 +5,11 @@ import io.embrace.android.embracesdk.LogExceptionType import io.embrace.android.embracesdk.Severity import io.embrace.android.embracesdk.internal.EmbraceInternalInterface import io.embrace.android.embracesdk.internal.FlutterInternalInterface +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.envelope.metadata.HostedSdkVersionInfo import io.embrace.android.embracesdk.internal.logging.EmbLogger +import io.opentelemetry.api.common.AttributeKey internal class FlutterInternalInterfaceImpl( private val embrace: EmbraceImpl, @@ -64,6 +67,10 @@ internal class FlutterInternalInterfaceImpl( exceptionType: LogExceptionType, ) { if (embrace.isStarted) { + val attrs = mutableMapOf, String>() + context?.let { attrs[embFlutterExceptionContext.attributeKey] = it } + library?.let { attrs[embFlutterExceptionLibrary.attributeKey] = it } + embrace.logMessage( Severity.ERROR, "Dart error", @@ -71,10 +78,9 @@ internal class FlutterInternalInterfaceImpl( null, stack, exceptionType, - context, - library, name, - message + message, + attrs ) } else { logger.logSdkNotInitialized("logDartError") diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt index a79bf41f6c..8b906bc85b 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/LogsApiDelegate.kt @@ -6,8 +6,8 @@ import io.embrace.android.embracesdk.internal.api.LogsApi import io.embrace.android.embracesdk.internal.injection.ModuleInitBootstrapper import io.embrace.android.embracesdk.internal.injection.embraceImplInject import io.embrace.android.embracesdk.internal.payload.PushNotificationBreadcrumb -import io.embrace.android.embracesdk.internal.utils.PropertyUtils.normalizeProperties import io.embrace.android.embracesdk.internal.utils.getSafeStackTrace +import io.opentelemetry.api.common.AttributeKey internal class LogsApiDelegate( bootstrapper: ModuleInitBootstrapper, @@ -22,17 +22,9 @@ internal class LogsApiDelegate( bootstrapper.featureModule.pushNotificationDataSource.dataSource } - override fun logInfo(message: String) { - logMessage(message, Severity.INFO) - } - - override fun logWarning(message: String) { - logMessage(message, Severity.WARNING) - } - - override fun logError(message: String) { - logMessage(message, Severity.ERROR) - } + override fun logInfo(message: String) = logMessage(message, Severity.INFO) + override fun logWarning(message: String) = logMessage(message, Severity.WARNING) + override fun logError(message: String) = logMessage(message, Severity.ERROR) override fun logMessage(message: String, severity: Severity) { logMessage(message, severity, null) @@ -81,9 +73,7 @@ internal class LogsApiDelegate( properties, null, null, - LogExceptionType.NONE, - null, - null + LogExceptionType.NONE ) } @@ -104,8 +94,6 @@ internal class LogsApiDelegate( throwable.getSafeStackTrace(), null, LogExceptionType.HANDLED, - null, - null, throwable.javaClass.simpleName, exceptionMessage ) @@ -126,8 +114,6 @@ internal class LogsApiDelegate( null, LogExceptionType.HANDLED, null, - null, - null, message ) } @@ -140,24 +126,24 @@ internal class LogsApiDelegate( stackTraceElements: Array?, customStackTrace: String?, logExceptionType: LogExceptionType, - context: String?, - library: String?, exceptionName: String? = null, exceptionMessage: String? = null, + customLogAttrs: Map, String> = emptyMap(), // TODO: gradually lift up & remove most params ) { if (sdkCallChecker.check("log_message")) { runCatching { + val attrs = mutableMapOf, String>() + logService?.log( message, severity, logExceptionType, - normalizeProperties(properties), + properties, stackTraceElements, customStackTrace, - context, - library, exceptionName, - exceptionMessage + exceptionMessage, + attrs.plus(customLogAttrs) ) sessionOrchestrator?.reportBackgroundActivityStateChange() } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/ReactNativeInternalInterfaceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/ReactNativeInternalInterfaceImpl.kt index 233f2ce8ba..90495d9a7e 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/ReactNativeInternalInterfaceImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/ReactNativeInternalInterfaceImpl.kt @@ -48,9 +48,7 @@ internal class ReactNativeInternalInterfaceImpl( properties, null, stacktrace, - LogExceptionType.HANDLED, - null, - null + LogExceptionType.HANDLED ) } else { logger.logSdkNotInitialized("log JS exception") diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/UnityInternalInterfaceImpl.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/UnityInternalInterfaceImpl.kt index cfd3586c6b..ee83388ecc 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/UnityInternalInterfaceImpl.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/internal/api/delegate/UnityInternalInterfaceImpl.kt @@ -64,8 +64,6 @@ internal class UnityInternalInterfaceImpl( null, stacktrace, exceptionType, - null, - null, name, message ) diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/EmbraceInternalInterfaceImplTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/EmbraceInternalInterfaceImplTest.kt index 14095f9cf4..aeeecada07 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/EmbraceInternalInterfaceImplTest.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/EmbraceInternalInterfaceImplTest.kt @@ -93,9 +93,7 @@ internal class EmbraceInternalInterfaceImplTest { emptyMap(), exception.stackTrace, null, - LogExceptionType.NONE, - null, - null + LogExceptionType.NONE ) } } diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/FlutterInternalInterfaceImplTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/FlutterInternalInterfaceImplTest.kt index f27b30f1c3..a136f29ad0 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/FlutterInternalInterfaceImplTest.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/FlutterInternalInterfaceImplTest.kt @@ -67,8 +67,6 @@ internal class FlutterInternalInterfaceImplTest { null, "stack", LogExceptionType.UNHANDLED, - "ctx", - "lib", "exception name", "message" ) @@ -87,8 +85,6 @@ internal class FlutterInternalInterfaceImplTest { null, "stack", LogExceptionType.HANDLED, - "ctx", - "lib", "exception name", "message" ) diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/UnityInternalInterfaceImplTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/UnityInternalInterfaceImplTest.kt index 5b768238e2..a748dc8a03 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/UnityInternalInterfaceImplTest.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/internal/api/UnityInternalInterfaceImplTest.kt @@ -66,8 +66,6 @@ internal class UnityInternalInterfaceImplTest { null, "stack", LogExceptionType.UNHANDLED, - null, - null, "name", "msg" ) @@ -86,8 +84,6 @@ internal class UnityInternalInterfaceImplTest { null, "stack", LogExceptionType.HANDLED, - null, - null, "name", "msg" ) diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeLogService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeLogService.kt index 1a9deb8440..9162fde9bb 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeLogService.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeLogService.kt @@ -3,6 +3,7 @@ package io.embrace.android.embracesdk.fakes import io.embrace.android.embracesdk.LogExceptionType import io.embrace.android.embracesdk.Severity import io.embrace.android.embracesdk.internal.logs.LogService +import io.opentelemetry.api.common.AttributeKey class FakeLogService : LogService { class LogData( @@ -12,8 +13,6 @@ class FakeLogService : LogService { val properties: Map?, val stackTraceElements: Array?, val customStackTrace: String?, - val context: String?, - val library: String?, val exceptionName: String?, val exceptionMessage: String?, ) @@ -29,10 +28,9 @@ class FakeLogService : LogService { properties: Map?, stackTraceElements: Array?, customStackTrace: String?, - context: String?, - library: String?, exceptionName: String?, exceptionMessage: String?, + customLogAttrs: Map, String> ) { loggedMessages.add( LogData( @@ -42,8 +40,6 @@ class FakeLogService : LogService { properties = properties, stackTraceElements = stackTraceElements, customStackTrace = customStackTrace, - context = context, - library = library, exceptionName = exceptionName, exceptionMessage = exceptionMessage ) @@ -54,7 +50,5 @@ class FakeLogService : LogService { return errorLogIds.count() } - override fun cleanCollections() { - TODO("Not yet implemented") - } + override fun cleanCollections() {} }