Add debounce to capturing screenshots by denrase · Pull Request #2368 · getsentry/sentry-dart

denrase · 2024-10-22T09:50:37Z

📜 Description

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

💡 Motivation and Context

Relates to #2360
Relates to #2368

💚 How did you test it?

Unit tests.

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

github-actions · 2024-10-22T09:51:06Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 4d5f535

codecov · 2024-10-22T09:52:53Z

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 ⚠️

Additional details and impacted files

@@            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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-10-22T11:17:49Z

iOS Performance metrics 🚀

	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
`6f3717a`	1259.84 ms	1280.90 ms	21.06 ms
`e6b16cd`	1226.20 ms	1246.52 ms	20.32 ms
`8fa3934`	1252.79 ms	1272.41 ms	19.62 ms
`cc80714`	1205.53 ms	1223.90 ms	18.37 ms
`0a82a1e`	1233.56 ms	1244.56 ms	11.00 ms
`3334ac1`	1259.22 ms	1275.40 ms	16.17 ms
`256df44`	1252.49 ms	1274.27 ms	21.78 ms
`cf91c9d`	1217.08 ms	1233.00 ms	15.92 ms
`b728df4`	1287.43 ms	1293.94 ms	6.51 ms
`61e71ec`	1243.14 ms	1257.21 ms	14.07 ms

App size

Revision	Plain	With Sentry	Diff
`6f3717a`	8.33 MiB	9.62 MiB	1.29 MiB
`e6b16cd`	8.33 MiB	9.54 MiB	1.22 MiB
`8fa3934`	8.09 MiB	9.07 MiB	1000.86 KiB
`cc80714`	8.33 MiB	9.40 MiB	1.07 MiB
`0a82a1e`	8.29 MiB	9.37 MiB	1.08 MiB
`3334ac1`	8.10 MiB	9.17 MiB	1.08 MiB
`256df44`	8.38 MiB	9.71 MiB	1.33 MiB
`cf91c9d`	8.33 MiB	9.40 MiB	1.07 MiB
`b728df4`	8.15 MiB	9.15 MiB	1020.72 KiB
`61e71ec`	8.33 MiB	9.40 MiB	1.07 MiB

Previous results on branch: feat/screenshot-debounce

Startup times

Revision	Plain	With Sentry	Diff
`b157a35`	1256.12 ms	1273.90 ms	17.78 ms
`e5f628a`	1227.47 ms	1229.73 ms	2.26 ms
`3d489d9`	1256.27 ms	1279.69 ms	23.42 ms
`9fce5c0`	1226.77 ms	1248.82 ms	22.05 ms
`e96bb8d`	1248.14 ms	1270.88 ms	22.73 ms
`5a9bcac`	1258.37 ms	1282.79 ms	24.42 ms

App size

Revision	Plain	With Sentry	Diff
`b157a35`	8.38 MiB	9.77 MiB	1.39 MiB
`e5f628a`	8.38 MiB	9.75 MiB	1.37 MiB
`3d489d9`	8.38 MiB	9.75 MiB	1.37 MiB
`9fce5c0`	8.38 MiB	9.77 MiB	1.39 MiB
`e96bb8d`	8.38 MiB	9.77 MiB	1.39 MiB
`5a9bcac`	8.38 MiB	9.77 MiB	1.39 MiB

github-actions · 2024-10-22T11:17:56Z

Android Performance metrics 🚀

	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
`1ac008b`	370.04 ms	435.58 ms	65.54 ms
`4ad2751`	374.58 ms	462.00 ms	87.42 ms
`43760f9`	321.78 ms	366.77 ms	44.99 ms
`fec92cc`	399.96 ms	479.26 ms	79.30 ms
`c9d3212`	301.34 ms	361.58 ms	60.24 ms
`5603ab2`	309.84 ms	345.20 ms	35.36 ms
`3e9fb0e`	329.14 ms	359.16 ms	30.02 ms
`6d317ea`	303.46 ms	356.06 ms	52.60 ms
`1a93825`	347.31 ms	424.54 ms	77.23 ms
`bffc2c5`	348.00 ms	399.89 ms	51.89 ms

App size

Revision	Plain	With Sentry	Diff
`1ac008b`	6.33 MiB	7.27 MiB	954.13 KiB
`4ad2751`	6.16 MiB	7.14 MiB	1010.86 KiB
`43760f9`	6.15 MiB	7.13 MiB	1000.49 KiB
`fec92cc`	6.35 MiB	7.33 MiB	1005.56 KiB
`c9d3212`	6.16 MiB	7.14 MiB	1010.90 KiB
`5603ab2`	5.94 MiB	6.95 MiB	1.01 MiB
`3e9fb0e`	5.94 MiB	6.95 MiB	1.01 MiB
`6d317ea`	5.94 MiB	6.92 MiB	1001.74 KiB
`1a93825`	6.27 MiB	7.20 MiB	956.36 KiB
`bffc2c5`	6.34 MiB	7.28 MiB	966.31 KiB

Previous results on branch: feat/screenshot-debounce

Startup times

Revision	Plain	With Sentry	Diff
`e96bb8d`	451.02 ms	488.94 ms	37.92 ms
`9fce5c0`	458.88 ms	484.86 ms	25.98 ms
`5a9bcac`	441.24 ms	476.58 ms	35.34 ms
`b157a35`	668.50 ms	743.42 ms	74.92 ms
`e5f628a`	451.42 ms	495.36 ms	43.94 ms
`3d489d9`	452.25 ms	498.58 ms	46.33 ms

App size

