Skip to content
Open
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -107,6 +110,15 @@ internal class CoreModule : IModule {
// Crash Uploader (crash handler is initialized directly in OneSignalImp for early initialization)
builder.register<OneSignalCrashUploaderWrapper>().provides<IStartableService>()

// 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<IObservabilityEventRecorder>()

// 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<MisconfiguredNotificationsManager>().provides<INotificationsManager>()
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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. */
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Comment thread
nan-li marked this conversation as resolved.
try {
previous?.shutdown()
} catch (t: Throwable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<ConfigModelStore>())
// 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<IObservabilityEventRecorder>()
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) {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<ILogTelemetryRemote>(relaxed = true)
val recorder = mockk<IObservabilityEventRecorder>()
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<ILogTelemetryRemote>(relaxed = true)
val second = mockk<ILogTelemetryRemote>(relaxed = true)
var calls = 0
val recorder = mockk<IObservabilityEventRecorder>()
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<IObservabilityEventRecorder>()
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<IObservabilityEventRecorder>(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. */
Expand Down
Loading