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: Restart replay session with mobile session #4085

Merged
merged 14 commits into from
Jun 20, 2024
Merged

Conversation

brustolin
Copy link
Contributor

📜 Description

Starts a new replay session when the mobile session is restarted. Usually when the user stays away from the app for more than 60 seconds.

💚 How did you test it?

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

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.

I added a few comments.

Copy link

github-actions bot commented Jun 19, 2024

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

Generated by 🚫 dangerJS against 7526a7e

Copy link

codecov bot commented Jun 19, 2024

Codecov Report

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

Project coverage is 91.174%. Comparing base (f1c36e0) to head (7526a7e).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4085       +/-   ##
=============================================
+ Coverage   91.065%   91.174%   +0.108%     
=============================================
  Files          610       610               
  Lines        47750     47860      +110     
=============================================
+ Hits         43484     43636      +152     
+ Misses        4266      4224       -42     
Files Coverage Δ
SentryTestUtils/TestTransportAdapter.swift 69.230% <ø> (ø)
Sources/Sentry/SentryHub.m 98.314% <100.000%> (+0.024%) ⬆️
Sources/Sentry/SentrySessionReplay.m 86.813% <100.000%> (+0.702%) ⬆️
...onReplay/SentrySessionReplayIntegrationTests.swift 97.872% <100.000%> (+8.831%) ⬆️
Sources/Sentry/SentrySessionReplayIntegration.m 94.174% <98.611%> (+3.852%) ⬆️

... and 23 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 f1c36e0...7526a7e. Read the comment docs.

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.

Thanks for including all the feedback, LGTM 🚀

Copy link

github-actions bot commented Jun 19, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1216.96 ms 1223.83 ms 6.87 ms
Size 21.58 KiB 670.97 KiB 649.39 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
42f4107 1224.52 ms 1244.77 ms 20.25 ms
aeec206 1211.31 ms 1229.18 ms 17.87 ms
caa37b6 1221.19 ms 1238.71 ms 17.52 ms
160a073 1260.72 ms 1270.10 ms 9.38 ms
b11cd87 1243.19 ms 1260.84 ms 17.65 ms
b064665 1230.04 ms 1254.08 ms 24.04 ms
90d17d3 1261.18 ms 1278.18 ms 17.00 ms
ea73af6 1230.96 ms 1244.98 ms 14.02 ms
a366f3b 1230.65 ms 1251.88 ms 21.23 ms
1656cf6 1229.59 ms 1245.52 ms 15.93 ms

App size

Revision Plain With Sentry Diff
42f4107 21.58 KiB 614.92 KiB 593.34 KiB
aeec206 20.76 KiB 434.88 KiB 414.12 KiB
caa37b6 21.58 KiB 424.34 KiB 402.76 KiB
160a073 22.85 KiB 408.88 KiB 386.03 KiB
b11cd87 21.58 KiB 631.85 KiB 610.27 KiB
b064665 22.85 KiB 413.42 KiB 390.57 KiB
90d17d3 20.76 KiB 432.17 KiB 411.41 KiB
ea73af6 20.76 KiB 425.75 KiB 404.99 KiB
a366f3b 21.58 KiB 614.73 KiB 593.15 KiB
1656cf6 21.58 KiB 546.19 KiB 524.61 KiB

Previous results on branch: feat/reset-SR

Startup times

Revision Plain With Sentry Diff
e5648b7 1217.84 ms 1237.00 ms 19.16 ms

App size

Revision Plain With Sentry Diff
e5648b7 21.58 KiB 671.02 KiB 649.44 KiB

@brustolin brustolin merged commit 20a828b into main Jun 20, 2024
66 checks passed
@brustolin brustolin deleted the feat/reset-SR branch June 20, 2024 09:40
philipphofmann added a commit that referenced this pull request Jun 20, 2024
Fixes compiling unit tests introduced with GH-4085.
philipphofmann added a commit that referenced this pull request Jun 20, 2024
Fixes compiling unit tests introduced with GH-4085.
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

3 participants