Merge pull request #2758 from element-hq/feature/bma/logRotation

Let the SDK manage the file log cleanup, and keep one week of log.

app/src/main/kotlin/io/element/android/x/initializer/TracingInitializer.kt

@@ -56,11 +56,13 @@ class TracingInitializer : Initializer<Unit> {
                 writesToLogcat = false,
                 writesToFilesConfiguration = WriteToFilesConfiguration.Enabled(
                     directory = bugReporter.logDirectory().absolutePath,
-                    filenamePrefix = "logs"
+                    filenamePrefix = "logs",
+                    filenameSuffix = null,
+                    // Keep a minimum of 1 week of log files.
+                    numberOfFiles = 7 * 24,
                 )
             )
         }
-        bugReporter.cleanLogDirectoryIfNeeded()
         bugReporter.setCurrentTracingFilter(tracingConfiguration.filterConfiguration.filter)
         tracingService.setupTracing(tracingConfiguration)
         // Also set env variable for rust back trace

changelog.d/2758.misc

@@ -0,0 +1 @@
+ Let the SDK manage the file log cleanup, and keep one week of log.

features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt

@@ -38,11 +38,6 @@ interface BugReporter {
         listener: BugReporterListener?
     )
 
-    /**
-     * Clean the log files if needed to avoid wasting disk space.
-     */
-    fun cleanLogDirectoryIfNeeded()
-
     /**
      * Provide the log directory.
      */

features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt

@@ -18,7 +18,6 @@ package io.element.android.features.rageshake.impl.reporter
 
 import android.content.Context
 import android.os.Build
-import android.text.format.DateUtils.DAY_IN_MILLIS
 import androidx.core.net.toFile
 import androidx.core.net.toUri
 import com.squareup.anvil.annotations.ContributesBinding
@@ -39,11 +38,8 @@ import io.element.android.libraries.di.SingleIn
 import io.element.android.libraries.matrix.api.SdkMetadata
 import io.element.android.libraries.network.useragent.UserAgentProvider
 import io.element.android.libraries.sessionstorage.api.SessionStore
-import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CancellationException
-import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.first
-import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 import okhttp3.Call
 import okhttp3.MediaType.Companion.toMediaTypeOrNull
@@ -75,8 +71,6 @@ class DefaultBugReporter @Inject constructor(
     @ApplicationContext private val context: Context,
     private val screenshotHolder: ScreenshotHolder,
     private val crashDataStore: CrashDataStore,
-    private val coroutineScope: CoroutineScope,
-    private val systemClock: SystemClock,
     private val coroutineDispatchers: CoroutineDispatchers,
     private val okHttpClient: Provider<OkHttpClient>,
     private val userAgentProvider: UserAgentProvider,
@@ -339,13 +333,6 @@ class DefaultBugReporter @Inject constructor(
         }
     }
 
-    override fun cleanLogDirectoryIfNeeded() {
-        coroutineScope.launch(coroutineDispatchers.io) {
-            // delete the log files older than 1 day, except the most recent one
-            deleteOldLogFiles(systemClock.epochMillis() - DAY_IN_MILLIS)
-        }
-    }
-
     suspend fun deleteAllFiles() {
         withContext(coroutineDispatchers.io) {
             getLogFiles().forEach { it.safeDelete() }
@@ -368,16 +355,6 @@ class DefaultBugReporter @Inject constructor(
         }.orEmpty()
     }
 
-    /**
-     * Delete the log files older than the given time except the most recent one.
-     * @param time the time in ms
-     */
-    private fun deleteOldLogFiles(time: Long) {
-        val logFiles = getLogFiles()
-        val oldLogFiles = logFiles.filter { it.lastModified() < time }
-        oldLogFiles.deleteAllExceptMostRecent()
-    }
-
     /**
      * Delete all the log files except the most recent one.
      */

features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt

@@ -52,10 +52,6 @@ class FakeBugReporter(val mode: FakeBugReporterMode = FakeBugReporterMode.Succes
         listener?.onUploadSucceed()
     }
 
-    override fun cleanLogDirectoryIfNeeded() {
-        // No op
-    }
-
     override fun logDirectory(): File {
         return File("fake")
     }

features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt

@@ -24,7 +24,6 @@ import io.element.android.libraries.matrix.test.FakeSdkMetadata
 import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.libraries.network.useragent.DefaultUserAgentProvider
 import io.element.android.libraries.sessionstorage.impl.memory.InMemorySessionStore
-import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
@@ -144,8 +143,6 @@ class DefaultBugReporterTest {
             context = RuntimeEnvironment.getApplication(),
             screenshotHolder = FakeScreenshotHolder(),
             crashDataStore = FakeCrashDataStore(),
-            coroutineScope = this,
-            systemClock = FakeSystemClock(),
             coroutineDispatchers = testCoroutineDispatchers(),
             okHttpClient = { OkHttpClient.Builder().build() },
             userAgentProvider = DefaultUserAgentProvider(buildMeta, FakeSdkMetadata("123456789")),

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/WriteToFilesConfiguration.kt

@@ -18,5 +18,10 @@ package io.element.android.libraries.matrix.api.tracing
 
 sealed interface WriteToFilesConfiguration {
     data object Disabled : WriteToFilesConfiguration
-    data class Enabled(val directory: String, val filenamePrefix: String) : WriteToFilesConfiguration
+    data class Enabled(
+        val directory: String,
+        val filenamePrefix: String,
+        val filenameSuffix: String?,
+        val numberOfFiles: Int?,
+    ) : WriteToFilesConfiguration
 }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt

@@ -33,15 +33,7 @@ class RustTracingService @Inject constructor(private val buildMeta: BuildMeta) :
         val rustTracingConfiguration = org.matrix.rustcomponents.sdk.TracingConfiguration(
             filter = tracingConfiguration.filterConfiguration.filter,
             writeToStdoutOrSystem = tracingConfiguration.writesToLogcat,
-            writeToFiles = when (val writeToFilesConfiguration = tracingConfiguration.writesToFilesConfiguration) {
+            writeToFiles = tracingConfiguration.writesToFilesConfiguration.toTracingFileConfiguration(),
-                is WriteToFilesConfiguration.Disabled -> null
-                is WriteToFilesConfiguration.Enabled -> TracingFileConfiguration(
-                    path = writeToFilesConfiguration.directory,
-                    filePrefix = writeToFilesConfiguration.filenamePrefix,
-                    fileSuffix = null,
-                    maxFiles = null,
-                )
-            },
         )
         org.matrix.rustcomponents.sdk.setupTracing(rustTracingConfiguration)
         Timber.v("Tracing config filter = $filter: ${filter.filter}")
@@ -51,3 +43,15 @@ class RustTracingService @Inject constructor(private val buildMeta: BuildMeta) :
         return RustTracingTree(retrieveFromStackTrace = buildMeta.isDebuggable)
     }
 }
+
+private fun WriteToFilesConfiguration.toTracingFileConfiguration(): TracingFileConfiguration? {
+    return when (this) {
+        is WriteToFilesConfiguration.Disabled -> null
+        is WriteToFilesConfiguration.Enabled -> TracingFileConfiguration(
+            path = directory,
+            filePrefix = filenamePrefix,
+            fileSuffix = filenameSuffix,
+            maxFiles = numberOfFiles?.toULong(),
+        )
+    }
+}