Skip to content

ref(session-replay): iOS: Use enableViewRendererV2 instead of the dep… #4815

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 7 commits into from
May 15, 2025

Conversation

antonis
Copy link
Collaborator

@antonis antonis commented May 7, 2025

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Use enableViewRendererV2 which defaults to true instead of the renamed/deprecated enableExperimentalViewRenderer

💡 Motivation and Context

enableExperimentalViewRenderer was renamed/deprecated in 8.50.0 with getsentry/sentry-cocoa#5054

💚 How did you test it?

CI, Manual

📝 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

🔮 Next steps

@antonis antonis marked this pull request as ready for review May 7, 2025 08:29
Copy link
Contributor

github-actions bot commented May 7, 2025

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 466.98 ms 452.85 ms -14.13 ms
Size 17.75 MiB 20.13 MiB 2.38 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d997097 470.23 ms 475.46 ms 5.23 ms
87d396c 463.52 ms 500.31 ms 36.79 ms
0c98663 437.75 ms 420.70 ms -17.05 ms
4a6664f 548.79 ms 585.00 ms 36.21 ms
0ebca77 414.93 ms 444.49 ms 29.56 ms
15c80ab+dirty 336.27 ms 350.58 ms 14.31 ms
ac41368 451.47 ms 453.67 ms 2.20 ms
d16beca 448.87 ms 447.20 ms -1.67 ms
e1e6bc7 479.96 ms 482.58 ms 2.62 ms
31fcca2 391.22 ms 414.78 ms 23.56 ms

App size

Revision Plain With Sentry Diff
d997097 17.75 MiB 20.11 MiB 2.36 MiB
87d396c 17.75 MiB 20.13 MiB 2.38 MiB
0c98663 17.75 MiB 20.12 MiB 2.37 MiB
4a6664f 17.73 MiB 19.94 MiB 2.21 MiB
0ebca77 17.73 MiB 19.95 MiB 2.21 MiB
15c80ab+dirty 17.73 MiB 20.04 MiB 2.31 MiB
ac41368 17.73 MiB 20.11 MiB 2.38 MiB
d16beca 17.74 MiB 20.10 MiB 2.36 MiB
e1e6bc7 17.75 MiB 20.13 MiB 2.38 MiB
31fcca2 17.73 MiB 19.90 MiB 2.17 MiB

Previous results on branch: antonis/ios-deprecate-experimentalViewRenderer

Startup times

Revision Plain With Sentry Diff
5501305 461.57 ms 472.17 ms 10.60 ms
50d030e 400.22 ms 431.93 ms 31.70 ms

App size

Revision Plain With Sentry Diff
5501305 17.75 MiB 20.13 MiB 2.38 MiB
50d030e 17.75 MiB 20.13 MiB 2.38 MiB

Copy link
Contributor

github-actions bot commented May 7, 2025

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 477.54 ms 499.30 ms 21.76 ms
Size 7.15 MiB 8.40 MiB 1.25 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
27ef4ee+dirty 296.71 ms 351.00 ms 54.29 ms
30189be+dirty 362.02 ms 386.80 ms 24.78 ms
9dabcce+dirty 359.66 ms 430.73 ms 71.08 ms
1332acb+dirty 385.00 ms 404.80 ms 19.80 ms
0677344+dirty 288.40 ms 391.44 ms 103.04 ms
0eacc98+dirty 393.31 ms 445.21 ms 51.90 ms
a5d86e1+dirty 380.86 ms 466.42 ms 85.56 ms
a38594f+dirty 393.83 ms 422.12 ms 28.29 ms
148f924+dirty 347.36 ms 389.13 ms 41.77 ms
d43a46b+dirty 417.65 ms 472.98 ms 55.33 ms

App size

