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

fix/screenshot masking during changes #2553

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Jan 7, 2025

  • capture screenshots in sequence until they're stable
  • improve timestamps
  • fix tests & future handling

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 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

🔮 Next steps

Copy link
Contributor

github-actions bot commented Jan 7, 2025

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/lib/src/screenshot/recorder.dart

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- fix/screenshot masking during changes ([#2553](https://github.com/getsentry/sentry-dart/pull/2553))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 4439231

Copy link

codecov bot commented Jan 7, 2025

Codecov Report

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

Project coverage is 92.05%. Comparing base (3a4de05) to head (4439231).

Files with missing lines Patch % Lines
flutter/lib/src/screenshot/retrier.dart 89.65% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2553      +/-   ##
==========================================
+ Coverage   87.21%   92.05%   +4.84%     
==========================================
  Files         257       90     -167     
  Lines        8847     3122    -5725     
==========================================
- Hits         7716     2874    -4842     
+ Misses       1131      248     -883     

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

Copy link
Contributor

github-actions bot commented Jan 7, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1253.86 ms 1273.80 ms 19.94 ms
Size 8.42 MiB 9.88 MiB 1.46 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c732386 1233.20 ms 1252.08 ms 18.88 ms
934f10e 1245.18 ms 1263.77 ms 18.59 ms
dd933d4 1238.73 ms 1252.43 ms 13.70 ms
e3d9076 1203.68 ms 1230.65 ms 26.97 ms
1b0c8a3 1259.30 ms 1282.78 ms 23.48 ms
02661eb 1244.45 ms 1252.37 ms 7.92 ms
1a93825 1257.25 ms 1261.55 ms 4.30 ms
00236a7 1250.06 ms 1274.00 ms 23.94 ms
8a10ab7 1226.49 ms 1250.52 ms 24.03 ms
3f23617 1261.93 ms 1286.10 ms 24.17 ms

App size

Revision Plain With Sentry Diff
c732386 8.28 MiB 9.33 MiB 1.05 MiB
934f10e 8.38 MiB 9.77 MiB 1.39 MiB
dd933d4 8.33 MiB 9.64 MiB 1.31 MiB
e3d9076 8.33 MiB 9.40 MiB 1.07 MiB
1b0c8a3 8.38 MiB 9.75 MiB 1.37 MiB
02661eb 8.42 MiB 9.86 MiB 1.44 MiB
1a93825 8.28 MiB 9.34 MiB 1.05 MiB
00236a7 8.38 MiB 9.77 MiB 1.39 MiB
8a10ab7 8.28 MiB 9.34 MiB 1.06 MiB
3f23617 8.16 MiB 9.17 MiB 1.01 MiB

Previous results on branch: fix/screenshot-masking-during-changes

Startup times

Revision Plain With Sentry Diff
d0c6c01 1238.92 ms 1250.12 ms 11.20 ms
9d4a3f9 1246.96 ms 1257.80 ms 10.84 ms
9bedc01 1238.04 ms 1267.28 ms 29.24 ms

App size

Revision Plain With Sentry Diff
d0c6c01 8.42 MiB 9.88 MiB 1.46 MiB
9d4a3f9 8.42 MiB 9.88 MiB 1.46 MiB
9bedc01 8.42 MiB 9.88 MiB 1.46 MiB

Copy link
Contributor

github-actions bot commented Jan 8, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 456.56 ms 554.10 ms 97.54 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4b943a1 348.17 ms 437.15 ms 88.98 ms
1596141 300.92 ms 368.94 ms 68.02 ms
afa6e2a 349.73 ms 428.48 ms 78.75 ms
9e7630d 449.43 ms 475.82 ms 26.39 ms
ee0ca56 355.35 ms 421.13 ms 65.78 ms
07cd9e8 427.71 ms 461.23 ms 33.52 ms
2e93bab 515.33 ms 558.79 ms 43.46 ms
9a5040f 490.15 ms 563.98 ms 73.83 ms
f735167 404.38 ms 412.57 ms 8.19 ms
3adbea9 395.16 ms 447.88 ms 52.71 ms

App size

Revision Plain With Sentry Diff
4b943a1 6.34 MiB 7.28 MiB 968.41 KiB
1596141 6.16 MiB 7.14 MiB 1003.98 KiB
afa6e2a 6.27 MiB 7.20 MiB 955.69 KiB
9e7630d 6.49 MiB 7.56 MiB 1.07 MiB
ee0ca56 6.33 MiB 7.30 MiB 992.52 KiB
07cd9e8 6.49 MiB 7.56 MiB 1.07 MiB
2e93bab 6.49 MiB 7.55 MiB 1.07 MiB
9a5040f 6.46 MiB 7.48 MiB 1.01 MiB
f735167 6.46 MiB 7.48 MiB 1.01 MiB
3adbea9 6.52 MiB 7.61 MiB 1.09 MiB

Previous results on branch: fix/screenshot-masking-during-changes

Startup times

Revision Plain With Sentry Diff
9bedc01 454.42 ms 500.27 ms 45.84 ms
d0c6c01 483.34 ms 564.12 ms 80.78 ms
9d4a3f9 442.54 ms 510.83 ms 68.29 ms

App size

Revision Plain With Sentry Diff
9bedc01 6.46 MiB 7.48 MiB 1.02 MiB
d0c6c01 6.46 MiB 7.48 MiB 1.01 MiB
9d4a3f9 6.46 MiB 7.48 MiB 1.02 MiB

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.

1 participant