Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

001 fix(ttd): Add TimeToDisplay integration to force fetch the data #4669

Merged
merged 41 commits into from
Mar 26, 2025

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Mar 19, 2025

📢 Type of change

  • Bugfix
  • Enhancement
  • Refactoring

📜 Description

This PR changes how TTID and TTFD data are retrieved from the native layer. The new approach relies on the retrieval of the data during the transaction processing. This ensures the data are added to the transaction if recorded on the native layer. This is more robust than reading data from the event emitted on the native layer which could be received in the JS after the transaction ended and thus the data are lost.

How often the data are lost depends on a given application and if transactions are often ended early or ended manually.

The timestamps are linked to the active span id (the transaction span id) and saved in a map with a max size to avoid leaking memory. If the timestamp would not be retrieved by JS (should only happen in unexpected scenarios).

💚 How did you test it?

sample app, e2e tests

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

…eady seen routes

- Introduced `enableTimeToInitialDisplayForPreloadedRoutes` option to the React Navigation integration.
- Updated logic to measure Time to Initial Display for routes that have already been seen.
- Added tests to verify functionality for preloaded routes in the tracing module.
@krystofwoldrich krystofwoldrich changed the title wip!: Add TimeToDisplay integration to force fetch the data fix(ttd): Add TimeToDisplay integration to force fetch the data Mar 19, 2025
Copy link
Contributor

github-actions bot commented Mar 19, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.52 ms 1239.90 ms 12.38 ms
Size 2.63 MiB 3.77 MiB 1.13 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0677344+dirty 1276.70 ms 1300.07 ms 23.37 ms
5f03ae9+dirty 1232.29 ms 1230.92 ms -1.37 ms
5571a20+dirty 1203.57 ms 1204.57 ms 1.00 ms
2ec71da+dirty 1225.85 ms 1231.57 ms 5.72 ms
d56b7dc+dirty 1225.20 ms 1233.59 ms 8.39 ms
7fd512a+dirty 1218.78 ms 1217.25 ms -1.53 ms
5446992+dirty 1273.28 ms 1276.68 ms 3.40 ms
b677956+dirty 1221.47 ms 1217.90 ms -3.57 ms
c314a21+dirty 1206.74 ms 1220.06 ms 13.32 ms
1c65324+dirty 1235.17 ms 1235.08 ms -0.09 ms

App size

Revision Plain With Sentry Diff
0677344+dirty 2.36 MiB 2.85 MiB 496.81 KiB
5f03ae9+dirty 2.63 MiB 3.68 MiB 1.05 MiB
5571a20+dirty 2.36 MiB 2.92 MiB 569.93 KiB
2ec71da+dirty 2.36 MiB 3.13 MiB 784.66 KiB
d56b7dc+dirty 2.63 MiB 3.75 MiB 1.12 MiB
7fd512a+dirty 2.36 MiB 3.10 MiB 753.35 KiB
5446992+dirty 2.36 MiB 2.88 MiB 531.94 KiB
b677956+dirty 2.63 MiB 3.75 MiB 1.12 MiB
c314a21+dirty 2.63 MiB 3.75 MiB 1.12 MiB
1c65324+dirty 2.36 MiB 3.04 MiB 698.64 KiB

Previous results on branch: kw-force-fetch-ttd

Startup times

Revision Plain With Sentry Diff
8a988e7+dirty 1228.35 ms 1240.64 ms 12.29 ms

App size

Revision Plain With Sentry Diff
8a988e7+dirty 2.63 MiB 3.77 MiB 1.13 MiB

Copy link
Contributor

github-actions bot commented Mar 19, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1212.59 ms 1227.76 ms 15.17 ms
Size 3.19 MiB 4.33 MiB 1.14 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0677344+dirty 1252.52 ms 1254.08 ms 1.56 ms
5f03ae9+dirty 1237.79 ms 1241.02 ms 3.23 ms
5571a20+dirty 1228.09 ms 1233.45 ms 5.36 ms
2ec71da+dirty 1230.29 ms 1239.50 ms 9.21 ms
d56b7dc+dirty 1211.17 ms 1224.25 ms 13.08 ms
7fd512a+dirty 1239.41 ms 1241.50 ms 2.09 ms
5446992+dirty 1249.94 ms 1254.80 ms 4.86 ms
b677956+dirty 1224.30 ms 1239.53 ms 15.23 ms
c314a21+dirty 1225.67 ms 1237.90 ms 12.22 ms
1c65324+dirty 1239.71 ms 1239.86 ms 0.15 ms

App size

Revision Plain With Sentry Diff
0677344+dirty 2.92 MiB 3.41 MiB 500.94 KiB
5f03ae9+dirty 3.19 MiB 4.25 MiB 1.06 MiB
5571a20+dirty 2.92 MiB 3.48 MiB 575.54 KiB
2ec71da+dirty 2.92 MiB 3.69 MiB 791.06 KiB
d56b7dc+dirty 3.19 MiB 4.32 MiB 1.13 MiB
7fd512a+dirty 2.92 MiB 3.66 MiB 758.62 KiB
5446992+dirty 2.92 MiB 3.44 MiB 535.26 KiB
b677956+dirty 3.19 MiB 4.32 MiB 1.13 MiB
c314a21+dirty 3.19 MiB 4.32 MiB 1.13 MiB
1c65324+dirty 2.92 MiB 3.61 MiB 705.56 KiB

Previous results on branch: kw-force-fetch-ttd

Startup times

