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

fix(stalls): Missing measurements when using new .end() span API #3737

Merged
merged 5 commits into from
Apr 16, 2024

Conversation

krystofwoldrich
Copy link
Member

📢 Type of change

  • Bugfix
  • Enhancement
  • Refactoring

📜 Description

This PR is a preparation for JS V8 and its new performance spans-only API. It fixes a small bug where .end methods were not patched.

This PR refactors StallTracking integration tests to avoid using Transaction and internal APIs.

This will enable us to refactor the integration in the future with confidence and minimal test changes.

📝 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

🔮 Next steps

@krystofwoldrich krystofwoldrich added this to the 6.0.0 milestone Apr 3, 2024
Copy link
Contributor

github-actions bot commented Apr 3, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 433.71 ms 474.20 ms 40.49 ms
Size 17.73 MiB 19.93 MiB 2.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
80b2ce3 385.02 ms 387.36 ms 2.34 ms
8900e1a+dirty 430.68 ms 456.13 ms 25.44 ms
d0bf494+dirty 375.37 ms 395.14 ms 19.77 ms
e2b64fe 316.88 ms 330.23 ms 13.35 ms
52a8031+dirty 311.55 ms 321.37 ms 9.82 ms
457e29f 398.10 ms 421.39 ms 23.29 ms
d7401ac+dirty 375.20 ms 383.51 ms 8.31 ms
12427f4 393.69 ms 414.84 ms 21.14 ms
9c48b2c 349.24 ms 385.96 ms 36.72 ms
d361d38 354.10 ms 381.69 ms 27.59 ms

App size

Revision Plain With Sentry Diff
80b2ce3 17.73 MiB 19.75 MiB 2.02 MiB
8900e1a+dirty 17.73 MiB 19.75 MiB 2.01 MiB
d0bf494+dirty 17.73 MiB 19.75 MiB 2.02 MiB
e2b64fe 17.73 MiB 19.80 MiB 2.07 MiB
52a8031+dirty 17.73 MiB 20.04 MiB 2.31 MiB
457e29f 17.73 MiB 19.84 MiB 2.10 MiB
d7401ac+dirty 17.73 MiB 19.75 MiB 2.02 MiB
12427f4 17.73 MiB 19.85 MiB 2.12 MiB
9c48b2c 17.73 MiB 19.80 MiB 2.07 MiB
d361d38 17.73 MiB 19.81 MiB 2.08 MiB

Previous results on branch: kw-fix-stalltracking-for-new-span-apis

Startup times

Revision Plain With Sentry Diff
aaae1a1 410.70 ms 451.12 ms 40.42 ms

App size

Revision Plain With Sentry Diff
aaae1a1 17.73 MiB 19.93 MiB 2.20 MiB

Copy link
Contributor

github-actions bot commented Apr 3, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 325.21 ms 354.57 ms 29.37 ms
Size 7.15 MiB 8.21 MiB 1.06 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e2b64fe+dirty 258.82 ms 304.26 ms 45.44 ms
9c48b2c+dirty 270.82 ms 321.12 ms 50.30 ms
22e31b6+dirty 295.75 ms 346.73 ms 50.98 ms
9a3ca65+dirty 344.96 ms 358.92 ms 13.96 ms
acadc0f+dirty 259.04 ms 304.67 ms 45.63 ms
e5c9b8b+dirty 335.40 ms 360.06 ms 24.67 ms
34aba08+dirty 331.79 ms 376.69 ms 44.91 ms
5571a20+dirty 359.52 ms 389.80 ms 30.28 ms
0db0c72+dirty 335.20 ms 351.06 ms 15.86 ms
e73f4ed+dirty 262.98 ms 311.02 ms 48.04 ms

App size

Revision Plain With Sentry Diff
e2b64fe+dirty 7.15 MiB 8.07 MiB 947.16 KiB
9c48b2c+dirty 7.15 MiB 8.07 MiB 947.16 KiB
22e31b6+dirty 7.15 MiB 8.10 MiB 981.29 KiB
9a3ca65+dirty 7.15 MiB 8.09 MiB 962.83 KiB
acadc0f+dirty 7.15 MiB 8.03 MiB 903.20 KiB
e5c9b8b+dirty 7.15 MiB 8.10 MiB 980.41 KiB
34aba08+dirty 7.15 MiB 8.07 MiB 946.13 KiB
5571a20+dirty 7.15 MiB 8.20 MiB 1.05 MiB
0db0c72+dirty 7.15 MiB 8.04 MiB 911.02 KiB
e73f4ed+dirty 7.15 MiB 8.09 MiB 965.94 KiB

