getsentry / sentry-dart

Sentry SDK for Dart and Flutter
https://sentry.io/for/flutter/
MIT License
766 stars 239 forks source link

Add debounce to capturing screenshots #2368

Closed denrase closed 3 days ago

denrase commented 1 month ago

:scroll: Description

Debounce screenshots made by sentry to avoid being removed by iOS watchdog.

Bildschirmfoto 2024-10-22 um 11 34 36

:bulb: Motivation and Context

Relates to #2360 Relates to #2368

:green_heart: How did you test it?

Unit tests.

:pencil: Checklist

github-actions[bot] commented 1 month ago
Messages
:book: Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by :no_entry_sign: dangerJS against 4d5f5356c9a8896627ffde21545256b24178bac3

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.81%. Comparing base (7f97e6c) to head (4d5f535). Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
flutter/lib/src/utils/timer_debouncer.dart 66.66% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2368 +/- ## ========================================== + Coverage 86.92% 91.81% +4.89% ========================================== Files 259 84 -175 Lines 9245 2897 -6348 ========================================== - Hits 8036 2660 -5376 + Misses 1209 237 -972 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 month ago

iOS Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 1243.80 ms 1274.37 ms 30.57 ms
Size 8.38 MiB 9.78 MiB 1.40 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
6f3717a084265d352ec706090ecfd2f3dc6337d5 1259.84 ms 1280.90 ms 21.06 ms
e6b16cd4aa51b0236a52f4c6380d53b6d1a1aee9 1226.20 ms 1246.52 ms 20.32 ms
8fa393466a5d478f281e46cbc11e66c2b3db12d8 1252.79 ms 1272.41 ms 19.62 ms
cc807145d186a99a58be0997ef14e4f55a27adc1 1205.53 ms 1223.90 ms 18.37 ms
0a82a1ee1ab16b08941011233f534636569d7543 1233.56 ms 1244.56 ms 11.00 ms
3334ac176f15a6d39b435960620ea9a4919aae79 1259.22 ms 1275.40 ms 16.17 ms
256df44bb670822fc3087877f775a3d82bb476b0 1252.49 ms 1274.27 ms 21.78 ms
cf91c9da10875feced1ae3de5a5cb4e9b0fa14f0 1217.08 ms 1233.00 ms 15.92 ms
b728df439473e146f8721c7555cd54728a93b6cb 1287.43 ms 1293.94 ms 6.51 ms
61e71ec84d02fc022b00615b4d0bae341a641502 1243.14 ms 1257.21 ms 14.07 ms

App size

Revision Plain With Sentry Diff
6f3717a084265d352ec706090ecfd2f3dc6337d5 8.33 MiB 9.62 MiB 1.29 MiB
e6b16cd4aa51b0236a52f4c6380d53b6d1a1aee9 8.33 MiB 9.54 MiB 1.22 MiB
8fa393466a5d478f281e46cbc11e66c2b3db12d8 8.09 MiB 9.07 MiB 1000.86 KiB
cc807145d186a99a58be0997ef14e4f55a27adc1 8.33 MiB 9.40 MiB 1.07 MiB
0a82a1ee1ab16b08941011233f534636569d7543 8.29 MiB 9.37 MiB 1.08 MiB
3334ac176f15a6d39b435960620ea9a4919aae79 8.10 MiB 9.17 MiB 1.08 MiB
256df44bb670822fc3087877f775a3d82bb476b0 8.38 MiB 9.71 MiB 1.33 MiB
cf91c9da10875feced1ae3de5a5cb4e9b0fa14f0 8.33 MiB 9.40 MiB 1.07 MiB
b728df439473e146f8721c7555cd54728a93b6cb 8.15 MiB 9.15 MiB 1020.72 KiB
61e71ec84d02fc022b00615b4d0bae341a641502 8.33 MiB 9.40 MiB 1.07 MiB

Previous results on branch: feat/screenshot-debounce

Startup times

Revision Plain With Sentry Diff
b157a358b38857ac818d10848bc928bdbf819e8b 1256.12 ms 1273.90 ms 17.78 ms
e5f628ad5813f9efe50100b0469490b6e2873f5e 1227.47 ms 1229.73 ms 2.26 ms
3d489d9dae7f8f15d52b9e542f78acb86d1858db 1256.27 ms 1279.69 ms 23.42 ms
9fce5c0ad625274f1847b5fe9afcfeaaadcf0f23 1226.77 ms 1248.82 ms 22.05 ms
e96bb8da03ed175ff22a28293107131f8ec3e49a 1248.14 ms 1270.88 ms 22.73 ms
5a9bcacc1ec02cb2d53e3a36c6713f349b021b21 1258.37 ms 1282.79 ms 24.42 ms

App size

Revision Plain With Sentry Diff
b157a358b38857ac818d10848bc928bdbf819e8b 8.38 MiB 9.77 MiB 1.39 MiB
e5f628ad5813f9efe50100b0469490b6e2873f5e 8.38 MiB 9.75 MiB 1.37 MiB
3d489d9dae7f8f15d52b9e542f78acb86d1858db 8.38 MiB 9.75 MiB 1.37 MiB
9fce5c0ad625274f1847b5fe9afcfeaaadcf0f23 8.38 MiB 9.77 MiB 1.39 MiB
e96bb8da03ed175ff22a28293107131f8ec3e49a 8.38 MiB 9.77 MiB 1.39 MiB
5a9bcacc1ec02cb2d53e3a36c6713f349b021b21 8.38 MiB 9.77 MiB 1.39 MiB
github-actions[bot] commented 1 month ago

