-
-
Notifications
You must be signed in to change notification settings - Fork 342
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(experimental): Initialize Android SDK from json configuration #4451
feat(experimental): Initialize Android SDK from json configuration #4451
Conversation
|
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af | 439.23 ms | 427.31 ms | -11.92 ms |
b75148e | 440.04 ms | 421.36 ms | -18.68 ms |
555070f | 438.67 ms | 428.30 ms | -10.37 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af | 17.75 MiB | 20.11 MiB | 2.37 MiB |
b75148e | 17.75 MiB | 20.11 MiB | 2.37 MiB |
555070f | 17.75 MiB | 20.11 MiB | 2.37 MiB |
Previous results on branch: antonis/init-from-json
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
66c8b49 | 430.41 ms | 442.98 ms | 12.57 ms |
f74989d | 480.30 ms | 505.59 ms | 25.29 ms |
236a90a | 417.78 ms | 432.00 ms | 14.22 ms |
d0ffabe | 497.57 ms | 478.45 ms | -19.12 ms |
3db962c | 459.06 ms | 458.69 ms | -0.38 ms |
69b6731 | 475.62 ms | 477.30 ms | 1.67 ms |
e57a9d7 | 419.65 ms | 425.09 ms | 5.43 ms |
4ea1d1e | 473.92 ms | 473.94 ms | 0.02 ms |
0f6176d | 450.36 ms | 432.60 ms | -17.76 ms |
dd8554c | 443.74 ms | 455.49 ms | 11.75 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
66c8b49 | 17.75 MiB | 20.11 MiB | 2.37 MiB |
f74989d | 17.75 MiB | 20.11 MiB | 2.37 MiB |
236a90a | 17.75 MiB | 20.11 MiB | 2.37 MiB |
d0ffabe | 17.75 MiB | 20.11 MiB | 2.37 MiB |
3db962c | 17.75 MiB | 20.11 MiB | 2.37 MiB |
69b6731 | 17.75 MiB | 20.11 MiB | 2.37 MiB |
e57a9d7 | 17.75 MiB | 20.11 MiB | 2.37 MiB |
4ea1d1e | 17.75 MiB | 20.11 MiB | 2.37 MiB |
0f6176d | 17.75 MiB | 20.11 MiB | 2.37 MiB |
dd8554c | 17.75 MiB | 20.11 MiB | 2.37 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af+dirty | 1209.44 ms | 1217.13 ms | 7.70 ms |
b75148e+dirty | 1221.53 ms | 1220.85 ms | -0.68 ms |
555070f+dirty | 1213.59 ms | 1217.79 ms | 4.20 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
b75148e+dirty | 2.63 MiB | 3.69 MiB | 1.06 MiB |
555070f+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
Previous results on branch: antonis/init-from-json
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 1223.15 ms | 1219.32 ms | -3.83 ms |
ee5a152+dirty | 1233.22 ms | 1227.27 ms | -5.96 ms |
f74989d+dirty | 1220.98 ms | 1219.00 ms | -1.98 ms |
4ea1d1e+dirty | 1231.00 ms | 1231.73 ms | 0.73 ms |
e57a9d7+dirty | 1228.56 ms | 1225.33 ms | -3.23 ms |
36893e3+dirty | 1211.35 ms | 1212.33 ms | 0.97 ms |
d0ffabe+dirty | 1210.75 ms | 1212.22 ms | 1.47 ms |
66c8b49+dirty | 1225.84 ms | 1217.93 ms | -7.90 ms |
0f6176d+dirty | 1213.33 ms | 1213.11 ms | -0.22 ms |
1646f54+dirty | 1237.86 ms | 1238.08 ms | 0.22 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
ee5a152+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
f74989d+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
4ea1d1e+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
e57a9d7+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
36893e3+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
d0ffabe+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
66c8b49+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
0f6176d+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
1646f54+dirty | 2.63 MiB | 3.68 MiB | 1.05 MiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af+dirty | 1213.08 ms | 1223.82 ms | 10.73 ms |
b75148e+dirty | 1202.72 ms | 1212.04 ms | 9.32 ms |
555070f+dirty | 1223.61 ms | 1227.57 ms | 3.96 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
208f4af+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
b75148e+dirty | 3.19 MiB | 4.25 MiB | 1.07 MiB |
555070f+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
Previous results on branch: antonis/init-from-json
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 1244.61 ms | 1246.22 ms | 1.61 ms |
ee5a152+dirty | 1209.06 ms | 1204.49 ms | -4.57 ms |
f74989d+dirty | 1246.20 ms | 1245.12 ms | -1.08 ms |
4ea1d1e+dirty | 1240.67 ms | 1246.71 ms | 6.04 ms |
e57a9d7+dirty | 1202.98 ms | 1203.71 ms | 0.73 ms |
36893e3+dirty | 1231.54 ms | 1231.40 ms | -0.15 ms |
d0ffabe+dirty | 1219.33 ms | 1223.08 ms | 3.76 ms |
66c8b49+dirty | 1226.60 ms | 1226.08 ms | -0.52 ms |
0f6176d+dirty | 1230.90 ms | 1238.56 ms | 7.66 ms |
1646f54+dirty | 1217.25 ms | 1210.30 ms | -6.95 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 3.19 MiB | 4.25 MiB | 1.07 MiB |
ee5a152+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
f74989d+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
4ea1d1e+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
e57a9d7+dirty | 3.19 MiB | 4.25 MiB | 1.07 MiB |
36893e3+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
d0ffabe+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
66c8b49+dirty | 3.19 MiB | 4.25 MiB | 1.07 MiB |
0f6176d+dirty | 3.19 MiB | 4.25 MiB | 1.07 MiB |
1646f54+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b75148e+dirty | 428.91 ms | 461.26 ms | 32.35 ms |
208f4af+dirty | 346.93 ms | 402.77 ms | 55.84 ms |
555070f+dirty | 388.25 ms | 424.44 ms | 36.19 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b75148e+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
208f4af+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
555070f+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
Previous results on branch: antonis/init-from-json
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 399.49 ms | 431.13 ms | 31.64 ms |
e57a9d7+dirty | 357.30 ms | 403.62 ms | 46.32 ms |
69b6731+dirty | 363.50 ms | 409.44 ms | 45.94 ms |
1646f54+dirty | 399.30 ms | 453.04 ms | 53.74 ms |
4ea1d1e+dirty | 384.08 ms | 403.66 ms | 19.58 ms |
dd8554c+dirty | 382.76 ms | 429.04 ms | 46.28 ms |
36893e3+dirty | 380.00 ms | 431.02 ms | 51.02 ms |
66c8b49+dirty | 384.27 ms | 429.34 ms | 45.07 ms |
d0ffabe+dirty | 395.28 ms | 436.64 ms | 41.36 ms |
f74989d+dirty | 265.81 ms | 320.56 ms | 54.75 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
3db962c+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
e57a9d7+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
69b6731+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
1646f54+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
4ea1d1e+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
dd8554c+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
36893e3+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
66c8b49+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
d0ffabe+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
f74989d+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
packages/core/android/src/main/java/io/sentry/react/RNSentrySDK.java
Outdated
Show resolved
Hide resolved
packages/core/android/src/main/java/io/sentry/react/RNSentrySDK.java
Outdated
Show resolved
Hide resolved
packages/core/android/src/main/java/io/sentry/react/RNSentryMapConverter.java
Outdated
Show resolved
Hide resolved
packages/core/android/src/main/java/io/sentry/react/RNSentryMapConverter.java
Outdated
Show resolved
Hide resolved
...dTester/app/src/androidTest/java/io/sentry/rnsentryandroidtester/RNSentryMapConverterTest.kt
Outdated
Show resolved
Hide resolved
packages/core/android/src/main/java/io/sentry/react/RNSentrySDK.java
Outdated
Show resolved
Hide resolved
Hey @krystofwoldrich 👋 |
packages/core/android/src/main/java/io/sentry/react/RNSentryStart.java
Outdated
Show resolved
Hide resolved
I think this should be moved to ReactDefaults. So the current activity is set regardless of the init with file or init without the file. |
The SDK name and version should be moved to ReactDefaults. So the correct name is set in both init with file and without file. |
packages/core/android/src/main/java/io/sentry/react/RNSentryStart.java
Outdated
Show resolved
Hide resolved
packages/core/RNSentryAndroidTester/app/src/androidTest/java/io/sentry/react/RNSentrySDKTest.kt
Show resolved
Hide resolved
I think the PR is almost done, I've just noticed two items that should also be moved to the react defaults. |
Thank you for the feedback Krystof 🙇
Updated with 43b26db
Updated with d2fd58f |
I've noticed the E2E tests started failing on this branch, is it possibly caused by some of the init changes? |
I think it is CI related since it started with a small internal change and the errors seems to be related with Maestro. I also noticed another deps PR with similar errors. Edit: The tests pass after rerunning today. I'll keep an eye on this. |
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.
Thank you! 🚀
📢 Type of change
Depends on #4445 and #4479
📜 Description
Adds the following methods that allow initialisation of Sentry Android in RN applications:
RNSentrySDK.init(Context context)
: Initialises using thesentry.options.json
file located in the root of the RN project.RNSentrySDK.init(Context context, Sentry.OptionsConfiguration<SentryAndroidOptions> configuration)
: Initialises with the passedconfiguration
and thesentry.options.json
if it exists.💡 Motivation and Context
Part of #3608
💚 How did you test it?
CI, Manual testing
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps