fix: only take unique error logs and control limit (#188)

amplitude · Mar 26, 2024 · cca7d79 · cca7d79
1 parent 7826f73
commit cca7d79
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 8 deletions.
diff --git a/core/src/main/java/com/amplitude/core/utilities/Diagnostics.kt b/core/src/main/java/com/amplitude/core/utilities/Diagnostics.kt
@@ -4,7 +4,11 @@ import java.util.Collections
 
 class Diagnostics() {
     private var malformedEvents: MutableList<String>? = null
-    private var errorLogs: MutableList<String>? = null
+    private var errorLogs: MutableSet<String> = Collections.synchronizedSet(mutableSetOf())
+
+    companion object {
+        private const val MAX_ERROR_LOGS = 10
+    }
 
     fun addMalformedEvent(event: String) {
         if (malformedEvents == null) {
@@ -14,14 +18,14 @@ class Diagnostics() {
     }
 
     fun addErrorLog(log: String) {
-        if (errorLogs == null) {
-            errorLogs = Collections.synchronizedList(mutableListOf())
+        errorLogs.add(log)
+        while (errorLogs.size > MAX_ERROR_LOGS) {
+            errorLogs.remove(errorLogs.first())
         }
-        errorLogs?.add(log)
     }
 
     fun hasDiagnostics(): Boolean {
-        return (malformedEvents != null && malformedEvents!!.isNotEmpty()) || (errorLogs != null && errorLogs!!.isNotEmpty())
+        return (malformedEvents != null && malformedEvents!!.isNotEmpty()) || errorLogs.isNotEmpty()
     }
 
     /**
@@ -36,12 +40,12 @@ class Diagnostics() {
         if (malformedEvents != null && malformedEvents!!.isNotEmpty()) {
             diagnostics["malformed_events"] = malformedEvents!!
         }
-        if (errorLogs != null && errorLogs!!.isNotEmpty()) {
-            diagnostics["error_logs"] = errorLogs!!
+        if (errorLogs.isNotEmpty()) {
+            diagnostics["error_logs"] = errorLogs.toList()
         }
         val result = diagnostics.toJSONObject().toString()
         malformedEvents?.clear()
-        errorLogs?.clear()
+        errorLogs.clear()
         return result
     }
 }
diff --git a/core/src/test/kotlin/com/amplitude/core/utilities/DiagnosticsTest.kt b/core/src/test/kotlin/com/amplitude/core/utilities/DiagnosticsTest.kt
@@ -22,6 +22,29 @@ class DiagnosticsTest {
         assertEquals("{\"error_logs\":[\"log\"]}", diagnostics.extractDiagnostics())
     }
 
+    @Test
+    fun `test duplicate error logs`() {
+        val diagnostics = Diagnostics()
+        diagnostics.addErrorLog("log")
+        diagnostics.addErrorLog("log")
+        assertTrue(diagnostics.hasDiagnostics())
+        assertEquals("{\"error_logs\":[\"log\"]}", diagnostics.extractDiagnostics())
+    }
+
+    @Test
+    fun `test we only take have 10 error logs if many`() {
+        val diagnostics = Diagnostics()
+        for (i in 1..15) {
+            diagnostics.addErrorLog("log$i")
+        }
+        assertTrue(diagnostics.hasDiagnostics())
+        assertEquals(
+            "{\"error_logs\":[\"log6\",\"log7\",\"log8\",\"log9\",\"log10\",\"log11\",\"log12\",\"log13\",\"log14\",\"log15\"]}",
+            diagnostics.extractDiagnostics(),
+        )
+        assertFalse(diagnostics.hasDiagnostics())
+    }
+
     @Test
     fun `test hasDiagnostics`() {
         val diagnostics = Diagnostics()