From 741676372dacf19fa5494add0193660fc099b565 Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Tue, 3 Jun 2025 19:25:46 -0400 Subject: [PATCH 1/2] add public log listener methods Adds the following new public methods: * OneSignal.Debug.addLogListener * OneSignal.Debug.removeLogListener These new methods provide a way to capture all SDK log entires at runtime, allowing the app developer to store these and/or send them to their server. The log listener is independent of logLevel, all message are always sent to listeners. --- .../java/com/onesignal/debug/IDebugManager.kt | 16 ++- .../java/com/onesignal/debug/ILogListener.kt | 5 + .../com/onesignal/debug/OneSignalLogEvent.kt | 6 + .../onesignal/debug/internal/DebugManager.kt | 9 ++ .../debug/internal/logging/Logging.kt | 38 ++++++- .../onesignal/debug/internal/LoggingTests.kt | 107 ++++++++++++++++++ 6 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/ILogListener.kt create mode 100644 OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/OneSignalLogEvent.kt create mode 100644 OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/IDebugManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/IDebugManager.kt index 2e9c038b63..7f8c637a3a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/IDebugManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/IDebugManager.kt @@ -3,17 +3,29 @@ package com.onesignal.debug /** * Access to debug the SDK in the event additional information is required to diagnose any * SDK-related issues. - * - * WARNING: This should not be used in a production setting. */ interface IDebugManager { /** * The log level the OneSignal SDK should be writing to the Android log. Defaults to [LogLevel.WARN]. + * WARNING: This should not be set higher than LogLevel.WARN in a production setting. */ var logLevel: LogLevel /** * The log level the OneSignal SDK should be showing as a modal. Defaults to [LogLevel.NONE]. + * WARNING: This should not be used in a production setting. */ var alertLevel: LogLevel + + /** + * Add a listener to receive all logging messages the SDK produces. + * Useful to capture and send logs to your server. + * NOTE: All log messages are always passed, logLevel has no effect on this. + */ + fun addLogListener(listener: ILogListener) + + /** + * Removes a listener added by addLogListener + */ + fun removeLogListener(listener: ILogListener) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/ILogListener.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/ILogListener.kt new file mode 100644 index 0000000000..2e838065e5 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/ILogListener.kt @@ -0,0 +1,5 @@ +package com.onesignal.debug + +fun interface ILogListener { + fun onLogEvent(event: OneSignalLogEvent) +} diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/OneSignalLogEvent.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/OneSignalLogEvent.kt new file mode 100644 index 0000000000..06478a91c7 --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/OneSignalLogEvent.kt @@ -0,0 +1,6 @@ +package com.onesignal.debug + +data class OneSignalLogEvent( + val level: LogLevel, + val entry: String, +) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/DebugManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/DebugManager.kt index 1067f0f7dc..a4fa31c733 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/DebugManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/DebugManager.kt @@ -1,6 +1,7 @@ package com.onesignal.debug.internal import com.onesignal.debug.IDebugManager +import com.onesignal.debug.ILogListener import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.logging.Logging @@ -21,4 +22,12 @@ internal class DebugManager() : IDebugManager { logLevel = LogLevel.WARN alertLevel = LogLevel.NONE } + + override fun addLogListener(listener: ILogListener) { + Logging.addListener(listener) + } + + override fun removeLogListener(listener: ILogListener) { + Logging.removeListener(listener) + } } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt index 1e085543ee..27bdec89f1 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt @@ -3,15 +3,20 @@ package com.onesignal.debug.internal.logging import android.app.AlertDialog import com.onesignal.common.threading.suspendifyOnMain import com.onesignal.core.internal.application.IApplicationService +import com.onesignal.debug.ILogListener import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent import java.io.PrintWriter import java.io.StringWriter +import java.util.concurrent.CopyOnWriteArraySet object Logging { private const val TAG = "OneSignal" var applicationService: IApplicationService? = null + private val logListeners = CopyOnWriteArraySet() + @JvmStatic var logLevel = LogLevel.WARN @@ -19,9 +24,7 @@ object Logging { var visualLogLevel = LogLevel.NONE @JvmStatic - fun atLogLevel(level: LogLevel): Boolean { - return level.compareTo(visualLogLevel) < 1 || level.compareTo(logLevel) < 1 - } + fun atLogLevel(level: LogLevel): Boolean = level.compareTo(visualLogLevel) < 1 || level.compareTo(logLevel) < 1 @JvmStatic fun verbose( @@ -112,7 +115,8 @@ object Logging { suspendifyOnMain { val currentActivity = applicationService?.current if (currentActivity != null) { - AlertDialog.Builder(currentActivity) + AlertDialog + .Builder(currentActivity) .setTitle(level.toString()) .setMessage(finalFullMessage) .show() @@ -122,5 +126,31 @@ object Logging { android.util.Log.e(TAG, "Error showing logging message.", t) } } + + callLogListeners(level, message, throwable) + } + + private fun callLogListeners( + level: LogLevel, + message: String, + throwable: Throwable?, + ) { + if (logListeners.isEmpty()) return + + var logEntry = message + if (throwable != null) { + logEntry += "\n" + android.util.Log.getStackTraceString(throwable) + } + for (listener in logListeners) { + listener.onLogEvent(OneSignalLogEvent(level, logEntry)) + } + } + + fun addListener(listener: ILogListener) { + logListeners.add(listener) + } + + fun removeListener(listener: ILogListener) { + logListeners.remove(listener) } } diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt new file mode 100644 index 0000000000..6da937957b --- /dev/null +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt @@ -0,0 +1,107 @@ +package com.onesignal.debug.internal + +import com.onesignal.debug.ILogListener +import com.onesignal.debug.LogLevel +import com.onesignal.debug.OneSignalLogEvent +import com.onesignal.debug.internal.logging.Logging +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe + +class TestLogLister : ILogListener { + val calls = ArrayList() + + override fun onLogEvent(event: OneSignalLogEvent) { + calls += event.entry + } +} + +class LoggingTests : FunSpec({ + beforeAny { + Logging.logLevel = LogLevel.NONE + } + + test("addListener") { + // Given + val listener = TestLogLister() + Logging.addListener(listener) + + // When + Logging.debug("test") + + // Then + listener.calls shouldBe arrayOf("test") + } + + test("addListener twice") { + // Given + val listener = TestLogLister() + Logging.addListener(listener) + Logging.addListener(listener) + + // When + Logging.debug("test") + + // Then + listener.calls shouldBe arrayOf("test") + } + + test("removeListener") { + // Given + val listener = TestLogLister() + Logging.addListener(listener) + Logging.removeListener(listener) + + // When + Logging.debug("test") + + // Then + listener.calls shouldBe arrayOf() + } + + test("removeListener twice") { + // Given + val listener = TestLogLister() + Logging.addListener(listener) + Logging.removeListener(listener) + Logging.removeListener(listener) + + // When + Logging.debug("test") + + // Then + listener.calls shouldBe arrayOf() + } + + test("addListener nested") { + // Given + val nestedListener = TestLogLister() + Logging.addListener { Logging.addListener(nestedListener) } + + // When + Logging.debug("test") + Logging.debug("test2") + Logging.debug("test3") + + // Then + nestedListener.calls shouldBe arrayOf("test2", "test3") + } + + test("removeListener nested") { + // Given + val calls = ArrayList() + var listener: ILogListener? = null + listener = + ILogListener { + calls += it.entry + Logging.removeListener(listener!!) + } + Logging.addListener(listener!!) + + // When + Logging.debug("test") + Logging.debug("test2") + + // Then + calls shouldBe arrayOf("test") + } +}) From 24ed61397f0e95e6256140b1ce5e39fee597179b Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Tue, 3 Jun 2025 19:49:08 -0400 Subject: [PATCH 2/2] Clean up Logging.log Split log level and alert level logging into their own methods to make the code easier to follow. --- .../debug/internal/logging/Logging.kt | 82 +++++++++++-------- .../onesignal/debug/internal/LoggingTests.kt | 14 +++- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt index 27bdec89f1..a4db03407a 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/debug/internal/logging/Logging.kt @@ -89,45 +89,59 @@ object Logging { throwable: Throwable?, ) { val fullMessage = "[${Thread.currentThread().name}] $message" - if (level.compareTo(logLevel) < 1) { - when (level) { - LogLevel.VERBOSE -> android.util.Log.v(TAG, fullMessage, throwable) - LogLevel.DEBUG -> android.util.Log.d(TAG, fullMessage, throwable) - LogLevel.INFO -> android.util.Log.i(TAG, fullMessage, throwable) - LogLevel.WARN -> android.util.Log.w(TAG, fullMessage, throwable) - LogLevel.ERROR, LogLevel.FATAL -> android.util.Log.e(TAG, message, throwable) - else -> {} - } + + logToLogcat(level, fullMessage, throwable) + showVisualLogging(level, fullMessage, throwable) + callLogListeners(level, fullMessage, throwable) + } + + private fun logToLogcat( + level: LogLevel, + message: String, + throwable: Throwable?, + ) { + if (level.compareTo(logLevel) >= 1) return + when (level) { + LogLevel.VERBOSE -> android.util.Log.v(TAG, message, throwable) + LogLevel.DEBUG -> android.util.Log.d(TAG, message, throwable) + LogLevel.INFO -> android.util.Log.i(TAG, message, throwable) + LogLevel.WARN -> android.util.Log.w(TAG, message, throwable) + LogLevel.ERROR, LogLevel.FATAL -> android.util.Log.e(TAG, message, throwable) + else -> {} } + } - if (level.compareTo(visualLogLevel) < 1 && applicationService?.current != null) { - try { - var fullMessage: String? = "$message\n".trimIndent() - if (throwable != null) { - fullMessage += throwable.message - val sw = StringWriter() - val pw = PrintWriter(sw) - throwable.printStackTrace(pw) - fullMessage += sw.toString() - } - val finalFullMessage = fullMessage - - suspendifyOnMain { - val currentActivity = applicationService?.current - if (currentActivity != null) { - AlertDialog - .Builder(currentActivity) - .setTitle(level.toString()) - .setMessage(finalFullMessage) - .show() - } + private fun showVisualLogging( + level: LogLevel, + message: String, + throwable: Throwable?, + ) { + if (level.compareTo(visualLogLevel) >= 1) return + + try { + var fullMessage: String? = "$message\n".trimIndent() + if (throwable != null) { + fullMessage += throwable.message + val sw = StringWriter() + val pw = PrintWriter(sw) + throwable.printStackTrace(pw) + fullMessage += sw.toString() + } + val finalFullMessage = fullMessage + + suspendifyOnMain { + val currentActivity = applicationService?.current + if (currentActivity != null) { + AlertDialog + .Builder(currentActivity) + .setTitle(level.toString()) + .setMessage(finalFullMessage) + .show() } - } catch (t: Throwable) { - android.util.Log.e(TAG, "Error showing logging message.", t) } + } catch (t: Throwable) { + android.util.Log.e(TAG, "Error showing logging message.", t) } - - callLogListeners(level, message, throwable) } private fun callLogListeners( diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt index 6da937957b..ca6ce9b308 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/LoggingTests.kt @@ -6,6 +6,7 @@ import com.onesignal.debug.OneSignalLogEvent import com.onesignal.debug.internal.logging.Logging import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldEndWith class TestLogLister : ILogListener { val calls = ArrayList() @@ -15,6 +16,11 @@ class TestLogLister : ILogListener { } } +infix fun > T.shouldHaveEachItemEndWith(expected: Array): T { + this.forEachIndexed { index, it -> it shouldEndWith expected[index] } + return this +} + class LoggingTests : FunSpec({ beforeAny { Logging.logLevel = LogLevel.NONE @@ -29,7 +35,7 @@ class LoggingTests : FunSpec({ Logging.debug("test") // Then - listener.calls shouldBe arrayOf("test") + listener.calls shouldHaveEachItemEndWith arrayOf("test") } test("addListener twice") { @@ -42,7 +48,7 @@ class LoggingTests : FunSpec({ Logging.debug("test") // Then - listener.calls shouldBe arrayOf("test") + listener.calls shouldHaveEachItemEndWith arrayOf("test") } test("removeListener") { @@ -83,7 +89,7 @@ class LoggingTests : FunSpec({ Logging.debug("test3") // Then - nestedListener.calls shouldBe arrayOf("test2", "test3") + nestedListener.calls shouldHaveEachItemEndWith arrayOf("test2", "test3") } test("removeListener nested") { @@ -102,6 +108,6 @@ class LoggingTests : FunSpec({ Logging.debug("test2") // Then - calls shouldBe arrayOf("test") + calls shouldHaveEachItemEndWith arrayOf("test") } })