Optimize diagnostics file management (#1194)

### Description This PR is the first part of a possible approach to avoid loading huge files in memory in diagnostics at once. Basically, in here we change to reading the file per lines instead of reading everything in one go. #### Behavior changes - Diagnostics will be Android 24+ only. - An important change in behavior is that, instead of using the number of events in the diagnostics file to set a limit, we use the file size. So if on SDK configuration we detect that it's reached X size, we would clear the whole file and start over. - When we reach the file size limit, we remove the whole file, instead of just the oldest events. - We remove the properties from the `MAX_EVENTS_STORED_LIMIT_REACHED` event since they don't make sense with the new approach. (This will require backend changes) - Now we will have a limit to the max number of diagnostics events in memory and per request.
RevenueCat · Aug 17, 2023 · 7feda65 · 7feda65
1 parent 0853414
commit 7feda65
Show file tree

Hide file tree

Showing 16 changed files with 340 additions and 115 deletions.
diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesConfiguration.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesConfiguration.kt
@@ -74,6 +74,8 @@ open class PurchasesConfiguration(builder: Builder) {
          * Examples of this information include response times, cache hits or error codes.
          * This information will be anonymized so it can't be traced back to the end-user.
          * The default value is false.
+         *
+         * Diagnostics is only available in Android API 24+
          */
         fun diagnosticsEnabled(diagnosticsEnabled: Boolean) = apply {
             this.diagnosticsEnabled = diagnosticsEnabled

diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt
@@ -31,11 +31,13 @@ import com.revenuecat.purchases.common.offlineentitlements.OfflineEntitlementsMa
 import com.revenuecat.purchases.common.offlineentitlements.PurchasedProductsFetcher
 import com.revenuecat.purchases.common.verification.SignatureVerificationMode
 import com.revenuecat.purchases.common.verification.SigningManager
+import com.revenuecat.purchases.common.warnLog
 import com.revenuecat.purchases.identity.IdentityManager
 import com.revenuecat.purchases.strings.ConfigureStrings
 import com.revenuecat.purchases.subscriberattributes.SubscriberAttributesManager
 import com.revenuecat.purchases.subscriberattributes.SubscriberAttributesPoster
 import com.revenuecat.purchases.subscriberattributes.caching.SubscriberAttributesCache
+import com.revenuecat.purchases.utils.isAndroidNOrNewer
 import java.net.URL
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.Executors
@@ -81,14 +83,16 @@ internal class PurchasesFactory(
 
             var diagnosticsFileHelper: DiagnosticsFileHelper? = null
             var diagnosticsTracker: DiagnosticsTracker? = null
-            if (diagnosticsEnabled) {
+            if (diagnosticsEnabled && isAndroidNOrNewer()) {
                 diagnosticsFileHelper = DiagnosticsFileHelper(FileHelper(context))
                 diagnosticsTracker = DiagnosticsTracker(
                     appConfig,
                     diagnosticsFileHelper,
                     DiagnosticsAnonymizer(Anonymizer()),
                     diagnosticsDispatcher,
                 )
+            } else if (diagnosticsEnabled) {
+                warnLog("Diagnostics are only supported on Android N or newer.")
             }
 
             val signatureVerificationMode = SignatureVerificationMode.fromEntitlementVerificationMode(
@@ -193,7 +197,7 @@ internal class PurchasesFactory(
             val offeringParser = OfferingParserFactory.createOfferingParser(store)
 
             var diagnosticsSynchronizer: DiagnosticsSynchronizer? = null
-            if (diagnosticsFileHelper != null && diagnosticsTracker != null) {
+            if (diagnosticsFileHelper != null && diagnosticsTracker != null && isAndroidNOrNewer()) {
                 diagnosticsSynchronizer = DiagnosticsSynchronizer(
                     diagnosticsFileHelper,
                     diagnosticsTracker,

diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesOrchestrator.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesOrchestrator.kt
@@ -55,6 +55,7 @@ import com.revenuecat.purchases.strings.IdentityStrings
 import com.revenuecat.purchases.strings.PurchaseStrings
 import com.revenuecat.purchases.strings.RestoreStrings
 import com.revenuecat.purchases.subscriberattributes.SubscriberAttributesManager
+import com.revenuecat.purchases.utils.isAndroidNOrNewer
 import java.net.URL
 import java.util.Collections
 
@@ -149,7 +150,10 @@ internal class PurchasesOrchestrator constructor(
             log(LogIntent.WARNING, ConfigureStrings.AUTO_SYNC_PURCHASES_DISABLED)
         }
 
-        diagnosticsSynchronizer?.syncDiagnosticsFileIfNeeded()
+        if (isAndroidNOrNewer()) {
+            diagnosticsSynchronizer?.clearDiagnosticsFileIfTooBig()
+            diagnosticsSynchronizer?.syncDiagnosticsFileIfNeeded()
+        }
     }
 
     /** @suppress */

diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/common/FileHelper.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/common/FileHelper.kt
@@ -1,15 +1,24 @@
 package com.revenuecat.purchases.common
 
 import android.content.Context
+import android.os.Build
+import androidx.annotation.RequiresApi
+import com.revenuecat.purchases.utils.sizeInKB
 import java.io.BufferedReader
 import java.io.File
 import java.io.FileInputStream
 import java.io.FileOutputStream
 import java.io.InputStreamReader
+import java.util.stream.Stream
 
 internal class FileHelper(
     private val applicationContext: Context,
 ) {
+    fun fileSizeInKB(filePath: String): Double {
+        val file = getFileInFilesDir(filePath)
+        return file.sizeInKB
+    }
+
     fun appendToFile(filePath: String, contentToAppend: String) {
         val file = getFileInFilesDir(filePath)
         file.parentFile?.mkdirs()
@@ -25,29 +34,26 @@ internal class FileHelper(
         return file.delete()
     }
 
-    fun readFilePerLines(filePath: String): List<String> {
-        val readLines = mutableListOf<String>()
-        val file = getFileInFilesDir(filePath)
-        FileInputStream(file).use { fileInputStream ->
-            InputStreamReader(fileInputStream).use { inputStreamReader ->
-                BufferedReader(inputStreamReader).use { bufferedReader ->
-                    readLines.addAll(bufferedReader.readLines())
-                }
-            }
+    // This is using a lambda with a Stream instead of returning the Stream itself. This is so we keep
+    // the responsibility of closing the bufferedReader to this class. Note that the Stream should
+    // be used synchronously, otherwise the bufferedReader will be closed before the stream is used.
+    @RequiresApi(Build.VERSION_CODES.N)
+    fun readFilePerLines(filePath: String, streamBlock: ((Stream<String>) -> Unit)) {
+        openBufferedReader(filePath) { bufferedReader ->
+            streamBlock(bufferedReader.lines())
         }
-        return readLines
     }
 
+    @RequiresApi(Build.VERSION_CODES.N)
     fun removeFirstLinesFromFile(filePath: String, numberOfLinesToRemove: Int) {
-        val readLines = readFilePerLines(filePath)
-        deleteFile(filePath)
-        val textToAppend = if (readLines.isEmpty() || numberOfLinesToRemove >= readLines.size) {
-            errorLog("Trying to remove $numberOfLinesToRemove from file with ${readLines.size} lines.")
-            ""
-        } else {
-            readLines.subList(numberOfLinesToRemove, readLines.size).joinToString(separator = "\n", postfix = "\n")
+        val textToAppend = StringBuilder()
+        readFilePerLines(filePath) { stream ->
+            stream.skip(numberOfLinesToRemove.toLong()).forEach { line ->
+                textToAppend.append(line).append("\n")
+            }
         }
-        appendToFile(filePath, textToAppend)
+        deleteFile(filePath)
+        appendToFile(filePath, textToAppend.toString())
     }
 
     /**
@@ -58,6 +64,17 @@ internal class FileHelper(
         return !file.exists() || file.length() == 0L
     }
 
+    private fun openBufferedReader(filePath: String, contentBlock: ((BufferedReader) -> Unit)) {
+        val file = getFileInFilesDir(filePath)
+        FileInputStream(file).use { fileInputStream ->
+            InputStreamReader(fileInputStream).use { inputStreamReader ->
+                BufferedReader(inputStreamReader).use { bufferedReader ->
+                    contentBlock(bufferedReader)
+                }
+            }
+        }
+    }
+
     private fun getFileInFilesDir(filePath: String): File {
         return File(getFilesDir(), filePath)
     }

diff --git a/...ases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsFileHelper.kt b/...ases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsFileHelper.kt
@@ -1,17 +1,27 @@
 package com.revenuecat.purchases.common.diagnostics
 
+import android.os.Build
+import androidx.annotation.RequiresApi
 import com.revenuecat.purchases.common.FileHelper
 import com.revenuecat.purchases.common.verboseLog
 import org.json.JSONObject
+import java.util.stream.Stream
 
 /**
  * All methods in this file should be executed within the diagnostics thread to ensure there are no threading issues.
  */
+@RequiresApi(Build.VERSION_CODES.N)
 internal class DiagnosticsFileHelper(
     private val fileHelper: FileHelper,
 ) {
     companion object {
         const val DIAGNOSTICS_FILE_PATH = "RevenueCat/diagnostics/diagnostic_entries.jsonl"
+        const val DIAGNOSTICS_FILE_LIMIT_IN_KB = 500
+    }
+
+    @Synchronized
+    fun isDiagnosticsFileTooBig(): Boolean {
+        return fileHelper.fileSizeInKB(DIAGNOSTICS_FILE_PATH) > DIAGNOSTICS_FILE_LIMIT_IN_KB
     }
 
     @Synchronized
@@ -32,11 +42,13 @@ internal class DiagnosticsFileHelper(
     }
 
     @Synchronized
-    fun readDiagnosticsFile(): List<JSONObject> {
-        return if (fileHelper.fileIsEmpty(DIAGNOSTICS_FILE_PATH)) {
-            emptyList()
+    fun readDiagnosticsFile(streamBlock: ((Stream<JSONObject>) -> Unit)) {
+        if (fileHelper.fileIsEmpty(DIAGNOSTICS_FILE_PATH)) {
+            streamBlock(Stream.empty())
         } else {
-            fileHelper.readFilePerLines(DIAGNOSTICS_FILE_PATH).map { JSONObject(it) }
+            fileHelper.readFilePerLines(DIAGNOSTICS_FILE_PATH) { stream ->
+                streamBlock(stream.map { JSONObject(it) })
+            }
         }
     }
 }
diff --git a/...es/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsSynchronizer.kt b/...es/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsSynchronizer.kt
@@ -2,12 +2,15 @@ package com.revenuecat.purchases.common.diagnostics
 
 import android.content.Context
 import android.content.SharedPreferences
+import android.os.Build
+import androidx.annotation.RequiresApi
 import androidx.annotation.VisibleForTesting
 import com.revenuecat.purchases.common.Backend
 import com.revenuecat.purchases.common.Dispatcher
 import com.revenuecat.purchases.common.verboseLog
 import org.json.JSONObject
 import java.io.IOException
+import java.util.stream.Collectors
 
 /**
  * This class is in charge of syncing all previously tracked diagnostics. All operations will be executed
@@ -16,6 +19,7 @@ import java.io.IOException
  *
  * If syncing diagnostics fails multiple times, we will delete any stored diagnostics data and start again.
  */
+@RequiresApi(Build.VERSION_CODES.N)
 internal class DiagnosticsSynchronizer(
     private val diagnosticsFileHelper: DiagnosticsFileHelper,
     private val diagnosticsTracker: DiagnosticsTracker,
@@ -31,7 +35,7 @@ internal class DiagnosticsSynchronizer(
         const val MAX_NUMBER_POST_RETRIES = 3
 
         @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
-        const val MAX_NUMBER_EVENTS = 1000
+        const val MAX_EVENTS_TO_SYNC_PER_REQUEST: Long = 200
 
         fun initializeSharedPreferences(context: Context): SharedPreferences =
             context.getSharedPreferences(
@@ -40,6 +44,14 @@ internal class DiagnosticsSynchronizer(
             )
     }
 
+    fun clearDiagnosticsFileIfTooBig() {
+        if (diagnosticsFileHelper.isDiagnosticsFileTooBig()) {
+            verboseLog("Diagnostics file is too big. Deleting it.")
+            diagnosticsTracker.trackMaxEventsStoredLimitReached()
+            resetDiagnosticsStatus()
+        }
+    }
+
     fun syncDiagnosticsFileIfNeeded() {
         enqueue {
             try {
@@ -91,15 +103,11 @@ internal class DiagnosticsSynchronizer(
     }
 
     private fun getEventsToSync(): List<JSONObject> {
-        val diagnosticsList = diagnosticsFileHelper.readDiagnosticsFile()
-        val diagnosticsInFileCount = diagnosticsList.size
-        if (diagnosticsInFileCount > MAX_NUMBER_EVENTS) {
-            val eventsToRemoveCount = diagnosticsInFileCount - MAX_NUMBER_EVENTS + 1
-            diagnosticsFileHelper.deleteOlderDiagnostics(eventsToRemoveCount)
-            diagnosticsTracker.trackMaxEventsStoredLimitReached(diagnosticsInFileCount, eventsToRemoveCount)
-            return diagnosticsFileHelper.readDiagnosticsFile()
+        var eventsToSync: List<JSONObject> = emptyList()
+        diagnosticsFileHelper.readDiagnosticsFile { stream ->
+            eventsToSync = stream.limit(MAX_EVENTS_TO_SYNC_PER_REQUEST).collect(Collectors.toList())
         }
-        return diagnosticsList
+        return eventsToSync
     }
 
     private fun enqueue(command: () -> Unit) {

diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsTracker.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsTracker.kt
@@ -6,6 +6,7 @@ import com.revenuecat.purchases.common.Dispatcher
 import com.revenuecat.purchases.common.networking.Endpoint
 import com.revenuecat.purchases.common.networking.HTTPResult
 import com.revenuecat.purchases.common.verboseLog
+import com.revenuecat.purchases.utils.isAndroidNOrNewer
 import java.io.IOException
 import kotlin.time.Duration
 
@@ -125,13 +126,10 @@ internal class DiagnosticsTracker(
         )
     }
 
-    fun trackMaxEventsStoredLimitReached(totalEventsStored: Int, eventsRemoved: Int, useCurrentThread: Boolean = true) {
+    fun trackMaxEventsStoredLimitReached(useCurrentThread: Boolean = true) {
         val event = DiagnosticsEntry.Event(
             name = DiagnosticsEventName.MAX_EVENTS_STORED_LIMIT_REACHED,
-            properties = mapOf(
-                "total_number_events_stored" to totalEventsStored,
-                "events_removed" to eventsRemoved,
-            ),
+            properties = mapOf(),
         )
         if (useCurrentThread) {
             trackEventInCurrentThread(event)
@@ -164,12 +162,14 @@ internal class DiagnosticsTracker(
     }
 
     internal fun trackEventInCurrentThread(diagnosticsEntry: DiagnosticsEntry) {
-        val anonymizedEvent = diagnosticsAnonymizer.anonymizeEntryIfNeeded(diagnosticsEntry)
-        verboseLog("Tracking diagnostics event: $anonymizedEvent")
-        try {
-            diagnosticsFileHelper.appendEntryToDiagnosticsFile(anonymizedEvent)
-        } catch (e: IOException) {
-            verboseLog("Error tracking diagnostics event: $e")
+        if (isAndroidNOrNewer()) {
+            val anonymizedEvent = diagnosticsAnonymizer.anonymizeEntryIfNeeded(diagnosticsEntry)
+            verboseLog("Tracking diagnostics event: $anonymizedEvent")
+            try {
+                diagnosticsFileHelper.appendEntryToDiagnosticsFile(anonymizedEvent)
+            } catch (e: IOException) {
+                verboseLog("Error tracking diagnostics event: $e")
+            }
         }
     }
 }
diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/utils/AndroidVersionUtils.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/utils/AndroidVersionUtils.kt
@@ -0,0 +1,5 @@
+package com.revenuecat.purchases.utils
+
+import android.os.Build
+
+fun isAndroidNOrNewer() = Build.VERSION.SDK_INT >= Build.VERSION_CODES.N
diff --git a/purchases/src/main/kotlin/com/revenuecat/purchases/utils/FileExtensions.kt b/purchases/src/main/kotlin/com/revenuecat/purchases/utils/FileExtensions.kt
@@ -0,0 +1,10 @@
+package com.revenuecat.purchases.utils
+
+import java.io.File
+
+private const val BYTE_UNIT_CONVERSION: Double = 1024.0
+
+val File.sizeInBytes: Long
+    get() = length()
+val File.sizeInKB: Double
+    get() = sizeInBytes / BYTE_UNIT_CONVERSION
diff --git a/purchases/src/test/java/com/revenuecat/purchases/BasePurchasesTest.kt b/purchases/src/test/java/com/revenuecat/purchases/BasePurchasesTest.kt
@@ -99,6 +99,9 @@ internal open class BasePurchasesTest {
         every {
             mockIdentityManager.configure(any())
         } just Runs
+        every {
+            mockDiagnosticsSynchronizer.clearDiagnosticsFileIfTooBig()
+        } just Runs
         every {
             mockDiagnosticsSynchronizer.syncDiagnosticsFileIfNeeded()
         } just Runs

diff --git a/purchases/src/test/java/com/revenuecat/purchases/PurchasesCommonTest.kt b/purchases/src/test/java/com/revenuecat/purchases/PurchasesCommonTest.kt
@@ -82,6 +82,11 @@ internal class PurchasesCommonTest: BasePurchasesTest() {
         assertThat(purchases.appUserID).isEqualTo(randomAppUserId)
     }
 
+    @Test
+    fun `diagnostics is cleared if diagnostics file too big on constructor`() {
+        verify(exactly = 1) { mockDiagnosticsSynchronizer.clearDiagnosticsFileIfTooBig() }
+    }
+
     @Test
     fun `diagnostics is synced if needed on constructor`() {
         verify(exactly = 1) { mockDiagnosticsSynchronizer.syncDiagnosticsFileIfNeeded() }