Revision Plain With Sentry Diff
8a988e7+dirty 1235.84 ms 1225.98 ms -9.86 ms

App size

Revision Plain With Sentry Diff
8a988e7+dirty 3.19 MiB 4.33 MiB 1.14 MiB

Copy link
Contributor

github-actions bot commented Mar 21, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 433.67 ms 465.84 ms 32.17 ms
Size 7.15 MiB 8.40 MiB 1.24 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b6da94a+dirty 371.63 ms 373.76 ms 2.12 ms
43c72ff+dirty 373.89 ms 368.56 ms -5.33 ms
d2c32bb+dirty 445.45 ms 497.85 ms 52.41 ms
a989877+dirty 383.04 ms 400.92 ms 17.88 ms
27ef4ee+dirty 296.71 ms 351.00 ms 54.29 ms
9433f35+dirty 265.50 ms 336.08 ms 70.58 ms
bab7feb+dirty 377.47 ms 415.60 ms 38.13 ms
416f465+dirty 407.26 ms 451.31 ms 44.05 ms
52c0562+dirty 401.23 ms 435.65 ms 34.42 ms
0c98663+dirty 385.91 ms 403.82 ms 17.91 ms

App size

Revision Plain With Sentry Diff
b6da94a+dirty 7.15 MiB 8.40 MiB 1.24 MiB
43c72ff+dirty 7.15 MiB 8.39 MiB 1.23 MiB
d2c32bb+dirty 7.15 MiB 8.35 MiB 1.20 MiB
a989877+dirty 7.15 MiB 8.35 MiB 1.20 MiB
27ef4ee+dirty 7.15 MiB 8.08 MiB 959.49 KiB
9433f35+dirty 7.15 MiB 8.08 MiB 959.34 KiB
bab7feb+dirty 7.15 MiB 8.38 MiB 1.23 MiB
416f465+dirty 7.15 MiB 8.37 MiB 1.22 MiB
52c0562+dirty 7.15 MiB 8.39 MiB 1.24 MiB
0c98663+dirty 7.15 MiB 8.39 MiB 1.23 MiB

Previous results on branch: kw-force-fetch-ttd

Startup times

Revision Plain With Sentry Diff
8a988e7+dirty 412.33 ms 414.21 ms 1.88 ms

App size

Revision Plain With Sentry Diff
8a988e7+dirty 7.15 MiB 8.40 MiB 1.24 MiB

Copy link
Contributor

github-actions bot commented Mar 24, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 432.02 ms 440.68 ms 8.66 ms
Size 17.75 MiB 20.13 MiB 2.38 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
62a750b 395.96 ms 423.36 ms 27.41 ms
0d3e677 422.82 ms 411.90 ms -10.92 ms
6e8584e 447.10 ms 474.71 ms 27.61 ms
83f6f6c 418.83 ms 410.94 ms -7.89 ms
1faf8e3 449.18 ms 432.08 ms -17.10 ms
c314a21 446.24 ms 439.98 ms -6.26 ms
f06c879 408.41 ms 424.54 ms 16.13 ms
cdf2f33 469.46 ms 462.17 ms -7.29 ms
ae7b03d 428.82 ms 412.33 ms -16.49 ms
9a3ca65+dirty 326.93 ms 330.14 ms 3.21 ms

App size

Revision Plain With Sentry Diff
62a750b 17.73 MiB 19.93 MiB 2.20 MiB
0d3e677 17.74 MiB 20.07 MiB 2.34 MiB
6e8584e 17.73 MiB 19.86 MiB 2.12 MiB
83f6f6c 17.74 MiB 20.09 MiB 2.35 MiB
1faf8e3 17.74 MiB 20.08 MiB 2.34 MiB
c314a21 17.75 MiB 20.12 MiB 2.37 MiB
f06c879 17.73 MiB 19.85 MiB 2.12 MiB
cdf2f33 17.74 MiB 20.08 MiB 2.34 MiB
ae7b03d 17.75 MiB 20.11 MiB 2.37 MiB
9a3ca65+dirty 17.73 MiB 20.04 MiB 2.31 MiB

Previous results on branch: kw-force-fetch-ttd

Startup times

Revision Plain With Sentry Diff
8a988e7 474.40 ms 461.68 ms -12.72 ms

App size

Revision Plain With Sentry Diff
8a988e7 17.75 MiB 20.13 MiB 2.38 MiB

@krystofwoldrich krystofwoldrich marked this pull request as ready for review March 24, 2025 10:05
@krystofwoldrich krystofwoldrich marked this pull request as draft March 24, 2025 10:33
@krystofwoldrich krystofwoldrich marked this pull request as ready for review March 24, 2025 14:18
Base automatically changed from kw-record-ttd-on-focus to main March 25, 2025 11:36
@krystofwoldrich krystofwoldrich changed the title fix(ttd): Add TimeToDisplay integration to force fetch the data 001 fix(ttd): Add TimeToDisplay integration to force fetch the data Mar 25, 2025
Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @krystofwoldrich 🎸
Apart from duplicate declaration issue (I've tested without it) everything LGTM and the app worked as expected on both platforms recording the ttid/ttfd.
Thank you for adding extensive test cases covering this change 🙇

@krystofwoldrich krystofwoldrich requested a review from antonis March 26, 2025 12:43
Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@krystofwoldrich krystofwoldrich enabled auto-merge (squash) March 26, 2025 14:17
@krystofwoldrich krystofwoldrich merged commit 9de3d45 into main Mar 26, 2025
62 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-force-fetch-ttd branch March 26, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants