feat: Include extra logs in error reports from orange release builds (#414)

Release builds normally strip out all logging to reduce the number of
disk writes and reduce UI jank.

These logs would still be useful in user error reports from orange
builds. To preseve them:

- Implement a simple `RingBuffer`.

- Create `TreeRing`, a `Timber` `Tree` logger that logs to a
`RingBuffer` instance in orange release builds.

- Create `TreeRingCollector`, called when ACRA reports are generated,
which includes the contents of the ring buffer in the report.

- Enable desugaring to allow the use of java.time libraries on older
Android versions.

- Disable ProGuard obfuscation of class names as the obfuscation adds
additional de-obfuscation steps when handling error reports from users.

app/build.gradle.kts

@@ -53,6 +53,10 @@ android {
         }
     }
 
+    compileOptions {
+        isCoreLibraryDesugaringEnabled = true
+    }
+
     packaging {
         resources.excludes.apply {
             add("LICENSE_OFL")
@@ -112,6 +116,8 @@ configurations {
 }
 
 dependencies {
+    coreLibraryDesugaring(libs.desugar.jdk.libs)
+
     // CachedTimelineRemoteMediator needs the @Transaction annotation from Room
     compileOnly(libs.bundles.room)
     testCompileOnly(libs.bundles.room)

app/lint-baseline.xml

@@ -2621,7 +2621,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="621"
+            line="623"
             column="17"/>
     </issue>
 
@@ -2632,7 +2632,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="624"
+            line="626"
             column="21"/>
     </issue>
 
@@ -2643,7 +2643,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="629"
+            line="631"
             column="17"/>
     </issue>
 
@@ -2654,7 +2654,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="633"
+            line="635"
             column="21"/>
     </issue>
 
@@ -2665,7 +2665,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="638"
+            line="640"
             column="17"/>
     </issue>
 
@@ -2676,7 +2676,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="641"
+            line="643"
             column="21"/>
     </issue>
 
@@ -2687,7 +2687,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="646"
+            line="648"
             column="17"/>
     </issue>
 
@@ -2698,7 +2698,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="649"
+            line="651"
             column="21"/>
     </issue>
 
@@ -2709,7 +2709,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="654"
+            line="656"
             column="17"/>
     </issue>
 
@@ -2720,7 +2720,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="657"
+            line="659"
             column="21"/>
     </issue>
 
@@ -2731,7 +2731,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="661"
+            line="663"
             column="17"/>
     </issue>
 
@@ -2742,7 +2742,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="664"
+            line="666"
             column="21"/>
     </issue>
 
@@ -2753,7 +2753,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="669"
+            line="671"
             column="17"/>
     </issue>
 
@@ -2764,7 +2764,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="672"
+            line="674"
             column="21"/>
     </issue>
 
@@ -2775,7 +2775,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="676"
+            line="678"
             column="17"/>
     </issue>
 
@@ -2786,7 +2786,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="680"
+            line="682"
             column="21"/>
     </issue>
 
@@ -2797,7 +2797,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="689"
+            line="691"
             column="17"/>
     </issue>
 
@@ -2808,7 +2808,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="692"
+            line="694"
             column="21"/>
     </issue>
 
@@ -2819,7 +2819,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="697"
+            line="699"
             column="17"/>
     </issue>
 
@@ -2830,7 +2830,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="700"
+            line="702"
             column="21"/>
     </issue>
 
@@ -2841,7 +2841,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="705"
+            line="707"
             column="17"/>
     </issue>
 
@@ -2852,7 +2852,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="708"
+            line="710"
             column="21"/>
     </issue>
 
@@ -2863,7 +2863,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="713"
+            line="715"
             column="17"/>
     </issue>
 
@@ -2874,7 +2874,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="716"
+            line="718"
             column="21"/>
     </issue>
 
@@ -2885,7 +2885,7 @@
         errorLine2="                    ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="723"
+            line="725"
             column="21"/>
     </issue>
 
@@ -2896,7 +2896,7 @@
         errorLine2="                        ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="726"
+            line="728"
             column="25"/>
     </issue>
 
@@ -2907,7 +2907,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="735"
+            line="737"
             column="17"/>
     </issue>
 
@@ -2918,7 +2918,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="738"
+            line="740"
             column="21"/>
     </issue>
 
@@ -2929,7 +2929,7 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="751"
+            line="753"
             column="17"/>
     </issue>
 
@@ -2940,7 +2940,7 @@
         errorLine2="                    ~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="755"
+            line="757"
             column="21"/>
     </issue>
 
@@ -2949,17 +2949,6 @@
         message="Access to `private` method `getBinding` of class `MainActivity` requires synthetic accessor"
         errorLine1="                binding.mainToolbar.title = tab.contentDescription"
         errorLine2="                ~~~~~~~">
-        <location
-            file="src/main/java/app/pachli/MainActivity.kt"
-            line="880"
-            column="17"/>
-    </issue>
-
-    <issue
-        id="SyntheticAccessor"
-        message="Access to `private` method `refreshComposeButtonState` of class `MainActivity` requires synthetic accessor"
-        errorLine1="                refreshComposeButtonState(tabAdapter, tab.position)"
-        errorLine2="                ~~~~~~~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
             line="882"
@@ -2973,7 +2962,18 @@
         errorLine2="                ~~~~~~~~~~~~~~~~~~~~~~~~~">
         <location
             file="src/main/java/app/pachli/MainActivity.kt"
-            line="893"
+            line="884"
+            column="17"/>
+    </issue>
+
+    <issue
+        id="SyntheticAccessor"
+        message="Access to `private` method `refreshComposeButtonState` of class `MainActivity` requires synthetic accessor"
+        errorLine1="                refreshComposeButtonState(tabAdapter, tab.position)"
+        errorLine2="                ~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/java/app/pachli/MainActivity.kt"
+            line="895"
             column="17"/>
     </issue>
 

app/proguard-rules.pro

@@ -52,6 +52,11 @@
     public *;
 }
 
+# Keep class names. Obfuscating them serves no purpose in an open source
+# project and adds an additional step to de-obfuscate them when managing user
+# error reports
+-keepnames class *
+
 # https://github.com/google/gson/blob/master/examples/android-proguard-example/proguard.cfg
 
 # Prevent proguard from stripping interface information from TypeAdapter, TypeAdapterFactory,

app/src/main/java/app/pachli/PachliApplication.kt

@@ -25,6 +25,7 @@ import androidx.work.ExistingPeriodicWorkPolicy
 import androidx.work.PeriodicWorkRequestBuilder
 import androidx.work.WorkManager
 import app.pachli.components.notifications.createWorkerNotificationChannel
+import app.pachli.core.activity.TreeRing
 import app.pachli.core.activity.initCrashReporter
 import app.pachli.core.preferences.AppTheme
 import app.pachli.core.preferences.NEW_INSTALL_SCHEMA_VERSION
@@ -81,7 +82,10 @@ class PachliApplication : Application() {
 
         AutoDisposePlugins.setHideProxies(false) // a small performance optimization
 
-        if (BuildConfig.DEBUG) Timber.plant(Timber.DebugTree())
+        when {
+            BuildConfig.DEBUG -> Timber.plant(Timber.DebugTree())
+            BuildConfig.FLAVOR_color == "orange" -> Timber.plant(TreeRing)
+        }
 
         // Migrate shared preference keys and defaults from version to version.
         val oldVersion = sharedPreferencesRepository.getInt(PrefKeys.SCHEMA_VERSION, NEW_INSTALL_SCHEMA_VERSION)

core/activity/build.gradle.kts

@@ -27,6 +27,10 @@ android {
         testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner"
         testInstrumentationRunnerArguments["disableAnalytics"] = "true"
     }
+
+    compileOptions {
+        isCoreLibraryDesugaringEnabled = true
+    }
 }
 
 dependencies {
@@ -50,6 +54,10 @@ dependencies {
     // Crash reporting in orange (Pachli Current) builds only
     orangeImplementation(libs.bundles.acra)
 
+    coreLibraryDesugaring(libs.desugar.jdk.libs)
+    orangeCompileOnly(libs.auto.service.annotations)
+    kspOrange(libs.auto.service.ksp)
+
     // BottomSheetActivityTest uses mockito
     testImplementation(libs.bundles.mockito)
 }

core/activity/src/blue/kotlin/app/pachli/core/activity/CrashReporter.kt

@@ -18,7 +18,14 @@
 package app.pachli.core.activity
 
 import android.app.Application
+import timber.log.Timber
 
 /** Do nothing in blue builds */
 fun initCrashReporter(app: Application) {}
 fun triggerCrashReport() {}
+
+object TreeRing : Timber.Tree() {
+    override fun log(priority: Int, tag: String?, message: String, t: Throwable?) {
+        // Do nothing
+    }
+}

core/activity/src/orange/kotlin/app/pachli/core/activity/CrashReporter.kt

@@ -18,12 +18,21 @@
 package app.pachli.core.activity
 
 import android.app.Application
+import android.content.Context
+import android.util.Log
 import app.pachli.core.designsystem.R as DR
+import com.google.auto.service.AutoService
+import java.time.Instant
 import org.acra.ACRA
+import org.acra.builder.ReportBuilder
+import org.acra.collector.Collector
+import org.acra.config.CoreConfiguration
 import org.acra.config.dialog
 import org.acra.config.mailSender
+import org.acra.data.CrashReportData
 import org.acra.data.StringFormat
 import org.acra.ktx.initAcra
+import timber.log.Timber
 
 /**
  * Initialise ACRA.
@@ -54,3 +63,64 @@ fun initCrashReporter(app: Application) {
 fun triggerCrashReport() {
     ACRA.errorReporter.handleException(null, false)
 }
+
+/**
+ * [Timber.Tree] that logs in to ring buffer, keeping the most recent 1,000 log
+ * entries.
+ */
+object TreeRing : Timber.DebugTree() {
+    /**
+     * Store the components of a log line without doing any formatting or other
+     * work at logging time.
+     */
+    data class LogEntry(
+        val instant: Instant,
+        val priority: Int,
+        val tag: String?,
+        val message: String,
+        val t: Throwable?,
+    )
+
+    val buffer = RingBuffer<LogEntry>(1000)
+
+    override fun log(priority: Int, tag: String?, message: String, t: Throwable?) {
+        buffer.add(LogEntry(Instant.now(), priority, tag, message, t))
+    }
+}
+
+/**
+ * Acra collector that appends the contents of the [TreeRing] log to the Acra
+ * report.
+ */
+@AutoService(Collector::class)
+class TreeRingCollector : Collector {
+    /** Map log priority values to characters to use when displaying the log */
+    private val priority = mapOf(
+        Log.VERBOSE to 'V',
+        Log.DEBUG to 'D',
+        Log.INFO to 'I',
+        Log.WARN to 'W',
+        Log.ERROR to 'E',
+        Log.ASSERT to 'A',
+    )
+
+    override fun collect(
+        context: Context,
+        config: CoreConfiguration,
+        reportBuilder: ReportBuilder,
+        crashReportData: CrashReportData,
+    ) {
+        crashReportData.put(
+            "TreeRing",
+            TreeRing.buffer.toList().joinToString("\n") {
+                "%s %c/%s: %s%s".format(
+                    it.instant.toString(),
+                    priority[it.priority] ?: '?',
+                    it.tag,
+                    it.message,
+                    it.t?.let { t -> " $t" } ?: "",
+                )
+            },
+        )
+    }
+}

core/activity/src/orange/kotlin/app/pachli/core/activity/RingBuffer.kt

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2024 Pachli Association
+ *
+ * This file is a part of Pachli.
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with Pachli; if not,
+ * see <http://www.gnu.org/licenses>.
+ */
+
+package app.pachli.core.activity
+
+import java.util.concurrent.atomic.AtomicInteger
+
+/**
+ * A ring buffer with space for [capacity] items. Adding new items to the buffer will
+ * drop older items.
+ */
+class RingBuffer<T>(private val capacity: Int) : Iterable<T> {
+    private val data: Array<Any?> = arrayOfNulls(capacity)
+    private var nItems: Int = 0
+    private var tail: Int = -1
+
+    private val head: Int
+        get() = if (nItems == data.size) (tail + 1) % nItems else 0
+
+    /** Number of items in the buffer */
+    val size: Int
+        get() = nItems
+
+    /** Add an item to the buffer, overwriting the oldest item in the buffer */
+    fun add(item: T) {
+        tail = (tail + 1) % data.size
+        data[tail] = item
+        if (nItems < data.size) nItems++
+    }
+
+    /** Get an item from the buffer */
+    @Suppress("UNCHECKED_CAST")
+    operator fun get(index: Int): T = when {
+        nItems == 0 || index > nItems || index < 0 -> throw IndexOutOfBoundsException("$index")
+        nItems == data.size -> data[(head + index) % data.size]
+        else -> data[index]
+    } as T
+
+    /** Buffer as a list. */
+    @Suppress("UNCHECKED_CAST")
+    fun toList(): List<T> = iterator().asSequence().toList()
+
+    override fun iterator(): Iterator<T> = object : Iterator<T> {
+        private val index: AtomicInteger = AtomicInteger(0)
+        override fun hasNext(): Boolean = index.get() < size
+        override fun next(): T = get(index.getAndIncrement())
+    }
+}

core/activity/src/testOrange/kotlin/RingBufferTest.kt

@@ -0,0 +1,95 @@
+
+import app.pachli.core.activity.RingBuffer
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+
+/*
+ * Copyright 2024 Pachli Association
+ *
+ * This file is a part of Pachli.
+ *
+ * This program is free software; you can redistribute it and/or modify it under the terms of the
+ * GNU General Public License as published by the Free Software Foundation; either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * Pachli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even
+ * the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
+ * Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along with Pachli; if not,
+ * see <http://www.gnu.org/licenses>.
+ */
+
+class RingBufferTest {
+    @Test
+    fun initialSize() {
+        val buffer = RingBuffer<Int>(10)
+        assertThat(buffer.size).isEqualTo(0)
+    }
+
+    @Test
+    fun `size increases when there is capacity`() {
+        val buffer = RingBuffer<Int>(10)
+        buffer.add(1)
+        assertThat(buffer.size).isEqualTo(1)
+    }
+
+    @Test
+    fun `size is bounded by capacity`() {
+        val buffer = RingBuffer<Int>(10)
+        repeat(20) { buffer.add(it) }
+        assertThat(buffer.size).isEqualTo(10)
+    }
+
+    @Test
+    fun `inserting elements cycles around`() {
+        val buffer = RingBuffer<Int>(3)
+        buffer.add(1)
+        assertThat(buffer[0]).isEqualTo(1)
+
+        buffer.add(2)
+        assertThat(buffer[0]).isEqualTo(1)
+        assertThat(buffer[1]).isEqualTo(2)
+
+        buffer.add(3)
+        assertThat(buffer[0]).isEqualTo(1)
+        assertThat(buffer[1]).isEqualTo(2)
+        assertThat(buffer[2]).isEqualTo(3)
+
+        // Exceed capacity, first item added is dropped
+        buffer.add(4)
+        assertThat(buffer[0]).isEqualTo(2)
+        assertThat(buffer[1]).isEqualTo(3)
+        assertThat(buffer[2]).isEqualTo(4)
+    }
+
+    @Test
+    fun `toList() returns expected list`() {
+        val buffer = RingBuffer<Int>(3)
+        repeat(3) { buffer.add(it) }
+
+        assertThat(buffer.toList()).isEqualTo(listOf(0, 1, 2))
+
+        buffer.add(3)
+        assertThat(buffer.toList()).isEqualTo(listOf(1, 2, 3))
+    }
+
+    @Test(expected = IndexOutOfBoundsException::class)
+    fun getZeroSizeException() {
+        RingBuffer<Int>(1)[0]
+    }
+
+    @Test(expected = IndexOutOfBoundsException::class)
+    fun getIndexOutOfBounds() {
+        val buffer = RingBuffer<Int>(1)
+        buffer.add(1)
+        buffer[10]
+    }
+
+    @Test(expected = IndexOutOfBoundsException::class)
+    fun getIndexNegative() {
+        val buffer = RingBuffer<Int>(1)
+        buffer.add(1)
+        buffer[-1]
+    }
+}

gradle/libs.versions.toml

@@ -26,9 +26,12 @@ androidx-work = "2.8.1"
 androidx-room = "2.6.1"
 app-update = "2.1.0"
 autodispose = "2.2.1"
+auto-service = "1.1.1"
+auto-service-ksp = "1.1.0"
 bouncycastle = "1.70"
 conscrypt = "2.5.2"
 coroutines = "1.7.3"
+desugar_jdk_libs = "2.0.3"
 diffx = "1.1.1"
 emoji2 = "1.3.0"
 espresso = "3.5.1"
@@ -135,8 +138,11 @@ app-update = { module = "com.google.android.play:app-update", version.ref = "app
 app-update-ktx = { module = "com.google.android.play:app-update-ktx", version.ref = "app-update" }
 autodispose-android-lifecycle = { module = "com.uber.autodispose2:autodispose-androidx-lifecycle", version.ref = "autodispose" }
 autodispose-core = { module = "com.uber.autodispose2:autodispose", version.ref = "autodispose" }
+auto-service-annotations = { module = "com.google.auto.service:auto-service-annotations", version.ref = "auto-service"}
+auto-service-ksp = { module = "dev.zacsweers.autoservice:auto-service-ksp", version.ref = "auto-service-ksp"}
 bouncycastle = { module = "org.bouncycastle:bcprov-jdk15on", version.ref = "bouncycastle" }
 conscrypt-android = { module = "org.conscrypt:conscrypt-android", version.ref = "conscrypt" }
+desugar_jdk_libs = { module = "com.android.tools:desugar_jdk_libs", version.ref = "desugar_jdk_libs" }
 diffx = { module = "org.pageseeder.diffx:pso-diffx", version.ref = "diffx" }
 espresso-core = { module = "androidx.test.espresso:espresso-core", version.ref = "espresso" }
 filemojicompat-core = { module = "de.c1710:filemojicompat", version.ref = "filemoji-compat" }