Add more performance checks #5767

jmartinesp · 2025-11-19T16:31:19Z

Content

Ensure long running transaction methods return an actual transaction we can modify. Also add AnalyticsService.getLongRunningTransaction and change AnalyticsService.stopLongRunningTransaction to removeLongRunningTransaction.
Then add AnalyticsRoomListStateWatcher to measure how long it takes the app to receive the up to date room list after coming back from a background state.
Add also AnalyticsColdStartWatcher to measure how long it takes the app to display the cached room list from a cold start.
Add NotificationTapOpensTimeline to check how long it takes for a notification tap to open a room.
Add OpenRoom to check how long it takes to open a room and display the initial timeline events.
Add transaction grouping so we can have trace trees like:

Motivation and context

Add some more metrics.

Tests

Testing any of the cases added above should add those events to Sentry.

Tested devices

Physical
Emulator
OS version(s): 16

Checklist

Changes have been tested on an Android device or Android emulator with API 24
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request title will be used in the release note, it clearly define what will change for the user
Pull request includes screenshots or videos if containing UI changes
You've made a self review of your PR

github-actions · 2025-11-19T16:36:33Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/WnNNm6

codecov · 2025-11-19T16:57:20Z

Codecov Report

❌ Patch coverage is 68.80000% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.47%. Comparing base (29d4dfb) to head (a7a4887).
⚠️ Report is 26 commits behind head on develop.

Files with missing lines	Patch %	Lines
...services/analytics/impl/DefaultAnalyticsService.kt	0.00%	6 Missing ⚠️
...roid/libraries/matrix/impl/room/RustRoomFactory.kt	0.00%	5 Missing ⚠️
...l/watchers/DefaultAnalyticsRoomListStateWatcher.kt	84.61%	1 Missing and 3 partials ⚠️
...ics/noop/watchers/NoopAnalyticsColdStartWatcher.kt	0.00%	4 Missing ⚠️
...id/services/analytics/noop/NoopAnalyticsService.kt	0.00%	3 Missing ⚠️
...noop/watchers/NoopAnalyticsRoomListStateWatcher.kt	0.00%	3 Missing ⚠️
...ticsproviders/sentry/SentryAnalyticsTransaction.kt	0.00%	3 Missing ⚠️
...s/analytics/api/AnalyticsLongRunningTransaction.kt	71.42%	2 Missing ⚠️
.../impl/watchers/DefaultAnalyticsColdStartWatcher.kt	89.47%	0 Missing and 2 partials ⚠️
...ics/test/watchers/FakeAnalyticsColdStartWatcher.kt	50.00%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5767      +/-   ##
===========================================
- Coverage    79.47%   79.47%   -0.01%     
===========================================
  Files         2447     2453       +6     
  Lines        65715    65824     +109     
  Branches      8372     8385      +13     
===========================================
+ Hits         52228    52311      +83     
- Misses       10495    10515      +20     
- Partials      2992     2998       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bmarty

Thanks! A few remarks, I'll let Ganfra do the final review

bmarty · 2025-11-24T08:12:35Z

