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: Screenshot Attachment #1088

Merged
merged 60 commits into from
Nov 9, 2022
Merged

Feat: Screenshot Attachment #1088

merged 60 commits into from
Nov 9, 2022

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Oct 24, 2022

📜 Description

Closes #556 (comment)

Add an option to add screenshots on errors. Based on the work of @ueman in #576 and the iOS implementation of @brustolin getsentry/sentry-cocoa#1751

Bildschirmfoto 2022-10-24 um 17 04 38

💡 Motivation and Context

  • Attach screenshots to errors.
  • Users just need to add the SentryWidget to enable this automatically.

💚 How did you test it?

  • Tested with the example app.
  • Added unit tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

For now SentryWidget only has screenshot support. Should we also provide an option to opt-out of screenshots through options?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2022

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

Generated by 🚫 dangerJS against 1b68c67

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2022

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 297.94 ms 343.49 ms 45.55 ms
Size 5.94 MiB 6.95 MiB 1.01 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
eecbbca 324.37 ms 352.49 ms 28.12 ms
5603ab2 309.84 ms 345.20 ms 35.36 ms
ef2f368 350.06 ms 429.44 ms 79.38 ms
3e5ee37 317.56 ms 366.84 ms 49.28 ms
0db91cc 327.85 ms 387.31 ms 59.46 ms
7ade5af 341.04 ms 386.84 ms 45.80 ms
af2d175 279.08 ms 312.37 ms 33.29 ms
f922f8f 332.31 ms 374.67 ms 42.37 ms
04db237 330.16 ms 428.38 ms 98.22 ms
72dfc83 298.62 ms 340.14 ms 41.52 ms

App size

Revision Plain With Sentry Diff
eecbbca 5.94 MiB 6.89 MiB 975.78 KiB
5603ab2 5.94 MiB 6.95 MiB 1.01 MiB
ef2f368 5.94 MiB 6.89 MiB 975.81 KiB
3e5ee37 5.94 MiB 6.92 MiB 1001.19 KiB
0db91cc 5.94 MiB 6.95 MiB 1.01 MiB
7ade5af 5.94 MiB 6.95 MiB 1.01 MiB
af2d175 5.94 MiB 6.92 MiB 1001.83 KiB
f922f8f 5.94 MiB 6.95 MiB 1.01 MiB
04db237 5.94 MiB 6.95 MiB 1.01 MiB
72dfc83 5.94 MiB 6.92 MiB 1001.71 KiB

Previous results on branch: feat/screenshot_atachment

Startup times

Revision Plain With Sentry Diff
f0392b4 353.59 ms 397.26 ms 43.67 ms
fca116e 308.92 ms 362.55 ms 53.63 ms
e477d14 333.90 ms 372.88 ms 38.98 ms
14b157f 330.22 ms 374.51 ms 44.29 ms
693b5ce 294.31 ms 344.84 ms 50.53 ms
760a472 320.79 ms 361.00 ms 40.21 ms
33aa55c 345.79 ms 397.82 ms 52.03 ms
ce8eb33 295.85 ms 329.85 ms 34.00 ms
4a4b0df 285.45 ms 343.50 ms 58.05 ms
e72e124 295.00 ms 354.50 ms 59.50 ms

App size

Revision Plain With Sentry Diff
f0392b4 5.94 MiB 6.95 MiB 1.01 MiB
fca116e 5.94 MiB 6.95 MiB 1.01 MiB
e477d14 5.94 MiB 6.92 MiB 1010.58 KiB
14b157f 5.94 MiB 6.92 MiB 1007.31 KiB
693b5ce 5.94 MiB 6.92 MiB 1007.34 KiB
760a472 5.94 MiB 6.95 MiB 1.01 MiB
33aa55c 5.94 MiB 6.93 MiB 1011.13 KiB
ce8eb33 5.94 MiB 6.92 MiB 1007.34 KiB
4a4b0df 5.94 MiB 6.93 MiB 1011.13 KiB
e72e124 5.94 MiB 6.93 MiB 1011.10 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2022

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1257.17 ms 1292.11 ms 34.94 ms
Size 8.15 MiB 9.15 MiB 1020.73 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d7758e8 1271.69 ms 1288.08 ms 16.39 ms
1c6eb5b 1277.85 ms 1285.71 ms 7.86 ms
72dfc83 1262.50 ms 1289.75 ms 27.25 ms
9c5aec6 1266.51 ms 1274.65 ms 8.14 ms
5603ab2 1268.47 ms 1280.73 ms 12.26 ms
6d317ea 1277.27 ms 1287.47 ms 10.20 ms
eb1a7c1 1281.25 ms 1295.40 ms 14.15 ms
4efee31 1270.33 ms 1285.75 ms 15.42 ms
559d28f 1265.04 ms 1288.96 ms 23.92 ms
0ceb89c 1252.02 ms 1271.78 ms 19.75 ms

App size