Revision	Plain	With Sentry	Diff
`e96bb8d`	6.49 MiB	7.56 MiB	1.07 MiB
`9fce5c0`	6.49 MiB	7.56 MiB	1.07 MiB
`5a9bcac`	6.49 MiB	7.56 MiB	1.07 MiB
`b157a35`	6.49 MiB	7.56 MiB	1.07 MiB
`e5f628a`	6.49 MiB	7.57 MiB	1.08 MiB
`3d489d9`	6.49 MiB	7.57 MiB	1.08 MiB

denrase · 2024-10-23T11:02:20Z

Closing due to discussion here: #2371 (comment)

# Conflicts: # flutter/lib/src/event_processor/screenshot_event_processor.dart

denrase · 2024-11-13T13:49:58Z

@buenaflor Looks like we have some provisioning profile issue.

buenaflor · 2024-11-14T11:43:04Z

@denrase on it

edit: ci is fixed

buenaflor · 2024-11-18T23:50:40Z

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 · 2024-11-19T15:52:40Z

@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 · 2024-11-19T16:00:18Z

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 · 2024-11-25T14:00:59Z

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

denrase · 2024-11-25T15:52:10Z

@buenaflor rdy for one more review round

buenaflor

🚀

Add debounce to ScreenshotWidget

77cce7c

add changelog entry

f540ecb

provide clock in tests

e5f628a

denrase added 4 commits October 22, 2024 13:37

remove unused import

235de7b

update clock at correct line

09c7f91

Merge branch 'main' into feat/screenshot-debounce

7345987

update cl

ba0b8e7

denrase marked this pull request as ready for review October 22, 2024 11:49

denrase requested review from buenaflor, krystofwoldrich, martinhaintz and stefanosiano as code owners October 22, 2024 11:49

martinhaintz reviewed Oct 22, 2024

View reviewed changes

Comment thread flutter/lib/src/event_processor/screenshot_event_processor.dart

use now from clock

3d489d9

denrase requested a review from martinhaintz October 22, 2024 12:19

denrase mentioned this pull request Oct 23, 2024

Handle backpressure earlier in pipeline #2371

Merged

6 tasks

denrase closed this Oct 23, 2024

denrase reopened this Nov 12, 2024

denrase self-assigned this Nov 12, 2024

denrase marked this pull request as draft November 12, 2024 08:58

denrase removed their assignment Nov 12, 2024

denrase mentioned this pull request Nov 12, 2024

Add screenshotCacheDuration to FlutterOptions #2387

Closed

6 tasks

denrase added 3 commits November 12, 2024 10:04

Merge branch 'main' into feat/screenshot-debounce

c088da8

# Conflicts: # flutter/lib/src/event_processor/screenshot_event_processor.dart

refactor debouncer to be similar to sentry-java impl

3a9d843

use debouncer in screenshot_event_processor

e710098

denrase added 2 commits November 13, 2024 12:25

add warning

2e43ad3

add ignores for analyzer

e96bb8d

buenaflor requested changes Nov 14, 2024

View reviewed changes

Comment thread flutter/lib/src/sentry_flutter_options.dart Outdated

Merge branch 'main' into feat/screenshot-debounce

5a9bcac

denrase requested a review from buenaflor November 18, 2024 09:25

denrase added 2 commits November 18, 2024 16:11

rename callback

f4c724d

Merge branch 'main' into feat/screenshot-debounce

9fce5c0

buenaflor reviewed Nov 18, 2024

View reviewed changes

Comment thread flutter/lib/src/sentry_flutter_options.dart Outdated

rename to beforeScreenshotCapture

b157a35

denrase requested a review from buenaflor November 18, 2024 17:00

buenaflor approved these changes Nov 18, 2024

View reviewed changes

denrase added 4 commits November 25, 2024 16:30

Merge branch 'main' into feat/screenshot-debounce

28b0857

cleanup

7ce37e4

fix cl

cd64cc1

move to features in cl

d245569

denrase requested a review from buenaflor November 25, 2024 15:51

buenaflor reviewed Nov 25, 2024

View reviewed changes

Comment thread CHANGELOG.md Outdated

denrase changed the title ~~Add debounce to ScreenshotWidget~~ Add debounce to capturing screenshots Nov 25, 2024

add sample to changelog

4d5f535

denrase requested a review from buenaflor November 25, 2024 16:34

buenaflor approved these changes Nov 25, 2024

View reviewed changes

denrase merged commit be08651 into main Nov 26, 2024

denrase deleted the feat/screenshot-debounce branch November 26, 2024 10:03

Uh oh!

Conversation

denrase commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

Uh oh!

github-actions Bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/screenshot-debounce

Startup times

App size

Uh oh!

github-actions Bot commented Oct 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/screenshot-debounce

Startup times

App size

Uh oh!

Uh oh!

denrase commented Oct 23, 2024

Uh oh!

denrase commented Nov 13, 2024

Uh oh!

buenaflor commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

buenaflor commented Nov 18, 2024

Uh oh!

denrase commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buenaflor commented Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

buenaflor commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

denrase commented Nov 25, 2024

Uh oh!

Uh oh!

buenaflor left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

denrase commented Oct 22, 2024 •

edited

Loading

github-actions Bot commented Oct 22, 2024 •

edited

Loading

codecov Bot commented Oct 22, 2024 •

edited

Loading

github-actions Bot commented Oct 22, 2024 •

edited

Loading

github-actions Bot commented Oct 22, 2024 •

edited

Loading

buenaflor commented Nov 14, 2024 •

edited

Loading

denrase commented Nov 19, 2024 •

edited

Loading

buenaflor commented Nov 19, 2024 •

edited

Loading

buenaflor commented Nov 25, 2024 •

edited

Loading