From c1c3227bde3e8bf3ccc00e8801921426b45a8cac Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 26 Apr 2024 12:28:24 +0200 Subject: [PATCH] Let the SDK manage the file log cleanup, and keep one week of log. --- .../x/initializer/TracingInitializer.kt | 6 +++-- .../rageshake/api/reporter/BugReporter.kt | 5 ---- .../impl/reporter/DefaultBugReporter.kt | 23 ------------------- .../impl/bugreport/FakeBugReporter.kt | 4 ---- .../impl/reporter/DefaultBugReporterTest.kt | 3 --- .../api/tracing/WriteToFilesConfiguration.kt | 7 +++++- .../matrix/impl/tracing/RustTracingService.kt | 22 ++++++++++-------- 7 files changed, 23 insertions(+), 47 deletions(-) diff --git a/app/src/main/kotlin/io/element/android/x/initializer/TracingInitializer.kt b/app/src/main/kotlin/io/element/android/x/initializer/TracingInitializer.kt index 7afcc49e6d..e037d2978f 100644 --- a/app/src/main/kotlin/io/element/android/x/initializer/TracingInitializer.kt +++ b/app/src/main/kotlin/io/element/android/x/initializer/TracingInitializer.kt @@ -56,11 +56,13 @@ class TracingInitializer : Initializer { 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 diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt index d8e05f947b..4f48871666 100644 --- a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/reporter/BugReporter.kt +++ b/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. */ diff --git a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt b/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt index 15a0adcf17..f81a755bd0 100755 --- a/features/rageshake/impl/src/main/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporter.kt +++ b/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, 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. */ diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt index 0a67a79f57..cce2d5d144 100644 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/bugreport/FakeBugReporter.kt +++ b/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") } diff --git a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt b/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt index 8d48220ab5..9d165ea844 100755 --- a/features/rageshake/impl/src/test/kotlin/io/element/android/features/rageshake/impl/reporter/DefaultBugReporterTest.kt +++ b/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")), diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/WriteToFilesConfiguration.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/WriteToFilesConfiguration.kt index 27b378846d..b814bdd5c4 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/tracing/WriteToFilesConfiguration.kt +++ b/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 } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt index c67cd600e6..c22b57f05e 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/tracing/RustTracingService.kt +++ b/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) { - is WriteToFilesConfiguration.Disabled -> null - is WriteToFilesConfiguration.Enabled -> TracingFileConfiguration( - path = writeToFilesConfiguration.directory, - filePrefix = writeToFilesConfiguration.filenamePrefix, - fileSuffix = null, - maxFiles = null, - ) - }, + writeToFiles = tracingConfiguration.writesToFilesConfiguration.toTracingFileConfiguration(), ) 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(), + ) + } +}