Skip to content

Conversation

krystofwoldrich
Copy link
Contributor

@krystofwoldrich krystofwoldrich commented Jul 2, 2024

📢 Type of change

  • New feature

📜 Description

This PR adds current screen on native scopes. This enables native replay integration to track the RN screens and use them them to fill the urls.field

💚 How did you test it?

unit tests, sample app

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

@krystofwoldrich krystofwoldrich changed the base branch from feat/replay to main July 2, 2024 10:44
Copy link
Contributor

github-actions bot commented Jul 2, 2024

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

Generated by 🚫 dangerJS against edad3f7

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 420.44 ms 458.24 ms 37.80 ms
Size 17.73 MiB 19.95 MiB 2.21 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
70e6261 482.65 ms 495.70 ms 13.05 ms
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
5571a20 410.55 ms 441.06 ms 30.51 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms
d0bf494+dirty 375.37 ms 395.14 ms 19.77 ms
148f924 492.65 ms 500.28 ms 7.63 ms
27ef4ee 317.40 ms 321.70 ms 4.30 ms
5bb8d5f 431.21 ms 459.40 ms 28.19 ms
2534337 394.15 ms 415.12 ms 20.97 ms
80b2ce3 385.02 ms 387.36 ms 2.34 ms

App size

Revision Plain With Sentry Diff
70e6261 17.73 MiB 19.94 MiB 2.21 MiB
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
5571a20 17.73 MiB 19.93 MiB 2.19 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB
d0bf494+dirty 17.73 MiB 19.75 MiB 2.02 MiB
148f924 17.73 MiB 19.94 MiB 2.21 MiB
27ef4ee 17.73 MiB 19.82 MiB 2.08 MiB
5bb8d5f 17.73 MiB 19.93 MiB 2.20 MiB
2534337 17.73 MiB 19.84 MiB 2.11 MiB
80b2ce3 17.73 MiB 19.75 MiB 2.02 MiB

Previous results on branch: kw/set-current-screen-on-native

Startup times

Revision Plain With Sentry Diff
48eef96 429.22 ms 460.06 ms 30.85 ms

App size

Revision Plain With Sentry Diff
48eef96 17.73 MiB 19.94 MiB 2.21 MiB

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 436.62 ms 491.63 ms 55.01 ms
Size 7.15 MiB 8.22 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
80b2ce3+dirty 271.29 ms 316.47 ms 45.18 ms
86d6d2c+dirty 267.21 ms 325.24 ms 58.04 ms
5571a20+dirty 359.52 ms 389.80 ms 30.28 ms
d0bf494+dirty 253.73 ms 308.23 ms 54.49 ms
22e31b6+dirty 295.75 ms 346.73 ms 50.98 ms
12427f4+dirty 379.48 ms 400.92 ms 21.44 ms
5a22220+dirty 384.61 ms 419.06 ms 34.45 ms
abb7058+dirty 320.78 ms 324.08 ms 3.30 ms
70e6261+dirty 395.08 ms 408.12 ms 13.04 ms
148f924+dirty 347.36 ms 389.13 ms 41.77 ms

App size

Revision Plain With Sentry Diff
80b2ce3+dirty 7.15 MiB 8.04 MiB 911.02 KiB
86d6d2c+dirty 7.15 MiB 8.09 MiB 962.69 KiB
5571a20+dirty 7.15 MiB 8.20 MiB 1.05 MiB
d0bf494+dirty 7.15 MiB 8.04 MiB 910.85 KiB
22e31b6+dirty 7.15 MiB 8.10 MiB 981.29 KiB
12427f4+dirty 7.15 MiB 8.12 MiB 997.78 KiB
5a22220+dirty 7.15 MiB 8.21 MiB 1.06 MiB
abb7058+dirty 7.15 MiB 8.10 MiB 980.40 KiB
70e6261+dirty 7.15 MiB 8.21 MiB 1.07 MiB
148f924+dirty 7.15 MiB 8.21 MiB 1.07 MiB

Previous results on branch: kw/set-current-screen-on-native

Startup times

Revision Plain With Sentry Diff
48eef96+dirty 391.38 ms 430.94 ms 39.56 ms

App size

Revision Plain With Sentry Diff
48eef96+dirty 7.15 MiB 8.22 MiB 1.07 MiB

Copy link
Contributor

