From 4142f4bf5e8530162e5034ab7cc291ba9f606935 Mon Sep 17 00:00:00 2001 From: Nan Date: Wed, 2 Sep 2026 20:21:37 -0700 Subject: [PATCH 1/8] feat: [SDK-5156] wire named events into the log pipeline Registers the shared KMP event recorder as ISdkEventRecorder, with its flag read wired to IFeatureManager, and hands it to LoggerLifecycleManager once the container exists. The manager attaches the recorder to the cached-config sink if one is already live, re-attaches it on every sink swap in startLogging, and detaches it in disableFeatures, beside the Logging sink swap. Events therefore use the crash gate (log_level present and not NONE), not the severity filter. The record call in DeviceGestureDetector lands separately, on top of the clipboard gesture from SDK-5088. Bumps the OneSignal-KMP-SDK submodule to fd1a057 (nan/sdk-5156), which adds SdkEvent, LogEventRecorder and the sdk_event_device_gesture_enabled flag. Re-point to the release tag once that KMP change ships. --- OneSignal-KMP-SDK | 2 +- .../java/com/onesignal/core/CoreModule.kt | 14 +++ .../IObservabilityLifecycleManager.kt | 8 ++ .../internal/LoggerLifecycleManager.kt | 34 ++++++ .../com/onesignal/internal/OneSignalImp.kt | 4 + .../LoggerLifecycleManagerFaultTest.kt | 44 +++++++ .../internal/LoggerLifecycleManagerTest.kt | 110 ++++++++++++++++++ 7 files changed, 215 insertions(+), 1 deletion(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index 7051a2485..fd1a057b4 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit 7051a248550c344345da7bdc820a471f0f6b1061 +Subproject commit fd1a057b4304ff248b2effda2de0e1e2f9c3c467 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..c33731289 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.ISdkEventRecorder +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,17 @@ internal class CoreModule : IModule { // Crash Uploader (crash handler is initialized directly in OneSignalImp for early initialization) builder.register().provides() + // Named events on the log pipeline. The shared recorder owns the flag check, the pre-sink + // queue and the session cap; this host wires the flag read to its feature manager and + // hands the recorder to the observability lifecycle manager, which attaches the sink. + builder.register { provider -> + val featureManager = provider.getService(IFeatureManager::class.java) + LoggerFactory.createEventRecorder( + isEnabled = { event -> featureManager.isEnabled(event.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..d00cec44e 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.ISdkEventRecorder /** * Narrow contract over the observability pipeline, so [OneSignalImp] holds it without depending @@ -12,4 +13,11 @@ internal interface IObservabilityLifecycleManager { /** Subscribes to config store change events so features react to fresh remote config. */ fun subscribeToConfigStore(configModelStore: ConfigModelStore) + + /** + * Hands over the recorder named events ship through, once the IoC container exists. The + * manager attaches it to the live remote sink straight away if there is one, and to every + * sink it installs afterwards. + */ + fun attachEventRecorder(recorder: ISdkEventRecorder) } 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..7f547ce8a 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.ISdkEventRecorder 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: ISdkEventRecorder? = null private var currentConfig: ObservabilityConfig? = null /** Level the live sink is actually filtering at, which is not always [currentConfig]'s level. */ @@ -107,6 +109,25 @@ internal class LoggerLifecycleManager( configModelStore.subscribe(this) } + /** + * The recorder arrives after bootstrap, and the cached-config sink may already be live from + * [initializeFromCachedConfig], so it attaches straight away in that case. Events recorded + * in the gap wait in the recorder's own queue; that queue, not subscriber order on the config + * store, is what carries them across. + */ + @Suppress("TooGenericExceptionCaught") + override fun attachEventRecorder(recorder: ISdkEventRecorder) { + synchronized(lock) { + eventRecorder = recorder + val telemetry = remoteTelemetry ?: return + try { + recorder.attach(telemetry) + } catch (t: Throwable) { + Logging.warn("OneSignal: Failed to attach the event recorder to the live sink: ${t.message}", t) + } + } + } + @Suppress("TooGenericExceptionCaught") override fun onModelReplaced(model: ConfigModel, tag: String) { if (tag != ModelChangeTags.HYDRATE) return @@ -232,6 +253,11 @@ internal class LoggerLifecycleManager( } catch (t: Throwable) { Logging.warn("OneSignal: Error unregistering logger crash handler: ${t.message}", t) } + try { + eventRecorder?.detach() + } catch (t: Throwable) { + Logging.warn("OneSignal: Error detaching the event recorder: ${t.message}", t) + } try { val telemetry = remoteTelemetry remoteTelemetry = null @@ -310,6 +336,14 @@ internal class LoggerLifecycleManager( remoteTelemetry = telemetry activeLogLevel = logLevel Logging.setLoggerTelemetry(telemetry, shouldSend) + // Events ride the same sink as Logging.* lines, so the recorder moves with it. Isolated + // like the shutdown below: the sink is already live, and a recorder fault must not + // report the level change as failed. + try { + eventRecorder?.attach(telemetry) + } 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..57b30783a 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.ISdkEventRecorder import com.onesignal.notifications.INotificationsManager import com.onesignal.session.ISessionManager import com.onesignal.session.SessionModule @@ -419,6 +420,9 @@ 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 recorder for named events lives in the container, so it can only be handed over + // now; the manager attaches it to the cached-config sink if that is already live. + observabilityManager?.attachEventRecorder(services.getService()) val result = resolveAppId(appId, configModel, preferencesService) if (result.failed) { 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..901c4d44a 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.ISdkEventRecorder import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.coEvery @@ -541,6 +542,49 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ manager.initializeFromCachedConfig() } + + // ===== The event recorder cannot take the pipeline down ===== + // It is a passenger on the sink: 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 sink 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() } 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 sink 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 a live sink — attachEventRecorder does not propagate") { + val recorder = mockk() + every { recorder.attach(any()) } throws RuntimeException("attach boom") + val manager = managerWith() + manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) + + manager.attachEventRecorder(recorder) + } }) /** 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..5eab957af 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 @@ -15,16 +15,24 @@ import com.onesignal.debug.internal.logging.Logging import com.onesignal.debug.internal.logging.logger.android.AndroidLogCrashHandler 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.ISdkEventRecorder 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 +266,108 @@ class LoggerLifecycleManagerTest : FunSpec({ runBlocking { delay(SINK_QUIET_MS) } coVerify(exactly = 0) { telemetry.emit(any()) } } + + // ===== Named events ride the remote sink ===== + // The recorder is handed over after bootstrap and has to follow every sink swap, so an event + // recorded before HYDRATE leaves through the sink HYDRATE installs. + + /** Every collaborator mocked, so only the hand-over and the sink 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 a sink 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 sink 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 sink 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.shutdown() + } + } + + test("a level change moves the event recorder to the replacement sink") { + 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. */ From 54db3c59345faee3a5b1b6e2223e630e378c964e Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 08:50:38 -0700 Subject: [PATCH 2/8] chore: [SDK-5156] tighten the event recorder comments and say remote telemetry Cuts the registration, hand-over and attach comments to the one constraint each site adds, keeps the explanatory test section headers, and renames the comments, warning text and test names to say remote telemetry rather than sink, matching what the lifecycle manager already calls it. Pre-existing uses of the word elsewhere in these files are left alone. --- .../main/java/com/onesignal/core/CoreModule.kt | 4 +--- .../internal/IObservabilityLifecycleManager.kt | 6 +----- .../onesignal/internal/LoggerLifecycleManager.kt | 13 +++++-------- .../java/com/onesignal/internal/OneSignalImp.kt | 3 +-- .../internal/LoggerLifecycleManagerFaultTest.kt | 8 ++++---- .../internal/LoggerLifecycleManagerTest.kt | 16 ++++++++-------- 6 files changed, 20 insertions(+), 30 deletions(-) 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 c33731289..9a5da3af5 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 @@ -110,9 +110,7 @@ internal class CoreModule : IModule { // Crash Uploader (crash handler is initialized directly in OneSignalImp for early initialization) builder.register().provides() - // Named events on the log pipeline. The shared recorder owns the flag check, the pre-sink - // queue and the session cap; this host wires the flag read to its feature manager and - // hands the recorder to the observability lifecycle manager, which attaches the sink. + // Named events; the observability lifecycle manager attaches the remote telemetry after bootstrap. builder.register { provider -> val featureManager = provider.getService(IFeatureManager::class.java) LoggerFactory.createEventRecorder( 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 d00cec44e..9fd368b1d 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 @@ -14,10 +14,6 @@ internal interface IObservabilityLifecycleManager { /** Subscribes to config store change events so features react to fresh remote config. */ fun subscribeToConfigStore(configModelStore: ConfigModelStore) - /** - * Hands over the recorder named events ship through, once the IoC container exists. The - * manager attaches it to the live remote sink straight away if there is one, and to every - * sink it installs afterwards. - */ + /** Attaches [recorder] to the live remote telemetry, if any, and to every one installed afterwards. */ fun attachEventRecorder(recorder: ISdkEventRecorder) } 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 7f547ce8a..001f53dd8 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 @@ -110,10 +110,8 @@ internal class LoggerLifecycleManager( } /** - * The recorder arrives after bootstrap, and the cached-config sink may already be live from - * [initializeFromCachedConfig], so it attaches straight away in that case. Events recorded - * in the gap wait in the recorder's own queue; that queue, not subscriber order on the config - * store, is what carries them across. + * The cached-config remote telemetry may already be live from [initializeFromCachedConfig], + * so attach at once when it is. */ @Suppress("TooGenericExceptionCaught") override fun attachEventRecorder(recorder: ISdkEventRecorder) { @@ -123,7 +121,7 @@ internal class LoggerLifecycleManager( try { recorder.attach(telemetry) } catch (t: Throwable) { - Logging.warn("OneSignal: Failed to attach the event recorder to the live sink: ${t.message}", t) + Logging.warn("OneSignal: Failed to attach the event recorder to the live remote telemetry: ${t.message}", t) } } } @@ -336,9 +334,8 @@ internal class LoggerLifecycleManager( remoteTelemetry = telemetry activeLogLevel = logLevel Logging.setLoggerTelemetry(telemetry, shouldSend) - // Events ride the same sink as Logging.* lines, so the recorder moves with it. Isolated - // like the shutdown below: the sink is already live, and a recorder fault must not - // report the level change as failed. + // Isolated like the shutdown below: the telemetry is already live, so a recorder fault + // must not fail the level change. try { eventRecorder?.attach(telemetry) } 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 57b30783a..4aa0f2ad1 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 @@ -420,8 +420,7 @@ 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 recorder for named events lives in the container, so it can only be handed over - // now; the manager attaches it to the cached-config sink if that is already live. + // The event recorder is a container service, so it can only be handed over now. observabilityManager?.attachEventRecorder(services.getService()) val result = resolveAppId(appId, configModel, preferencesService) 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 901c4d44a..f199a7846 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 @@ -544,10 +544,10 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ } // ===== The event recorder cannot take the pipeline down ===== - // It is a passenger on the sink: a fault in it must not fail the level change, block the + // 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 sink still comes up and is torn down") { + 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") @@ -561,7 +561,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ verify { telemetry.shutdown() } } - test("event recorder attach throws during a level change — the new sink is still adopted") { + 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 @@ -577,7 +577,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ calls shouldBe 2 } - test("event recorder attach throws against a live sink — attachEventRecorder does not propagate") { + test("event recorder attach throws against live telemetry — attachEventRecorder does not propagate") { val recorder = mockk() every { recorder.attach(any()) } throws RuntimeException("attach boom") val manager = managerWith() 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 5eab957af..73b4ccf6a 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 @@ -267,11 +267,11 @@ class LoggerLifecycleManagerTest : FunSpec({ coVerify(exactly = 0) { telemetry.emit(any()) } } - // ===== Named events ride the remote sink ===== - // The recorder is handed over after bootstrap and has to follow every sink swap, so an event - // recorded before HYDRATE leaves through the sink HYDRATE installs. + // ===== Named 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 sink swaps are observable. */ + /** 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 = @@ -288,7 +288,7 @@ class LoggerLifecycleManagerTest : FunSpec({ remoteTelemetryFactory = remoteTelemetryFactory, ) - test("the event recorder waits for a sink and attaches to the one HYDRATE installs") { + 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 }) @@ -302,7 +302,7 @@ class LoggerLifecycleManagerTest : FunSpec({ verify(exactly = 1) { recorder.attach(telemetry) } } - test("the event recorder attaches at once when the cached-config sink is already live") { + 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) @@ -316,7 +316,7 @@ class LoggerLifecycleManagerTest : FunSpec({ verify(exactly = 1) { recorder.attach(telemetry) } } - test("disabling detaches the event recorder before the sink shuts down") { + 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 }) @@ -332,7 +332,7 @@ class LoggerLifecycleManagerTest : FunSpec({ } } - test("a level change moves the event recorder to the replacement sink") { + 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 From 56aecd240b3e36a65dfc1a78ddf42f0bf0fb85d0 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 07:58:25 -0700 Subject: [PATCH 3/8] fix: [SDK-5156] follow the recorder API change and log the recorder's transitions Review follow-ups on the event recorder wiring, against KMP 868d457: - detach passes the telemetry it is leaving, so the shared recorder on either platform only ever drops the instance that is attached. - The hand-over logs whether telemetry was already live, and attach and detach log at INFO like the other components, so a release log capture can say whether the recorder was ever attached. - A different recorder handed over later takes the live telemetry and the one it replaces is detached; the same recorder twice is not detached. - The init-path lookup is fail-open with a WARN, like the observability calls around it. - CoreModule passes the gate the factory now takes. New tests: the real KMP recorder wired the way CoreModule wires it, proving an event recorded before HYDRATE ships through the telemetry HYDRATE installs while an INFO log line does not; the replaced-recorder hand-over; an enable retry attaching once; and the throwing-attach case now proves the recorder was kept. --- OneSignal-KMP-SDK | 2 +- .../java/com/onesignal/core/CoreModule.kt | 2 +- .../internal/LoggerLifecycleManager.kt | 29 ++++++++-- .../com/onesignal/internal/OneSignalImp.kt | 12 +++- .../LoggerLifecycleManagerFaultTest.kt | 27 ++++++++- .../internal/LoggerLifecycleManagerTest.kt | 58 ++++++++++++++++++- 6 files changed, 118 insertions(+), 12 deletions(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index fd1a057b4..ea52cfff1 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit fd1a057b4304ff248b2effda2de0e1e2f9c3c467 +Subproject commit ea52cfff10c4f44ed28a5e87cc210a14a3544d52 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 9a5da3af5..8ea4881e6 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 @@ -114,7 +114,7 @@ internal class CoreModule : IModule { builder.register { provider -> val featureManager = provider.getService(IFeatureManager::class.java) LoggerFactory.createEventRecorder( - isEnabled = { event -> featureManager.isEnabled(event.flag) }, + gate = { event -> featureManager.isEnabled(event.flag) }, logger = AndroidLogger(), ) }.provides() 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 001f53dd8..8eaa1c2d7 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 @@ -111,15 +111,27 @@ internal class LoggerLifecycleManager( /** * The cached-config remote telemetry may already be live from [initializeFromCachedConfig], - * so attach at once when it is. + * 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: ISdkEventRecorder) { synchronized(lock) { + val previous = eventRecorder eventRecorder = recorder - val telemetry = remoteTelemetry ?: return + 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) } @@ -237,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 @@ -252,7 +265,12 @@ internal class LoggerLifecycleManager( Logging.warn("OneSignal: Error unregistering logger crash handler: ${t.message}", t) } try { - eventRecorder?.detach() + 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) } @@ -337,7 +355,10 @@ internal class LoggerLifecycleManager( // Isolated like the shutdown below: the telemetry is already live, so a recorder fault // must not fail the level change. try { - eventRecorder?.attach(telemetry) + 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) } 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 4aa0f2ad1..7d784efc6 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 @@ -420,8 +420,14 @@ 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. - observabilityManager?.attachEventRecorder(services.getService()) + // The event recorder is a container service, so it can only be handed over now. Fail-open + // like the observability calls around it: telemetry must not be able to fail init. + val eventRecorder = services.getServiceOrNull() + if (eventRecorder != null) { + observabilityManager?.attachEventRecorder(eventRecorder) + } else { + Logging.warn("OneSignal: event recorder unavailable, named events will not ship") + } val result = resolveAppId(appId, configModel, preferencesService) if (result.failed) { @@ -443,7 +449,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 f199a7846..404c5e326 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 @@ -551,7 +551,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ val telemetry = mockk(relaxed = true) val recorder = mockk() every { recorder.attach(any()) } throws RuntimeException("attach boom") - every { recorder.detach() } throws RuntimeException("detach boom") + every { recorder.detach(any()) } throws RuntimeException("detach boom") val manager = managerWith(remoteTelemetry = { telemetry }) manager.attachEventRecorder(recorder) @@ -577,13 +577,36 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ calls shouldBe 2 } - test("event recorder attach throws against live telemetry — attachEventRecorder does not propagate") { + 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()) } } }) 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 73b4ccf6a..eaa236002 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,6 +13,7 @@ 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 @@ -20,6 +21,9 @@ import com.onesignal.logger.ILogTelemetry import com.onesignal.logger.ILogTelemetryRemote import com.onesignal.logger.ILoggerPlatformProvider import com.onesignal.logger.ISdkEventRecorder +import com.onesignal.logger.LogRecord +import com.onesignal.logger.LoggerFactory +import com.onesignal.logger.SdkEvent import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe @@ -327,11 +331,63 @@ class LoggerLifecycleManagerTest : FunSpec({ verifyOrder { recorder.attach(telemetry) - recorder.detach() + 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.createEventRecorder({ true }, AndroidLogger()) + val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) + manager.attachEventRecorder(recorder) + + recorder.record(SdkEvent.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() From 54da3b5ac3c1eaa6c255d9af6fb8cfb31bf6f24c Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 08:01:58 -0700 Subject: [PATCH 4/8] chore: [SDK-5156] pin the KMP submodule to the equality-based detach --- OneSignal-KMP-SDK | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index ea52cfff1..2c4628cd2 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit ea52cfff10c4f44ed28a5e87cc210a14a3544d52 +Subproject commit 2c4628cd2d28649f0429cc513266e5bafcfe8de2 From 733ce002aad32c647108d4177b2260ea74c6a263 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 16:45:30 -0700 Subject: [PATCH 5/8] refactor: [SDK-5156] rename the event recorder types to ObservabilityEvent Follows the KMP rename (4f9c246): ISdkEventRecorder is now IObservabilityEventRecorder and the factory is createObservabilityEventRecorder. "Event" already means custom, notification and in-app events in this SDK, and a single log record inside the logger module; these are the SDK reporting on itself through the observability pipeline, next to IObservabilityLifecycleManager. Pins the submodule to the rename. --- OneSignal-KMP-SDK | 2 +- .../java/com/onesignal/core/CoreModule.kt | 8 +++--- .../IObservabilityLifecycleManager.kt | 4 +-- .../internal/LoggerLifecycleManager.kt | 6 ++-- .../com/onesignal/internal/OneSignalImp.kt | 6 ++-- .../LoggerLifecycleManagerFaultTest.kt | 10 +++---- .../internal/LoggerLifecycleManagerTest.kt | 28 +++++++++---------- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index 2c4628cd2..4f9c2462a 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit 2c4628cd2d28649f0429cc513266e5bafcfe8de2 +Subproject commit 4f9c2462a1e150fe9aa11c8e814f2e0afee326c5 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 8ea4881e6..22ff5108b 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 @@ -47,7 +47,7 @@ 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.ISdkEventRecorder +import com.onesignal.logger.IObservabilityEventRecorder import com.onesignal.logger.LoggerFactory import com.onesignal.notifications.INotificationsManager import com.onesignal.notifications.internal.MisconfiguredNotificationsManager @@ -110,14 +110,14 @@ internal class CoreModule : IModule { // Crash Uploader (crash handler is initialized directly in OneSignalImp for early initialization) builder.register().provides() - // Named events; the observability lifecycle manager attaches the remote telemetry after bootstrap. + // Observability events; the observability lifecycle manager attaches the remote telemetry after bootstrap. builder.register { provider -> val featureManager = provider.getService(IFeatureManager::class.java) - LoggerFactory.createEventRecorder( + LoggerFactory.createObservabilityEventRecorder( gate = { event -> featureManager.isEnabled(event.flag) }, logger = AndroidLogger(), ) - }.provides() + }.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. 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 9fd368b1d..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,7 +1,7 @@ package com.onesignal.internal import com.onesignal.core.internal.config.ConfigModelStore -import com.onesignal.logger.ISdkEventRecorder +import com.onesignal.logger.IObservabilityEventRecorder /** * Narrow contract over the observability pipeline, so [OneSignalImp] holds it without depending @@ -15,5 +15,5 @@ internal interface IObservabilityLifecycleManager { fun subscribeToConfigStore(configModelStore: ConfigModelStore) /** Attaches [recorder] to the live remote telemetry, if any, and to every one installed afterwards. */ - fun attachEventRecorder(recorder: ISdkEventRecorder) + 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 8eaa1c2d7..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,7 +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.ISdkEventRecorder +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. */ @@ -82,7 +82,7 @@ internal class LoggerLifecycleManager( private var crashHandler: ILogCrashHandler? = null private var anrDetector: ILogAnrDetector? = null private var remoteTelemetry: ILogTelemetryRemote? = null - private var eventRecorder: ISdkEventRecorder? = 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. */ @@ -115,7 +115,7 @@ internal class LoggerLifecycleManager( * it replaces is detached so it is not left pointing at telemetry this manager will shut down. */ @Suppress("TooGenericExceptionCaught") - override fun attachEventRecorder(recorder: ISdkEventRecorder) { + override fun attachEventRecorder(recorder: IObservabilityEventRecorder) { synchronized(lock) { val previous = eventRecorder eventRecorder = recorder 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 7d784efc6..bc083b2c4 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,7 +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.ISdkEventRecorder +import com.onesignal.logger.IObservabilityEventRecorder import com.onesignal.notifications.INotificationsManager import com.onesignal.session.ISessionManager import com.onesignal.session.SessionModule @@ -422,11 +422,11 @@ internal class OneSignalImp : IOneSignal, observabilityManager?.subscribeToConfigStore(services.getService()) // The event recorder is a container service, so it can only be handed over now. Fail-open // like the observability calls around it: telemetry must not be able to fail init. - val eventRecorder = services.getServiceOrNull() + val eventRecorder = services.getServiceOrNull() if (eventRecorder != null) { observabilityManager?.attachEventRecorder(eventRecorder) } else { - Logging.warn("OneSignal: event recorder unavailable, named events will not ship") + Logging.warn("OneSignal: event recorder unavailable, observability events will not ship") } val result = resolveAppId(appId, configModel, preferencesService) 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 404c5e326..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,7 +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.ISdkEventRecorder +import com.onesignal.logger.IObservabilityEventRecorder import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.mockk.coEvery @@ -549,7 +549,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ test("event recorder attach and detach throw — the telemetry still comes up and is torn down") { val telemetry = mockk(relaxed = true) - val recorder = mockk() + val recorder = mockk() every { recorder.attach(any()) } throws RuntimeException("attach boom") every { recorder.detach(any()) } throws RuntimeException("detach boom") val manager = managerWith(remoteTelemetry = { telemetry }) @@ -565,7 +565,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ val first = mockk(relaxed = true) val second = mockk(relaxed = true) var calls = 0 - val recorder = mockk() + val recorder = mockk() every { recorder.attach(any()) } throws RuntimeException("attach boom") val manager = managerWith(remoteTelemetry = { if (calls++ == 0) first else second }) manager.attachEventRecorder(recorder) @@ -578,7 +578,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ } test("event recorder attach throws against live telemetry — attachEventRecorder does not propagate and the recorder is kept") { - val recorder = mockk() + val recorder = mockk() every { recorder.attach(any()) } throws RuntimeException("attach boom") val manager = managerWith() manager.onModelReplaced(enabledConfig(), ModelChangeTags.HYDRATE) @@ -592,7 +592,7 @@ class LoggerLifecycleManagerFaultTest : FunSpec({ test("an enable retry after a partial failure attaches the event recorder once") { var crashHandlerAttempts = 0 - val recorder = mockk(relaxed = true) + val recorder = mockk(relaxed = true) val manager = managerWith( crashHandler = { 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 eaa236002..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 @@ -20,10 +20,10 @@ import com.onesignal.logger.ILogHttpSender import com.onesignal.logger.ILogTelemetry import com.onesignal.logger.ILogTelemetryRemote import com.onesignal.logger.ILoggerPlatformProvider -import com.onesignal.logger.ISdkEventRecorder +import com.onesignal.logger.IObservabilityEventRecorder import com.onesignal.logger.LogRecord import com.onesignal.logger.LoggerFactory -import com.onesignal.logger.SdkEvent +import com.onesignal.logger.ObservabilityEvent import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe @@ -271,7 +271,7 @@ class LoggerLifecycleManagerTest : FunSpec({ coVerify(exactly = 0) { telemetry.emit(any()) } } - // ===== Named events ride the remote telemetry ===== + // ===== 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. @@ -294,7 +294,7 @@ class LoggerLifecycleManagerTest : FunSpec({ 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 recorder = mockk(relaxed = true) val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) manager.initializeFromCachedConfig() @@ -308,7 +308,7 @@ class LoggerLifecycleManagerTest : FunSpec({ 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 recorder = mockk(relaxed = true) val cachedProvider = mockk(relaxed = true) every { cachedProvider.isRemoteLoggingEnabled } returns true every { cachedProvider.remoteLogLevel } returns "ERROR" @@ -322,7 +322,7 @@ class LoggerLifecycleManagerTest : FunSpec({ test("disabling detaches the event recorder before the telemetry shuts down") { val telemetry = mockk(relaxed = true) - val recorder = 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) @@ -338,8 +338,8 @@ class LoggerLifecycleManagerTest : FunSpec({ 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 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) @@ -355,7 +355,7 @@ class LoggerLifecycleManagerTest : FunSpec({ test("handing over the same event recorder twice does not detach it") { val telemetry = mockk(relaxed = true) - val recorder = 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) @@ -372,11 +372,11 @@ class LoggerLifecycleManagerTest : FunSpec({ val emitted = CompletableDeferred() val telemetry = mockk(relaxed = true) coEvery { telemetry.emit(any()) } answers { emitted.complete(firstArg()); Unit } - val recorder = LoggerFactory.createEventRecorder({ true }, AndroidLogger()) + val recorder = LoggerFactory.createObservabilityEventRecorder({ true }, AndroidLogger()) val manager = mutedManager(remoteTelemetryFactory = { _, _ -> telemetry }) manager.attachEventRecorder(recorder) - recorder.record(SdkEvent.DEVICE_GESTURE, mapOf("gesture.result" to "copied")) + 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") @@ -389,7 +389,7 @@ class LoggerLifecycleManagerTest : FunSpec({ } test("a level change moves the event recorder to the replacement telemetry") { - val recorder = mockk(relaxed = true) + val recorder = mockk(relaxed = true) val attached = mutableListOf() every { recorder.attach(capture(attached)) } just Runs val manager = mutedManager() @@ -403,7 +403,7 @@ class LoggerLifecycleManagerTest : FunSpec({ } test("repeating the same config does not re-attach the event recorder") { - val recorder = mockk(relaxed = true) + val recorder = mockk(relaxed = true) val manager = mutedManager() manager.attachEventRecorder(recorder) @@ -415,7 +415,7 @@ class LoggerLifecycleManagerTest : FunSpec({ test("the event recorder never attaches when the SDK level is unsupported") { ObservabilitySdkSupport.isSupported = false - val recorder = mockk(relaxed = true) + val recorder = mockk(relaxed = true) val manager = mutedManager() manager.initializeFromCachedConfig() manager.attachEventRecorder(recorder) From 65f3142d377cf465ad3ddfb9d578a2b793dd7d2d Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 17:11:52 -0700 Subject: [PATCH 6/8] chore: [SDK-5156] pin the KMP submodule to the flag key without _enabled --- OneSignal-KMP-SDK | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index 4f9c2462a..cb13ea8de 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit 4f9c2462a1e150fe9aa11c8e814f2e0afee326c5 +Subproject commit cb13ea8de0e13411a8e16173ea97c59ae34f72de From 3ac95f5dc80fbc2ea8cb12ad078ac9ea44fe42e9 Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 17:34:00 -0700 Subject: [PATCH 7/8] refactor: [SDK-5156] answer flag lookups instead of gating events Follows KMP 31a7759: each event carries its own gate policy, so the host only implements IFeatureFlagReader, "is this flag on", and CoreModule wires that to IFeatureManager. Pins the submodule to the policy change. --- OneSignal-KMP-SDK | 2 +- .../core/src/main/java/com/onesignal/core/CoreModule.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/OneSignal-KMP-SDK b/OneSignal-KMP-SDK index cb13ea8de..31a7759bb 160000 --- a/OneSignal-KMP-SDK +++ b/OneSignal-KMP-SDK @@ -1 +1 @@ -Subproject commit cb13ea8de0e13411a8e16173ea97c59ae34f72de +Subproject commit 31a7759bba9711ae485b9fdcf5a6e5bbf25da432 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 22ff5108b..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 @@ -114,7 +114,7 @@ internal class CoreModule : IModule { builder.register { provider -> val featureManager = provider.getService(IFeatureManager::class.java) LoggerFactory.createObservabilityEventRecorder( - gate = { event -> featureManager.isEnabled(event.flag) }, + flags = { flag -> featureManager.isEnabled(flag) }, logger = AndroidLogger(), ) }.provides() From 86a804f8cb8de7e7b6e234692fd876a989cc309b Mon Sep 17 00:00:00 2001 From: Nan Date: Thu, 3 Sep 2026 23:40:08 -0700 Subject: [PATCH 8/8] docs: [SDK-5156] say why the recorder hand-over needs no catch-all --- .../src/main/java/com/onesignal/internal/OneSignalImp.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 bc083b2c4..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 @@ -420,8 +420,9 @@ 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. Fail-open - // like the observability calls around it: telemetry must not be able to fail init. + // 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)