-
-
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
Disable enableNative if Native SDK was not initialized #3099
Conversation
|
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
86d6d2c+dirty | 332.90 ms | 352.45 ms | 19.55 ms |
ad6c299 | 375.94 ms | 382.02 ms | 6.08 ms |
e73f4ed+dirty | 332.96 ms | 354.33 ms | 21.37 ms |
8900e1a+dirty | 430.68 ms | 456.13 ms | 25.44 ms |
d0bf494+dirty | 375.37 ms | 395.14 ms | 19.77 ms |
76d1baf+dirty | 335.72 ms | 355.52 ms | 19.80 ms |
34aba08 | 328.10 ms | 342.84 ms | 14.74 ms |
80b2ce3 | 385.02 ms | 387.36 ms | 2.34 ms |
b1e8712 | 462.11 ms | 465.71 ms | 3.60 ms |
dadc233+dirty | 333.78 ms | 343.94 ms | 10.16 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
86d6d2c+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
ad6c299 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
e73f4ed+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
8900e1a+dirty | 17.73 MiB | 19.75 MiB | 2.01 MiB |
d0bf494+dirty | 17.73 MiB | 19.75 MiB | 2.02 MiB |
76d1baf+dirty | 17.73 MiB | 20.04 MiB | 2.31 MiB |
34aba08 | 17.73 MiB | 19.80 MiB | 2.07 MiB |
80b2ce3 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
b1e8712 | 17.73 MiB | 19.75 MiB | 2.02 MiB |
dadc233+dirty | 17.73 MiB | 19.75 MiB | 2.02 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 1256.02 ms | 1265.14 ms | 9.12 ms |
86d6d2c+dirty | 1267.55 ms | 1286.21 ms | 18.66 ms |
e73f4ed+dirty | 1243.27 ms | 1244.52 ms | 1.25 ms |
8900e1a+dirty | 1210.27 ms | 1218.66 ms | 8.39 ms |
d0bf494+dirty | 1289.40 ms | 1298.40 ms | 9.00 ms |
0db0c72+dirty | 1275.02 ms | 1285.84 ms | 10.82 ms |
76d1baf+dirty | 1244.10 ms | 1268.52 ms | 24.42 ms |
34aba08+dirty | 1276.78 ms | 1308.52 ms | 31.74 ms |
ad6c299+dirty | 1244.76 ms | 1260.10 ms | 15.34 ms |
dadc233+dirty | 1223.20 ms | 1236.88 ms | 13.68 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 2.36 MiB | 2.84 MiB | 488.84 KiB |
86d6d2c+dirty | 2.36 MiB | 2.82 MiB | 462.82 KiB |
e73f4ed+dirty | 2.36 MiB | 2.82 MiB | 469.44 KiB |
8900e1a+dirty | 2.36 MiB | 2.83 MiB | 479.25 KiB |
d0bf494+dirty | 2.36 MiB | 2.83 MiB | 481.15 KiB |
0db0c72+dirty | 2.36 MiB | 2.84 MiB | 487.01 KiB |
76d1baf+dirty | 2.36 MiB | 2.82 MiB | 469.45 KiB |
34aba08+dirty | 2.36 MiB | 2.85 MiB | 495.32 KiB |
ad6c299+dirty | 2.36 MiB | 2.84 MiB | 488.85 KiB |
dadc233+dirty | 2.36 MiB | 2.84 MiB | 486.85 KiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 1284.11 ms | 1297.82 ms | 13.71 ms |
86d6d2c+dirty | 1291.62 ms | 1296.80 ms | 5.18 ms |
e73f4ed+dirty | 1282.90 ms | 1309.30 ms | 26.40 ms |
8900e1a+dirty | 1268.36 ms | 1273.04 ms | 4.68 ms |
d0bf494+dirty | 1266.20 ms | 1267.52 ms | 1.32 ms |
0db0c72+dirty | 1258.88 ms | 1262.52 ms | 3.64 ms |
76d1baf+dirty | 1245.00 ms | 1257.76 ms | 12.76 ms |
34aba08+dirty | 1268.58 ms | 1276.80 ms | 8.22 ms |
ad6c299+dirty | 1248.50 ms | 1248.88 ms | 0.38 ms |
dadc233+dirty | 1266.52 ms | 1282.55 ms | 16.03 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 2.92 MiB | 3.40 MiB | 494.15 KiB |
86d6d2c+dirty | 2.92 MiB | 3.37 MiB | 464.31 KiB |
e73f4ed+dirty | 2.92 MiB | 3.38 MiB | 475.71 KiB |
8900e1a+dirty | 2.92 MiB | 3.39 MiB | 485.96 KiB |
d0bf494+dirty | 2.92 MiB | 3.40 MiB | 488.08 KiB |
0db0c72+dirty | 2.92 MiB | 3.40 MiB | 492.71 KiB |
76d1baf+dirty | 2.92 MiB | 3.38 MiB | 475.74 KiB |
34aba08+dirty | 2.92 MiB | 3.41 MiB | 499.03 KiB |
ad6c299+dirty | 2.92 MiB | 3.40 MiB | 494.12 KiB |
dadc233+dirty | 2.92 MiB | 3.40 MiB | 492.53 KiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 322.55 ms | 331.84 ms | 9.29 ms |
86d6d2c+dirty | 267.21 ms | 325.24 ms | 58.04 ms |
e73f4ed+dirty | 262.98 ms | 311.02 ms | 48.04 ms |
8900e1a+dirty | 371.40 ms | 377.70 ms | 6.31 ms |
d0bf494+dirty | 253.73 ms | 308.23 ms | 54.49 ms |
0db0c72+dirty | 335.20 ms | 351.06 ms | 15.86 ms |
76d1baf+dirty | 339.02 ms | 408.65 ms | 69.63 ms |
34aba08+dirty | 331.79 ms | 376.69 ms | 44.91 ms |
ad6c299+dirty | 336.47 ms | 362.89 ms | 26.42 ms |
dadc233+dirty | 363.19 ms | 370.37 ms | 7.18 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b1e8712+dirty | 7.15 MiB | 8.04 MiB | 912.27 KiB |
86d6d2c+dirty | 7.15 MiB | 8.09 MiB | 962.69 KiB |
e73f4ed+dirty | 7.15 MiB | 8.09 MiB | 965.94 KiB |
8900e1a+dirty | 7.15 MiB | 8.03 MiB | 901.79 KiB |
d0bf494+dirty | 7.15 MiB | 8.04 MiB | 910.85 KiB |
0db0c72+dirty | 7.15 MiB | 8.04 MiB | 911.02 KiB |
76d1baf+dirty | 7.15 MiB | 8.09 MiB | 964.41 KiB |
34aba08+dirty | 7.15 MiB | 8.07 MiB | 946.13 KiB |
ad6c299+dirty | 7.15 MiB | 8.04 MiB | 912.17 KiB |
dadc233+dirty | 7.15 MiB | 8.04 MiB | 910.84 KiB |
Is this somehow a dupe of #3093 ? |
Seems to be a different case. PR #3093 fixes this flow where this PR fixes the initial value of enableNative to false if the NativeTransport isn't available. |
Let's add one or two tests into it('falls back to fetch if native flag enabled but native not available', () => {
(NATIVE.isNativeAvailable as jest.Mock).mockImplementation(() => false);
init({ enableNative: true });
expect(usedOptions()?.enableNative).toEqual(false);
expect(NATIVE.isNativeAvailable).toBeCalled();
}); Besides that, it looks good, thanks for the events examples. PS.: I've noticed none of the stack traces is symbolicated, but I see symbolicate calls in the breadcrumbs, strange. |
issue in regard to the stack trace was created here: #3104 |
Co-authored-by: Kryštof Woldřich <31292499+krystofwoldrich@users.noreply.github.com>
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.
Looks good. Thank you! 🚀🚀
sets the flag
EnableNative
ifmakeNativeTransportFactory
failed to be created.Platform.OS could be 'Android' on Expo so we can not rely on that value in order to set EnableNative to true/false
Validated issues:
https://sentry-sdks.sentry.io/issues/4227272790/events/e80ef7a5799c43c69e9d69c53df24b16/?project=5428561
https://sentry-sdks.sentry.io/issues/4227268747/events/47a07470db4641ea90e5c7f124e0c623/?project=5428561
https://sentry-sdks.sentry.io/issues/4227271973/events/d94ead040c684d459b172e1458c8504e/?project=5428561
Close #3027