-
-
Notifications
You must be signed in to change notification settings - Fork 246
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 ScreenshotIntegration
, WidgetsBindingIntegration
and SentryWidget
in multi-view apps
#2366
Conversation
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2366 +/- ##
==========================================
+ Coverage 88.98% 92.09% +3.10%
==========================================
Files 262 91 -171
Lines 8906 2946 -5960
==========================================
- Hits 7925 2713 -5212
+ Misses 981 233 -748 ☔ View full report in Codecov by Sentry. |
Android Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
905bf99 | 459.73 ms | 505.26 ms | 45.53 ms |
464b4d0 | 320.71 ms | 380.02 ms | 59.31 ms |
ea60f10 | 456.23 ms | 494.48 ms | 38.25 ms |
1b0c8a3 | 479.78 ms | 521.18 ms | 41.40 ms |
e893df5 | 310.60 ms | 380.58 ms | 69.98 ms |
0a23f98 | 377.19 ms | 416.18 ms | 39.00 ms |
691aa3b | 370.43 ms | 466.28 ms | 95.85 ms |
3e5ee37 | 317.56 ms | 366.84 ms | 49.28 ms |
ca7f531 | 395.69 ms | 497.82 ms | 102.12 ms |
4c13d97 | 455.34 ms | 509.42 ms | 54.08 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
905bf99 | 6.49 MiB | 7.56 MiB | 1.07 MiB |
464b4d0 | 6.06 MiB | 7.03 MiB | 990.27 KiB |
ea60f10 | 6.49 MiB | 7.55 MiB | 1.07 MiB |
1b0c8a3 | 6.49 MiB | 7.57 MiB | 1.08 MiB |
e893df5 | 6.06 MiB | 7.09 MiB | 1.03 MiB |
0a23f98 | 6.06 MiB | 7.03 MiB | 996.97 KiB |
691aa3b | 5.94 MiB | 6.96 MiB | 1.02 MiB |
3e5ee37 | 5.94 MiB | 6.92 MiB | 1001.19 KiB |
ca7f531 | 6.33 MiB | 7.26 MiB | 949.75 KiB |
4c13d97 | 6.49 MiB | 7.56 MiB | 1.07 MiB |
Previous results on branch: feat/multiview-aware
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2baf2a4 | 414.65 ms | 505.75 ms | 91.10 ms |
be2b2de | 411.65 ms | 507.57 ms | 95.93 ms |
df169ab | 452.48 ms | 559.46 ms | 106.98 ms |
1d57db2 | 542.49 ms | 649.24 ms | 106.75 ms |
72556e0 | 401.20 ms | 502.86 ms | 101.66 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2baf2a4 | 6.46 MiB | 7.48 MiB | 1.02 MiB |
be2b2de | 6.46 MiB | 7.48 MiB | 1.03 MiB |
df169ab | 6.46 MiB | 7.48 MiB | 1.03 MiB |
1d57db2 | 6.46 MiB | 7.48 MiB | 1.03 MiB |
72556e0 | 6.46 MiB | 7.48 MiB | 1.02 MiB |
iOS Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
95c69e3 | 1223.94 ms | 1245.29 ms | 21.35 ms |
09c1f55 | 1258.11 ms | 1280.45 ms | 22.34 ms |
49a149b | 1296.47 ms | 1320.20 ms | 23.73 ms |
bffc2c5 | 1239.76 ms | 1264.67 ms | 24.92 ms |
b49bf00 | 1248.00 ms | 1260.35 ms | 12.35 ms |
a609134 | 1254.50 ms | 1265.08 ms | 10.58 ms |
d1488a1 | 1234.02 ms | 1244.98 ms | 10.96 ms |
a24a1db | 1257.71 ms | 1271.57 ms | 13.87 ms |
905bf99 | 1240.84 ms | 1271.47 ms | 30.63 ms |
9da696d | 1253.91 ms | 1267.68 ms | 13.77 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
95c69e3 | 8.33 MiB | 9.64 MiB | 1.31 MiB |
09c1f55 | 8.38 MiB | 9.74 MiB | 1.36 MiB |
49a149b | 8.15 MiB | 9.12 MiB | 986.26 KiB |
bffc2c5 | 8.33 MiB | 9.40 MiB | 1.07 MiB |
b49bf00 | 8.10 MiB | 9.08 MiB | 1004.36 KiB |
a609134 | 8.16 MiB | 9.16 MiB | 1.01 MiB |
d1488a1 | 8.42 MiB | 9.89 MiB | 1.47 MiB |
a24a1db | 8.42 MiB | 9.91 MiB | 1.49 MiB |
905bf99 | 8.38 MiB | 9.74 MiB | 1.36 MiB |
9da696d | 8.42 MiB | 9.91 MiB | 1.49 MiB |
Previous results on branch: feat/multiview-aware
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2baf2a4 | 1255.74 ms | 1276.96 ms | 21.22 ms |
72556e0 | 1254.58 ms | 1269.53 ms | 14.95 ms |
be2b2de | 1263.27 ms | 1272.61 ms | 9.35 ms |
df169ab | 1239.88 ms | 1259.89 ms | 20.02 ms |
1d57db2 | 1245.47 ms | 1253.96 ms | 8.49 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
2baf2a4 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
72556e0 | 8.42 MiB | 9.91 MiB | 1.49 MiB |
be2b2de | 8.42 MiB | 9.91 MiB | 1.49 MiB |
df169ab | 8.42 MiB | 9.91 MiB | 1.49 MiB |
1d57db2 | 8.42 MiB | 9.91 MiB | 1.49 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.
not sure how it's gonna work but any chance we can set up some simple tests for this?
I wanna make sure that this doesn't regress in future flutter versions
I'll add an integration test for this |
# Conflicts: # flutter/lib/src/sentry_flutter.dart
ScreenshotIntegration
, WidgetsBindingIntegration
and SentryWidget
in multi-view applications.
ScreenshotIntegration
, WidgetsBindingIntegration
and SentryWidget
in multi-view applications.ScreenshotIntegration
, WidgetsBindingIntegration
and SentryWidget
in multi-view apps
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! wdyt is the effort of putting in an integration test in example/integration_test
? @denrase
@buenaflor what do you recommend should this test cover? |
@denrase I was thinking about creating a new app that has multiview enabled with an integration test that checks if the relevant sentry integrations are disabled but probably thats too much work, wdyt otherwise what we could do is add an addition web integration_test case where we check that these integrations I have already set up integration tests that run on web, see |
📜 Description
Disable ScreenshotIntegration, WidgetsBindingIntegration and SentryWidget in multi-view applications.
To find out if we are in a multi-view app, we check the
PlatformDispatcher.instance.implicitView
return value.In a non-multiview app
PlatformDispatcher.instance.implicitView
returns aFlutterView
object, in multi-view it returns null. See Flutter Docs for more infos.💡 Motivation and Context
Based on the discussion here.
💚 How did you test it?
Unit tests.
📝 Checklist
sendDefaultPii
is enabled