-
-
Notifications
You must be signed in to change notification settings - Fork 263
Fix context to native sync for sentry context types #3012
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3012 +/- ##
==========================================
- Coverage 87.74% 87.73% -0.02%
==========================================
Files 286 286
Lines 9335 9340 +5
==========================================
+ Hits 8191 8194 +3
- Misses 1144 1146 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c1bb00f | 1265.14 ms | 1290.85 ms | 25.71 ms |
f172c4d | 1350.66 ms | 1408.49 ms | 57.83 ms |
0be962b | 1264.10 ms | 1281.16 ms | 17.06 ms |
6325c3b | 1266.52 ms | 1291.06 ms | 24.54 ms |
e1897c5 | 1252.61 ms | 1276.48 ms | 23.87 ms |
aa950e9 | 1275.17 ms | 1295.33 ms | 20.16 ms |
a49594a | 1284.83 ms | 1313.29 ms | 28.45 ms |
1131914 | 1277.20 ms | 1300.20 ms | 23.00 ms |
7659cbe | 1246.70 ms | 1265.88 ms | 19.17 ms |
3f23617 | 1261.93 ms | 1286.10 ms | 24.17 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
c1bb00f | 8.09 MiB | 9.07 MiB | 1001.06 KiB |
f172c4d | 8.33 MiB | 9.62 MiB | 1.29 MiB |
0be962b | 8.10 MiB | 9.16 MiB | 1.07 MiB |
6325c3b | 8.16 MiB | 9.17 MiB | 1.01 MiB |
e1897c5 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
aa950e9 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
a49594a | 8.16 MiB | 9.16 MiB | 1.00 MiB |
1131914 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
7659cbe | 8.42 MiB | 9.89 MiB | 1.47 MiB |
3f23617 | 8.16 MiB | 9.17 MiB | 1.01 MiB |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8ced2dc | 295.58 ms | 336.49 ms | 40.91 ms |
4b29d6e | 386.80 ms | 430.86 ms | 44.06 ms |
5f443de | 412.30 ms | 491.67 ms | 79.37 ms |
04db237 | 330.16 ms | 428.38 ms | 98.22 ms |
689d2fd | 378.62 ms | 430.48 ms | 51.86 ms |
6572f8d | 302.35 ms | 348.10 ms | 45.75 ms |
ccc09e4 | 308.21 ms | 357.74 ms | 49.54 ms |
7954fb3 | 459.38 ms | 500.72 ms | 41.34 ms |
33f99c9 | 668.33 ms | 735.08 ms | 66.76 ms |
dd5521e | 454.51 ms | 538.29 ms | 83.78 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
8ced2dc | 6.06 MiB | 7.03 MiB | 990.29 KiB |
4b29d6e | 6.33 MiB | 7.26 MiB | 946.14 KiB |
5f443de | 6.35 MiB | 7.34 MiB | 1008.00 KiB |
04db237 | 5.94 MiB | 6.95 MiB | 1.01 MiB |
689d2fd | 6.06 MiB | 7.10 MiB | 1.04 MiB |
6572f8d | 6.15 MiB | 7.13 MiB | 999.97 KiB |
ccc09e4 | 5.94 MiB | 6.95 MiB | 1.01 MiB |
7954fb3 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
33f99c9 | 6.54 MiB | 7.53 MiB | 1017.45 KiB |
dd5521e | 6.44 MiB | 7.50 MiB | 1.06 MiB |
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.
lgtm 👍
📜 Description
Fixes an issue where we'd call toString() on sentry context types when synging to native. This in turn broke load contexts integration, as a map was expected, and instead it was the toString() string.
💡 Motivation and Context
Fixes #3006
💚 How did you test it?
Unit tests. Ran on sample app.
📝 Checklist
sendDefaultPii
is enabled