Android Performance metrics :rocket:

  Plain With Sentry Diff
Startup time 469.60 ms 521.49 ms 51.89 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
1ac008b595f39f8eb8d7c8d67df3f9db827b7c1e 370.04 ms 435.58 ms 65.54 ms
4ad2751b7824f5312c9ddd3374520e03aaf107b3 374.58 ms 462.00 ms 87.42 ms
43760f991fd70985663607059a73897fe3123966 321.78 ms 366.77 ms 44.99 ms
fec92cc66e26e4c4884781314753c8f4e739539d 399.96 ms 479.26 ms 79.30 ms
c9d3212f2912cb01a1648be804a482edeee6b791 301.34 ms 361.58 ms 60.24 ms
5603ab240ccb170067baf128345a92757ee5de64 309.84 ms 345.20 ms 35.36 ms
3e9fb0ef61113312be4c21d9168d4043eca6c921 329.14 ms 359.16 ms 30.02 ms
6d317ea0c2dcfba0d8300bc7693d2dc292486d01 303.46 ms 356.06 ms 52.60 ms
1a938255a9c0a1073ccebbdb23a70ea2d435013e 347.31 ms 424.54 ms 77.23 ms
bffc2c57d27178214d7b567c9abbf911a4f61801 348.00 ms 399.89 ms 51.89 ms

App size

Revision Plain With Sentry Diff
1ac008b595f39f8eb8d7c8d67df3f9db827b7c1e 6.33 MiB 7.27 MiB 954.13 KiB
4ad2751b7824f5312c9ddd3374520e03aaf107b3 6.16 MiB 7.14 MiB 1010.86 KiB
43760f991fd70985663607059a73897fe3123966 6.15 MiB 7.13 MiB 1000.49 KiB
fec92cc66e26e4c4884781314753c8f4e739539d 6.35 MiB 7.33 MiB 1005.56 KiB
c9d3212f2912cb01a1648be804a482edeee6b791 6.16 MiB 7.14 MiB 1010.90 KiB
5603ab240ccb170067baf128345a92757ee5de64 5.94 MiB 6.95 MiB 1.01 MiB
3e9fb0ef61113312be4c21d9168d4043eca6c921 5.94 MiB 6.95 MiB 1.01 MiB
6d317ea0c2dcfba0d8300bc7693d2dc292486d01 5.94 MiB 6.92 MiB 1001.74 KiB
1a938255a9c0a1073ccebbdb23a70ea2d435013e 6.27 MiB 7.20 MiB 956.36 KiB
bffc2c57d27178214d7b567c9abbf911a4f61801 6.34 MiB 7.28 MiB 966.31 KiB

Previous results on branch: feat/screenshot-debounce

Startup times

Revision Plain With Sentry Diff
e96bb8da03ed175ff22a28293107131f8ec3e49a 451.02 ms 488.94 ms 37.92 ms
9fce5c0ad625274f1847b5fe9afcfeaaadcf0f23 458.88 ms 484.86 ms 25.98 ms
5a9bcacc1ec02cb2d53e3a36c6713f349b021b21 441.24 ms 476.58 ms 35.34 ms
b157a358b38857ac818d10848bc928bdbf819e8b 668.50 ms 743.42 ms 74.92 ms
e5f628ad5813f9efe50100b0469490b6e2873f5e 451.42 ms 495.36 ms 43.94 ms
3d489d9dae7f8f15d52b9e542f78acb86d1858db 452.25 ms 498.58 ms 46.33 ms

App size

Revision Plain With Sentry Diff
e96bb8da03ed175ff22a28293107131f8ec3e49a 6.49 MiB 7.56 MiB 1.07 MiB
9fce5c0ad625274f1847b5fe9afcfeaaadcf0f23 6.49 MiB 7.56 MiB 1.07 MiB
5a9bcacc1ec02cb2d53e3a36c6713f349b021b21 6.49 MiB 7.56 MiB 1.07 MiB
b157a358b38857ac818d10848bc928bdbf819e8b 6.49 MiB 7.56 MiB 1.07 MiB
e5f628ad5813f9efe50100b0469490b6e2873f5e 6.49 MiB 7.57 MiB 1.08 MiB
3d489d9dae7f8f15d52b9e542f78acb86d1858db 6.49 MiB 7.57 MiB 1.08 MiB
denrase commented 1 month ago

Closing due to discussion here: https://github.com/getsentry/sentry-dart/pull/2371#issuecomment-2431741695

buenaflor commented 2 weeks ago

as mentioned, let's deprecate beforeSendScreenshot and advise users to use beforeScreenshotCapture instead

denrase commented 2 weeks ago

@buenaflor Looks like we have some provisioning profile issue.

buenaflor commented 2 weeks ago

@denrase on it

edit: ci is fixed

buenaflor commented 1 week ago

before capture callback looks like is added to the redact screenshot pr: https://github.com/getsentry/sentry-dart/pull/2361/files, _options.screenshot.beforeCapture although I'm not sure whether we should keep our approach with beforeCaptureScreenshot, might be easier to manage?

denrase commented 1 week ago

@buenaflor Lets wait then when the other PR is merged. It still uses the BeforeScreenshotCallback, which we should change to BeforeCaptureCallback then. Do other SDKs also nest options?

buenaflor commented 1 week ago

I'm not sure if we should nest the before screenshot options tbh, probably would not be consistent with other sdks and the before capture view hierarchy

but I don't know if we're going to nest for every sdk

buenaflor commented 4 days ago

screenshot redaction is merged, we can continue here, they changed it to root options

denrase commented 4 days ago

@buenaflor rdy for one more review round