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
1 change: 1 addition & 0 deletions libraries/push/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ dependencies {
testImplementation(projects.features.networkmonitor.test)
testImplementation(projects.services.appnavstate.impl)
testImplementation(projects.services.appnavstate.test)
testImplementation(projects.services.analytics.test)
testImplementation(projects.services.toolbox.impl)
testImplementation(projects.services.toolbox.test)
testImplementation(projects.libraries.featureflag.test)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableMess
import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent
import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent
import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.analytics.api.finishLongRunningTransaction
import kotlinx.coroutines.flow.first
import timber.log.Timber

Expand All @@ -36,6 +39,7 @@ class NotificationRenderer(
private val notificationDataFactory: NotificationDataFactory,
private val enterpriseService: EnterpriseService,
private val sessionStore: SessionStore,
private val analyticsService: AnalyticsService,
) {
suspend fun render(
currentUser: MatrixUser,
Expand Down Expand Up @@ -124,6 +128,12 @@ class NotificationRenderer(
notification = summaryNotification.notification
)
}

for (event in eventsToProcess) {
// Finish long-running transaction
val uploaded = analyticsService.finishLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(event.eventId.value))
Timber.d("Push-to-notification for event ${event.eventId} uploaded: $uploaded")
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import io.element.android.libraries.pushproviders.api.PushData
import io.element.android.libraries.pushproviders.api.PushHandler
import io.element.android.libraries.pushstore.api.UserPushStoreFactory
import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
import io.element.android.services.analytics.api.AnalyticsService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
Expand Down Expand Up @@ -69,6 +71,7 @@ class DefaultPushHandler(
private val fallbackNotificationFactory: FallbackNotificationFactory,
private val syncOnNotifiableEvent: SyncOnNotifiableEvent,
private val featureFlagService: FeatureFlagService,
private val analyticsService: AnalyticsService,
) : PushHandler {
init {
processPushEventResults()
Expand Down Expand Up @@ -215,6 +218,13 @@ class DefaultPushHandler(
* @param providerInfo the provider info.
*/
override suspend fun handle(pushData: PushData, providerInfo: String) {
// Start measuring how long it takes to display a notification from when the push is received
Timber.d("Calculating push-to-notification for event ${pushData.eventId}")
val parent = analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(pushData.eventId.value))
if (featureFlagService.isFeatureEnabled(FeatureFlags.SyncNotificationsWithWorkManager)) {
analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(pushData.eventId.value), parent)
}

Timber.tag(loggerTag.value).d("## handling pushData: ${pushData.roomId}/${pushData.eventId}")
if (buildMeta.lowPrivacyLoggingEnabled) {
Timber.tag(loggerTag.value).d("## pushData: $pushData")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import dev.zacsweers.metro.ContributesIntoMap
import dev.zacsweers.metro.binding
import io.element.android.features.networkmonitor.api.NetworkMonitor
import io.element.android.features.networkmonitor.api.NetworkStatus
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.core.extensions.runCatchingExceptions
import io.element.android.libraries.di.annotations.ApplicationContext
import io.element.android.libraries.matrix.api.auth.SessionRestorationException
Expand All @@ -31,10 +30,13 @@ import io.element.android.libraries.push.impl.notifications.NotificationResolver
import io.element.android.libraries.workmanager.api.WorkManagerScheduler
import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory
import io.element.android.libraries.workmanager.api.di.WorkerKey
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
import io.element.android.services.analytics.api.AnalyticsService
import io.element.android.services.analytics.api.finishLongRunningTransaction
import io.element.android.services.analytics.api.recordTransaction
import io.element.android.services.toolbox.api.sdk.BuildVersionSdkIntProvider
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withContext
import kotlinx.coroutines.withTimeoutOrNull
import timber.log.Timber
import kotlin.time.Duration.Companion.seconds
Expand All @@ -48,22 +50,45 @@ class FetchNotificationsWorker(
private val queue: NotificationResolverQueue,
private val workManagerScheduler: WorkManagerScheduler,
private val syncOnNotifiableEvent: SyncOnNotifiableEvent,
private val coroutineDispatchers: CoroutineDispatchers,
private val workerDataConverter: SyncNotificationsWorkerDataConverter,
private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider,
private val analyticsService: AnalyticsService,
) : CoroutineWorker(context, workerParams) {
override suspend fun doWork(): Result = withContext(coroutineDispatchers.io) {
override suspend fun doWork(): Result {
Timber.d("FetchNotificationsWorker started")
val requests = workerDataConverter.deserialize(inputData) ?: return@withContext Result.failure()
val requests = workerDataConverter.deserialize(inputData) ?: return Result.failure()
// Wait for network to be available, but not more than 10 seconds
val networkTimeoutSpans = requests.mapNotNull { request ->
val parent = analyticsService.getLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(request.eventId.value))
parent?.startChild("Waiting for network connectivity", "await_network")
}
val hasNetwork = withTimeoutOrNull(10.seconds) {
networkMonitor.connectivity.first { it == NetworkStatus.Connected }
} != null

for (span in networkTimeoutSpans) {
span.finish()
}

if (!hasNetwork) {
Timber.w("No network, retrying later")
return@withContext Result.retry()
for (request in requests) {
val eventId = request.eventId.value
analyticsService.finishLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(eventId))
val parent = analyticsService.getLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(eventId))
// Since we're retrying, start a new transaction
analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(eventId), parent)
}
return Result.retry()
}

val pendingAnalyticTransactions = requests.mapNotNull { request ->
analyticsService.finishLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(request.eventId.value))
val parent = analyticsService.getLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(request.eventId.value))
val transactionName = "WorkManager to event fetched"
parent?.startChild(transactionName)?.let { request.eventId to it }
}.toMap()

val failedSyncForSessions = mutableMapOf<SessionId, Throwable>()

val groupedRequests = requests.groupBy { it.sessionId }.toMutableMap()
Expand All @@ -72,10 +97,17 @@ class FetchNotificationsWorker(
eventResolver.resolveEvents(sessionId, notificationRequests)
.fold(
onSuccess = { result ->
for ((_, transaction) in pendingAnalyticTransactions) {
transaction.finish()
}
// Update the resolved results in the queue
(queue.results as MutableSharedFlow).emit(requests to result)
},
onFailure = {
for ((_, transaction) in pendingAnalyticTransactions) {
transaction.attachError(it)
transaction.finish()
}
failedSyncForSessions[sessionId] = it
Timber.e(it, "Failed to resolve notification events for session $sessionId")
}
Expand All @@ -92,6 +124,18 @@ class FetchNotificationsWorker(
continue
}
val requestsToRetry = groupedRequests[failedSessionId] ?: continue

for (request in requestsToRetry) {
val failedTransaction = pendingAnalyticTransactions[request.eventId]
failedTransaction?.attachError(exception)
failedTransaction?.finish()

val eventId = request.eventId.value
val parent = analyticsService.getLongRunningTransaction(AnalyticsLongRunningTransaction.PushToNotification(eventId))
// Since we're retrying, start a new transaction
analyticsService.startLongRunningTransaction(AnalyticsLongRunningTransaction.PushToWorkManager(eventId), parent)
}

Timber.d("Re-scheduling ${requestsToRetry.size} failed notification requests for session $failedSessionId")
workManagerScheduler.submit(
SyncNotificationWorkManagerRequest(
Expand All @@ -106,9 +150,11 @@ class FetchNotificationsWorker(

Timber.d("Notifications processed successfully")

performOpportunisticSyncIfNeeded(groupedRequests)
analyticsService.recordTransaction("Opportunistic sync", "opportunistic_sync") {
performOpportunisticSyncIfNeeded(groupedRequests)
}

Result.success()
return Result.success()
}

private suspend fun performOpportunisticSyncIfNeeded(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
import io.element.android.libraries.sessionstorage.test.observer.FakeSessionObserver
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.services.appnavstate.api.AppNavigationState
import io.element.android.services.appnavstate.api.AppNavigationStateService
import io.element.android.services.appnavstate.test.FakeAppNavigationStateService
Expand Down Expand Up @@ -508,6 +509,7 @@ fun TestScope.createDefaultNotificationDrawerManager(
sessionStore: SessionStore = InMemorySessionStore(),
enterpriseService: EnterpriseService = FakeEnterpriseService(),
sessionObserver: SessionObserver = FakeSessionObserver(),
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
): DefaultNotificationDrawerManager {
return DefaultNotificationDrawerManager(
notificationDisplayer = notificationDisplayer,
Expand All @@ -521,6 +523,7 @@ fun TestScope.createDefaultNotificationDrawerManager(
),
enterpriseService = enterpriseService,
sessionStore = sessionStore,
analyticsService = analyticsService,
),
appNavigationStateService = appNavigationStateService,
coroutineScope = backgroundScope,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import io.element.android.libraries.push.impl.notifications.fixtures.anInviteNot
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
import io.element.android.libraries.sessionstorage.api.SessionStore
import io.element.android.libraries.sessionstorage.test.InMemorySessionStore
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import kotlinx.coroutines.test.runTest
Expand Down Expand Up @@ -122,9 +123,11 @@ fun createNotificationRenderer(
notificationDataFactory: NotificationDataFactory = FakeNotificationDataFactory(),
enterpriseService: EnterpriseService = FakeEnterpriseService(),
sessionStore: SessionStore = InMemorySessionStore(),
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
) = NotificationRenderer(
notificationDisplayer = notificationDisplayer,
notificationDataFactory = notificationDataFactory,
enterpriseService = enterpriseService,
sessionStore = sessionStore,
analyticsService = analyticsService,
)
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushSto
import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.FakePushClientSecret
import io.element.android.libraries.workmanager.api.WorkManagerRequest
import io.element.android.libraries.workmanager.test.FakeWorkManagerScheduler
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider
import io.element.android.services.toolbox.test.systemclock.FakeSystemClock
import io.element.android.tests.testutils.lambda.any
Expand Down Expand Up @@ -683,6 +684,7 @@ class DefaultPushHandlerTest {
syncOnNotifiableEvent: SyncOnNotifiableEvent = SyncOnNotifiableEvent {},
featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(initialState = mapOf(FeatureFlags.SyncNotificationsWithWorkManager.key to false)),
workManagerScheduler: FakeWorkManagerScheduler = FakeWorkManagerScheduler(),
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
): DefaultPushHandler {
return DefaultPushHandler(
onNotifiableEventReceived = FakeOnNotifiableEventReceived(onNotifiableEventsReceived),
Expand Down Expand Up @@ -715,6 +717,7 @@ class DefaultPushHandlerTest {
),
syncOnNotifiableEvent = syncOnNotifiableEvent,
featureFlagService = featureFlagService,
analyticsService = analyticsService,
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import io.element.android.libraries.push.test.notifications.FakeNotificationReso
import io.element.android.libraries.workmanager.api.WorkManagerRequest
import io.element.android.libraries.workmanager.api.di.MetroWorkerFactory
import io.element.android.libraries.workmanager.test.FakeWorkManagerScheduler
import io.element.android.services.analytics.test.FakeAnalyticsService
import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceTimeBy
Expand Down Expand Up @@ -168,6 +168,7 @@ class FetchNotificationWorkerTest {
),
workManagerScheduler: FakeWorkManagerScheduler = FakeWorkManagerScheduler(),
syncOnNotifiableEvent: SyncOnNotifiableEvent = SyncOnNotifiableEvent {},
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
) = FetchNotificationsWorker(
workerParams = createWorkerParams(workDataOf("requests" to input)),
context = InstrumentationRegistry.getInstrumentation().context,
Expand All @@ -176,9 +177,9 @@ class FetchNotificationWorkerTest {
queue = queue,
workManagerScheduler = workManagerScheduler,
syncOnNotifiableEvent = syncOnNotifiableEvent,
coroutineDispatchers = testCoroutineDispatchers(),
workerDataConverter = SyncNotificationsWorkerDataConverter(DefaultJsonProvider()),
buildVersionSdkIntProvider = FakeBuildVersionSdkIntProvider(33),
analyticsService = analyticsService,
)

private fun TestScope.createWorkerParams(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ sealed class AnalyticsLongRunningTransaction(
data object LoadJoinedRoomFlow : AnalyticsLongRunningTransaction("Load joined room UI", "ui.load")
data object LoadMessagesUi : AnalyticsLongRunningTransaction("Load messages UI", "ui.load")
data object DisplayFirstTimelineItems : AnalyticsLongRunningTransaction("Get and display first timeline items", null)
data class PushToNotification(val eventId: String) : AnalyticsLongRunningTransaction(AnalyticsTransactions.pushToNotification)
data class PushToWorkManager(val eventId: String) : AnalyticsLongRunningTransaction("Push to WorkManager")
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,12 @@ fun AnalyticsService.cancelLongRunningTransaction(
fun AnalyticsService.finishLongRunningTransaction(
longRunningTransaction: AnalyticsLongRunningTransaction,
action: (AnalyticsTransaction) -> Unit = {},
) {
removeLongRunningTransaction(longRunningTransaction)?.let {
): Boolean {
return removeLongRunningTransaction(longRunningTransaction)?.let {
action(it)
it.finish()
}
true
} ?: false
}

inline fun <T> AnalyticsService.inBridgeSdkSpan(parentTraceId: String?, block: (AnalyticsSdkSpan) -> T): T {
Expand Down
1 change: 1 addition & 0 deletions services/analytics/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ dependencies {
testImplementation(projects.services.analytics.test)
testImplementation(projects.services.analyticsproviders.test)
testImplementation(projects.services.appnavstate.test)
testImplementation(projects.features.networkmonitor.test)
testImplementation(projects.services.toolbox.test)
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ object AnalyticsTransactions {
operation = "ux",
description = "Send to sent state in timeline",
)

val pushToNotification = TransactionDefinition(
name = "Push to notification",
operation = "push_to_notification",
description = "Time from receiving a push notification until it's displayed",
)
}

data class TransactionDefinition(
Expand Down
Loading