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

Use appLaunchedInForeground to determine invalid app start data on Android #4146

Merged
merged 10 commits into from
Oct 9, 2024

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Oct 3, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Checks appLaunchedInForeground to determine invalid app start data on Android.
Returns null in the fetchNativeAppStart promise when the app has not started in foreground. The cases that a null is returned are already handled in the code due to the iOS implementation that returns nil for invalid measurements.

Note: There is already a check in place for launches longer than 1 min.

💡 Motivation and Context

Fixes #3995

💚 How did you test it?

Added tests for the RNSentryModuleImpl.fetchNativeAppStart function logic and relied on existing unit test:

and the sentry-java tests.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

@antonis antonis changed the title Use appLaunchedInForeground to determinate invalid app start data on Android Use appLaunchedInForeground to determine invalid app start data on Android Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1211.42 ms 1222.85 ms 11.43 ms
Size 2.36 MiB 3.14 MiB 793.31 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5a22220+dirty 1209.49 ms 1220.94 ms 11.45 ms
f06c879+dirty 1252.64 ms 1259.66 ms 7.02 ms
484813b+dirty 1222.45 ms 1220.79 ms -1.66 ms
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
5bb8d5f+dirty 1235.47 ms 1237.39 ms 1.92 ms
d0bf494+dirty 1289.40 ms 1298.40 ms 9.00 ms
e73d82f+dirty 1207.52 ms 1216.73 ms 9.21 ms
e73f4ed+dirty 1243.27 ms 1244.52 ms 1.25 ms
baa882f+dirty 1218.00 ms 1227.04 ms 9.04 ms
457e29f+dirty 1253.94 ms 1269.18 ms 15.24 ms

App size

Revision Plain With Sentry Diff
5a22220+dirty 2.36 MiB 2.92 MiB 570.21 KiB
f06c879+dirty 2.36 MiB 2.88 MiB 530.42 KiB
484813b+dirty 2.36 MiB 3.08 MiB 734.18 KiB
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
5bb8d5f+dirty 2.36 MiB 2.92 MiB 570.22 KiB
d0bf494+dirty 2.36 MiB 2.83 MiB 481.15 KiB
e73d82f+dirty 2.36 MiB 3.08 MiB 734.23 KiB
e73f4ed+dirty 2.36 MiB 2.82 MiB 469.44 KiB
baa882f+dirty 2.36 MiB 3.08 MiB 731.91 KiB
457e29f+dirty 2.36 MiB 2.87 MiB 520.67 KiB

Previous results on branch: antonis/3995-appLaunchedInForeground

Startup times

Revision Plain With Sentry Diff
4dc8131+dirty 1238.65 ms 1233.29 ms -5.36 ms

App size

Revision Plain With Sentry Diff
4dc8131+dirty 2.36 MiB 3.08 MiB 734.28 KiB

Copy link
Contributor

github-actions bot commented Oct 3, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.29 ms 1227.80 ms -10.49 ms
Size 2.92 MiB 3.69 MiB 794.15 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5a22220+dirty 1246.18 ms 1249.61 ms 3.43 ms
f06c879+dirty 1285.14 ms 1285.86 ms 0.72 ms
484813b+dirty 1225.07 ms 1221.00 ms -4.07 ms
c398f67+dirty 1227.31 ms 1230.00 ms 2.69 ms
5bb8d5f+dirty 1215.04 ms 1217.52 ms 2.48 ms
d0bf494+dirty 1266.20 ms 1267.52 ms 1.32 ms
e73d82f+dirty 1231.20 ms 1228.81 ms -2.40 ms
e73f4ed+dirty 1282.90 ms 1309.30 ms 26.40 ms
baa882f+dirty 1235.48 ms 1229.02 ms -6.46 ms
457e29f+dirty 1256.71 ms 1258.50 ms 1.79 ms

App size

Revision Plain With Sentry Diff
5a22220+dirty 2.92 MiB 3.48 MiB 575.81 KiB
f06c879+dirty 2.92 MiB 3.44 MiB 533.24 KiB
484813b+dirty 2.92 MiB 3.64 MiB 740.56 KiB
c398f67+dirty 2.92 MiB 3.60 MiB 701.89 KiB
5bb8d5f+dirty 2.92 MiB 3.48 MiB 575.85 KiB
d0bf494+dirty 2.92 MiB 3.40 MiB 488.08 KiB
e73d82f+dirty 2.92 MiB 3.64 MiB 740.56 KiB
e73f4ed+dirty 2.92 MiB 3.38 MiB 475.71 KiB
baa882f+dirty 2.92 MiB 3.64 MiB 738.56 KiB
457e29f+dirty 2.92 MiB 3.43 MiB 524.75 KiB

