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: Session Replay is GA #4384

Merged
merged 12 commits into from
Jan 3, 2025
Merged

feat: Session Replay is GA #4384

merged 12 commits into from
Jan 3, 2025

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Dec 17, 2024

🛑 Requires

  • Bump to Cocoa with SR GA
  • Bump to Android with SR GA

📜 Description

This PR adds SR GA changelog entry and deprecates _experiments options which will be removed in the next major version.

(I choose to deprecate options in _experiments and not to remove them, as with removal the SR would silently stop working for our beta users.)

This PR updates the native implementation to use GA options of Sentry Cocoa and Android.

💚 How did you test it?

updated tests, native implementation is waiting for the native SDKs update

📝 Checklist

  • 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.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

@krystofwoldrich krystofwoldrich requested review from kahest and removed request for antonis and lucas-zimerman December 17, 2024 11:08
Copy link
Contributor

github-actions bot commented Dec 17, 2024

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

Generated by 🚫 dangerJS against f96f2de

Copy link
Contributor

github-actions bot commented Dec 17, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1239.81 ms 1235.73 ms -4.08 ms
Size 2.63 MiB 3.68 MiB 1.05 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
5a22220+dirty 1209.49 ms 1220.94 ms 11.45 ms
f06c879+dirty 1252.64 ms 1259.66 ms 7.02 ms
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
5bb8d5f+dirty 1235.47 ms 1237.39 ms 1.92 ms
4cc5c27+dirty 1211.45 ms 1214.60 ms 3.16 ms
b95b8af+dirty 1221.39 ms 1228.52 ms 7.13 ms
dadc233+dirty 1223.20 ms 1236.88 ms 13.68 ms
1faf8e3+dirty 1214.87 ms 1222.83 ms 7.97 ms
7bc4d75+dirty 1233.40 ms 1229.56 ms -3.83 ms
1332acb+dirty 1230.53 ms 1234.54 ms 4.01 ms

App size

Revision Plain With Sentry Diff
5a22220+dirty 2.36 MiB 2.92 MiB 570.21 KiB
f06c879+dirty 2.36 MiB 2.88 MiB 530.42 KiB
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
5bb8d5f+dirty 2.36 MiB 2.92 MiB 570.22 KiB
4cc5c27+dirty 2.36 MiB 3.04 MiB 698.52 KiB
b95b8af+dirty 2.36 MiB 3.14 MiB 793.32 KiB
dadc233+dirty 2.36 MiB 2.84 MiB 486.85 KiB
1faf8e3+dirty 2.36 MiB 3.08 MiB 736.75 KiB
7bc4d75+dirty 2.36 MiB 3.10 MiB 752.58 KiB
1332acb+dirty 2.36 MiB 3.11 MiB 759.86 KiB

Copy link
Contributor

github-actions bot commented Dec 17, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.47 ms 1215.58 ms 1.11 ms
Size 3.19 MiB 4.25 MiB 1.06 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
575f9da+dirty 1272.00 ms 1284.38 ms 12.38 ms
57448c5+dirty 1244.65 ms 1249.20 ms 4.55 ms
6c56eb1+dirty 1240.96 ms 1243.41 ms 2.45 ms
8900e1a+dirty 1268.36 ms 1273.04 ms 4.68 ms
acadc0f+dirty 1271.12 ms 1272.28 ms 1.16 ms
9a3ca65+dirty 1276.40 ms 1279.14 ms 2.74 ms
52c0562+dirty 1233.94 ms 1226.29 ms -7.65 ms
e73d82f+dirty 1231.20 ms 1228.81 ms -2.40 ms
1332acb+dirty 1243.98 ms 1241.12 ms -2.86 ms
457e29f+dirty 1256.71 ms 1258.50 ms 1.79 ms

App size

Revision Plain With Sentry Diff
575f9da+dirty 2.92 MiB 3.43 MiB 524.26 KiB
57448c5+dirty 2.92 MiB 3.66 MiB 756.65 KiB
6c56eb1+dirty 2.92 MiB 3.67 MiB 772.48 KiB
8900e1a+dirty 2.92 MiB 3.39 MiB 485.96 KiB
acadc0f+dirty 2.92 MiB 3.39 MiB 487.34 KiB
9a3ca65+dirty 2.92 MiB 3.37 MiB 464.32 KiB
52c0562+dirty 2.92 MiB 3.69 MiB 794.15 KiB
e73d82f+dirty 2.92 MiB 3.64 MiB 740.56 KiB
1332acb+dirty 2.92 MiB 3.67 MiB 772.45 KiB
457e29f+dirty 2.92 MiB 3.43 MiB 524.75 KiB

Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Thank you for deprecating _experiments for SR 🙇
The changes on this PR LGTM 🚀
I restrain from approving till the Cocoa and Android SDKs land and I can fully test the changes.