Revision Plain With Sentry Diff
27ef4ee+dirty 7.15 MiB 8.08 MiB 959.49 KiB
30189be+dirty 7.15 MiB 8.38 MiB 1.23 MiB
9dabcce+dirty 7.15 MiB 8.37 MiB 1.22 MiB
1332acb+dirty 7.15 MiB 8.37 MiB 1.22 MiB
0677344+dirty 7.15 MiB 8.07 MiB 949.80 KiB
0eacc98+dirty 7.15 MiB 8.38 MiB 1.23 MiB
a5d86e1+dirty 7.15 MiB 8.35 MiB 1.20 MiB
a38594f+dirty 7.15 MiB 8.38 MiB 1.23 MiB
148f924+dirty 7.15 MiB 8.21 MiB 1.07 MiB
d43a46b+dirty 7.15 MiB 8.34 MiB 1.19 MiB

Previous results on branch: antonis/ios-deprecate-experimentalViewRenderer

Startup times

Revision Plain With Sentry Diff
50d030e+dirty 394.00 ms 390.15 ms -3.85 ms
5501305+dirty 424.60 ms 438.47 ms 13.87 ms

App size

Revision Plain With Sentry Diff
50d030e+dirty 7.15 MiB 8.40 MiB 1.25 MiB
5501305+dirty 7.15 MiB 8.40 MiB 1.25 MiB

Copy link
Contributor

github-actions bot commented May 7, 2025

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1223.96 ms 1233.52 ms 9.56 ms
Size 2.63 MiB 3.80 MiB 1.17 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b8ff156+dirty 1236.51 ms 1234.53 ms -1.98 ms
d8e8c67+dirty 1222.57 ms 1226.22 ms 3.65 ms
9c48b2c+dirty 1246.96 ms 1255.73 ms 8.77 ms
abb7058+dirty 1255.42 ms 1268.86 ms 13.44 ms
3853f43+dirty 1221.82 ms 1242.64 ms 20.82 ms
d197b5c+dirty 1217.61 ms 1242.66 ms 25.05 ms
c10f417+dirty 1211.27 ms 1212.85 ms 1.59 ms
b7eb05d+dirty 1215.71 ms 1221.38 ms 5.67 ms
776f0b5+dirty 1221.61 ms 1222.02 ms 0.41 ms
1d86dd6+dirty 1249.71 ms 1279.16 ms 29.45 ms

App size

Revision Plain With Sentry Diff
b8ff156+dirty 2.36 MiB 3.11 MiB 759.80 KiB
d8e8c67+dirty 2.36 MiB 3.12 MiB 779.40 KiB
9c48b2c+dirty 2.36 MiB 2.85 MiB 495.77 KiB
abb7058+dirty 2.36 MiB 2.87 MiB 520.42 KiB
3853f43+dirty 2.36 MiB 2.85 MiB 499.81 KiB
d197b5c+dirty 2.36 MiB 2.82 MiB 462.86 KiB
c10f417+dirty 2.63 MiB 3.79 MiB 1.16 MiB
b7eb05d+dirty 2.63 MiB 3.75 MiB 1.12 MiB
776f0b5+dirty 2.63 MiB 3.76 MiB 1.13 MiB
1d86dd6+dirty 2.36 MiB 2.89 MiB 535.43 KiB

Previous results on branch: antonis/ios-deprecate-experimentalViewRenderer

Startup times

Revision Plain With Sentry Diff
5501305+dirty 1228.22 ms 1233.49 ms 5.27 ms
50d030e+dirty 1227.36 ms 1226.98 ms -0.38 ms

App size

Revision Plain With Sentry Diff
5501305+dirty 2.63 MiB 3.79 MiB 1.16 MiB
50d030e+dirty 2.63 MiB 3.79 MiB 1.16 MiB

Copy link
Collaborator

@lucas-zimerman lucas-zimerman left a comment

Choose a reason for hiding this comment

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

Nice addition!
I added a small wording that will help on the reading, after resolving that comment, LGTM!

Copy link
Contributor