Previous results on branch: antonis/3995-appLaunchedInForeground

Startup times

Revision Plain With Sentry Diff
4dc8131+dirty 1225.57 ms 1238.31 ms 12.75 ms

App size

Revision Plain With Sentry Diff
4dc8131+dirty 2.92 MiB 3.64 MiB 740.71 KiB

@antonis antonis marked this pull request as ready for review October 4, 2024 08:57
Copy link
Contributor

github-actions bot commented Oct 4, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 430.10 ms 485.76 ms 55.66 ms
Size 7.15 MiB 8.38 MiB 1.23 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9c48b2c+dirty 270.82 ms 321.12 ms 50.30 ms
0677344+dirty 288.40 ms 391.44 ms 103.04 ms
c398f67+dirty 315.08 ms 345.60 ms 30.52 ms
62a750b+dirty 370.78 ms 376.73 ms 5.96 ms
700cbf4+dirty 411.71 ms 485.52 ms 73.81 ms
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
5bb8d5f+dirty 356.71 ms 389.65 ms 32.94 ms
70e6261+dirty 395.08 ms 408.12 ms 13.04 ms
575f9da+dirty 337.15 ms 370.47 ms 33.32 ms
4a6664f+dirty 357.02 ms 394.91 ms 37.89 ms

App size

Revision Plain With Sentry Diff
9c48b2c+dirty 7.15 MiB 8.07 MiB 947.16 KiB
0677344+dirty 7.15 MiB 8.07 MiB 949.80 KiB
c398f67+dirty 7.15 MiB 8.21 MiB 1.07 MiB
62a750b+dirty 7.15 MiB 8.21 MiB 1.06 MiB
700cbf4+dirty 7.15 MiB 8.34 MiB 1.19 MiB
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
5bb8d5f+dirty 7.15 MiB 8.21 MiB 1.06 MiB
70e6261+dirty 7.15 MiB 8.21 MiB 1.07 MiB
575f9da+dirty 7.15 MiB 8.10 MiB 979.68 KiB
4a6664f+dirty 7.15 MiB 8.22 MiB 1.07 MiB

Previous results on branch: antonis/3995-appLaunchedInForeground

Startup times

Revision Plain With Sentry Diff
4dc8131+dirty 421.44 ms 455.98 ms 34.54 ms

App size

Revision Plain With Sentry Diff
4dc8131+dirty 7.15 MiB 8.34 MiB 1.19 MiB

Copy link
Contributor

github-actions bot commented Oct 4, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 499.02 ms 501.48 ms 2.46 ms
Size 17.73 MiB 20.07 MiB 2.33 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
e2b64fe 316.88 ms 330.23 ms 13.35 ms
b1e8712 462.11 ms 465.71 ms 3.60 ms
abb7058 370.27 ms 389.58 ms 19.31 ms
3ffcddd 302.92 ms 315.80 ms 12.88 ms
4cc5c27 460.04 ms 496.32 ms 36.28 ms
76d1baf+dirty 335.72 ms 355.52 ms 19.80 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms
d43a46b 454.22 ms 477.79 ms 23.57 ms
d0bf494+dirty 375.37 ms 395.14 ms 19.77 ms

App size

Revision Plain With Sentry Diff
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
e2b64fe 17.73 MiB 19.80 MiB 2.07 MiB
b1e8712 17.73 MiB 19.75 MiB 2.02 MiB
abb7058 17.73 MiB 19.83 MiB 2.10 MiB
3ffcddd 17.73 MiB 19.75 MiB 2.02 MiB
4cc5c27 17.73 MiB 19.95 MiB 2.21 MiB
76d1baf+dirty 17.73 MiB 20.04 MiB 2.31 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB
d43a46b 17.73 MiB 20.06 MiB 2.33 MiB
d0bf494+dirty 17.73 MiB 19.75 MiB 2.02 MiB

@antonis antonis marked this pull request as draft October 4, 2024 10:28
@antonis antonis marked this pull request as ready for review October 4, 2024 10:58
@krystofwoldrich
Copy link
Member

Thank you, I'll review this in the afternoon.

Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

looks good to me, thank you for adding the Native tests!

@antonis antonis merged commit b95b8af into main Oct 9, 2024
63 of 64 checks passed
@antonis antonis deleted the antonis/3995-appLaunchedInForeground branch October 9, 2024 06:52
@krystofwoldrich
Copy link
Member

Thank you @lucas-zimerman for the review and thanks @antonis for the impl and the tests. I bit late but looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use appLaunchedInForeground to determinate invalid app start data on Android
3 participants