NIT: also found a few more places that we may want to update:

@romtsn
Copy link
Member

romtsn commented Jan 3, 2025

@antonis I've updated the native SDKs and also adjusted the code to make it compiling. Also addressed your nit about the leftovers that use the deprecated options. Could you PTAL once more? All build failures look like a flake to me.

Here's also a relevant Android replay from this branch: https://sentry-sdks.sentry.io/replays/86ec775d2df04b21bd6c4af47478556d/?project=5428561&query=&referrer=%2Freplays%2F%3AreplaySlug%2F&statsPeriod=1h&t_main=tags&yAxis=count%28%29&t=0

I don't have the latest ios simulator, so if you could test it on iOS that would be great!

@antonis
Copy link
Collaborator

antonis commented Jan 3, 2025

Hey @romtsn 👋
Looking at it now.

All build failures look like a flake to me.

The metrics failures are unrelated with this PR #4413 and I'm trying to tackle them with #4414

@antonis antonis self-requested a review January 3, 2025 13:20
Copy link
Contributor

github-actions bot commented Jan 3, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 416.24 ms 504.31 ms 88.07 ms
Size 17.74 MiB 20.10 MiB 2.36 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e2b64fe 316.88 ms 330.23 ms 13.35 ms
9c48b2c 349.24 ms 385.96 ms 36.72 ms
d43a46b 454.22 ms 477.79 ms 23.57 ms
8ab11b6 462.10 ms 450.40 ms -11.70 ms
0ebca77 414.93 ms 444.49 ms 29.56 ms
62a750b 395.96 ms 423.36 ms 27.41 ms
0d3e677 422.82 ms 411.90 ms -10.92 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms
1332acb 493.40 ms 475.20 ms -18.21 ms
9282172 486.12 ms 467.70 ms -18.42 ms

App size

Revision Plain With Sentry Diff
e2b64fe 17.73 MiB 19.80 MiB 2.07 MiB
9c48b2c 17.73 MiB 19.80 MiB 2.07 MiB
d43a46b 17.73 MiB 20.06 MiB 2.33 MiB
8ab11b6 17.74 MiB 20.09 MiB 2.35 MiB
0ebca77 17.73 MiB 19.95 MiB 2.21 MiB
62a750b 17.73 MiB 19.93 MiB 2.20 MiB
0d3e677 17.74 MiB 20.07 MiB 2.34 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB
1332acb 17.74 MiB 20.09 MiB 2.35 MiB
9282172 17.74 MiB 20.09 MiB 2.35 MiB

Copy link
Collaborator

@antonis antonis left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
Thank you for your work on this @krystofwoldrich and for helping move this forward @romtsn 🙇

I took the liberty of pushing a minor test fix with c12f51e

Copy link
Contributor

github-actions bot commented Jan 3, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 417.11 ms 450.38 ms 33.27 ms
Size 7.15 MiB 8.38 MiB 1.23 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
690220d+dirty 492.10 ms 473.88 ms -18.23 ms
8e80789+dirty 464.48 ms 551.10 ms 86.63 ms
d8e8c67+dirty 363.77 ms 408.66 ms 44.89 ms
484813b+dirty 411.65 ms 456.02 ms 44.37 ms
13f280b+dirty 376.13 ms 406.41 ms 30.28 ms
8ae23a7+dirty 398.10 ms 411.48 ms 13.38 ms
12427f4+dirty 379.48 ms 400.92 ms 21.44 ms
1faf8e3+dirty 412.83 ms 470.49 ms 57.66 ms
0677344+dirty 288.40 ms 391.44 ms 103.04 ms
63ed251+dirty 485.02 ms 531.16 ms 46.14 ms

App size

Revision Plain With Sentry Diff
690220d+dirty 7.15 MiB 8.36 MiB 1.21 MiB
8e80789+dirty 7.15 MiB 8.37 MiB 1.22 MiB
d8e8c67+dirty 7.15 MiB 8.38 MiB 1.23 MiB
484813b+dirty 7.15 MiB 8.34 MiB 1.19 MiB
13f280b+dirty 7.15 MiB 8.35 MiB 1.20 MiB
8ae23a7+dirty 7.15 MiB 8.35 MiB 1.20 MiB
12427f4+dirty 7.15 MiB 8.12 MiB 997.78 KiB
1faf8e3+dirty 7.15 MiB 8.35 MiB 1.20 MiB
0677344+dirty 7.15 MiB 8.07 MiB 949.80 KiB
63ed251+dirty 7.15 MiB 8.35 MiB 1.20 MiB

@antonis antonis merged commit 5dff5ee into main Jan 3, 2025
69 of 70 checks passed
@antonis antonis deleted the kw/update-replay-for-ga branch January 3, 2025 15:36
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.

4 participants