Copy link
Contributor

github-actions bot commented Apr 3, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1220.02 ms 1222.57 ms 2.55 ms
Size 2.36 MiB 2.92 MiB 570.04 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0db0c72+dirty 1275.02 ms 1285.84 ms 10.82 ms
9433f35+dirty 1246.94 ms 1271.45 ms 24.52 ms
76d1baf+dirty 1244.10 ms 1268.52 ms 24.42 ms
e2b64fe+dirty 1232.22 ms 1255.20 ms 22.98 ms
8900e1a+dirty 1210.27 ms 1218.66 ms 8.39 ms
e73f4ed+dirty 1243.27 ms 1244.52 ms 1.25 ms
3853f43+dirty 1221.82 ms 1242.64 ms 20.82 ms
27ef4ee+dirty 1293.52 ms 1296.08 ms 2.56 ms
d7401ac+dirty 1252.38 ms 1275.04 ms 22.66 ms
22e31b6+dirty 1253.62 ms 1265.96 ms 12.34 ms

App size

Revision Plain With Sentry Diff
0db0c72+dirty 2.36 MiB 2.84 MiB 487.01 KiB
9433f35+dirty 2.36 MiB 2.85 MiB 499.80 KiB
76d1baf+dirty 2.36 MiB 2.82 MiB 469.45 KiB
e2b64fe+dirty 2.36 MiB 2.85 MiB 495.80 KiB
8900e1a+dirty 2.36 MiB 2.83 MiB 479.25 KiB
e73f4ed+dirty 2.36 MiB 2.82 MiB 469.44 KiB
3853f43+dirty 2.36 MiB 2.85 MiB 499.81 KiB
27ef4ee+dirty 2.36 MiB 2.85 MiB 500.03 KiB
d7401ac+dirty 2.36 MiB 2.83 MiB 481.14 KiB
22e31b6+dirty 2.36 MiB 2.87 MiB 520.67 KiB

Copy link
Contributor

github-actions bot commented Apr 3, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1229.15 ms 1241.15 ms 12.00 ms
Size 2.92 MiB 3.48 MiB 575.63 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0db0c72+dirty 1258.88 ms 1262.52 ms 3.64 ms
9433f35+dirty 1232.24 ms 1232.74 ms 0.50 ms
76d1baf+dirty 1245.00 ms 1257.76 ms 12.76 ms
e2b64fe+dirty 1285.78 ms 1297.56 ms 11.78 ms
8900e1a+dirty 1268.36 ms 1273.04 ms 4.68 ms
e73f4ed+dirty 1282.90 ms 1309.30 ms 26.40 ms
3853f43+dirty 1271.74 ms 1278.04 ms 6.30 ms
27ef4ee+dirty 1236.41 ms 1244.90 ms 8.49 ms
d7401ac+dirty 1288.10 ms 1289.54 ms 1.44 ms
22e31b6+dirty 1276.55 ms 1278.12 ms 1.57 ms

App size

Revision Plain With Sentry Diff
0db0c72+dirty 2.92 MiB 3.40 MiB 492.71 KiB
9433f35+dirty 2.92 MiB 3.41 MiB 503.55 KiB
76d1baf+dirty 2.92 MiB 3.38 MiB 475.74 KiB
e2b64fe+dirty 2.92 MiB 3.41 MiB 499.97 KiB
8900e1a+dirty 2.92 MiB 3.39 MiB 485.96 KiB
e73f4ed+dirty 2.92 MiB 3.38 MiB 475.71 KiB
3853f43+dirty 2.92 MiB 3.41 MiB 503.54 KiB
27ef4ee+dirty 2.92 MiB 3.41 MiB 503.72 KiB
d7401ac+dirty 2.92 MiB 3.40 MiB 488.06 KiB
22e31b6+dirty 2.92 MiB 3.43 MiB 524.74 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.

Need to run prettier on stalltracking.test.ts but other than that LGTM!

@krystofwoldrich krystofwoldrich enabled auto-merge (squash) April 16, 2024 14:05
@krystofwoldrich krystofwoldrich merged commit 5ce5307 into main Apr 16, 2024
45 of 48 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-fix-stalltracking-for-new-span-apis branch April 16, 2024 14:06
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