From 76f1340ffd0fd2959f64f75b847606244194176a Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Tue, 14 Jan 2025 11:55:36 +0000 Subject: [PATCH 1/3] wip: support embrace hosted attachments --- .../internal/injection/LogModuleImpl.kt | 3 +- .../internal/logs/EmbraceLogService.kt | 11 ++ .../embracesdk/internal/logs/LogService.kt | 7 +- .../PayloadResurrectionServiceImpl.kt | 4 +- .../session/orchestrator/PayloadStore.kt | 2 +- .../session/orchestrator/V1PayloadStore.kt | 2 +- .../session/orchestrator/V2PayloadStore.kt | 9 +- .../internal/logs/EmbraceLogServiceTest.kt | 24 +++- .../orchestrator/V2PayloadStoreTest.kt | 11 ++ .../delivery/SupportedEnvelopeType.kt | 3 +- .../OkHttpRequestExecutionService.kt | 93 ++++++++---- .../delivery/intake/IntakeServiceImpl.kt | 9 +- .../delivery/storage/AttachmentStorage.kt | 22 +++ .../storage/CachedLogEnvelopeStoreImpl.kt | 4 +- .../delivery/intake/IntakeServiceImplTest.kt | 35 ++++- .../features/FileAttachmentFeatureTest.kt | 135 ++++++++++-------- .../session/PeriodicSessionCacheTest.kt | 2 +- .../internal/api/delegate/LogsApiDelegate.kt | 14 +- .../embracesdk/fakes/FakeIntakeService.kt | 5 - .../embracesdk/fakes/FakeLogService.kt | 2 + .../fakes/FakePayloadStorageService.kt | 9 +- .../embracesdk/fakes/FakePayloadStore.kt | 4 +- .../fakes/FakeRequestExecutionService.kt | 2 +- .../fakes/injection/FakeLogModule.kt | 4 +- 24 files changed, 298 insertions(+), 118 deletions(-) create mode 100644 embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/AttachmentStorage.kt diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/LogModuleImpl.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/LogModuleImpl.kt index 4eb169d5f5..f244c95c56 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/LogModuleImpl.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/injection/LogModuleImpl.kt @@ -62,7 +62,8 @@ internal class LogModuleImpl( EmbraceLogService( essentialServiceModule.logWriter, configModule.configService, - essentialServiceModule.sessionPropertiesService + essentialServiceModule.sessionPropertiesService, + deliveryModule.payloadStore, ) } 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 6dfe02e372..6b8a1d44a4 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 @@ -11,8 +11,11 @@ import io.embrace.android.embracesdk.internal.arch.schema.TelemetryAttributes import io.embrace.android.embracesdk.internal.capture.session.SessionPropertiesService import io.embrace.android.embracesdk.internal.config.ConfigService import io.embrace.android.embracesdk.internal.config.behavior.REDACTED_LABEL +import io.embrace.android.embracesdk.internal.logs.attachments.Attachment import io.embrace.android.embracesdk.internal.opentelemetry.embExceptionHandling import io.embrace.android.embracesdk.internal.payload.AppFramework +import io.embrace.android.embracesdk.internal.payload.Envelope +import io.embrace.android.embracesdk.internal.session.orchestrator.PayloadStore 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 @@ -26,6 +29,7 @@ class EmbraceLogService( private val logWriter: LogWriter, private val configService: ConfigService, private val sessionPropertiesService: SessionPropertiesService, + private val payloadStore: PayloadStore?, ) : LogService { private val behavior = configService.logMessageBehavior @@ -41,6 +45,7 @@ class EmbraceLogService( logExceptionType: LogExceptionType, properties: Map?, customLogAttrs: Map, String>, + logAttachment: Attachment.EmbraceHosted?, ) { val redactedProperties = redactSensitiveProperties(normalizeProperties(properties)) val attrs = createTelemetryAttributes(redactedProperties, customLogAttrs) @@ -53,6 +58,12 @@ class EmbraceLogService( if (logExceptionType != LogExceptionType.NONE) { attrs.setAttribute(embExceptionHandling, logExceptionType.value) } + + logAttachment?.let { + val envelope = Envelope(data = Pair(it.id, it.bytes)) + payloadStore?.storeAttachment(envelope) + } + addLogEventData( message = message, severity = severity, 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 654d11c42c..f6c14e93c5 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 @@ -2,6 +2,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.logs.attachments.Attachment import io.embrace.android.embracesdk.internal.session.MemoryCleanerListener import io.opentelemetry.api.common.AttributeKey @@ -12,11 +13,6 @@ interface LogService : MemoryCleanerListener { /** * Creates a remote log. - * - * @param message the message to 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 */ fun log( message: String, @@ -24,6 +20,7 @@ interface LogService : MemoryCleanerListener { logExceptionType: LogExceptionType, properties: Map? = null, customLogAttrs: Map, String> = emptyMap(), + logAttachment: Attachment.EmbraceHosted? = null, ) /** diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt index 76473bb1b8..6c9b316fbd 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/resurrection/PayloadResurrectionServiceImpl.kt @@ -97,7 +97,7 @@ internal class PayloadResurrectionServiceImpl( inputStream = GZIPInputStream( cacheStorageService.loadPayloadAsStream(cachedCrashEnvelopeMetadata) ), - type = SupportedEnvelopeType.CRASH.serializedType + type = checkNotNull(SupportedEnvelopeType.CRASH.serializedType) ).also { cacheStorageService.delete(cachedCrashEnvelopeMetadata) } @@ -157,7 +157,7 @@ internal class PayloadResurrectionServiceImpl( SupportedEnvelopeType.SESSION -> { val deadSession = serializer.fromJson>( inputStream = GZIPInputStream(cacheStorageService.loadPayloadAsStream(this)), - type = envelopeType.serializedType + type = checkNotNull(envelopeType.serializedType) ) val sessionId = deadSession.getSessionId() diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/PayloadStore.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/PayloadStore.kt index 5e3a065ea0..fd3ee1e65b 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/PayloadStore.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/PayloadStore.kt @@ -31,7 +31,7 @@ interface PayloadStore : CrashTeardownHandler { /** * Stores a log attachment. */ - fun storeAttachment(envelope: Envelope) + fun storeAttachment(envelope: Envelope>) /** * Stores an empty payload-type-less crash envelope for future use. One one cached version of this should diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V1PayloadStore.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V1PayloadStore.kt index de2b128dd2..68d47042fd 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V1PayloadStore.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V1PayloadStore.kt @@ -31,7 +31,7 @@ class V1PayloadStore( } } - override fun storeAttachment(envelope: Envelope) { + override fun storeAttachment(envelope: Envelope>) { // ignored - v1 doesn't support attachments } diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStore.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStore.kt index 347b88d8b6..e0a8bd3b29 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStore.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStore.kt @@ -41,7 +41,14 @@ internal class V2PayloadStore( intakeService.take(envelope, createMetadata(type, payloadType = payloadType)) } - override fun storeAttachment(envelope: Envelope) { + override fun storeAttachment(envelope: Envelope>) { + intakeService.take( + envelope, + createMetadata( + type = SupportedEnvelopeType.ATTACHMENT, + payloadType = PayloadType.ATTACHMENT + ) + ) } override fun cacheEmptyCrashEnvelope(envelope: Envelope) { 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 1680bda5f1..ffb48dcae8 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 @@ -4,6 +4,7 @@ import io.embrace.android.embracesdk.LogExceptionType import io.embrace.android.embracesdk.Severity import io.embrace.android.embracesdk.fakes.FakeConfigService import io.embrace.android.embracesdk.fakes.FakeLogWriter +import io.embrace.android.embracesdk.fakes.FakePayloadStore import io.embrace.android.embracesdk.fakes.FakeSessionPropertiesService import io.embrace.android.embracesdk.fakes.behavior.FakeLogMessageBehavior import io.embrace.android.embracesdk.fakes.config.FakeInstrumentedConfig @@ -14,6 +15,7 @@ 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.logs.attachments.Attachment import io.embrace.android.embracesdk.internal.payload.AppFramework import io.opentelemetry.semconv.incubating.LogIncubatingAttributes import org.junit.Assert.assertEquals @@ -28,6 +30,7 @@ internal class EmbraceLogServiceTest { private lateinit var fakeLogWriter: FakeLogWriter private lateinit var fakeSessionPropertiesService: FakeSessionPropertiesService private lateinit var fakeConfigService: FakeConfigService + private lateinit var payloadStore: FakePayloadStore @Before fun setUp() { @@ -38,7 +41,7 @@ internal class EmbraceLogServiceTest { ) fakeSessionPropertiesService = FakeSessionPropertiesService() fakeLogWriter = FakeLogWriter() - + payloadStore = FakePayloadStore() logService = createEmbraceLogService() } @@ -46,6 +49,7 @@ internal class EmbraceLogServiceTest { logWriter = fakeLogWriter, configService = fakeConfigService, sessionPropertiesService = fakeSessionPropertiesService, + payloadStore = payloadStore, ) @Test @@ -237,4 +241,22 @@ internal class EmbraceLogServiceTest { // then the correct number of error logs is returned assertEquals(5, logService.getErrorLogsCount()) } + + @Test + fun `log with attachment`() { + val bytes = ByteArray(2) + val msg = "message" + logService.log( + message = msg, + severity = Severity.INFO, + logExceptionType = LogExceptionType.NONE, + logAttachment = Attachment.EmbraceHosted(bytes) { true }, + ) + + // then the sensitive key is redacted + val log = fakeLogWriter.logEvents.single() + assertEquals(msg, log.message) + val attachment = payloadStore.storedAttachments.single() + assertEquals(bytes, attachment.data.second) + } } diff --git a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStoreTest.kt b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStoreTest.kt index ffdc06e5c3..7bf22c0624 100644 --- a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStoreTest.kt +++ b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/session/orchestrator/V2PayloadStoreTest.kt @@ -110,6 +110,17 @@ class V2PayloadStoreTest { assertEquals(System.NetworkCapturedRequest.value, getLastLogMetadata().payloadType.value) } + @Test + fun `test log attachment`() { + val envelope = Envelope(data = Pair("test", ByteArray(5))) + store.storeAttachment(envelope) + + val intake = intakeService.getIntakes>().single() + assertSame(envelope, intake.envelope) + assertEquals("p4_1692201601000_fakeuuid_fakeProcessId_true_attachment_v1.json", intake.metadata.filename) + assertEquals(0, intakeService.shutdownCount) + } + private fun storeLogWithType(type: TelemetryType) { val envelope = Envelope( data = LogPayload( diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/SupportedEnvelopeType.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/SupportedEnvelopeType.kt index dd7217dc89..6793422ea7 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/SupportedEnvelopeType.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/SupportedEnvelopeType.kt @@ -8,13 +8,14 @@ import java.lang.reflect.Type * Enumerates the different types of telemetry that are supported when persisting to disk. */ enum class SupportedEnvelopeType( - val serializedType: Type, + val serializedType: Type?, val priority: String, val endpoint: Endpoint, ) { CRASH(Envelope.logEnvelopeType, "p1", Endpoint.LOGS), SESSION(Envelope.sessionEnvelopeType, "p3", Endpoint.SESSIONS), + ATTACHMENT(null, "p4", Endpoint.ATTACHMENT), LOG(Envelope.logEnvelopeType, "p5", Endpoint.LOGS), BLOB(Envelope.logEnvelopeType, "p7", Endpoint.LOGS); diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/execution/OkHttpRequestExecutionService.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/execution/OkHttpRequestExecutionService.kt index 04f7d90f24..747e18ff88 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/execution/OkHttpRequestExecutionService.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/execution/OkHttpRequestExecutionService.kt @@ -5,18 +5,22 @@ import io.embrace.android.embracesdk.internal.comms.api.Endpoint import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType import io.embrace.android.embracesdk.internal.delivery.debug.DeliveryTracer import io.embrace.android.embracesdk.internal.delivery.execution.ExecutionResult.Companion.getResult +import io.embrace.android.embracesdk.internal.delivery.storage.loadAttachment import io.embrace.android.embracesdk.internal.logging.EmbLogger import io.embrace.android.embracesdk.internal.logging.InternalErrorType import okhttp3.Headers.Companion.toHeaders import okhttp3.MediaType.Companion.toMediaType +import okhttp3.MultipartBody import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.RequestBody +import okhttp3.RequestBody.Companion.toRequestBody import okio.BufferedSink import okio.buffer import okio.source import java.io.IOException import java.io.InputStream +import java.util.zip.GZIPInputStream class OkHttpRequestExecutionService( private val okHttpClient: OkHttpClient, @@ -28,23 +32,21 @@ class OkHttpRequestExecutionService( private val deliveryTracer: DeliveryTracer? = null, ) : RequestExecutionService { + private companion object { + private val mediaType = "application/json".toMediaType() + } + override fun attemptHttpRequest( payloadStream: () -> InputStream, envelopeType: SupportedEnvelopeType, payloadType: String, ): ExecutionResult { - val apiRequest = envelopeType.endpoint.getApiRequestFromEndpoint() - val requestBody = ApiRequestBody(payloadStream) - val request = Request.Builder() - .url(apiRequest.url) - .headers( - apiRequest - .getHeaders() - .plus("X-EM-TYPES" to payloadType) - .toHeaders() - ) - .post(requestBody) - .build() + val multipart = envelopeType.endpoint == Endpoint.ATTACHMENT + val apiRequest = envelopeType.endpoint.getApiRequestFromEndpoint(multipart) + val request = when { + multipart -> prepareMultipartRequest(payloadStream, apiRequest) + else -> prepareRequest(payloadStream, apiRequest, payloadType) + } var executionError: Throwable? = null val httpCallResponse = try { @@ -72,26 +74,67 @@ class OkHttpRequestExecutionService( } } - private fun Endpoint.getApiRequestFromEndpoint(): ApiRequestV2 = ApiRequestV2( + private fun prepareRequest( + payloadStream: () -> InputStream, + apiRequest: ApiRequestV2, + payloadType: String, + ): Request { + val request = Request.Builder() + .url(apiRequest.url) + .headers( + apiRequest + .getHeaders() + .plus("X-EM-TYPES" to payloadType) + .toHeaders() + ) + .post(ApiRequestBody(payloadStream)) + .build() + return request + } + + private fun prepareMultipartRequest( + payloadStream: () -> InputStream, + apiRequest: ApiRequestV2, + ): Request { + GZIPInputStream(payloadStream()).use { + val attachment = loadAttachment(it) ?: throw IOException("Failed to load attachment") + val builder = MultipartBody.Builder() + .setType(MultipartBody.FORM) + .addFormDataPart("app_id", checkNotNull(apiRequest.appId)) + .addFormDataPart("attachment_id", attachment.second) + .addFormDataPart("file", "file", attachment.first.toRequestBody()) + + val request = Request.Builder() + .url(apiRequest.url) + .post(builder.build()) + .build() + return request + } + } + + private fun Endpoint.getApiRequestFromEndpoint(multipart: Boolean): ApiRequestV2 = ApiRequestV2( url = "$coreBaseUrl${this.path}", appId = appId, deviceId = lazyDeviceId.value, - contentEncoding = "gzip", + contentEncoding = when { + multipart -> null + else -> "gzip" + }, + contentType = when { + multipart -> "multipart/form-data" + else -> "application/json" + }, userAgent = "Embrace/a/$embraceVersionName" ) - private companion object { - private val mediaType = "application/json".toMediaType() - - class ApiRequestBody( - private val payloadStream: () -> InputStream, - ) : RequestBody() { - override fun contentType() = mediaType + class ApiRequestBody( + private val payloadStream: () -> InputStream, + ) : RequestBody() { + override fun contentType() = mediaType - override fun writeTo(sink: BufferedSink) { - payloadStream().source().buffer().use { source -> - sink.writeAll(source) - } + override fun writeTo(sink: BufferedSink) { + payloadStream().source().buffer().use { source -> + sink.writeAll(source) } } } diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImpl.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImpl.kt index 75cf8cb781..e100e08ef8 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImpl.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImpl.kt @@ -5,6 +5,7 @@ import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType import io.embrace.android.embracesdk.internal.delivery.debug.DeliveryTracer import io.embrace.android.embracesdk.internal.delivery.scheduling.SchedulingService import io.embrace.android.embracesdk.internal.delivery.storage.PayloadStorageService +import io.embrace.android.embracesdk.internal.delivery.storage.storeAttachment import io.embrace.android.embracesdk.internal.logging.EmbLogger import io.embrace.android.embracesdk.internal.logging.InternalErrorType import io.embrace.android.embracesdk.internal.payload.Envelope @@ -45,6 +46,7 @@ class IntakeServiceImpl( } } + @Suppress("UNCHECKED_CAST") private fun processIntake( intake: Envelope<*>, metadata: StoredTelemetryMetadata, @@ -55,7 +57,12 @@ class IntakeServiceImpl( else -> cacheStorageService } service.store(metadata) { stream -> - serializer.toJson(intake, metadata.envelopeType.serializedType, stream) + if (metadata.envelopeType.serializedType != null) { + serializer.toJson(intake, metadata.envelopeType.serializedType, stream) + } else { // payload doesn't require serialization + val pair = intake.data as Pair + storeAttachment(stream, pair.second, pair.first) + } } val lastReference = cacheReferences[metadata.envelopeType] diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/AttachmentStorage.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/AttachmentStorage.kt new file mode 100644 index 0000000000..e32e533568 --- /dev/null +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/AttachmentStorage.kt @@ -0,0 +1,22 @@ +package io.embrace.android.embracesdk.internal.delivery.storage + +import java.io.InputStream +import java.io.OutputStream + +fun storeAttachment(stream: OutputStream, attachment: ByteArray, id: String) { + stream.use { + it.write(id.toByteArray()) + it.write("\n".toByteArray()) + it.write(attachment) + } +} + +fun loadAttachment(stream: InputStream): Pair? { + stream.use { + val contents = it.readBytes() + val start = contents.indexOfFirst { byte -> byte == '\n'.code.toByte() } + val id = String(contents.sliceArray(0 until start)) + val attachment = contents.sliceArray(start + 1 until contents.size) + return Pair(attachment, id) + } +} diff --git a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/CachedLogEnvelopeStoreImpl.kt b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/CachedLogEnvelopeStoreImpl.kt index a21795d6b5..f2150a0097 100644 --- a/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/CachedLogEnvelopeStoreImpl.kt +++ b/embrace-android-delivery/src/main/kotlin/io/embrace/android/embracesdk/internal/delivery/storage/CachedLogEnvelopeStoreImpl.kt @@ -37,7 +37,7 @@ class CachedLogEnvelopeStoreImpl( resource, metadata ), - storedTelemetryMetadata.envelopeType.serializedType, + checkNotNull(storedTelemetryMetadata.envelopeType.serializedType), stream ) } @@ -48,7 +48,7 @@ class CachedLogEnvelopeStoreImpl( fileStorageService.loadPayloadAsStream(storedTelemetryMetadata)?.let { inputStream -> serializer.fromJson>( inputStream = inputStream, - type = storedTelemetryMetadata.envelopeType.serializedType + type = checkNotNull(storedTelemetryMetadata.envelopeType.serializedType) ) } }.getOrNull() diff --git a/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImplTest.kt b/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImplTest.kt index 0fcf5442ff..192c4d44cc 100644 --- a/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImplTest.kt +++ b/embrace-android-delivery/src/test/kotlin/io/embrace/android/embracesdk/internal/delivery/intake/IntakeServiceImplTest.kt @@ -8,6 +8,7 @@ import io.embrace.android.embracesdk.fakes.FakeSchedulingService import io.embrace.android.embracesdk.fakes.TestPlatformSerializer import io.embrace.android.embracesdk.internal.delivery.PayloadType import io.embrace.android.embracesdk.internal.delivery.StoredTelemetryMetadata +import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType.ATTACHMENT import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType.BLOB import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType.CRASH import io.embrace.android.embracesdk.internal.delivery.SupportedEnvelopeType.LOG @@ -25,6 +26,7 @@ import io.embrace.android.embracesdk.internal.worker.PriorityWorker import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -57,6 +59,9 @@ class IntakeServiceImplTest { private val logEnvelope = Envelope( data = LogPayload(logs = listOf(Log(body = "Log data"))) ) + private val attachmentEnvelope = Envelope( + data = Pair("my-id", ByteArray(2)) + ) private val sessionDataExpected = run { val baos = ByteArrayOutputStream() serializer.toJson(sessionEnvelope, Envelope.sessionEnvelopeType, GZIPOutputStream(baos)) @@ -70,11 +75,15 @@ class IntakeServiceImplTest { private val clock = FakeClock() private val sessionMetadata = StoredTelemetryMetadata(clock.now(), UUID, PROCESS_ID, SESSION) + private val attachmentMetadata = + StoredTelemetryMetadata(clock.now(), UUID, PROCESS_ID, ATTACHMENT, payloadType = PayloadType.ATTACHMENT) private val logMetadata = StoredTelemetryMetadata(clock.now(), UUID, PROCESS_ID, LOG, payloadType = PayloadType.LOG) private val networkMetadata = StoredTelemetryMetadata(clock.now(), UUID, PROCESS_ID, BLOB) private val crashMetadata = StoredTelemetryMetadata(clock.now(), UUID, PROCESS_ID, CRASH) private val sessionMetadata2 = StoredTelemetryMetadata(clock.apply { tick(100L) }.now(), UUID, PROCESS_ID, SESSION) private val logMetadata2 = StoredTelemetryMetadata(clock.apply { tick(100L) }.now(), UUID, PROCESS_ID, LOG) + private val attachmentMetadata2 = + StoredTelemetryMetadata(clock.apply { tick(100L) }.now(), UUID, PROCESS_ID, ATTACHMENT) private val networkMetadata2 = StoredTelemetryMetadata(clock.apply { tick(100L) }.now(), UUID, PROCESS_ID, BLOB) private val crashMetadata2 = StoredTelemetryMetadata(clock.apply { tick(100L) }.now(), UUID, PROCESS_ID, CRASH) @@ -137,6 +146,24 @@ class IntakeServiceImplTest { assertEquals(1, schedulingService.payloadIntakeCount) } + @Test + fun `take attachment`() { + intakeService.take(attachmentEnvelope, attachmentMetadata) + executorService.runCurrentlyBlocked() + + // assert filename is valid & contains correct metadata + val filename = payloadStorageService.storedFilenames().single() + val metadata = StoredTelemetryMetadata.fromFilename(filename).getOrThrow() + assertEquals(ATTACHMENT, metadata.envelopeType) + + // assert payload was stored + val obj = payloadStorageService.storedPayloads().single() + assertNotNull(obj) + + // assert scheduling service was notified + assertEquals(1, schedulingService.payloadIntakeCount) + } + @Test fun `take session`() { intakeService.take(sessionEnvelope, sessionMetadata) @@ -231,8 +258,10 @@ class IntakeServiceImplTest { take(sessionEnvelope, sessionMetadata) take(logEnvelope, crashMetadata) take(logEnvelope, networkMetadata) + take(attachmentEnvelope, attachmentMetadata) take(logEnvelope, logMetadata) clock.tick(1000) + take(attachmentEnvelope, attachmentMetadata2) take(logEnvelope, logMetadata2) take(logEnvelope, networkMetadata2) take(logEnvelope, crashMetadata2) @@ -242,15 +271,15 @@ class IntakeServiceImplTest { latch.countDown() } worker.shutdownAndWait(1000) - assertEquals(8, payloadStorageService.storedPayloadCount()) - assertEquals(8, payloadStorageService.storeCount.get()) + assertEquals(10, payloadStorageService.storedPayloadCount()) + assertEquals(10, payloadStorageService.storeCount.get()) // assert payloads were prioritised in the expected order val observedTypes = payloadStorageService.storedFilenames().map { val metadata = StoredTelemetryMetadata.fromFilename(it).getOrThrow() metadata.envelopeType } - val expected = listOf(CRASH, CRASH, SESSION, SESSION, LOG, LOG, BLOB, BLOB) + val expected = listOf(CRASH, CRASH, SESSION, SESSION, ATTACHMENT, ATTACHMENT, LOG, LOG, BLOB, BLOB) assertEquals(expected, observedTypes) } diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt index 5f01074c47..5ce25b9055 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt +++ b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt @@ -9,6 +9,7 @@ import io.embrace.android.embracesdk.testframework.actions.EmbraceActionInterfac import io.embrace.android.embracesdk.testframework.assertions.assertOtelLogReceived import io.embrace.android.embracesdk.testframework.assertions.getLastLog import io.embrace.android.embracesdk.testframework.assertions.getOtelSeverity +import io.embrace.android.embracesdk.testframework.server.FormPart import java.util.UUID import org.junit.Assert.assertEquals import org.junit.Assert.assertNull @@ -37,18 +38,15 @@ internal class FileAttachmentFeatureTest { val size = 509L val url = "https://example.com/my-file.txt" val id = attachmentId.toString() - testRule.runTest( - testCaseAction = { - recordSession { - logWithUserHostedAttachment(id, url, size) - } - }, - assertAction = { - val log = getSingleLogEnvelope().getLastLog() - assertUserHostedLogSent(log, size, url, id) - assertNull(log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE)) + testRule.runTest(testCaseAction = { + recordSession { + logWithUserHostedAttachment(id, url, size) } - ) + }, assertAction = { + val log = getSingleLogEnvelope().getLastLog() + assertUserHostedLogSent(log, size, url, id) + assertNull(log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE)) + }) } @Test @@ -56,64 +54,69 @@ internal class FileAttachmentFeatureTest { val size = 509L val url = "https://example.com/my-file.txt" val id = attachmentId.toString() - testRule.runTest( - testCaseAction = { - recordSession { - repeat(6) { - logWithUserHostedAttachment(id, url, size) - } + val limit = 5 + testRule.runTest(testCaseAction = { + recordSession { + repeat(limit + 1) { + logWithUserHostedAttachment(id, url, size) } - recordSession { - repeat(6) { - logWithUserHostedAttachment(id, url, size) - } + } + recordSession { + repeat(limit + 1) { + logWithUserHostedAttachment(id, url, size) } - }, - assertAction = { - val logs = getLogEnvelopes(2).flatMap { checkNotNull(it.data.logs) } - assertEquals(12, logs.size) + } + }, assertAction = { + val logs = getLogEnvelopes(2).flatMap { checkNotNull(it.data.logs) } + assertEquals((limit * 2) + 2, logs.size) - logs.forEachIndexed { k, log -> - assertUserHostedLogSent(log, size, url, id) + logs.forEachIndexed { k, log -> + assertUserHostedLogSent(log, size, url, id) - val errCode = log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE) - val expectedCode = when (k) { - 5, 11 -> "OVER_MAX_ATTACHMENTS" - else -> null - } - assertEquals(expectedCode, errCode) + val errCode = log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE) + val expectedCode = when (k) { + limit, limit + limit + 1 -> "OVER_MAX_ATTACHMENTS" + else -> null } + assertEquals(expectedCode, errCode) } - ) + }) } @Test fun `log message with embrace hosted file attachment`() { - testRule.runTest( - testCaseAction = { - recordSession { - embrace.logMessage( - message = "test message", - severity = Severity.INFO, - properties = mapOf("key" to "value"), - attachment = "Hello, world!".toByteArray() - ) - } - }, - assertAction = { - val log = getSingleLogEnvelope().getLastLog() - assertOtelLogReceived( - logReceived = log, - expectedMessage = "test message", - expectedSeverityNumber = getOtelSeverity(Severity.INFO).severityNumber, - expectedSeverityText = Severity.INFO.name, - expectedState = "foreground", - expectedProperties = mapOf( - "key" to "value", - ), + val byteArray = "Hello, world!".toByteArray() + testRule.runTest(testCaseAction = { + recordSession { + embrace.logMessage( + message = "test message", + severity = Severity.INFO, + properties = mapOf("key" to "value"), + attachment = byteArray ) } + }, assertAction = { + val parts = getSingleAttachment() + verifyAttachment(parts, byteArray) + + val log = getSingleLogEnvelope().getLastLog() + assertEmbraceHostedLogSent(log, byteArray.size.toLong()) + }) + } + + private fun verifyAttachment( + parts: List, + byteArray: ByteArray, + appId: String = "abcde" + ) { + assertEquals("form-data; name=\"app_id\"", parts[0].contentDisposition) + assertEquals(appId, parts[0].data) + assertEquals("form-data; name=\"attachment_id\"", parts[1].contentDisposition) + checkNotNull(UUID.fromString(parts[1].data)) + assertEquals( + "form-data; name=\"file\"; filename=\"file\"", parts[2].contentDisposition ) + assertEquals(String(byteArray), parts[2].data) } private fun assertUserHostedLogSent( @@ -138,6 +141,26 @@ internal class FileAttachmentFeatureTest { ) } + private fun assertEmbraceHostedLogSent( + log: Log, + size: Long, + ) { + assertOtelLogReceived( + logReceived = log, + expectedMessage = "test message", + expectedSeverityNumber = getOtelSeverity(Severity.INFO).severityNumber, + expectedSeverityText = Severity.INFO.name, + expectedState = "foreground", + expectedTimeMs = null, + expectedProperties = mapOf( + "key" to "value", + ATTR_KEY_SIZE to size.toString(), + ), + ) + val id = log.attributes?.findAttributeValue(ATTR_KEY_ID) + checkNotNull(UUID.fromString(id)) + } + private fun EmbraceActionInterface.logWithUserHostedAttachment( id: String, url: String, diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/session/PeriodicSessionCacheTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/session/PeriodicSessionCacheTest.kt index 72bd16ed25..83507ea4b5 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/session/PeriodicSessionCacheTest.kt +++ b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/session/PeriodicSessionCacheTest.kt @@ -103,7 +103,7 @@ internal class PeriodicSessionCacheTest { val inputStream = checkNotNull(cacheStorageService.loadPayloadAsStream(metadata)) return TestPlatformSerializer().fromJson( inputStream, - SupportedEnvelopeType.SESSION.serializedType + checkNotNull(SupportedEnvelopeType.SESSION.serializedType) ) } 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 20db41f2e9..c4b821b701 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 @@ -7,7 +7,6 @@ import io.embrace.android.embracesdk.internal.injection.ModuleInitBootstrapper import io.embrace.android.embracesdk.internal.injection.embraceImplInject import io.embrace.android.embracesdk.internal.logs.attachments.Attachment import io.embrace.android.embracesdk.internal.logs.attachments.Attachment.EmbraceHosted -import io.embrace.android.embracesdk.internal.logs.attachments.Attachment.UserHosted import io.embrace.android.embracesdk.internal.payload.PushNotificationBreadcrumb import io.embrace.android.embracesdk.internal.serialization.truncatedStacktrace import io.embrace.android.embracesdk.internal.utils.getSafeStackTrace @@ -175,17 +174,20 @@ internal class LogsApiDelegate( stacktrace?.let { attrs[ExceptionAttributes.EXCEPTION_STACKTRACE] = it } if (attachment != null) { - when (attachment) { - is EmbraceHosted -> {} // TODO: add support - is UserHosted -> attrs.putAll(attachment.attributes.mapKeys { it.key.attributeKey }) - } + attrs.putAll(attachment.attributes.mapKeys { it.key.attributeKey }) + } + + val logAttachment = when { + attachment is EmbraceHosted && attachment.shouldAttemptUpload() -> attachment + else -> null } logService?.log( message, severity, logExceptionType, properties, - attrs.plus(customLogAttrs) + attrs.plus(customLogAttrs), + logAttachment ) sessionOrchestrator?.reportBackgroundActivityStateChange() } diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeIntakeService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeIntakeService.kt index e170e1728e..0b42b86f5a 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeIntakeService.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeIntakeService.kt @@ -3,8 +3,6 @@ package io.embrace.android.embracesdk.fakes import io.embrace.android.embracesdk.internal.delivery.StoredTelemetryMetadata import io.embrace.android.embracesdk.internal.delivery.intake.IntakeService import io.embrace.android.embracesdk.internal.payload.Envelope -import io.embrace.android.embracesdk.internal.payload.LogPayload -import io.embrace.android.embracesdk.internal.payload.SessionPayload class FakeIntakeService : IntakeService { @@ -14,9 +12,6 @@ class FakeIntakeService : IntakeService { @Suppress("UNCHECKED_CAST") inline fun getIntakes(complete: Boolean = true): List> { - if (T::class != SessionPayload::class && T::class != LogPayload::class) { - error("Unsupported type: ${T::class}") - } val dst = when (complete) { true -> intakeList false -> cacheList 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 f127be26fc..2fd46dd2a8 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.embrace.android.embracesdk.internal.logs.attachments.Attachment import io.opentelemetry.api.common.AttributeKey class FakeLogService : LogService { @@ -23,6 +24,7 @@ class FakeLogService : LogService { logExceptionType: LogExceptionType, properties: Map?, customLogAttrs: Map, String>, + logAttachment: Attachment.EmbraceHosted?, ) { loggedMessages.add( LogData( diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStorageService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStorageService.kt index 22a2aba9b8..4e9c9179fb 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStorageService.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStorageService.kt @@ -1,5 +1,6 @@ package io.embrace.android.embracesdk.fakes +import io.embrace.android.embracesdk.internal.delivery.PayloadType import io.embrace.android.embracesdk.internal.delivery.StoredTelemetryMetadata import io.embrace.android.embracesdk.internal.delivery.storage.PayloadStorageService import io.embrace.android.embracesdk.internal.injection.SerializationAction @@ -30,7 +31,11 @@ class FakePayloadStorageService( } val baos = ByteArrayOutputStream() - action(GZIPOutputStream(baos)) + if (metadata.payloadType != PayloadType.ATTACHMENT) { + action(GZIPOutputStream(baos)) + } else { + action(baos) + } cachedPayloads[metadata] = baos.toByteArray() } @@ -58,7 +63,7 @@ class FakePayloadStorageService( fun addPayload(metadata: StoredTelemetryMetadata, data: T) { store(metadata) { stream -> - serializer.toJson(data, metadata.envelopeType.serializedType, stream) + serializer.toJson(data, checkNotNull(metadata.envelopeType.serializedType), stream) } } diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStore.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStore.kt index ddbb074a86..d9b043725a 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStore.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakePayloadStore.kt @@ -13,7 +13,7 @@ class FakePayloadStore( val storedSessionPayloads = mutableListOf, TransitionType>>() val storedLogPayloads = mutableListOf, Boolean>>() - val storedAttachments = mutableListOf>() + val storedAttachments = mutableListOf>>() val cachedSessionPayloads = mutableListOf>() val cachedEmptyCrashPayloads = mutableListOf>() var crashCount: Int = 0 @@ -34,7 +34,7 @@ class FakePayloadStore( storedLogPayloads.add(Pair(envelope, attemptImmediateRequest)) } - override fun storeAttachment(envelope: Envelope) { + override fun storeAttachment(envelope: Envelope>) { storedAttachments.add(envelope) } diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeRequestExecutionService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeRequestExecutionService.kt index 1956564887..48fb71e709 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeRequestExecutionService.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeRequestExecutionService.kt @@ -40,7 +40,7 @@ class FakeRequestExecutionService( ): ExecutionResult { exceptionOnExecution?.run { throw this } val bufferedStream = GZIPInputStream(payloadStream()) - val envelope: Envelope<*> = serializer.fromJson(bufferedStream, envelopeType.serializedType) + val envelope: Envelope<*> = serializer.fromJson(bufferedStream, checkNotNull(envelopeType.serializedType)) processEnvelope(envelope) return responseAction(envelope) } diff --git a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/injection/FakeLogModule.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/injection/FakeLogModule.kt index 10deb2d4d6..9dc9cde3cc 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/injection/FakeLogModule.kt +++ b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/injection/FakeLogModule.kt @@ -6,6 +6,7 @@ import io.embrace.android.embracesdk.fakes.FakeLogWriter import io.embrace.android.embracesdk.fakes.FakeNetworkCaptureDataSource import io.embrace.android.embracesdk.fakes.FakeNetworkCaptureService import io.embrace.android.embracesdk.fakes.FakeNetworkLoggingService +import io.embrace.android.embracesdk.fakes.FakePayloadStore import io.embrace.android.embracesdk.fakes.FakeSessionPropertiesService import io.embrace.android.embracesdk.internal.injection.LogModule import io.embrace.android.embracesdk.internal.logs.EmbraceLogService @@ -22,7 +23,8 @@ class FakeLogModule( override val logService: LogService = EmbraceLogService( FakeLogWriter(), FakeConfigService(), - FakeSessionPropertiesService() + FakeSessionPropertiesService(), + FakePayloadStore(), ), ) : LogModule { From 19e6abcdfdaef4b62ac0035ca7897f3f66b8773a Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Thu, 16 Jan 2025 08:04:28 +0000 Subject: [PATCH 2/3] api: remove size parameter from user hosted log messages --- .../api/embrace-android-api.api | 2 +- .../embracesdk/internal/api/LogsApi.kt | 2 -- .../internal/logs/attachments/Attachment.kt | 30 +++++++--------- .../logs/attachments/AttachmentErrorCode.kt | 1 - .../logs/attachments/AttachmentService.kt | 2 -- .../logs/attachments/AttachmentTest.kt | 35 +++---------------- .../api/embrace-android-sdk.api | 2 +- .../features/FileAttachmentFeatureTest.kt | 18 ++++------ .../io/embrace/android/embracesdk/Embrace.kt | 3 +- .../embrace/android/embracesdk/EmbraceImpl.kt | 2 -- .../internal/api/delegate/LogsApiDelegate.kt | 3 +- 11 files changed, 27 insertions(+), 73 deletions(-) diff --git a/embrace-android-api/api/embrace-android-api.api b/embrace-android-api/api/embrace-android-api.api index 7057798578..102b173ed1 100644 --- a/embrace-android-api/api/embrace-android-api.api +++ b/embrace-android-api/api/embrace-android-api.api @@ -81,7 +81,7 @@ public abstract interface class io/embrace/android/embracesdk/internal/api/LogsA public abstract fun logInfo (Ljava/lang/String;)V public abstract fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;)V public abstract fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;)V - public abstract fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;J)V + public abstract fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;)V public abstract fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;[B)V public abstract fun logPushNotification (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;Ljava/lang/Boolean;Ljava/lang/Boolean;)V public abstract fun logWarning (Ljava/lang/String;)V diff --git a/embrace-android-api/src/main/kotlin/io/embrace/android/embracesdk/internal/api/LogsApi.kt b/embrace-android-api/src/main/kotlin/io/embrace/android/embracesdk/internal/api/LogsApi.kt index 9c9533f7b9..fb93a14902 100644 --- a/embrace-android-api/src/main/kotlin/io/embrace/android/embracesdk/internal/api/LogsApi.kt +++ b/embrace-android-api/src/main/kotlin/io/embrace/android/embracesdk/internal/api/LogsApi.kt @@ -61,7 +61,6 @@ public interface LogsApi { * @param properties the properties to attach to the log message * @param attachmentId a UUID that identifies the attachment * @param attachmentUrl a URL that gives the location of the attachment - * @param attachmentSize the size of the attachment in bytes */ public fun logMessage( message: String, @@ -69,7 +68,6 @@ public interface LogsApi { properties: Map?, attachmentId: String, attachmentUrl: String, - attachmentSize: Long, ) /** diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/Attachment.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/Attachment.kt index bcdb71f8b0..0a5923ba46 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/Attachment.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/Attachment.kt @@ -14,10 +14,7 @@ import java.util.UUID /** * Holds attributes that describe an attachment to a log record. */ -sealed class Attachment( - val size: Long, - val id: String, -) { +sealed class Attachment(val id: String) { internal companion object { private const val LIMIT_MB = 1 * 1024 * 1024 @@ -26,11 +23,9 @@ sealed class Attachment( abstract val attributes: Map protected fun constructAttributes( - size: Long, id: String, - errorCode: AttachmentErrorCode? = null + errorCode: AttachmentErrorCode? = null, ): Map = mapOf( - embAttachmentSize to size.toString(), embAttachmentId to id, embAttachmentErrorCode to errorCode?.name ).toNonNullMap() @@ -40,20 +35,22 @@ sealed class Attachment( */ class EmbraceHosted( val bytes: ByteArray, - counter: () -> Boolean + counter: () -> Boolean, ) : Attachment( - bytes.size.toLong(), UUID.randomUUID().toString() ) { + private val size: Long = bytes.size.toLong() + private val errorCode: AttachmentErrorCode? = when { !counter() -> OVER_MAX_ATTACHMENTS size > LIMIT_MB -> ATTACHMENT_TOO_LARGE else -> null } - override val attributes: Map = - constructAttributes(size, id, errorCode) + override val attributes: Map = constructAttributes(id, errorCode).plus( + embAttachmentSize to size.toString(), + ) fun shouldAttemptUpload(): Boolean = errorCode == null } @@ -62,24 +59,21 @@ sealed class Attachment( * An attachment that is uploaded to a user-supplied backend. */ class UserHosted( - size: Long, id: String, val url: String, counter: () -> Boolean, - ) : Attachment(size, id) { + ) : Attachment(id) { private val errorCode: AttachmentErrorCode? = when { !counter() -> OVER_MAX_ATTACHMENTS - size < 0 -> UNKNOWN url.isEmpty() -> UNKNOWN isNotUuid() -> UNKNOWN else -> null } - override val attributes: Map = - constructAttributes(size, id, errorCode).plus( - embAttachmentUrl to url - ) + override val attributes: Map = constructAttributes(id, errorCode).plus( + embAttachmentUrl to url + ) private fun isNotUuid(): Boolean = try { UUID.fromString(id) diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentErrorCode.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentErrorCode.kt index 73e7aac571..58bca21841 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentErrorCode.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentErrorCode.kt @@ -5,7 +5,6 @@ package io.embrace.android.embracesdk.internal.logs.attachments */ enum class AttachmentErrorCode { ATTACHMENT_TOO_LARGE, - UNSUCCESSFUL_UPLOAD, OVER_MAX_ATTACHMENTS, UNKNOWN } diff --git a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentService.kt b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentService.kt index 6759367b84..249dde6e91 100644 --- a/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentService.kt +++ b/embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentService.kt @@ -16,9 +16,7 @@ class AttachmentService(private val limit: Int = 5) : MemoryCleanerListener { fun createAttachment( attachmentId: String, attachmentUrl: String, - attachmentSize: Long, ): UserHosted = UserHosted( - attachmentSize, attachmentId, attachmentUrl, ::incrementAndCheckAttachmentLimit diff --git a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentTest.kt b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentTest.kt index b5fc4d9dc8..b596adc767 100644 --- a/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentTest.kt +++ b/embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/logs/attachments/AttachmentTest.kt @@ -73,57 +73,34 @@ internal class AttachmentTest { @Test fun `create user hosted attachment`() { - val attachment = UserHosted(SIZE, ID, URL, counter) + val attachment = UserHosted(ID, URL, counter) attachment.assertUserHostedAttributesMatch() } - @Test - fun `user hosted attachment empty size`() { - val size: Long = 0 - val attachment = UserHosted(size, ID, URL, counter) - attachment.assertUserHostedAttributesMatch(size = size) - } - - @Test - fun `user hosted attachment invalid size`() { - val size: Long = -1 - val attachment = UserHosted(size, ID, URL, counter) - attachment.assertUserHostedAttributesMatch(size = size, errorCode = UNKNOWN) - } - @Test fun `user hosted attachment invalid url`() { val url = "" - val attachment = UserHosted(SIZE, ID, url, counter) + val attachment = UserHosted(ID, url, counter) attachment.assertUserHostedAttributesMatch(url = url, errorCode = UNKNOWN) } @Test fun `user hosted attachment invalid ID`() { val id = "my-id" - val attachment = UserHosted(SIZE, id, URL, counter) + val attachment = UserHosted(id, URL, counter) attachment.assertUserHostedAttributesMatch(id = id, errorCode = UNKNOWN) } - @Test - fun `user hosted attachment has no max size constraints`() { - val size = 5000000L // 50MiB - val attachment = UserHosted(size, ID, URL, counter) - attachment.assertUserHostedAttributesMatch(size = size) - } - @Test fun `user hosted attachment exceeds session limit`() { var limit = true val smallCounter: () -> Boolean = { limit } - val attachment = UserHosted(SIZE, ID, URL, smallCounter) + val attachment = UserHosted(ID, URL, smallCounter) attachment.assertUserHostedAttributesMatch() - val size = -1L limit = false - val limitedAttachment = UserHosted(size, ID, URL, smallCounter) + val limitedAttachment = UserHosted(ID, URL, smallCounter) limitedAttachment.assertUserHostedAttributesMatch( - size = size, errorCode = OVER_MAX_ATTACHMENTS ) } @@ -140,12 +117,10 @@ internal class AttachmentTest { } private fun UserHosted.assertUserHostedAttributesMatch( - size: Long = SIZE, url: String = URL, id: String = ID, errorCode: AttachmentErrorCode? = null, ) { - assertEquals(size, checkNotNull(attributes[embAttachmentSize]).toLong()) assertEquals(id, checkNotNull(attributes[embAttachmentId])) assertEquals(errorCode?.toString(), attributes[embAttachmentErrorCode]) assertEquals(url, checkNotNull(attributes[embAttachmentUrl])) diff --git a/embrace-android-sdk/api/embrace-android-sdk.api b/embrace-android-sdk/api/embrace-android-sdk.api index 5fe6c8c8be..eef1dbf903 100644 --- a/embrace-android-sdk/api/embrace-android-sdk.api +++ b/embrace-android-sdk/api/embrace-android-sdk.api @@ -40,7 +40,7 @@ public final class io/embrace/android/embracesdk/Embrace : io/embrace/android/em public fun logInfo (Ljava/lang/String;)V public fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;)V public fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;)V - public fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;J)V + public fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;Ljava/lang/String;Ljava/lang/String;)V public fun logMessage (Ljava/lang/String;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;[B)V public fun logPushNotification (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/Integer;Ljava/lang/Boolean;Ljava/lang/Boolean;)V public fun logWarning (Ljava/lang/String;)V diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt index 5ce25b9055..52c52b6239 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt +++ b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt @@ -35,35 +35,33 @@ internal class FileAttachmentFeatureTest { @Test fun `log message with user hosted file attachment`() { - val size = 509L val url = "https://example.com/my-file.txt" val id = attachmentId.toString() testRule.runTest(testCaseAction = { recordSession { - logWithUserHostedAttachment(id, url, size) + logWithUserHostedAttachment(id, url) } }, assertAction = { val log = getSingleLogEnvelope().getLastLog() - assertUserHostedLogSent(log, size, url, id) + assertUserHostedLogSent(log, url, id) assertNull(log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE)) }) } @Test fun `user hosted file attachment exceeding limit`() { - val size = 509L val url = "https://example.com/my-file.txt" val id = attachmentId.toString() val limit = 5 testRule.runTest(testCaseAction = { recordSession { repeat(limit + 1) { - logWithUserHostedAttachment(id, url, size) + logWithUserHostedAttachment(id, url) } } recordSession { repeat(limit + 1) { - logWithUserHostedAttachment(id, url, size) + logWithUserHostedAttachment(id, url) } } }, assertAction = { @@ -71,7 +69,7 @@ internal class FileAttachmentFeatureTest { assertEquals((limit * 2) + 2, logs.size) logs.forEachIndexed { k, log -> - assertUserHostedLogSent(log, size, url, id) + assertUserHostedLogSent(log, url, id) val errCode = log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE) val expectedCode = when (k) { @@ -107,7 +105,7 @@ internal class FileAttachmentFeatureTest { private fun verifyAttachment( parts: List, byteArray: ByteArray, - appId: String = "abcde" + appId: String = "abcde", ) { assertEquals("form-data; name=\"app_id\"", parts[0].contentDisposition) assertEquals(appId, parts[0].data) @@ -121,7 +119,6 @@ internal class FileAttachmentFeatureTest { private fun assertUserHostedLogSent( log: Log, - size: Long, url: String, id: String, ) { @@ -134,7 +131,6 @@ internal class FileAttachmentFeatureTest { expectedTimeMs = null, expectedProperties = mapOf( "key" to "value", - ATTR_KEY_SIZE to size.toString(), ATTR_KEY_URL to url, ATTR_KEY_ID to id, ), @@ -164,7 +160,6 @@ internal class FileAttachmentFeatureTest { private fun EmbraceActionInterface.logWithUserHostedAttachment( id: String, url: String, - size: Long, ) { embrace.logMessage( message = "test message", @@ -172,7 +167,6 @@ internal class FileAttachmentFeatureTest { properties = mapOf("key" to "value"), attachmentId = id, attachmentUrl = url, - attachmentSize = size, ) } } diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/Embrace.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/Embrace.kt index c323038157..4e2a8db912 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/Embrace.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/Embrace.kt @@ -154,9 +154,8 @@ public class Embrace private constructor( properties: Map?, attachmentId: String, attachmentUrl: String, - attachmentSize: Long, ) { - impl.logMessage(message, severity, properties, attachmentId, attachmentUrl, attachmentSize) + impl.logMessage(message, severity, properties, attachmentId, attachmentUrl) } override fun logException(throwable: Throwable) { 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 26a6c91ab9..fd6bb20116 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 @@ -338,7 +338,6 @@ internal class EmbraceImpl @JvmOverloads constructor( properties: Map?, attachmentId: String, attachmentUrl: String, - attachmentSize: Long, ) { logsApiDelegate.logMessage( message, @@ -346,7 +345,6 @@ internal class EmbraceImpl @JvmOverloads constructor( properties, attachmentId, attachmentUrl, - attachmentSize ) } 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 c4b821b701..97b8841c4d 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 @@ -105,9 +105,8 @@ internal class LogsApiDelegate( properties: Map?, attachmentId: String, attachmentUrl: String, - attachmentSize: Long, ) { - val obj = attachmentService?.createAttachment(attachmentId, attachmentUrl, attachmentSize) ?: return + val obj = attachmentService?.createAttachment(attachmentId, attachmentUrl) ?: return logMessageImpl( severity = severity, message = message, From 606090a98ff2167827324b6318e4ca298dda6fe9 Mon Sep 17 00:00:00 2001 From: Jamie Lynch Date: Thu, 16 Jan 2025 10:45:10 +0000 Subject: [PATCH 3/3] test: add integration tests for file attachments --- .../features/FileAttachmentFeatureTest.kt | 138 ++++++++++++++++-- 1 file changed, 122 insertions(+), 16 deletions(-) diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt index 52c52b6239..6138607f2a 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt +++ b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/testcases/features/FileAttachmentFeatureTest.kt @@ -13,6 +13,7 @@ import io.embrace.android.embracesdk.testframework.server.FormPart import java.util.UUID import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -49,19 +50,16 @@ internal class FileAttachmentFeatureTest { } @Test - fun `user hosted file attachment exceeding limit`() { - val url = "https://example.com/my-file.txt" + fun `user hosted file attachment exceeding session limit`() { + val url = "https://example.com/another-file.txt" val id = attachmentId.toString() val limit = 5 testRule.runTest(testCaseAction = { - recordSession { - repeat(limit + 1) { - logWithUserHostedAttachment(id, url) - } - } - recordSession { - repeat(limit + 1) { - logWithUserHostedAttachment(id, url) + repeat(2) { + recordSession { + repeat(limit + 1) { + logWithUserHostedAttachment(id, url) + } } } }, assertAction = { @@ -86,12 +84,7 @@ internal class FileAttachmentFeatureTest { val byteArray = "Hello, world!".toByteArray() testRule.runTest(testCaseAction = { recordSession { - embrace.logMessage( - message = "test message", - severity = Severity.INFO, - properties = mapOf("key" to "value"), - attachment = byteArray - ) + logWithEmbraceHostedAttachment(byteArray) } }, assertAction = { val parts = getSingleAttachment() @@ -102,6 +95,108 @@ internal class FileAttachmentFeatureTest { }) } + @Test + fun `embrace hosted file attachment exceeding session limit`() { + val byteArray = ByteArray(1024 * 16) + val limit = 5 + testRule.runTest(testCaseAction = { + repeat(2) { + recordSession { + repeat(limit + 1) { + logWithEmbraceHostedAttachment(byteArray) + } + } + } + }, assertAction = { + val logs = getLogEnvelopes(2).flatMap { checkNotNull(it.data.logs) } + val logSize = (limit * 2) + 2 + assertEquals(logSize, logs.size) + + logs.forEachIndexed { k, log -> + assertEmbraceHostedLogSent(log, byteArray.size.toLong()) + + val errCode = log.attributes?.findAttributeValue(ATTR_KEY_ERR_CODE) + val expectedCode = when (k) { + limit, limit + limit + 1 -> "OVER_MAX_ATTACHMENTS" + else -> null + } + assertEquals(expectedCode, errCode) + } + + val attachments = getAttachments(10) + attachments.forEach { parts -> + verifyAttachment(parts, byteArray) + } + }) + } + + @Test + fun `embrace hosted file attachment equalling size limit`() { + val byteArray = ByteArray(1024 * 1024) + testRule.runTest(testCaseAction = { + recordSession { + logWithEmbraceHostedAttachment(byteArray) + } + }, assertAction = { + val parts = getSingleAttachment() + verifyAttachment(parts, byteArray) + + val log = getSingleLogEnvelope().getLastLog() + assertEmbraceHostedLogSent(log, byteArray.size.toLong()) + }) + } + + @Test + fun `embrace hosted file attachment exceeding size limit`() { + val byteArray = ByteArray(2 * 1024 * 1024) + testRule.runTest(testCaseAction = { + recordSession { + logWithEmbraceHostedAttachment(byteArray) + } + }, assertAction = { + val log = getSingleLogEnvelope().getLastLog() + assertEmbraceHostedLogSent(log, byteArray.size.toLong()) + assertTrue(getAttachments(0).isEmpty()) + }) + } + + @Test + fun `embrace multiple file attachments`() { + val a = "apple".toByteArray() + val b = "banana".toByteArray() + val c = "cherry".toByteArray() + + testRule.runTest(testCaseAction = { + recordSession { + logWithEmbraceHostedAttachment(a) + logWithEmbraceHostedAttachment(b) + logWithEmbraceHostedAttachment(c) + } + }, assertAction = { + val expectedSize = 3 + val logs = getLogEnvelopes(1).flatMap { checkNotNull(it.data.logs) } + assertEquals(expectedSize, logs.size) + + val attachments = getAttachments(expectedSize) + assertEquals(expectedSize, attachments.size) + + val aId = logs[0].attributes?.findAttributeValue(ATTR_KEY_ID) + val bId = logs[1].attributes?.findAttributeValue(ATTR_KEY_ID) + val cId = logs[2].attributes?.findAttributeValue(ATTR_KEY_ID) + + val attachmentA = attachments.single { it[1].data == aId } + val attachmentB = attachments.single { it[1].data == bId } + val attachmentC = attachments.single { it[1].data == cId } + + verifyAttachment(attachmentA, a) + verifyAttachment(attachmentB, b) + verifyAttachment(attachmentC, c) + + val uniqueIds = logs.distinctBy { it.attributes?.findAttributeValue(ATTR_KEY_ID) } + assertEquals(expectedSize, uniqueIds.size) + }) + } + private fun verifyAttachment( parts: List, byteArray: ByteArray, @@ -169,4 +264,15 @@ internal class FileAttachmentFeatureTest { attachmentUrl = url, ) } + + private fun EmbraceActionInterface.logWithEmbraceHostedAttachment( + byteArray: ByteArray, + ) { + embrace.logMessage( + message = "test message", + severity = Severity.INFO, + properties = mapOf("key" to "value"), + attachment = byteArray + ) + } }