app/src/main/kotlin/io/element/android/x/intent/DefaultIntentProvider.kt

        return Intent(context, MainActivity::class.java).apply {
            action = Intent.ACTION_VIEW
            data = deepLinkCreator.create(sessionId, roomId, threadId, eventId).toUri()
+            putExtra("from_notification", true)


Maybe create a const for the String?
Also this code is also invoked by DefaultNotificationConversationService so the entry point will not be a notification. But I guess we do not want to distinguish this case for now.

Handled in 5bd4c4c. This should also ensure the value is only included for notifications.

bmarty · 2025-11-24T08:15:17Z

appnav/src/main/kotlin/io/element/android/appnav/NotLoggedInFlowNode.kt


    override fun onBuilt() {
        super.onBuilt()
+        analyticsColdStartWatcher.whenLoggingIn()


Note: This will be also invoked when adding an account.

That's fine, this just wants to cancel the cold start time check. By the time you try to add an account, the check should already have finished or been canceled.

bmarty · 2025-11-24T08:19:04Z

features/home/impl/build.gradle.kts


 dependencies {
    implementation(projects.appconfig)
+    implementation(projects.appnav)


I'd move AnalyticsColdStartWatcher to the analytics api module to avoid adding this dependency.

Also FakeAnalyticsColdStartWatcher can be moved to :analytics:test

Done in bfbe01e.

bmarty · 2025-11-24T08:27:38Z

...tlin/io/element/android/services/analytics/impl/watchers/DefaultAnalyticsColdStartWatcher.kt

+    override fun whenLoggingIn() {
+        if (isColdStart.getAndSet(false)) {
+            analyticsService.removeLongRunningTransaction(ColdStartUntilCachedRoomList)
+            Timber.d("Canceled cold start check: user is logging in")


Neat: it seems that this is the only case where the returned value of removeLongRunningTransaction is not finished. Maybe add a parameter to removeLongRunningTransactionlike andFinish: Boolean = true? This parameter will be false here.

I'd rather just return the transaction so the developers can do what they need to them, including extra parameters if needed. If we add this, we'd be returning a transaction that's already finished, so it makes no sense to return it at all since you won't be able to alter what's sent to Sentry.

You make a point!
But maybe for clarity the API could be (or we could have extension on AnalyticsService like):

// Will just remove the transaction analyticsService.cancelLongRunningTransaction(...) // Will remove the transaction and finish it analyticsService.finishLongRunningTransaction(...)

Added these helper methods in 1a79bb3.

bmarty · 2025-11-24T08:35:05Z

...tlin/io/element/android/services/analytics/impl/watchers/DefaultAnalyticsColdStartWatcher.kt

+                    }
+                }
+            }
+            .catch { Timber.w(it.message) }


There is a warning here, maybe change to:

Suggested change

.catch { Timber.w(it.message) }

.catch { Timber.w(it, "Error") }

Maybe the error above was only to debug?

That's actually what I wanted, since I don't expect any actual error. I just emit the error above to cancel the flow, then use the log to just notify we've done that. I can't remember if there's a better way to cancel a hot flow from inside the flow, to be honest.

OK, the error will happen when the user gives their consent later from the settings, no?
I am running the code to check this.

EDIT: so yes, it's the case. An error will occur (and will be caught) when the user opt-in for analytics in the settings.

jmartinesp · 2025-11-25T20:49:50Z

I was busy with some Rust these past couple of days, but I'll continue working on this tomorrow. @ganfra having your review too before I make any changes would be nice, so I don't have to change the code twice.

ganfra · 2025-11-26T13:10:14Z

appnav/src/main/kotlin/io/element/android/appnav/analytics/AnalyticsRoomListStateWatcher.kt

+            return
+        }
+
+        val coroutineScope = CoroutineScope(sessionCoroutineScope.coroutineContext + dispatchers.computation)


Use sessionCoroutineScope.childScope?
Also I'd prefer we create the child scope in the constructor and use an atomic boolean to know if it's already started.

So we'd store the jobs for the launched flows in another variable(s) so we can cancel them on stop()? I used the nullable CoroutineScope because cancelling it will also cancel all their children.

Used childScope in bfbe01e.

jmartinesp · 2025-11-27T11:52:28Z

Some build/quality checks are failing but those are working locally. Maybe it's a cache issue?

Rebasing in case it helps with the cache issue 🤞 .

Also add `AnalyticsService.getLongRunningTransaction` and change `AnalyticsService.stopLongRunningTransaction` to `removeLongRunningTransaction`.

We want to measure how long it takes the SDK to update the room list when the app comes back from being in background. Note we don't want to check this in cold starts, only warm ones.

…om list is displayed

…view

…xtras` Allow `IntentProvider` to receive extras and `PendingIntentFactory` to send them.

…vice.finishLongRunningTransaction`

…creating and cancelling the same one

…actually rendered the timeline event we need to focus on

sonarqubecloud · 2025-11-28T10:38:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ganfra

Thanks, LGTM!

jmartinesp added the PR-Misc For other changes label Nov 19, 2025

jmartinesp force-pushed the misc/add-warm-room-list-performance-check branch 2 times, most recently from e40ef51 to 1ebe4f8 Compare November 20, 2025 11:46

jmartinesp changed the title ~~Add check to measure room list update time in warm app starts~~ Add more performance checks Nov 20, 2025

jmartinesp force-pushed the misc/add-warm-room-list-performance-check branch from 1ebe4f8 to d069e33 Compare November 20, 2025 11:47

jmartinesp marked this pull request as ready for review November 20, 2025 12:04

jmartinesp requested a review from a team as a code owner November 20, 2025 12:04

jmartinesp requested review from ganfra and removed request for a team November 20, 2025 12:04

bmarty reviewed Nov 24, 2025

View reviewed changes

ganfra reviewed Nov 26, 2025

View reviewed changes

jmartinesp requested a review from bmarty November 27, 2025 11:29

jmartinesp added 10 commits November 27, 2025 13:07

Have long running analytic transactions return actual transactions.

1d7d32a

Also add `AnalyticsService.getLongRunningTransaction` and change `AnalyticsService.stopLongRunningTransaction` to `removeLongRunningTransaction`.

Add a 'warm' room list performance check:

b03bd44

We want to measure how long it takes the SDK to update the room list when the app comes back from being in background. Note we don't want to check this in cold starts, only warm ones.

Add a performance check for notification tap -> populated timeline time

c2a9aa4

Add another performance check for cold start time until the cached ro…

5c4e835

…om list is displayed

Add OpenRoom long running analytic transaction

f552512

Add transaction trees for opening a room so we can have a nice trace …

6358990

…view

Add tests for DefaultAnalyticsColdStartWatcher

693ca17

Move analytic watchers to :services:analytics

f4225cb

Extract from_notification extra to a constant in `IntentNavigationE…

d977f11

…xtras` Allow `IntentProvider` to receive extras and `PendingIntentFactory` to send them.

Add AnalyticsService.cancelLongRunningTransaction and `AnalyticsSer…

faa4cb5

…vice.finishLongRunningTransaction`

jmartinesp force-pushed the misc/add-warm-room-list-performance-check branch from 1a79bb3 to faa4cb5 Compare November 27, 2025 12:08

jmartinesp added 3 commits November 27, 2025 13:29

Add Noop variants for the new watcher interfaces in :analytics:api

93927f2

Reuse coroutine in DefaultAnalyticsRoomListStateWatcher instead of …

10c0120

…creating and cancelling the same one

Make sure we don't finish NotificationTapOpensTimeline until we've …

a7a4887

…actually rendered the timeline event we need to focus on

ganfra approved these changes Nov 28, 2025

View reviewed changes

jmartinesp merged commit f265a47 into develop Nov 28, 2025
29 of 32 checks passed

jmartinesp deleted the misc/add-warm-room-list-performance-check branch November 28, 2025 13:15

	.catch { Timber.w(it.message) }
	.catch { Timber.w(it, "Error") }

Add more performance checks #5767

Add more performance checks #5767

Uh oh!

Conversation

jmartinesp commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Motivation and context

Tests

Tested devices

Checklist

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmartinesp commented Nov 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmartinesp Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmartinesp commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Nov 28, 2025

Quality Gate passed

Uh oh!

ganfra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmartinesp commented Nov 19, 2025 •

edited

Loading

github-actions bot commented Nov 19, 2025 •

edited

Loading

codecov bot commented Nov 19, 2025 •

edited

Loading

bmarty Nov 24, 2025 •

edited

Loading

bmarty Nov 24, 2025 •

edited

Loading

jmartinesp Nov 27, 2025 •

edited

Loading

jmartinesp commented Nov 27, 2025 •

edited

Loading