github-actions bot commented May 8, 2025

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1215.73 ms 1218.79 ms 3.05 ms
Size 3.19 MiB 4.37 MiB 1.18 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b8ff156+dirty 1238.92 ms 1239.57 ms 0.66 ms
d8e8c67+dirty 1235.58 ms 1233.44 ms -2.15 ms
9c48b2c+dirty 1253.39 ms 1256.30 ms 2.91 ms
abb7058+dirty 1260.28 ms 1266.56 ms 6.28 ms
3853f43+dirty 1271.74 ms 1278.04 ms 6.30 ms
d197b5c+dirty 1234.80 ms 1249.20 ms 14.40 ms
c10f417+dirty 1218.61 ms 1225.41 ms 6.80 ms
b7eb05d+dirty 1234.69 ms 1242.52 ms 7.83 ms
776f0b5+dirty 1227.16 ms 1225.45 ms -1.71 ms
1d86dd6+dirty 1289.25 ms 1293.36 ms 4.11 ms

App size

Revision Plain With Sentry Diff
b8ff156+dirty 2.92 MiB 3.67 MiB 772.38 KiB
d8e8c67+dirty 2.92 MiB 3.69 MiB 790.53 KiB
9c48b2c+dirty 2.92 MiB 3.41 MiB 499.97 KiB
abb7058+dirty 2.92 MiB 3.43 MiB 524.53 KiB
3853f43+dirty 2.92 MiB 3.41 MiB 503.54 KiB
d197b5c+dirty 2.92 MiB 3.37 MiB 464.41 KiB
c10f417+dirty 3.19 MiB 4.36 MiB 1.17 MiB
b7eb05d+dirty 3.19 MiB 4.32 MiB 1.13 MiB
776f0b5+dirty 3.19 MiB 4.33 MiB 1.14 MiB
1d86dd6+dirty 2.92 MiB 3.44 MiB 538.27 KiB

Previous results on branch: antonis/ios-deprecate-experimentalViewRenderer

Startup times

Revision Plain With Sentry Diff
5501305+dirty 1218.80 ms 1223.55 ms 4.76 ms
50d030e+dirty 1230.02 ms 1233.53 ms 3.51 ms

App size

Revision Plain With Sentry Diff
5501305+dirty 3.19 MiB 4.36 MiB 1.17 MiB
50d030e+dirty 3.19 MiB 4.36 MiB 1.17 MiB

@@ -27,8 +27,7 @@ + (void)updateOptions:(NSMutableDictionary *)options
@"errorSampleRate" : options[@"replaysOnErrorSampleRate"] ?: [NSNull null],
@"maskAllImages" : replayOptions[@"maskAllImages"] ?: [NSNull null],
@"maskAllText" : replayOptions[@"maskAllText"] ?: [NSNull null],
@"enableExperimentalViewRenderer" : replayOptions[@"enableExperimentalViewRenderer"]
?: [NSNull null],
@"enableViewRendererV2" : replayOptions[@"enableViewRendererV2"] ?: @YES,
Copy link
Member

@krystofwoldrich krystofwoldrich May 8, 2025

Choose a reason for hiding this comment

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

I would keep here ?: [NSNull null], to avoid baking as little logic as possible in here.

If null cocoa uses its default if not values is coming from RN (user or default).

Copy link
Member

Choose a reason for hiding this comment

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

I see sentry-cocoa set's it to false. That was an oversight, I've talked to @philprime and we will update this in cocoa.

So I would hold off this PR until cocoa hotfix release.

Copy link
Collaborator Author

@antonis antonis May 8, 2025

Choose a reason for hiding this comment

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

So I would hold off this PR until cocoa hotfix release.

Sounds good @krystofwoldrich 👍

I would keep here ?: [NSNull null], to avoid baking as little logic as possible in here.

Updated with 88280eb (The failing Native Tests / ios should pass when Cocoa is updated)

Choose a reason for hiding this comment

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

Related PR to fix the inconsistency in sentry-cocoa:
getsentry/sentry-cocoa#5210

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Cocoa change was merged with 8.50.2 #4830

# Conflicts:
#	packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryReplayOptionsTests.swift
@antonis antonis removed the Blocked label May 15, 2025
@antonis antonis merged commit 8dffe6b into main May 15, 2025
95 of 96 checks passed
@antonis antonis deleted the antonis/ios-deprecate-experimentalViewRenderer branch May 15, 2025 08:32
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