diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt index 130cbbb9ab..e8ea18f2b8 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt @@ -20,6 +20,7 @@ import com.datadog.android.BuildConfig import com.datadog.android.Datadog import com.datadog.android.DatadogSite import com.datadog.android.api.InternalLogger +import com.datadog.android.api.feature.Feature import com.datadog.android.api.storage.RawBatchEvent import com.datadog.android.core.configuration.BackPressureStrategy import com.datadog.android.core.configuration.BatchProcessingLevel @@ -234,7 +235,8 @@ internal class CoreFeature( private val lastViewEventFileWriter: FileWriter by lazy { BatchFileReaderWriter.create( internalLogger = internalLogger, - encryption = localDataEncryption + encryption = localDataEncryption, + featureName = Feature.RUM_FEATURE_NAME ) } @@ -436,7 +438,7 @@ internal class CoreFeature( if (lastViewEventFile == null) return null val reader = - BatchFileReaderWriter.create(internalLogger, localDataEncryption) + BatchFileReaderWriter.create(internalLogger, localDataEncryption, Feature.RUM_FEATURE_NAME) val content = reader.readData(lastViewEventFile) return if (content.isEmpty()) { null diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt index bee08bf1b5..35a6dd48e6 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt @@ -377,7 +377,8 @@ internal class SdkFeature( pendingOrchestrator = fileOrchestrator.pendingOrchestrator, batchEventsReaderWriter = BatchFileReaderWriter.create( internalLogger = internalLogger, - encryption = coreFeature.localDataEncryption + encryption = coreFeature.localDataEncryption, + featureName = featureName ), batchMetadataReaderWriter = FileReaderWriter.create( internalLogger = internalLogger, @@ -454,7 +455,7 @@ internal class SdkFeature( val flusher = DataFlusher( contextProvider, fileOrchestrator, - BatchFileReaderWriter.create(internalLogger, coreFeature.localDataEncryption), + BatchFileReaderWriter.create(internalLogger, coreFeature.localDataEncryption, wrappedFeature.name), FileReaderWriter.create(internalLogger, coreFeature.localDataEncryption), FileMover(internalLogger), internalLogger diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt index 22c30bdba5..1d6683bdf1 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt @@ -60,6 +60,7 @@ internal class ConsentAwareStorage( return AsyncEventWriteScope(executorService, NoOpEventBatchWriter(), writeLock, featureName, internalLogger) } val writer = FileEventBatchWriter( + featureName = featureName, fileOrchestrator = orchestrator, eventsWriter = batchEventsReaderWriter, metadataReaderWriter = batchMetadataReaderWriter, diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt index 7abbc1eeb3..ece27f1a8e 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt @@ -16,10 +16,12 @@ import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig import com.datadog.android.core.internal.persistence.file.FileReaderWriter import com.datadog.android.core.internal.persistence.file.FileWriter import com.datadog.android.core.internal.persistence.file.existsSafe +import com.datadog.android.internal.telemetry.TelemetryAttributes import java.io.File import java.util.Locale internal class FileEventBatchWriter( + private val featureName: String, private val fileOrchestrator: FileOrchestrator, private val eventsWriter: FileWriter, private val metadataReaderWriter: FileReaderWriter, @@ -88,14 +90,18 @@ internal class FileEventBatchWriter( if (eventSize > filePersistenceConfig.maxItemSize) { internalLogger.log( InternalLogger.Level.ERROR, - InternalLogger.Target.USER, - { + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + messageBuilder = { ERROR_LARGE_DATA.format( Locale.US, eventSize, filePersistenceConfig.maxItemSize ) - } + }, + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to featureName, + TelemetryAttributes.EVENT_DROPPED_BYTES to eventSize + ) ) return false } diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriter.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriter.kt index 85533e1b17..3b02601820 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriter.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriter.kt @@ -18,8 +18,12 @@ internal interface BatchFileReaderWriter : FileWriter, BatchFileR * Creates either plain [PlainBatchFileReaderWriter] or [PlainBatchFileReaderWriter] wrapped in * [EncryptedBatchReaderWriter] if encryption is provided. */ - fun create(internalLogger: InternalLogger, encryption: Encryption?): BatchFileReaderWriter { - val readerWriter = PlainBatchFileReaderWriter(internalLogger) + fun create( + internalLogger: InternalLogger, + encryption: Encryption?, + featureName: String + ): BatchFileReaderWriter { + val readerWriter = PlainBatchFileReaderWriter(internalLogger, featureName) return if (encryption == null) { readerWriter } else { diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriter.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriter.kt index 83bab04dd5..98b3423cd1 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriter.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriter.kt @@ -11,6 +11,7 @@ import com.datadog.android.api.InternalLogger import com.datadog.android.api.storage.RawBatchEvent import com.datadog.android.core.internal.persistence.file.lengthSafe import com.datadog.android.core.internal.utils.use +import com.datadog.android.internal.telemetry.TelemetryAttributes import java.io.File import java.io.IOException import java.io.InputStream @@ -23,7 +24,8 @@ import kotlin.math.max * Stores data in the TLV format as meta+data, use only for RUM/Log/Trace events. */ internal class PlainBatchFileReaderWriter( - private val internalLogger: InternalLogger + private val internalLogger: InternalLogger, + private val featureName: String ) : BatchFileReaderWriter { // region FileWriter @@ -68,18 +70,20 @@ internal class PlainBatchFileReaderWriter( readFileData(file) } catch (e: IOException) { internalLogger.log( - InternalLogger.Level.ERROR, - listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), - { ERROR_READ.format(Locale.US, file.path) }, - e + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), + messageBuilder = { ERROR_READ.format(Locale.US, file.path) }, + throwable = e, + additionalProperties = mapOf(TelemetryAttributes.FEATURE_NAME to featureName) ) emptyList() } catch (e: SecurityException) { internalLogger.log( - InternalLogger.Level.ERROR, - listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), - { ERROR_READ.format(Locale.US, file.path) }, - e + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), + messageBuilder = { ERROR_READ.format(Locale.US, file.path) }, + throwable = e, + additionalProperties = mapOf(TelemetryAttributes.FEATURE_NAME to featureName) ) emptyList() } @@ -173,9 +177,10 @@ internal class PlainBatchFileReaderWriter( if (remaining != 0 || (inputLength > 0 && result.isEmpty())) { internalLogger.log( - InternalLogger.Level.ERROR, - listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), - { WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) } + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + messageBuilder = { WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) }, + additionalProperties = mapOf(TelemetryAttributes.FEATURE_NAME to featureName) ) } diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt index d79bf7cb1c..87eb853224 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt @@ -45,15 +45,15 @@ internal class TLVBlock( private fun logEntrySizeExceededError(entrySize: Int, maxEntrySize: Int) { internalLogger.log( - target = InternalLogger.Target.MAINTAINER, - level = InternalLogger.Level.WARN, + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), messageBuilder = { BYTE_LENGTH_EXCEEDED_ERROR.format(Locale.US, maxEntrySize, entrySize) } ) } internal companion object { // The maximum length of data (Value) in TLV block defining key data. - private const val MAXIMUM_DATA_SIZE_MB = 10 * 1024 * 1024 // 10 mb + internal const val MAXIMUM_DATA_SIZE_MB = 10 * 1024 * 1024 // 10 mb internal const val BYTE_LENGTH_EXCEEDED_ERROR = "DataBlock length exceeds limit of %s bytes, was %s" } diff --git a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt index a5f8db3683..2a7491b611 100644 --- a/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt +++ b/dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt @@ -17,7 +17,8 @@ import java.util.Locale internal class TLVBlockFileReader( val internalLogger: InternalLogger, - val fileReaderWriter: FileReaderWriter + val fileReaderWriter: FileReaderWriter, + private val maxEntrySize: Int = TLVBlock.MAXIMUM_DATA_SIZE_MB ) { @WorkerThread internal fun read( @@ -76,6 +77,7 @@ internal class TLVBlockFileReader( ) } + @Suppress("ReturnCount") private fun readData(inputArray: ByteArray, currentIndex: Int): TLVResult? { val lengthBlockSize = Int.SIZE_BYTES var newIndex = currentIndex + lengthBlockSize @@ -89,6 +91,11 @@ internal class TLVBlockFileReader( val lengthData = lengthInBytes.toInt() + if (lengthData !in 0..maxEntrySize) { + logBlockSizeExceededError(lengthData) + return null + } + val dataBytes = inputArray.copyOfRangeSafe(newIndex, newIndex + lengthData) @@ -113,6 +120,16 @@ internal class TLVBlockFileReader( ) } + private fun logBlockSizeExceededError(lengthData: Int) { + internalLogger.log( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + messageBuilder = { + CORRUPT_DATA_LENGTH_ERROR.format(Locale.US, maxEntrySize, lengthData) + } + ) + } + private fun logFailedToDeserializeError() { internalLogger.log( target = InternalLogger.Target.MAINTAINER, @@ -129,5 +146,10 @@ internal class TLVBlockFileReader( internal companion object { internal const val CORRUPT_TLV_HEADER_TYPE_ERROR = "TLV header corrupt. Invalid type %s" internal const val FAILED_TO_DESERIALIZE_ERROR = "Failed to deserialize TLV data length" + + // Read-side counterpart of TLVBlock.BYTE_LENGTH_EXCEEDED_ERROR (write/serialize side). + // Kept distinct so dropped blocks can be told apart by direction in telemetry. + internal const val CORRUPT_DATA_LENGTH_ERROR = + "DataBlock length read from file is invalid or exceeds limit of %s bytes, was %s" } } diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt index 8e11b73977..95405ce3e0 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt @@ -16,6 +16,7 @@ import android.net.ConnectivityManager import android.os.Process import com.datadog.android.Datadog import com.datadog.android.api.InternalLogger +import com.datadog.android.api.feature.Feature import com.datadog.android.api.storage.RawBatchEvent import com.datadog.android.core.configuration.Configuration import com.datadog.android.core.internal.net.info.BroadcastReceiverNetworkInfoProvider @@ -1285,7 +1286,7 @@ internal class CoreFeatureTest { legacyNdkViewEventFile.parentFile?.mkdirs() BatchFileReaderWriter - .create(internalLogger = mock(), encryption = null) + .create(internalLogger = mock(), encryption = null, featureName = Feature.RUM_FEATURE_NAME) .writeData( legacyNdkViewEventFile, RawBatchEvent(fakeViewEvent.toString().toByteArray()), diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriterTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriterTest.kt index 88b81c3459..79adbc7986 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriterTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriterTest.kt @@ -17,6 +17,7 @@ import com.datadog.android.core.internal.persistence.file.FileOrchestrator import com.datadog.android.core.internal.persistence.file.FilePersistenceConfig import com.datadog.android.core.internal.persistence.file.FileReaderWriter import com.datadog.android.core.internal.persistence.file.FileWriter +import com.datadog.android.internal.telemetry.TelemetryAttributes import com.datadog.android.utils.forge.Configurator import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.Forge @@ -80,9 +81,13 @@ internal class FileEventBatchWriterTest { @Forgery lateinit var fakeEventType: EventType + @StringForgery + lateinit var fakeFeatureName: String + @BeforeEach fun `set up`() { testedWriter = FileEventBatchWriter( + featureName = fakeFeatureName, fileOrchestrator = mockFileOrchestrator, eventsWriter = mockBatchWriter, metadataReaderWriter = mockMetaReaderWriter, @@ -192,11 +197,15 @@ internal class FileEventBatchWriterTest { mockInternalLogger.verifyLog( InternalLogger.Level.ERROR, - InternalLogger.Target.USER, + listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), ERROR_LARGE_DATA.format( Locale.US, batchEvent.data.size, maxItemSize + ), + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to fakeFeatureName, + TelemetryAttributes.EVENT_DROPPED_BYTES to batchEvent.data.size ) ) } diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriterTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriterTest.kt index 7007439aad..5bdbaf0c17 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriterTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriterTest.kt @@ -9,6 +9,7 @@ package com.datadog.android.core.internal.persistence.file.batch import com.datadog.android.api.InternalLogger import com.datadog.android.security.Encryption import com.datadog.android.utils.forge.Configurator +import fr.xgouchet.elmyr.annotation.StringForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension import org.assertj.core.api.Assertions.assertThat @@ -32,10 +33,13 @@ internal class BatchFileReaderWriterTest { @Mock lateinit var mockInternalLogger: InternalLogger + @StringForgery + lateinit var fakeFeatureName: String + @Test fun `M create BatchFileReaderWriter W create() { without encryption }`() { // When - val readerWriter = BatchFileReaderWriter.create(mockInternalLogger, null) + val readerWriter = BatchFileReaderWriter.create(mockInternalLogger, null, fakeFeatureName) // Then assertThat(readerWriter) .isInstanceOf(PlainBatchFileReaderWriter::class.java) @@ -47,7 +51,8 @@ internal class BatchFileReaderWriterTest { val mockEncryption = mock() val readerWriter = BatchFileReaderWriter.create( mockInternalLogger, - mockEncryption + mockEncryption, + fakeFeatureName ) // Then diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriterTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriterTest.kt index 02de90f68e..de84fa9463 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriterTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriterTest.kt @@ -8,6 +8,7 @@ package com.datadog.android.core.internal.persistence.file.batch import com.datadog.android.api.InternalLogger import com.datadog.android.api.storage.RawBatchEvent +import com.datadog.android.internal.telemetry.TelemetryAttributes import com.datadog.android.utils.forge.Configurator import com.datadog.android.utils.verifyLog import fr.xgouchet.elmyr.Forge @@ -67,6 +68,9 @@ internal class PlainBatchFileReaderWriterTest { @Mock lateinit var mockInternalLogger: InternalLogger + @StringForgery + lateinit var fakeFeatureName: String + private lateinit var fakeSrcDir: File private lateinit var fakeDstDir: File @@ -74,7 +78,7 @@ internal class PlainBatchFileReaderWriterTest { fun `set up`() { fakeSrcDir = File(fakeRootDirectory, fakeSrcDirName) fakeDstDir = File(fakeRootDirectory, fakeDstDirName) - testedReaderWriter = PlainBatchFileReaderWriter(mockInternalLogger) + testedReaderWriter = PlainBatchFileReaderWriter(mockInternalLogger, fakeFeatureName) } // region writeData @@ -285,7 +289,10 @@ internal class PlainBatchFileReaderWriterTest { InternalLogger.Level.ERROR, listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), PlainBatchFileReaderWriter.ERROR_READ.format(Locale.US, file.path), - FileNotFoundException::class.java + FileNotFoundException::class.java, + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to fakeFeatureName + ) ) } @@ -306,7 +313,10 @@ internal class PlainBatchFileReaderWriterTest { InternalLogger.Level.ERROR, listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), PlainBatchFileReaderWriter.ERROR_READ.format(Locale.US, file.path), - FileNotFoundException::class.java + FileNotFoundException::class.java, + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to fakeFeatureName + ) ) } @@ -327,7 +337,10 @@ internal class PlainBatchFileReaderWriterTest { mockInternalLogger.verifyLog( InternalLogger.Level.ERROR, listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), - PlainBatchFileReaderWriter.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) + PlainBatchFileReaderWriter.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path), + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to fakeFeatureName + ) ) } @@ -411,7 +424,10 @@ internal class PlainBatchFileReaderWriterTest { mockInternalLogger.verifyLog( InternalLogger.Level.ERROR, listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), - PlainBatchFileReaderWriter.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path) + PlainBatchFileReaderWriter.WARNING_NOT_ALL_DATA_READ.format(Locale.US, file.path), + additionalProperties = mapOf( + TelemetryAttributes.FEATURE_NAME to fakeFeatureName + ) ) } diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt index eb72af2bba..407f4b50c1 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt @@ -31,6 +31,7 @@ import org.mockito.kotlin.whenever import org.mockito.quality.Strictness import java.io.File import java.nio.ByteBuffer +import java.util.Locale @Extensions( ExtendWith(MockitoExtension::class), @@ -167,6 +168,66 @@ internal class TLVBlockFileReaderTest { ) } + @Test + fun `M return empty array and warn W read() { declared block length exceeds limit }`( + @IntForgery(min = 1, max = 8) fakeMaxEntrySize: Int, + @IntForgery(min = 9) fakeDeclaredLength: Int + ) { + // Given + val fakeBlock = ByteBuffer.allocate(Short.SIZE_BYTES + Int.SIZE_BYTES) + .putShort(TLVBlockType.DATA.rawValue.toShort()) + .putInt(fakeDeclaredLength) + .array() + whenever(mockFileReaderWriter.readData(mockFile)).thenReturn(fakeBlock) + testedReader = TLVBlockFileReader( + fileReaderWriter = mockFileReaderWriter, + internalLogger = mockInternalLogger, + maxEntrySize = fakeMaxEntrySize + ) + + // When + val result = testedReader.read(mockFile) + + // Then + assertThat(result).isEmpty() + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = TLVBlockFileReader.CORRUPT_DATA_LENGTH_ERROR + .format(Locale.US, fakeMaxEntrySize, fakeDeclaredLength) + ) + } + + @Test + fun `M return empty array and warn W read() { declared block length is negative }`( + @IntForgery(min = 1) fakeMaxEntrySize: Int, + @IntForgery(max = 0) fakeNegativeLength: Int + ) { + // Given + val fakeBlock = ByteBuffer.allocate(Short.SIZE_BYTES + Int.SIZE_BYTES) + .putShort(TLVBlockType.DATA.rawValue.toShort()) + .putInt(fakeNegativeLength) + .array() + whenever(mockFileReaderWriter.readData(mockFile)).thenReturn(fakeBlock) + testedReader = TLVBlockFileReader( + fileReaderWriter = mockFileReaderWriter, + internalLogger = mockInternalLogger, + maxEntrySize = fakeMaxEntrySize + ) + + // When + val result = testedReader.read(mockFile) + + // Then + assertThat(result).isEmpty() + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = TLVBlockFileReader.CORRUPT_DATA_LENGTH_ERROR + .format(Locale.US, fakeMaxEntrySize, fakeNegativeLength) + ) + } + private fun createVersionBytes(fakeVersion: Int): ByteArray { val versionType = TLVBlockType.VERSION_CODE.rawValue.toShort() fakeVersionBytes = ByteBuffer.allocate(Int.SIZE_BYTES).putInt(fakeVersion).array() diff --git a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt index a174e71bb1..4914cbd932 100644 --- a/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt +++ b/dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt @@ -112,8 +112,8 @@ internal class TLVBlockTest { // Then val stringCaptor = argumentCaptor<() -> String>() verify(mockInternalLogger).log( - level = eq(InternalLogger.Level.WARN), - target = eq(InternalLogger.Target.MAINTAINER), + level = eq(InternalLogger.Level.ERROR), + targets = eq(listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY)), stringCaptor.capture(), anyOrNull(), anyOrNull(), diff --git a/dd-sdk-android-internal/api/apiSurface b/dd-sdk-android-internal/api/apiSurface index b5e5aa80c3..6a31f914e7 100644 --- a/dd-sdk-android-internal/api/apiSurface +++ b/dd-sdk-android-internal/api/apiSurface @@ -231,6 +231,9 @@ sealed class com.datadog.android.internal.telemetry.InternalTelemetryEvent enum Mode - DEFAULT_HEADERS - CUSTOM +object com.datadog.android.internal.telemetry.TelemetryAttributes + const val FEATURE_NAME: String + const val EVENT_DROPPED_BYTES: String enum com.datadog.android.internal.telemetry.TracingHeaderType - DATADOG - B3 diff --git a/dd-sdk-android-internal/api/dd-sdk-android-internal.api b/dd-sdk-android-internal/api/dd-sdk-android-internal.api index e254577a65..722d942a24 100644 --- a/dd-sdk-android-internal/api/dd-sdk-android-internal.api +++ b/dd-sdk-android-internal/api/dd-sdk-android-internal.api @@ -520,6 +520,12 @@ public final class com/datadog/android/internal/telemetry/InternalTelemetryEvent public static fun values ()[Lcom/datadog/android/internal/telemetry/InternalTelemetryEvent$ResourceHeadersTrackingConfigured$Mode; } +public final class com/datadog/android/internal/telemetry/TelemetryAttributes { + public static final field EVENT_DROPPED_BYTES Ljava/lang/String; + public static final field FEATURE_NAME Ljava/lang/String; + public static final field INSTANCE Lcom/datadog/android/internal/telemetry/TelemetryAttributes; +} + public final class com/datadog/android/internal/telemetry/TracingHeaderType : java/lang/Enum { public static final field B3 Lcom/datadog/android/internal/telemetry/TracingHeaderType; public static final field B3MULTI Lcom/datadog/android/internal/telemetry/TracingHeaderType; diff --git a/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/telemetry/TelemetryAttributes.kt b/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/telemetry/TelemetryAttributes.kt new file mode 100644 index 0000000000..9060d43141 --- /dev/null +++ b/dd-sdk-android-internal/src/main/java/com/datadog/android/internal/telemetry/TelemetryAttributes.kt @@ -0,0 +1,24 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ +package com.datadog.android.internal.telemetry + +/** + * Attribute keys attached to internal telemetry error events (as `additionalProperties`) so that + * dropped-event diagnostics stay consistent and queryable across features and across the read/write + * persistence paths. + */ +object TelemetryAttributes { + + /** + * Name of the feature ("rum", "logs", "tracing", ...) whose event was affected. + */ + const val FEATURE_NAME: String = "feature.name" + + /** + * Serialized size, in bytes, of an event that was dropped because it exceeded the size limit. + */ + const val EVENT_DROPPED_BYTES: String = "event.dropped_bytes" +} diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt index 09c34bf1fb..e2967b67f4 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt @@ -409,7 +409,8 @@ internal class RumFeature( RumEventSerializer(sdkCore.internalLogger) ), eventMetaSerializer = RumEventMetaSerializer(), - sdkCore = sdkCore + sdkCore = sdkCore, + maxItemSize = storageConfiguration.maxItemSize ) } diff --git a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt index 1736fb03b9..e022589bc7 100644 --- a/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt +++ b/features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt @@ -7,6 +7,7 @@ package com.datadog.android.rum.internal.domain import androidx.annotation.WorkerThread +import com.datadog.android.api.InternalLogger import com.datadog.android.api.storage.DataWriter import com.datadog.android.api.storage.EventBatchWriter import com.datadog.android.api.storage.EventType @@ -14,17 +15,27 @@ import com.datadog.android.api.storage.RawBatchEvent import com.datadog.android.core.InternalSdkCore import com.datadog.android.core.persistence.Serializer import com.datadog.android.core.persistence.serializeToByteArray +import com.datadog.android.internal.telemetry.TelemetryAttributes import com.datadog.android.rum.internal.domain.event.RumEventMeta +import com.datadog.android.rum.model.ActionEvent +import com.datadog.android.rum.model.ErrorEvent +import com.datadog.android.rum.model.LongTaskEvent +import com.datadog.android.rum.model.ResourceEvent import com.datadog.android.rum.model.ViewEvent +import com.datadog.android.rum.model.VitalAppLaunchEvent +import com.datadog.android.rum.model.VitalOperationStepEvent +import java.util.Locale internal class RumDataWriter( internal val eventSerializer: Serializer, private val eventMetaSerializer: Serializer, - private val sdkCore: InternalSdkCore + private val sdkCore: InternalSdkCore, + private val maxItemSize: Long ) : DataWriter { // region DataWriter + @Suppress("ReturnCount") @WorkerThread override fun write(writer: EventBatchWriter, element: Any, eventType: EventType): Boolean { val byteArray = eventSerializer.serializeToByteArray( @@ -32,6 +43,31 @@ internal class RumDataWriter( sdkCore.internalLogger ) ?: return false + if (byteArray.size > maxItemSize) { + val rumEventType = when (element) { + is ViewEvent -> element.type + is ActionEvent -> element.type + is ErrorEvent -> element.type + is ResourceEvent -> element.type + is LongTaskEvent -> element.type + is VitalOperationStepEvent -> element.type + is VitalAppLaunchEvent -> element.type + else -> "unknown" + } + sdkCore.internalLogger.log( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + messageBuilder = { + ERROR_LARGE_EVENT.format(Locale.US, rumEventType, byteArray.size, maxItemSize) + }, + additionalProperties = mapOf( + RUM_EVENT_TYPE to rumEventType, + TelemetryAttributes.EVENT_DROPPED_BYTES to byteArray.size + ) + ) + return false + } + val batchEvent = if (element is ViewEvent) { val hasAccessibility = element.view.accessibility != null @@ -75,5 +111,11 @@ internal class RumDataWriter( companion object { val EMPTY_BYTE_ARRAY = ByteArray(0) + internal const val ERROR_LARGE_EVENT = + "RUM event of type [%s] was dropped, serialized size %d bytes exceeds limit of %d bytes" + + // RUM-specific telemetry attribute: the RUM event type ("view", "action", ...) of a + // dropped event. Kept in the RUM module since it is meaningful only for RUM. + internal const val RUM_EVENT_TYPE = "rum.event.type" } } diff --git a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt index 16ef328b87..f7aa2fb654 100644 --- a/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt +++ b/features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt @@ -11,14 +11,20 @@ import com.datadog.android.api.storage.EventBatchWriter import com.datadog.android.api.storage.EventType import com.datadog.android.api.storage.RawBatchEvent import com.datadog.android.core.persistence.Serializer +import com.datadog.android.internal.telemetry.TelemetryAttributes +import com.datadog.android.rum.internal.domain.RumDataWriter.Companion.ERROR_LARGE_EVENT +import com.datadog.android.rum.internal.domain.RumDataWriter.Companion.RUM_EVENT_TYPE import com.datadog.android.rum.internal.domain.event.RumEventMeta import com.datadog.android.rum.model.ActionEvent import com.datadog.android.rum.model.ErrorEvent import com.datadog.android.rum.model.LongTaskEvent import com.datadog.android.rum.model.ResourceEvent import com.datadog.android.rum.model.ViewEvent +import com.datadog.android.rum.model.VitalAppLaunchEvent +import com.datadog.android.rum.model.VitalOperationStepEvent import com.datadog.android.rum.utils.config.GlobalRumMonitorTestConfiguration import com.datadog.android.rum.utils.forge.Configurator +import com.datadog.android.utils.verifyLog import com.datadog.tools.unit.annotations.TestConfigurationsProvider import com.datadog.tools.unit.extensions.TestConfigurationExtension import com.datadog.tools.unit.extensions.config.TestConfiguration @@ -43,6 +49,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import org.mockito.quality.Strictness +import java.util.Locale @Extensions( ExtendWith(MockitoExtension::class), @@ -75,9 +82,12 @@ internal class RumDataWriterTest { @Forgery lateinit var fakeEventType: EventType + private var fakeMaxItemSize: Long = Long.MAX_VALUE + @BeforeEach fun `set up`() { fakeSerializedData = fakeSerializedEvent.toByteArray(Charsets.UTF_8) + fakeMaxItemSize = fakeSerializedData.size.toLong() + 1 whenever( mockEventBatchWriter.write( @@ -91,7 +101,8 @@ internal class RumDataWriterTest { testedWriter = RumDataWriter( mockSerializer, mockEventMetaSerializer, - rumMonitor.mockSdkCore + rumMonitor.mockSdkCore, + fakeMaxItemSize ) } @@ -305,6 +316,238 @@ internal class RumDataWriterTest { assertThat(metaData.hasAccessibility).isTrue } + // region size-exceeded drop tests + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {ViewEvent exceeds maxItemSize}`( + @Forgery fakeEvent: ViewEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = ERROR_LARGE_EVENT.format(Locale.US, "view", fakeSerializedData.size, fakeSerializedData.size - 1), + additionalProperties = mapOf( + RUM_EVENT_TYPE to "view", + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {ActionEvent exceeds maxItemSize}`( + @Forgery fakeEvent: ActionEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT + .format(Locale.US, "action", fakeSerializedData.size, fakeSerializedData.size - 1) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to "action", + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {ErrorEvent exceeds maxItemSize}`( + @Forgery fakeEvent: ErrorEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT + .format(Locale.US, "error", fakeSerializedData.size, fakeSerializedData.size - 1) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to "error", + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {ResourceEvent exceeds maxItemSize}`( + @Forgery fakeEvent: ResourceEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT + .format(Locale.US, "resource", fakeSerializedData.size, fakeSerializedData.size - 1) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to "resource", + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {LongTaskEvent exceeds maxItemSize}`( + @Forgery fakeEvent: LongTaskEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT + .format(Locale.US, "long_task", fakeSerializedData.size, fakeSerializedData.size - 1) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to "long_task", + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {VitalOperationStepEvent exceeds maxItemSize}`( + @Forgery fakeEvent: VitalOperationStepEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT.format( + Locale.US, + fakeEvent.type, + fakeSerializedData.size, + fakeSerializedData.size - 1 + ) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to fakeEvent.type, + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + + @Test + fun `M log ERROR to USER+TELEMETRY W write() {VitalAppLaunchEvent exceeds maxItemSize}`( + @Forgery fakeEvent: VitalAppLaunchEvent + ) { + // Given + whenever(mockSerializer.serialize(fakeEvent)) doReturn fakeSerializedEvent + testedWriter = RumDataWriter( + mockSerializer, + mockEventMetaSerializer, + rumMonitor.mockSdkCore, + fakeSerializedData.size.toLong() - 1 + ) + + // When + val result = testedWriter.write(mockEventBatchWriter, fakeEvent, fakeEventType) + + // Then + val expectedMsg = ERROR_LARGE_EVENT.format( + Locale.US, + fakeEvent.type, + fakeSerializedData.size, + fakeSerializedData.size - 1 + ) + assertThat(result).isFalse + verifyNoInteractions(mockEventBatchWriter) + mockInternalLogger.verifyLog( + level = InternalLogger.Level.ERROR, + targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), + message = expectedMsg, + additionalProperties = mapOf( + RUM_EVENT_TYPE to fakeEvent.type, + TelemetryAttributes.EVENT_DROPPED_BYTES to fakeSerializedData.size + ) + ) + } + // endregion companion object {