-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
feat(profiling): Add RN/Android mixed profiles #3397
Conversation
…symbolicated-profiles
…symbolicated-profiles
…symbolicated-profiles
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
12427f4 | 393.69 ms | 414.84 ms | 21.14 ms |
5446992 | 403.40 ms | 426.70 ms | 23.30 ms |
dadc233+dirty | 333.78 ms | 343.94 ms | 10.16 ms |
22e31b6 | 396.48 ms | 419.64 ms | 23.16 ms |
f06c879 | 408.41 ms | 424.54 ms | 16.13 ms |
b1e8712 | 462.11 ms | 465.71 ms | 3.60 ms |
34aba08 | 328.10 ms | 342.84 ms | 14.74 ms |
70caa60+dirty | 299.00 ms | 321.02 ms | 22.02 ms |
e73f4ed+dirty | 332.96 ms | 354.33 ms | 21.37 ms |
2534337 | 394.15 ms | 415.12 ms | 20.97 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
12427f4 | 17.73 MiB | 19.85 MiB | 2.12 MiB |
5446992 | 17.73 MiB | 19.85 MiB | 2.12 MiB |
dadc233+dirty | 17.73 MiB | 19.75 MiB | 2.02 MiB |
22e31b6 | 17.73 MiB | 19.84 MiB | 2.10 MiB |
f06c879 | 17.73 MiB | 19.85 MiB | 2.12 MiB |
b1e8712 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
34aba08 | 17.73 MiB | 19.80 MiB | 2.07 MiB |
70caa60+dirty | 17.73 MiB | 19.75 MiB | 2.01 MiB |
e73f4ed+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
2534337 | 17.73 MiB | 19.84 MiB | 2.11 MiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
abb7058+dirty | 320.78 ms | 324.08 ms | 3.30 ms |
dadc233+dirty | 363.19 ms | 370.37 ms | 7.18 ms |
ad6c299+dirty | 336.47 ms | 362.89 ms | 26.42 ms |
d361d38+dirty | 257.72 ms | 318.76 ms | 61.04 ms |
e2b64fe+dirty | 258.82 ms | 304.26 ms | 45.44 ms |
e5c9b8b+dirty | 335.40 ms | 360.06 ms | 24.67 ms |
457e29f+dirty | 591.49 ms | 612.96 ms | 21.47 ms |
2534337+dirty | 597.14 ms | 665.04 ms | 67.90 ms |
70caa60+dirty | 308.83 ms | 393.06 ms | 84.23 ms |
5446992+dirty | 371.61 ms | 390.00 ms | 18.39 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
abb7058+dirty | 7.15 MiB | 8.10 MiB | 980.40 KiB |
dadc233+dirty | 7.15 MiB | 8.04 MiB | 910.84 KiB |
ad6c299+dirty | 7.15 MiB | 8.04 MiB | 912.17 KiB |
d361d38+dirty | 7.15 MiB | 8.08 MiB | 959.34 KiB |
e2b64fe+dirty | 7.15 MiB | 8.07 MiB | 947.16 KiB |
e5c9b8b+dirty | 7.15 MiB | 8.10 MiB | 980.41 KiB |
457e29f+dirty | 7.15 MiB | 8.10 MiB | 981.29 KiB |
2534337+dirty | 7.15 MiB | 8.11 MiB | 988.68 KiB |
70caa60+dirty | 7.15 MiB | 8.03 MiB | 901.79 KiB |
5446992+dirty | 7.15 MiB | 8.12 MiB | 999.45 KiB |
This PR is ready to be reviewed. Merge only after Sentry UI can display the JS profile attached to the Android profile. Add note for Self hosted users to the changelog |
@krystofwoldrich on the profiling ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@krystofwoldrich at a high level it looks good.
@JonasBa can give a more detailed look at the js/ts
profiler specifically.
Let me pull in @stefanosiano as well for the android
profiler.
📢 Type of change
📜 Description
This PR enabled the RN SDK to send Android Profiles with an attached SentryProfile. In this case a Hermes Profile.
The Java Module implementation depends on
sentry-java
changes:readBytesFromFile
for use in Hybrid SDKs sentry-java#3052getProguardUuid
sentry-java#3054💡 Motivation and Context
💚 How did you test it?
RN sample app
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps
Release:
readBytesFromFile
for use in Hybrid SDKs sentry-java#3052getProguardUuid
sentry-java#3054Blocked by