-
-
Notifications
You must be signed in to change notification settings - Fork 342
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 Resumes replay when the app becomes active #4303
Conversation
Nice, such a small change. Is it possible to tests that the new session is created before the replay starts, I assume that was the root issue? That the replay start before the new session so it was appended to the previous session? |
We have tests to ensure that both session tracking and session replay end with the same type of notification. Therefore, the order in which they are called does not matter; both actions will occur before the next frame, which prevents a new session segment from being sent. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4303 +/- ##
=============================================
+ Coverage 91.648% 91.685% +0.036%
=============================================
Files 616 617 +1
Lines 50124 50212 +88
Branches 18041 18124 +83
=============================================
+ Hits 45938 46037 +99
+ Misses 4094 4084 -10
+ Partials 92 91 -1
... and 19 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚀
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e070f8a | 1236.47 ms | 1250.50 ms | 14.03 ms |
1ee5a37 | 1209.37 ms | 1222.04 ms | 12.68 ms |
7f691b5 | 1233.94 ms | 1243.80 ms | 9.86 ms |
38b36f5 | 1218.48 ms | 1246.61 ms | 28.13 ms |
7cd187e | 1196.51 ms | 1226.04 ms | 29.53 ms |
98cca71 | 1226.66 ms | 1242.94 ms | 16.28 ms |
c8dbe73 | 1235.49 ms | 1250.57 ms | 15.08 ms |
bb5dc7d | 1238.55 ms | 1247.44 ms | 8.89 ms |
f1d1166 | 1237.54 ms | 1256.06 ms | 18.52 ms |
b483671 | 1217.20 ms | 1236.82 ms | 19.62 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e070f8a | 21.58 KiB | 546.20 KiB | 524.62 KiB |
1ee5a37 | 21.58 KiB | 670.39 KiB | 648.81 KiB |
7f691b5 | 20.76 KiB | 420.55 KiB | 399.79 KiB |
38b36f5 | 21.58 KiB | 616.35 KiB | 594.77 KiB |
7cd187e | 20.76 KiB | 401.66 KiB | 380.90 KiB |
98cca71 | 22.85 KiB | 411.14 KiB | 388.29 KiB |
c8dbe73 | 21.58 KiB | 615.91 KiB | 594.33 KiB |
bb5dc7d | 22.85 KiB | 412.98 KiB | 390.13 KiB |
f1d1166 | 21.58 KiB | 630.74 KiB | 609.16 KiB |
b483671 | 20.76 KiB | 434.72 KiB | 413.96 KiB |
Changing the SR resume from "app enter foreground" to "app became active" to align with session tracker.
This will also prevent the problem happening in here.