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

feat: Add beforeCaptureScreenshot Callback #4016

Merged
merged 1 commit into from
May 27, 2024

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented May 27, 2024

📜 Description

Add a callback to the options to decide if the SDK should capture a screenshot or not.

PR for updating the docs: getsentry/sentry-docs#10152.

💡 Motivation and Context

Fixes GH-3137

💚 How did you test it?

Unit tests and sample app.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • 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.
  • 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

@philipphofmann philipphofmann changed the title feat: Add beforeCaptureScreenshotCallback feat: Add beforeCaptureScreenshot Callback May 27, 2024
Copy link

github-actions bot commented May 27, 2024

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

Generated by 🚫 dangerJS against cdbc68e

Add a callback to the options to decide if the SDK should capture a
screenshot or not.

Fixes GH-3137
Copy link

codecov bot commented May 27, 2024

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.854%. Comparing base (b189fbf) to head (cdbc68e).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4016       +/-   ##
=============================================
- Coverage   90.879%   90.854%   -0.026%     
=============================================
  Files          599       598        -1     
  Lines        46664     46601       -63     
  Branches     16713     16660       -53     
=============================================
- Hits         42408     42339       -69     
- Misses        4075      4080        +5     
- Partials       181       182        +1     
Files Coverage Δ
Sources/Sentry/SentryOptions.m 99.206% <100.000%> (+0.004%) ⬆️
Sources/Sentry/SentryScreenshotIntegration.m 94.444% <100.000%> (+0.505%) ⬆️
.../Screenshot/SentryScreenshotIntegrationTests.swift 99.166% <100.000%> (+0.084%) ⬆️
Tests/SentryTests/SentryOptionsTest.m 98.187% <66.666%> (-0.435%) ⬇️

... and 14 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 b189fbf...cdbc68e. Read the comment docs.

Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1238.61 ms 1256.10 ms 17.49 ms
Size 21.58 KiB 638.25 KiB 616.67 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5d6ce0e 1227.57 ms 1241.08 ms 13.51 ms
96eb740 1231.27 ms 1243.44 ms 12.17 ms
105a36c 1227.37 ms 1245.36 ms 17.99 ms
1b09e3f 1227.24 ms 1242.19 ms 14.95 ms
45d3ca5 1238.53 ms 1263.09 ms 24.55 ms
1734d1b 1198.69 ms 1221.62 ms 22.93 ms
0dfdaaa 1226.88 ms 1248.82 ms 21.94 ms
dfde86d 1215.12 ms 1221.30 ms 6.18 ms
b868c3a 1238.45 ms 1254.98 ms 16.53 ms
b35ccd0 1224.59 ms 1241.08 ms 16.49 ms

App size

Revision Plain With Sentry Diff
5d6ce0e 22.85 KiB 405.38 KiB 382.53 KiB
96eb740 21.58 KiB 572.50 KiB 550.92 KiB
105a36c 22.85 KiB 414.09 KiB 391.24 KiB
1b09e3f 21.58 KiB 614.93 KiB 593.34 KiB
45d3ca5 20.76 KiB 427.54 KiB 406.78 KiB
1734d1b 21.58 KiB 418.82 KiB 397.24 KiB
0dfdaaa 20.76 KiB 434.56 KiB 413.79 KiB
dfde86d 21.58 KiB 414.57 KiB 392.99 KiB
b868c3a 22.85 KiB 411.17 KiB 388.32 KiB
b35ccd0 21.58 KiB 573.14 KiB 551.56 KiB

@philipphofmann philipphofmann merged commit 86b89b9 into main May 27, 2024
68 of 71 checks passed
@philipphofmann philipphofmann deleted the feat/before-screenshot-callback branch May 27, 2024 10:13
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.

Add BeforeCaptureCallback - Screenshot
2 participants