diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index 3f10b22fa..7fde98e6b 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit 3f10b22fa218fe0468f4e7491ec0b7cdb652b786 +Subproject commit 7fde98e6ba0fb5f692ca606c53f7ccd7e051602c diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt index 3592aa32c..1a1105af9 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/CoreModule.kt @@ -42,10 +42,13 @@ import com.onesignal.core.internal.startup.IStartableService import com.onesignal.core.internal.time.ITime import com.onesignal.core.internal.time.impl.Time import com.onesignal.debug.internal.crash.OneSignalCrashUploaderWrapper +import com.onesignal.debug.internal.logging.logger.android.AndroidLogger import com.onesignal.inAppMessages.IInAppMessagesManager import com.onesignal.inAppMessages.internal.MisconfiguredIAMManager import com.onesignal.location.ILocationManager import com.onesignal.location.internal.MisconfiguredLocationManager +import com.onesignal.logger.IObservabilityEventRecorder +import com.onesignal.logger.LoggerFactory import com.onesignal.notifications.INotificationsManager import com.onesignal.notifications.internal.MisconfiguredNotificationsManager import com.onesignal.user.internal.jwt.JwtTokenStore @@ -107,6 +110,15 @@ internal class CoreModule : IModule { // Crash Uploader (crash handler is initialized directly in OneSignalImp for early initialization) builder.register().provides() + // Observability events; the observability lifecycle manager attaches the remote telemetry after bootstrap. + builder.register { provider -> + val featureManager = provider.getService(IFeatureManager::class.java) + LoggerFactory.createObservabilityEventRecorder( + flags = { flag -> featureManager.isEnabled(flag) }, + logger = AndroidLogger(), + ) + }.provides() + // Register dummy services in the event they are not configured. These dummy services // will throw an error message if the associated functionality is attempted to be used. builder.register().provides() diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/IObservabilityLifecycleManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/IObservabilityLifecycleManager.kt index 25bc97cb2..7847d30d8 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/IObservabilityLifecycleManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/IObservabilityLifecycleManager.kt @@ -1,6 +1,7 @@ package com.onesignal.internal import com.onesignal.core.internal.config.ConfigModelStore +import com.onesignal.logger.IObservabilityEventRecorder /** * Narrow contract over the observability pipeline, so [OneSignalImp] holds it without depending @@ -12,4 +13,7 @@ internal interface IObservabilityLifecycleManager { /** Subscribes to config store change events so features react to fresh remote config. */ fun subscribeToConfigStore(configModelStore: ConfigModelStore) + + /** Attaches [recorder] to the live remote telemetry, if any, and to every one installed afterwards. */ + fun attachEventRecorder(recorder: IObservabilityEventRecorder) } diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/LoggerLifecycleManager.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/LoggerLifecycleManager.kt index c3b868af1..1029449aa 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/LoggerLifecycleManager.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/LoggerLifecycleManager.kt @@ -25,6 +25,7 @@ import com.onesignal.logger.ILogHttpSender import com.onesignal.logger.ILogTelemetryRemote import com.onesignal.logger.ILogger import com.onesignal.logger.ILoggerPlatformProvider +import com.onesignal.logger.IObservabilityEventRecorder import com.onesignal.logger.LoggerFactory /** Shared by the crash-handler and ANR-detector defaults, which each report through their own reporter. */ @@ -81,6 +82,7 @@ internal class LoggerLifecycleManager( private var crashHandler: ILogCrashHandler? = null private var anrDetector: ILogAnrDetector? = null private var remoteTelemetry: ILogTelemetryRemote? = null + private var eventRecorder: IObservabilityEventRecorder? = null private var currentConfig: ObservabilityConfig? = null /** Level the live sink is actually filtering at, which is not always [currentConfig]'s level. */ @@ -107,6 +109,35 @@ internal class LoggerLifecycleManager( configModelStore.subscribe(this) } + /** + * The cached-config remote telemetry may already be live from [initializeFromCachedConfig], + * so attach at once when it is. A different recorder arriving later takes over, and the one + * it replaces is detached so it is not left pointing at telemetry this manager will shut down. + */ + @Suppress("TooGenericExceptionCaught") + override fun attachEventRecorder(recorder: IObservabilityEventRecorder) { + synchronized(lock) { + val previous = eventRecorder + eventRecorder = recorder + val telemetry = remoteTelemetry + Logging.debug("OneSignal: event recorder handed over, remote telemetry is ${if (telemetry == null) "not live yet" else "already live"}") + if (telemetry == null) return + if (previous != null && previous !== recorder) { + try { + previous.detach(telemetry) + } catch (t: Throwable) { + Logging.warn("OneSignal: Error detaching the replaced event recorder: ${t.message}", t) + } + } + try { + recorder.attach(telemetry) + Logging.info("OneSignal: event recorder attached to the live remote telemetry") + } catch (t: Throwable) { + Logging.warn("OneSignal: Failed to attach the event recorder to the live remote telemetry: ${t.message}", t) + } + } + } + @Suppress("TooGenericExceptionCaught") override fun onModelReplaced(model: ConfigModel, tag: String) { if (tag != ModelChangeTags.HYDRATE) return @@ -218,6 +249,7 @@ internal class LoggerLifecycleManager( Logging.info("OneSignal: Disabling logger module features") // Clear each reference before the teardown call: a collaborator that throws on the way // down would otherwise leave its field set, and the start guards would treat it as live. + // The event recorder is the exception: it is kept so the next enable re-attaches it. try { val detector = anrDetector anrDetector = null @@ -232,6 +264,16 @@ internal class LoggerLifecycleManager( } catch (t: Throwable) { Logging.warn("OneSignal: Error unregistering logger crash handler: ${t.message}", t) } + try { + val recorder = eventRecorder + val telemetry = remoteTelemetry + if (recorder != null && telemetry != null) { + recorder.detach(telemetry) + Logging.info("OneSignal: event recorder detached from the remote telemetry") + } + } catch (t: Throwable) { + Logging.warn("OneSignal: Error detaching the event recorder: ${t.message}", t) + } try { val telemetry = remoteTelemetry remoteTelemetry = null @@ -310,6 +352,16 @@ internal class LoggerLifecycleManager( remoteTelemetry = telemetry activeLogLevel = logLevel Logging.setLoggerTelemetry(telemetry, shouldSend) + // Isolated like the shutdown below: the telemetry is already live, so a recorder fault + // must not fail the level change. + try { + eventRecorder?.let { + it.attach(telemetry) + Logging.info("OneSignal: event recorder attached to the remote telemetry at level $logLevel") + } + } catch (t: Throwable) { + Logging.warn("OneSignal: Failed to attach the event recorder: ${t.message}", t) + } try { previous?.shutdown() } catch (t: Throwable) { diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt index 77af9a91a..8af456549 100644 --- a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt +++ b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt @@ -31,6 +31,7 @@ import com.onesignal.debug.internal.logging.Logging import com.onesignal.debug.internal.logging.logger.android.getCrashStoragePath import com.onesignal.inAppMessages.IInAppMessagesManager import com.onesignal.location.ILocationManager +import com.onesignal.logger.IObservabilityEventRecorder import com.onesignal.notifications.INotificationsManager import com.onesignal.session.ISessionManager import com.onesignal.session.SessionModule @@ -419,6 +420,15 @@ internal class OneSignalImp : IOneSignal, // Now that the IoC container is ready, subscribe the observability lifecycle // manager to config store events so it reacts to fresh remote config. observabilityManager?.subscribeToConfigStore(services.getService()) + // The event recorder is a container service, so it can only be handed over now. The resolve + // cannot fail in practice (bootstrap already built the feature manager it needs), and the + // hand-over is fail-open inside the manager. + val eventRecorder = services.getServiceOrNull() + if (eventRecorder != null) { + observabilityManager?.attachEventRecorder(eventRecorder) + } else { + Logging.warn("OneSignal: event recorder unavailable, observability events will not ship") + } val result = resolveAppId(appId, configModel, preferencesService) if (result.failed) { @@ -440,7 +450,7 @@ internal class OneSignalImp : IOneSignal, return true } catch (e: Exception) { // Any unchecked throw from initEssentials / bootstrapServices / subscribeToConfigStore / - // updateConfig / userSwitcher.initUser / startupService.scheduleStart would otherwise + // attachEventRecorder / updateConfig / userSwitcher.initUser / startupService.scheduleStart would otherwise // leave initState at IN_PROGRESS forever and `suspendCompletion` uncompleted — // accessors and re-entrant suspend callers (e.g. SyncJobService) would deadlock on // `await()`. Reach a terminal state via [completeInit] (atomic state+completion) and diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerFaultTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerFaultTest.kt index fe80650a7..0362e364c 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerFaultTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerFaultTest.kt @@ -16,6 +16,7 @@ import com.onesignal.logger.ILogFileStore import com.onesignal.logger.ILogTelemetryRemote import com.onesignal.logger.ILogger import com.onesignal.logger.ILoggerPlatformProvider +import com.onesignal.logger.IObservabilityEventRecorder import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.coEvery @@ -541,6 +542,72 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ manager.initializeFromCachedConfig() } + + // ===== The event recorder cannot take the pipeline down ===== + // It rides the remote telemetry: a fault in it must not fail the level change, block the + // teardown, or reach the init path that hands it over. + + test("event recorder attach and detach throw — the telemetry still comes up and is torn down") { + val telemetry = mockk(relaxed = true) + val recorder = mockk() + every { recorder.attach(any()) } throws RuntimeException("attach boom") + every { recorder.detach(any()) } throws RuntimeException("detach boom") + val manager = managerWith(remoteTelemetry = { telemetry }) + manager.attachEventRecorder(recorder) + + manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) + manager.onModelReplaced(disabledConfig(), ModelChangeTags.HYDRATE) + + verify { telemetry.shutdown() } + } + + test("event recorder attach throws during a level change — the new telemetry is still adopted") { + val first = mockk(relaxed = true) + val second = mockk(relaxed = true) + var calls = 0 + val recorder = mockk() + every { recorder.attach(any()) } throws RuntimeException("attach boom") + val manager = managerWith(remoteTelemetry = { if (calls++ == 0) first else second }) + manager.attachEventRecorder(recorder) + + manager.onModelReplaced(enabledConfig(LogLevel.ERROR), ModelChangeTags.HYDRATE) + manager.onModelReplaced(enabledConfig(LogLevel.WARN), ModelChangeTags.HYDRATE) + + verify { first.shutdown() } + calls shouldBe 2 + } + + test("event recorder attach throws against live telemetry — attachEventRecorder does not propagate and the recorder is kept") { + val recorder = mockk() + every { recorder.attach(any()) } throws RuntimeException("attach boom") + val manager = managerWith() + manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) + + manager.attachEventRecorder(recorder) + + // The fault must not have dropped the hand-over: the next level change still attaches. + manager.onModelReplaced(enabledConfig(LogLevel.WARN), ModelChangeTags.HYDRATE) + verify(exactly = 2) { recorder.attach(any()) } + } + + test("an enable retry after a partial failure attaches the event recorder once") { + var crashHandlerAttempts = 0 + val recorder = mockk(relaxed = true) + val manager = + managerWith( + crashHandler = { + if (crashHandlerAttempts++ == 0) throw RuntimeException("first crash handler boom") + mockk(relaxed = true) + }, + ) + manager.attachEventRecorder(recorder) + + manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) + manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) + + crashHandlerAttempts shouldBe 2 + verify(exactly = 1) { recorder.attach(any()) } + } }) /** Generous upper bound on a signal we expect; only a hang burns the full budget. */ diff --git a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerTest.kt b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerTest.kt index fb8ef9653..ccf8eec12 100644 --- a/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerTest.kt +++ b/OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/LoggerLifecycleManagerTest.kt @@ -13,18 +13,30 @@ import com.onesignal.debug.LogLevel import com.onesignal.debug.internal.crash.ObservabilitySdkSupport import com.onesignal.debug.internal.logging.Logging import com.onesignal.debug.internal.logging.logger.android.AndroidLogCrashHandler +import com.onesignal.debug.internal.logging.logger.android.AndroidLogger import com.onesignal.logger.ILogAnrDetector import com.onesignal.logger.ILogFileStore +import com.onesignal.logger.ILogHttpSender +import com.onesignal.logger.ILogTelemetry import com.onesignal.logger.ILogTelemetryRemote +import com.onesignal.logger.ILoggerPlatformProvider +import com.onesignal.logger.IObservabilityEventRecorder +import com.onesignal.logger.LogRecord +import com.onesignal.logger.LoggerFactory +import com.onesignal.logger.ObservabilityEvent import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe import io.kotest.matchers.types.shouldBeInstanceOf +import io.mockk.Runs import io.mockk.clearMocks import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.verify +import io.mockk.verifyOrder import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking @@ -258,6 +270,160 @@ class LoggerLifecycleManagerTest : FunSpec({ runBlocking { delay(SINK_QUIET_MS) } coVerify(exactly = 0) { telemetry.emit(any()) } } + + // ===== Observability events ride the remote telemetry ===== + // The recorder is handed over after bootstrap and has to follow every telemetry swap, so an + // event recorded before HYDRATE leaves through the telemetry HYDRATE installs. + + /** Every collaborator mocked, so only the hand-over and the telemetry swaps are observable. */ + fun mutedManager( + platformProvider: ILoggerPlatformProvider = mockk(relaxed = true), + remoteTelemetryFactory: (ILoggerPlatformProvider, ILogHttpSender) -> ILogTelemetryRemote = + { _, _ -> mockk(relaxed = true) }, + ): LoggerLifecycleManager = + LoggerLifecycleManager( + context = context, + featureManagerProvider = { featureManager }, + platformProviderFactory = { _, _ -> platformProvider }, + logger = mockk(relaxed = true), + fileStoreFactory = { mockk(relaxed = true) }, + crashHandlerFactory = { _, _, _ -> mockk(relaxed = true) }, + anrDetectorFactory = { _, _, _ -> mockk(relaxed = true) }, + remoteTelemetryFactory = remoteTelemetryFactory, + ) + + test("the event recorder waits for remote telemetry and attaches to the one HYDRATE installs") { + val telemetry = mockk(relaxed = true) + val recorder = mockk(relaxed = true) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.initializeFromCachedConfig() + + manager.attachEventRecorder(recorder) + verify(exactly = 0) { recorder.attach(any()) } + + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + verify(exactly = 1) { recorder.attach(telemetry) } + } + + test("the event recorder attaches at once when the cached-config telemetry is already live") { + val telemetry = mockk(relaxed = true) + val recorder = mockk(relaxed = true) + val cachedProvider = mockk(relaxed = true) + every { cachedProvider.isRemoteLoggingEnabled } returns true + every { cachedProvider.remoteLogLevel } returns "ERROR" + val manager = mutedManager(platformProvider = cachedProvider, remoteTelemetryFactory = { _, _ -> telemetry }) + manager.initializeFromCachedConfig() + + manager.attachEventRecorder(recorder) + + verify(exactly = 1) { recorder.attach(telemetry) } + } + + test("disabling detaches the event recorder before the telemetry shuts down") { + val telemetry = mockk(relaxed = true) + val recorder = mockk(relaxed = true) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.attachEventRecorder(recorder) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + manager.onModelReplaced(configWith(isEnabled = false, logLevel = null), ModelChangeTags.HYDRATE) + + verifyOrder { + recorder.attach(telemetry) + recorder.detach(telemetry) + telemetry.shutdown() + } + } + + test("a different event recorder handed over later takes the live telemetry from the first") { + val telemetry = mockk(relaxed = true) + val first = mockk(relaxed = true) + val second = mockk(relaxed = true) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.attachEventRecorder(first) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + manager.attachEventRecorder(second) + + verifyOrder { + first.attach(telemetry) + first.detach(telemetry) + second.attach(telemetry) + } + } + + test("handing over the same event recorder twice does not detach it") { + val telemetry = mockk(relaxed = true) + val recorder = mockk(relaxed = true) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.attachEventRecorder(recorder) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + manager.attachEventRecorder(recorder) + + verify(exactly = 0) { recorder.detach(any()) } + verify(exactly = 2) { recorder.attach(telemetry) } + } + + test("an event recorded before HYDRATE ships through the telemetry HYDRATE installs, unlike an INFO log line") { + // The only Android-side proof of the composition until the gesture detector calls record: + // the real KMP recorder, wired the way CoreModule wires it, against a mocked telemetry. + val emitted = CompletableDeferred() + val telemetry = mockk(relaxed = true) + coEvery { telemetry.emit(any()) } answers { emitted.complete(firstArg()); Unit } + val recorder = LoggerFactory.createObservabilityEventRecorder({ true }, AndroidLogger()) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.attachEventRecorder(recorder) + + recorder.record(ObservabilityEvent.DEVICE_GESTURE, mapOf("gesture.result" to "copied")) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + Logging.info("filtered out at level ERROR") + + val record = runBlocking { withTimeout(SINK_TIMEOUT_MS) { emitted.await() } } + record.body shouldBe "sdk.device_gesture" + record.attributes["event.name"] shouldBe "sdk.device_gesture" + record.attributes["gesture.result"] shouldBe "copied" + runBlocking { delay(SINK_QUIET_MS) } + coVerify(exactly = 1) { telemetry.emit(any()) } + } + + test("a level change moves the event recorder to the replacement telemetry") { + val recorder = mockk(relaxed = true) + val attached = mutableListOf() + every { recorder.attach(capture(attached)) } just Runs + val manager = mutedManager() + manager.attachEventRecorder(recorder) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.WARN), ModelChangeTags.HYDRATE) + + attached.size shouldBe 2 + attached[0] shouldNotBe attached[1] + } + + test("repeating the same config does not re-attach the event recorder") { + val recorder = mockk(relaxed = true) + val manager = mutedManager() + manager.attachEventRecorder(recorder) + + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + verify(exactly = 1) { recorder.attach(any()) } + } + + test("the event recorder never attaches when the SDK level is unsupported") { + ObservabilitySdkSupport.isSupported = false + val recorder = mockk(relaxed = true) + val manager = mutedManager() + manager.initializeFromCachedConfig() + manager.attachEventRecorder(recorder) + + manager.onModelReplaced(configWith(isEnabled = true, logLevel = LogLevel.ERROR), ModelChangeTags.HYDRATE) + + verify(exactly = 0) { recorder.attach(any()) } + } }) /** Generous upper bound on a signal we expect; only a hang burns the full budget. */