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

fix(test): Flaky E2E Android Metrics App Start Time #4292

Closed
wants to merge 38 commits into from

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented Nov 19, 2024

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

⚠️ Depends on: getsentry/action-app-sdk-overhead-metrics#23

📜 Description

Since the main activity of the application is displayed before RN start executing and thus before the JS SDK inits this PR:

💡 Motivation and Context

Fixes #3413

💚 How did you test it?

CI

📝 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

#skip-changelog

Copy link
Contributor

github-actions bot commented Nov 19, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1221.79 ms 1229.43 ms 7.64 ms
Size 2.36 MiB 3.12 MiB 779.44 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
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
5bb8d5f+dirty 1235.47 ms 1237.39 ms 1.92 ms
4cc5c27+dirty 1211.45 ms 1214.60 ms 3.16 ms
b95b8af+dirty 1221.39 ms 1228.52 ms 7.13 ms
dadc233+dirty 1223.20 ms 1236.88 ms 13.68 ms
1faf8e3+dirty 1214.87 ms 1222.83 ms 7.97 ms
7bc4d75+dirty 1233.40 ms 1229.56 ms -3.83 ms
1332acb+dirty 1230.53 ms 1234.54 ms 4.01 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
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
5bb8d5f+dirty 2.36 MiB 2.92 MiB 570.22 KiB
4cc5c27+dirty 2.36 MiB 3.04 MiB 698.52 KiB
b95b8af+dirty 2.36 MiB 3.14 MiB 793.32 KiB
dadc233+dirty 2.36 MiB 2.84 MiB 486.85 KiB
1faf8e3+dirty 2.36 MiB 3.08 MiB 736.75 KiB
7bc4d75+dirty 2.36 MiB 3.10 MiB 752.58 KiB
1332acb+dirty 2.36 MiB 3.11 MiB 759.86 KiB

Copy link
Contributor

github-actions bot commented Nov 19, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1228.53 ms 1234.81 ms 6.28 ms
Size 2.92 MiB 3.69 MiB 790.63 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
c398f67+dirty 1227.31 ms 1230.00 ms 2.69 ms
5bb8d5f+dirty 1215.04 ms 1217.52 ms 2.48 ms
4cc5c27+dirty 1220.43 ms 1215.13 ms -5.30 ms
b95b8af+dirty 1235.60 ms 1242.06 ms 6.46 ms
dadc233+dirty 1266.52 ms 1282.55 ms 16.03 ms
1faf8e3+dirty 1223.38 ms 1220.56 ms -2.82 ms
7bc4d75+dirty 1222.13 ms 1216.39 ms -5.74 ms
1332acb+dirty 1243.98 ms 1241.12 ms -2.86 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
c398f67+dirty 2.92 MiB 3.60 MiB 701.89 KiB
5bb8d5f+dirty 2.92 MiB 3.48 MiB 575.85 KiB
4cc5c27+dirty 2.92 MiB 3.61 MiB 705.47 KiB
b95b8af+dirty 2.92 MiB 3.69 MiB 794.16 KiB
dadc233+dirty 2.92 MiB 3.40 MiB 492.53 KiB
1faf8e3+dirty 2.92 MiB 3.64 MiB 742.61 KiB
7bc4d75+dirty 2.92 MiB 3.66 MiB 757.15 KiB
1332acb+dirty 2.92 MiB 3.67 MiB 772.45 KiB

@antonis
Copy link
Collaborator Author

antonis commented Nov 20, 2024

Though the AppReadyActivity is launched I failed to detect it with Appium getsentry/action-app-sdk-overhead-metrics#23. I'll stop for now and revisit the issue later.

@antonis
Copy link
Collaborator Author

antonis commented Nov 20, 2024

A better simpler approach could be to just emit the app ready timestamp from RN and then parse it from appium to calculate the total load time.

2024-11-20 20:46:18.098 16742-16837 ReactNativeJS           com.testappplain                     I  'AppReadyTimestampMs:', 1732128378092

@antonis
Copy link
Collaborator Author

antonis commented Nov 21, 2024

A better simpler approach could be to just emit the app ready timestamp from RN and then parse it from appium to calculate the total load time.

This failed too since capturing the logs on CI did not work (metric lib branch https://github.com/antonis/action-app-sdk-overhead-metrics/commits/antonis/add-log-timestamp-regex/).

I'm stopping this effort for now.

@antonis antonis closed this Nov 21, 2024
@vaind
Copy link
Collaborator

vaind commented Nov 21, 2024

A better simpler approach could be to just emit the app ready timestamp from RN and then parse it from appium to calculate the total load time.

Yes, this looks like a good idea. We may not even need to post the timestamp, just any message and we can grab log timestamps from logcat

@antonis
Copy link
Collaborator Author

antonis commented Nov 21, 2024

Yes, this looks like a good idea.

Thank you for iterating on this @vaind 🙇

What I tried with this approach is:

This didn't work either since the log wasn't emitted or captured on ci (it worked on my machine with a release build 😅).

I guess the reason might be similar to why capturing the 2nd activity did not work.

We may not even need to post the timestamp, just any message and we can grab log timestamps from logcat

Good idea. This can simplify the implementation further.

Any ideas on what I might be missing are more than welcome 🙇

@antonis antonis reopened this Nov 21, 2024
@krystofwoldrich
Copy link
Member

It looks like we have not find any reliable improvement to the app start metrics measurements.

Should we close this for now?

@antonis
Copy link
Collaborator Author

antonis commented Jan 16, 2025

It looks like we have not find any reliable improvement to the app start metrics measurements.

Yep, I'm still facing issue with reliably detecting the app initialisation after JS initialises

Should we close this for now?

Good idea 👍 We can reopen or create a new PR when we have a solution.

@antonis antonis closed this Jan 16, 2025
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.

Flaky E2E Metrics App Start Time
3 participants