feat(screenshot): Add masking options for screenshot provider
:scroll: Description
- Adds
SentryScreenshotOptionsto configure the masking rules of screenshots taken for captured whenattachScreenshotsis true, and also for user feedback. The reference was sentry-flutter which uses aoptions.privacyapproach. As I do not want to touch the options of session replay to consolidate into oneprivacyblock, I added it as separate options. My thoughts behind this was also that a user might not want their PII show up in replays, but is fine to consent to PII submission when using the user feedback form. This is hereby delegate to the SDK user. - We can consider merging the options for screenshots and session replay in a future (major) release.
- I added a
getScreenshotProviderForOptionsto the dependency container. Originally I wanted to use the same logic asgetANRTrackerwith a single lazy initialized instance, but but that can be wrong if options change later on. As we need to fix this eventually I instead introduced a map with weak references to the screenshot provider using the options as the key. - Adds screenshot options to
SentrySDKWrapperandSentrySDKOverrides - Adds session replay options to
SentrySDKWrapperandSentrySDKOverridesbecause I had to turn it off during development - Uses explicity
nonnulland_Nullableannotations so we can have more null-safety. - This PR also enables the mask renderer V2 by default, as it is already enabled for session replay by default already
:bulb: Motivation and Context
Closes #4831 Closes #4832
:green_heart: How did you test it?
- I used to xcscheme arguments to enable and disable masking for screenshots. Then I tapped on
capture exceptionand verified the screenshot in Sentry is masked or unmasked. - Unit tests
:pencil: Checklist
You have to check all boxes before merging:
- [ ] I added tests to verify the changes.
- [x] No new PII added or SDK only sends newly added PII if
sendDefaultPIIis enabled. - [ ] I updated the docs if needed.
- [x] I updated the wizard if needed.
- [x] Review from the native team if needed.
- [x] No breaking change or entry added to the changelog.
- [x] No breaking change for hybrid SDKs or communicated to hybrid SDKs.
| 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 fcb15dede94b13de9f7c8c1cae8222192949d2ae
Codecov Report
:x: Patch coverage is 80.32787% with 24 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 86.585%. Comparing base (c174e5e) to head (fcb15de).
:warning: Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #5401 +/- ##
=============================================
- Coverage 86.670% 86.585% -0.085%
=============================================
Files 429 432 +3
Lines 36857 36952 +95
Branches 17398 17422 +24
=============================================
+ Hits 31944 31995 +51
- Misses 4868 4912 +44
Partials 45 45
| Files with missing lines | Coverage Δ | |
|---|---|---|
| SentryTestUtils/TestSentryViewPhotographer.swift | 100.000% <100.000%> (ø) |
|
| SentryTestUtils/WeakReference.swift | 100.000% <100.000%> (ø) |
|
| Sources/Sentry/SentryOptions.m | 97.468% <100.000%> (+0.008%) |
:arrow_up: |
| ...egrations/Screenshot/SentryScreenshotOptions.swift | 100.000% <100.000%> (ø) |
|
| ...tegrations/SessionReplay/SentryReplayOptions.swift | 100.000% <ø> (ø) |
|
| SentryTestUtils/TestSentryViewRenderer.swift | 88.888% <88.888%> (ø) |
|
| Sources/Sentry/SentryDependencyContainer.m | 89.406% <94.736%> (+0.822%) |
:arrow_up: |
| Sources/Sentry/PrivateSentrySDKOnly.m | 23.786% <0.000%> (-0.234%) |
:arrow_down: |
| Sources/Sentry/SentryScreenshotIntegration.m | 88.461% <50.000%> (-3.376%) |
:arrow_down: |
| Sources/Sentry/SentryUserFeedbackIntegration.m | 22.727% <0.000%> (-2.273%) |
:arrow_down: |
| ... and 2 more |
... and 10 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update c174e5e...fcb15de. Read the comment docs.
Performance metrics :rocket:
| Plain | With Sentry | Diff | |
|---|---|---|---|
| Startup time | 1231.94 ms | 1256.41 ms | 24.47 ms |
| Size | 23.75 KiB | 958.59 KiB | 934.84 KiB |
Baseline results on branch: main
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| aadbffecb9456d05f0ffd68a15ecf6418b611d7a | 1228.73 ms | 1251.59 ms | 22.86 ms |
| 331dad69050678bbb80653e9f54d323190b0f95e | 1210.40 ms | 1242.06 ms | 31.67 ms |
| 1fe932f4570f89448236c9195253f6cc2f8daa57 | 1231.92 ms | 1253.44 ms | 21.52 ms |
| 701b301b0777a6cbe3bd97f45be62cb72edd1eb7 | 1226.10 ms | 1245.57 ms | 19.47 ms |
| 3cdbc22dbeb82864b9eaf7e03ed756b0c14a8a5d | 1231.63 ms | 1251.06 ms | 19.43 ms |
| 162cd7fe64d43a5dfd715b4d49fa04748acf451e | 1230.59 ms | 1256.76 ms | 26.16 ms |
| d1c0538b609161ffeaba47e9c1c952c80a3c6297 | 1227.49 ms | 1246.96 ms | 19.47 ms |
| 83d27f63a97748cdb36c1b4d485879e0baafd07c | 1233.56 ms | 1259.24 ms | 25.68 ms |
| 22b6996c73cfaa36ff18f44e779c9d7ef826622a | 1234.00 ms | 1263.24 ms | 29.24 ms |
| a2a3bfb93dea0fe83ec1622318f5c5b9edc553c7 | 1227.94 ms | 1261.26 ms | 33.32 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| aadbffecb9456d05f0ffd68a15ecf6418b611d7a | 23.75 KiB | 912.77 KiB | 889.02 KiB |
| 331dad69050678bbb80653e9f54d323190b0f95e | 23.75 KiB | 928.12 KiB | 904.37 KiB |
| 1fe932f4570f89448236c9195253f6cc2f8daa57 | 23.75 KiB | 913.63 KiB | 889.88 KiB |
| 701b301b0777a6cbe3bd97f45be62cb72edd1eb7 | 23.75 KiB | 867.16 KiB | 843.41 KiB |
| 3cdbc22dbeb82864b9eaf7e03ed756b0c14a8a5d | 23.75 KiB | 928.14 KiB | 904.40 KiB |
| 162cd7fe64d43a5dfd715b4d49fa04748acf451e | 23.75 KiB | 908.39 KiB | 884.64 KiB |
| d1c0538b609161ffeaba47e9c1c952c80a3c6297 | 23.75 KiB | 928.87 KiB | 905.12 KiB |
| 83d27f63a97748cdb36c1b4d485879e0baafd07c | 23.75 KiB | 928.88 KiB | 905.13 KiB |
| 22b6996c73cfaa36ff18f44e779c9d7ef826622a | 23.75 KiB | 908.02 KiB | 884.27 KiB |
| a2a3bfb93dea0fe83ec1622318f5c5b9edc553c7 | 23.75 KiB | 872.67 KiB | 848.92 KiB |
Previous results on branch: philprime/disable-screenshot-masking
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8ab12dd5d064063bc776aae00488c01e1f682ba | 1229.24 ms | 1262.60 ms | 33.35 ms |
| 76744f7b0bd9996a50099ccc393a1d589e33a390 | 1216.65 ms | 1238.20 ms | 21.55 ms |
| 54777d72cbe8ae6a5d14bb28b148a667682e889a | 1227.56 ms | 1258.04 ms | 30.48 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a8ab12dd5d064063bc776aae00488c01e1f682ba | 23.75 KiB | 933.85 KiB | 910.11 KiB |
| 76744f7b0bd9996a50099ccc393a1d589e33a390 | 23.75 KiB | 931.87 KiB | 908.12 KiB |
| 54777d72cbe8ae6a5d14bb28b148a667682e889a | 23.75 KiB | 933.85 KiB | 910.10 KiB |
@cursor review
@cursor review
After an internal discussion between @philipphofmann and myself, it was decided that even though the SentryScreenshotSource is bound to options, I will not introduce a per-options solution for the dependency container in this PR.