Skip to content

feat: Replay for crashes #4171

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

Merged
merged 29 commits into from
Jul 29, 2024
Merged

feat: Replay for crashes #4171

merged 29 commits into from
Jul 29, 2024

Conversation

brustolin
Copy link
Contributor

@brustolin brustolin commented Jul 16, 2024

📜 Description

Send the replay for the final moments before a crash.

💚 How did you test it?

Sample and Unit tests

📝 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

Sorry, something went wrong.

brustolin added 8 commits July 9, 2024 08:36
wip

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini
wip

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini
wip

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini
wip

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini

Verified

This commit was signed with the committer’s verified signature.
pietroalbini Pietro Albini
Copy link
Contributor

github-actions bot commented Jul 16, 2024

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

Generated by 🚫 dangerJS against 5355385

Copy link
Contributor

github-actions bot commented Jul 16, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 16, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 16, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1233.36 ms 1246.98 ms 13.62 ms
Size 21.58 KiB 693.49 KiB 671.91 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0af45a9 1225.75 ms 1245.38 ms 19.63 ms
4259afd 1234.04 ms 1256.76 ms 22.72 ms
7fe37ab 1228.92 ms 1243.86 ms 14.94 ms
2719ce6 1211.75 ms 1237.16 ms 25.41 ms
d3edf46 1213.00 ms 1227.46 ms 14.46 ms
1bd0055 1207.57 ms 1223.10 ms 15.53 ms
ba4d16f 1236.90 ms 1252.12 ms 15.22 ms
556c407 1256.56 ms 1274.60 ms 18.04 ms
42ef6ba 1234.35 ms 1252.29 ms 17.94 ms
2de284c 1234.92 ms 1254.45 ms 19.53 ms

App size

Revision Plain With Sentry Diff
0af45a9 21.58 KiB 631.19 KiB 609.61 KiB
4259afd 20.76 KiB 419.70 KiB 398.94 KiB
7fe37ab 21.58 KiB 542.28 KiB 520.70 KiB
2719ce6 20.76 KiB 435.13 KiB 414.37 KiB
d3edf46 20.76 KiB 436.65 KiB 415.89 KiB
1bd0055 20.76 KiB 420.22 KiB 399.46 KiB
ba4d16f 20.76 KiB 437.12 KiB 416.36 KiB
556c407 22.85 KiB 413.98 KiB 391.13 KiB
42ef6ba 21.58 KiB 417.86 KiB 396.28 KiB
2de284c 21.58 KiB 542.39 KiB 520.80 KiB

Previous results on branch: feat(SR)/replay-for-crashes

Startup times

Revision Plain With Sentry Diff
c4968e8 1235.15 ms 1240.56 ms 5.41 ms
4e094da 1234.02 ms 1249.94 ms 15.92 ms
2087b3a 1209.91 ms 1229.59 ms 19.68 ms
24df457 1234.08 ms 1247.56 ms 13.48 ms
2dfce84 1235.27 ms 1251.76 ms 16.49 ms
8ce2896 1236.67 ms 1253.31 ms 16.63 ms
8c670b9 1234.15 ms 1245.26 ms 11.11 ms
e849a60 1225.57 ms 1232.11 ms 6.53 ms

App size

Revision Plain With Sentry Diff
c4968e8 21.58 KiB 693.35 KiB 671.77 KiB
4e094da 21.58 KiB 693.24 KiB 671.66 KiB
2087b3a 21.58 KiB 692.47 KiB 670.89 KiB
24df457 21.58 KiB 693.66 KiB 672.08 KiB
2dfce84 21.58 KiB 692.33 KiB 670.75 KiB
8ce2896 21.58 KiB 693.65 KiB 672.07 KiB
8c670b9 21.58 KiB 693.33 KiB 671.75 KiB
e849a60 21.58 KiB 693.51 KiB 671.92 KiB

Copy link
Contributor

github-actions bot commented Jul 17, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

Attention: Patch coverage is 88.71473% with 36 lines in your changes missing coverage. Please review.

Project coverage is 91.388%. Comparing base (061c982) to head (e13488d).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4171       +/-   ##
=============================================
- Coverage   91.402%   91.388%   -0.015%     
=============================================
  Files          606       606               
  Lines        48690     48875      +185     
  Branches     17567     17596       +29     
=============================================
+ Hits         44504     44666      +162     
- Misses        4093      4116       +23     
  Partials        93        93               
Files Coverage Δ
SentryTestUtils/TestHub.swift 75.862% <100.000%> (+3.862%) ⬆️
Sources/Sentry/SentryClient.m 98.588% <100.000%> (+0.014%) ⬆️
Sources/Sentry/SentryNetworkTracker.m 96.825% <100.000%> (+0.010%) ⬆️
Sources/Sentry/SentryOptions.m 99.250% <100.000%> (ø)
Sources/Sentry/SentryTraceContext.m 92.783% <100.000%> (+1.030%) ⬆️
...grations/SessionReplay/SentryReplayRecording.swift 100.000% <100.000%> (ø)
...erformance/Network/SentryNetworkTrackerTests.swift 98.452% <100.000%> (ø)
...ions/SessionReplay/SentryOnDemandReplayTests.swift 97.938% <100.000%> (ø)
...tions/SessionReplay/SentrySessionReplayTests.swift 97.633% <100.000%> (ø)
Tests/SentryTests/PrivateSentrySDKOnlyTests.swift 99.625% <100.000%> (+0.001%) ⬆️
... and 10 more

... and 17 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 061c982...e13488d. Read the comment docs.

Copy link
Contributor

github-actions bot commented Jul 17, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 18, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 18, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

@brustolin brustolin marked this pull request as ready for review July 18, 2024 14:36
Copy link
Contributor

github-actions bot commented Jul 23, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 23, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

First pass just looking at the code. I plan to come back around and try running the tests and poking things.

Co-authored-by: Andrew McKnight <andrew.mcknight@sentry.io>
Copy link
Contributor

github-actions bot commented Jul 24, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 24, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 24, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Contributor

github-actions bot commented Jul 24, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

I have a couple outstanding questions about threading and UIScene but don't think those should hold up this PR at this point; we should continue that discussion separately and fix any issues we uncover in a subsequent PR. Thanks for the work here!

Copy link
Contributor

github-actions bot commented Jul 25, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Copy link
Contributor

github-actions bot commented Jul 25, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Sorry, something went wrong.

@brustolin brustolin merged commit fb2bfe8 into main Jul 29, 2024
@brustolin brustolin deleted the feat(SR)/replay-for-crashes branch July 29, 2024 07:27
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.

None yet

4 participants