github-actions bot commented Jul 2, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.75 ms 1235.57 ms 0.82 ms
Size 2.36 MiB 3.04 MiB 698.50 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
148f924+dirty 1214.76 ms 1215.73 ms 0.97 ms
31fcca2+dirty 1209.17 ms 1216.21 ms 7.04 ms
3ffcddd+dirty 1244.47 ms 1264.14 ms 19.67 ms
27ef4ee+dirty 1293.52 ms 1296.08 ms 2.56 ms
e5c9b8b+dirty 1258.57 ms 1267.32 ms 8.75 ms
e2b64fe+dirty 1232.22 ms 1255.20 ms 22.98 ms
1d86dd6+dirty 1249.71 ms 1279.16 ms 29.45 ms
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
575f9da+dirty 1266.22 ms 1274.84 ms 8.62 ms
2534337+dirty 1225.08 ms 1230.26 ms 5.17 ms

App size

Revision Plain With Sentry Diff
148f924+dirty 2.36 MiB 3.04 MiB 696.25 KiB
31fcca2+dirty 2.36 MiB 2.90 MiB 552.95 KiB
3ffcddd+dirty 2.36 MiB 2.84 MiB 489.60 KiB
27ef4ee+dirty 2.36 MiB 2.85 MiB 500.03 KiB
e5c9b8b+dirty 2.36 MiB 2.87 MiB 520.43 KiB
e2b64fe+dirty 2.36 MiB 2.85 MiB 495.80 KiB
1d86dd6+dirty 2.36 MiB 2.89 MiB 535.43 KiB
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
575f9da+dirty 2.36 MiB 2.87 MiB 520.20 KiB
2534337+dirty 2.36 MiB 2.88 MiB 525.47 KiB

Previous results on branch: kw/set-current-screen-on-native

Startup times

Revision Plain With Sentry Diff
48eef96+dirty 1215.77 ms 1219.65 ms 3.88 ms

App size

Revision Plain With Sentry Diff
48eef96+dirty 2.36 MiB 3.04 MiB 698.53 KiB

Copy link
Contributor

github-actions bot commented Jul 2, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.15 ms 1221.19 ms 2.04 ms
Size 2.92 MiB 3.61 MiB 705.38 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
148f924+dirty 1220.72 ms 1221.30 ms 0.58 ms
31fcca2+dirty 1222.04 ms 1226.51 ms 4.47 ms
3ffcddd+dirty 1272.22 ms 1273.98 ms 1.76 ms
27ef4ee+dirty 1236.41 ms 1244.90 ms 8.49 ms
e5c9b8b+dirty 1276.90 ms 1280.92 ms 4.02 ms
e2b64fe+dirty 1285.78 ms 1297.56 ms 11.78 ms
1d86dd6+dirty 1289.25 ms 1293.36 ms 4.11 ms
c398f67+dirty 1227.31 ms 1230.00 ms 2.69 ms
575f9da+dirty 1272.00 ms 1284.38 ms 12.38 ms
2534337+dirty 1220.87 ms 1221.47 ms 0.60 ms

App size

Revision Plain With Sentry Diff
148f924+dirty 2.92 MiB 3.60 MiB 701.88 KiB
31fcca2+dirty 2.92 MiB 3.46 MiB 557.31 KiB
3ffcddd+dirty 2.92 MiB 3.40 MiB 494.39 KiB
27ef4ee+dirty 2.92 MiB 3.41 MiB 503.72 KiB
e5c9b8b+dirty 2.92 MiB 3.43 MiB 524.50 KiB
e2b64fe+dirty 2.92 MiB 3.41 MiB 499.97 KiB
1d86dd6+dirty 2.92 MiB 3.44 MiB 538.27 KiB
c398f67+dirty 2.92 MiB 3.60 MiB 701.89 KiB
575f9da+dirty 2.92 MiB 3.43 MiB 524.26 KiB
2534337+dirty 2.92 MiB 3.43 MiB 529.76 KiB

Previous results on branch: kw/set-current-screen-on-native

Startup times

Revision Plain With Sentry Diff
48eef96+dirty 1225.78 ms 1223.26 ms -2.52 ms

App size

Revision Plain With Sentry Diff
48eef96+dirty 2.92 MiB 3.61 MiB 705.41 KiB

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.

Left some suggestions that will not change the code logic so I am approving it, LGTM!

@krystofwoldrich
Copy link
Contributor Author

@lucas-zimerman Thank you for the review, at first I thought the same, we can simplify the null check, but sadly no.

The maps can contain key with null values.

        WritableMap test = new WritableNativeMap();
        test.putString("keyWithNullValue", null);

@krystofwoldrich krystofwoldrich merged commit 8a28af8 into main Jul 3, 2024
@krystofwoldrich krystofwoldrich deleted the kw/set-current-screen-on-native branch July 3, 2024 15:47
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.

2 participants