Revision Plain With Sentry Diff
d7758e8 8.15 MiB 9.12 MiB 989.76 KiB
1c6eb5b 8.15 MiB 9.12 MiB 986.27 KiB
72dfc83 8.15 MiB 9.12 MiB 987.30 KiB
9c5aec6 8.15 MiB 9.12 MiB 986.23 KiB
5603ab2 8.15 MiB 9.12 MiB 990.57 KiB
6d317ea 8.15 MiB 9.12 MiB 986.26 KiB
eb1a7c1 8.15 MiB 9.13 MiB 1000.10 KiB
4efee31 8.15 MiB 9.12 MiB 991.35 KiB
559d28f 8.15 MiB 9.12 MiB 987.32 KiB
0ceb89c 8.15 MiB 9.12 MiB 989.78 KiB

Previous results on branch: feat/screenshot_atachment

Startup times

Revision Plain With Sentry Diff
0c43605 1251.48 ms 1267.72 ms 16.24 ms
693b5ce 1284.82 ms 1301.45 ms 16.63 ms
e477d14 1265.68 ms 1284.53 ms 18.85 ms
f35689f 1264.02 ms 1284.87 ms 20.85 ms
760a472 1249.83 ms 1267.69 ms 17.86 ms
33aa55c 1258.41 ms 1276.41 ms 18.00 ms
ce8eb33 1273.43 ms 1289.42 ms 15.99 ms
16dede7 1262.96 ms 1276.55 ms 13.59 ms
9c9d5f0 1269.86 ms 1289.38 ms 19.52 ms
4a4b0df 1274.14 ms 1277.94 ms 3.79 ms

App size

Revision Plain With Sentry Diff
0c43605 8.15 MiB 9.14 MiB 1008.79 KiB
693b5ce 8.15 MiB 9.14 MiB 1004.91 KiB
e477d14 8.15 MiB 9.14 MiB 1008.83 KiB
f35689f 8.15 MiB 9.14 MiB 1008.45 KiB
760a472 8.15 MiB 9.14 MiB 1007.68 KiB
33aa55c 8.15 MiB 9.14 MiB 1008.45 KiB
ce8eb33 8.15 MiB 9.14 MiB 1004.87 KiB
16dede7 8.15 MiB 9.15 MiB 1020.72 KiB
9c9d5f0 8.15 MiB 9.14 MiB 1005.00 KiB
4a4b0df 8.15 MiB 9.14 MiB 1008.46 KiB

@ueman ueman mentioned this pull request Oct 24, 2022
5 tasks
@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2022

Codecov Report

Base: 95.55% // Head: 91.66% // Decreases project coverage by -3.88% ⚠️

Coverage data is based on head (1b68c67) compared to base (3a69405).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1088      +/-   ##
==========================================
- Coverage   95.55%   91.66%   -3.89%     
==========================================
  Files           2        9       +7     
  Lines          45      192     +147     
==========================================
+ Hits           43      176     +133     
- Misses          2       16      +14     
Impacted Files Coverage Δ
dio/lib/src/sentry_dio_client_adapter.dart 77.77% <0.00%> (ø)
dio/lib/src/failed_request_interceptor.dart 100.00% <0.00%> (ø)
dio/lib/src/sentry_dio_extension.dart 83.33% <0.00%> (ø)
dio/lib/src/sentry_transformer.dart 94.44% <0.00%> (ø)
dio/lib/src/dio_event_processor.dart 96.72% <0.00%> (ø)
dio/lib/src/breadcrumb_client_adapter.dart 80.00% <0.00%> (ø)
dio/lib/src/tracing_client_adapter.dart 85.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@denrase
Copy link
Collaborator Author

denrase commented Oct 25, 2022

@marandaneto There seems to be an issue with PackageInfoPlugin.kt on Android. Also saw this on min version test, have you seen this elsewhere?

@denrase denrase marked this pull request as ready for review October 25, 2022 11:41
@denrase
Copy link
Collaborator Author

denrase commented Nov 8, 2022

@marandaneto Just tested a release build of the example app on an iPhone 13 Pro. Feature works as expected. https://sentry.io/organizations/sentry-sdks/issues/3679081037/?project=5428562&query=is%3Aunresolved&referrer=issue-stream

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

Thanks @ueman for the PoC and thanks @denrase for the final impl.

@marandaneto
Copy link
Contributor

@marandaneto Just tested a release build of the example app on an iPhone 13 Pro. Feature works as expected. sentry.io/organizations/sentry-sdks/issues/3679081037/?project=5428562&query=is%3Aunresolved&referrer=issue-stream

Tested on Android, iOS and Web with canvaskit, all good =)

@marandaneto marandaneto enabled auto-merge (squash) November 9, 2022 10:02
@marandaneto marandaneto merged commit b728df4 into main Nov 9, 2022
@marandaneto marandaneto deleted the feat/screenshot_atachment branch November 9, 2022 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: EA 🏁
Development

Successfully merging this pull request may close these issues.

Support screenshot feature (via attachments)
4 participants