Skip to content
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

ref: Mask screenshots for errors #4623

Merged
merged 13 commits into from
Dec 19, 2024
Merged

ref: Mask screenshots for errors #4623

merged 13 commits into from
Dec 19, 2024

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Dec 12, 2024

📜 Description

Using api created for SR to mask a error screenshot

image

💡 Motivation and Context

Closes #3802

💚 How did you test it?

Samples

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

github-actions bot commented Dec 12, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1232.80 ms 1255.41 ms 22.60 ms
Size 22.30 KiB 761.10 KiB 738.80 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2af280d 1226.46 ms 1229.81 ms 3.35 ms
0759ae9 1207.82 ms 1218.06 ms 10.24 ms
9acdca2 1242.45 ms 1249.63 ms 7.18 ms
c0f08e7 1230.67 ms 1246.31 ms 15.63 ms
ddc9b9a 1210.00 ms 1234.18 ms 24.18 ms
c78683b 1246.71 ms 1258.70 ms 11.99 ms
f1a6589 1253.67 ms 1269.06 ms 15.39 ms
32e64d1 6272.19 ms 6299.50 ms 27.31 ms
aa45f36 1226.39 ms 1253.22 ms 26.84 ms
6c31077 1233.80 ms 1245.34 ms 11.54 ms

App size

Revision Plain With Sentry Diff
2af280d 20.76 KiB 435.22 KiB 414.46 KiB
0759ae9 21.58 KiB 671.00 KiB 649.42 KiB
9acdca2 22.84 KiB 401.44 KiB 378.59 KiB
c0f08e7 21.58 KiB 573.84 KiB 552.26 KiB
ddc9b9a 20.76 KiB 420.40 KiB 399.64 KiB
c78683b 20.76 KiB 435.24 KiB 414.48 KiB
f1a6589 22.85 KiB 408.87 KiB 386.02 KiB
32e64d1 20.76 KiB 433.18 KiB 412.42 KiB
aa45f36 21.58 KiB 616.73 KiB 595.15 KiB
6c31077 22.84 KiB 401.65 KiB 378.81 KiB

Previous results on branch: fix/redactscreenshots

Startup times

Revision Plain With Sentry Diff
f63da06 1222.54 ms 1240.85 ms 18.31 ms
1b9890c 1231.16 ms 1252.02 ms 20.86 ms
da115f7 1214.24 ms 1244.00 ms 29.76 ms
28af916 1213.06 ms 1247.86 ms 34.79 ms
79a596d 1242.64 ms 1254.19 ms 11.55 ms

App size

Revision Plain With Sentry Diff
f63da06 22.31 KiB 758.25 KiB 735.95 KiB
1b9890c 22.31 KiB 757.13 KiB 734.83 KiB
da115f7 22.31 KiB 757.13 KiB 734.83 KiB
28af916 22.31 KiB 757.59 KiB 735.28 KiB
79a596d 22.31 KiB 757.48 KiB 735.18 KiB

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 98.76543% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.592%. Comparing base (6da2718) to head (874900c).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Sentry/PrivateSentrySDKOnly.mm 0.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4623       +/-   ##
=============================================
+ Coverage   90.590%   90.592%   +0.002%     
=============================================
  Files          620       621        +1     
  Lines        71074     71094       +20     
  Branches     25292     25306       +14     
=============================================
+ Hits         64386     64406       +20     
+ Misses        6596      6595        -1     
- Partials        92        93        +1     
Files with missing lines Coverage Δ
Sources/Sentry/SentryDependencyContainer.m 96.710% <ø> (ø)
Sources/Sentry/SentryOptions.m 98.717% <100.000%> (+0.007%) ⬆️
Sources/Sentry/SentryScreenshot.m 75.609% <100.000%> (+1.925%) ⬆️
Sources/Sentry/SentryScreenshotIntegration.m 92.156% <ø> (ø)
Sources/Swift/Protocol/SentryRedactOptions.swift 100.000% <100.000%> (ø)
Sources/Swift/Tools/SentryViewPhotographer.swift 89.090% <100.000%> (+1.335%) ⬆️
Sources/Sentry/PrivateSentrySDKOnly.mm 16.080% <0.000%> (ø)

... and 7 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 data
Powered by Codecov. Last update 6da2718...874900c. Read the comment docs.

@brustolin brustolin marked this pull request as ready for review December 16, 2024 10:31
@philprime
Copy link
Contributor

Changing the SentryViewPhotographer.swift might affect the SDK crashes @kahest showed me last week with an exception caused with the path handling. Not sure if we have an issue for that yet.

@kahest
Copy link
Member

kahest commented Dec 16, 2024

Changing the SentryViewPhotographer.swift might affect the SDK crashes @kahest showed me last week with an exception caused with the path handling. Not sure if we have an issue for that yet.

This is the GH issue for the crash in SentryViewPhotographer.swift: #4558

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brustolin brustolin merged commit 950adcc into main Dec 19, 2024
68 of 70 checks passed
@brustolin brustolin deleted the fix/redactscreenshots branch December 19, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redact Screenshots via View Hierarchy
6 participants