Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Comment thread
nan-li marked this conversation as resolved.
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)
Comment thread
nan-li marked this conversation as resolved.
} 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