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(ios): Disable failed requests capture on iOS #4250

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Nov 8, 2024

📢 Type of change

  • Bugfix

📜 Description

This PR disabled capture failed request on iOS.

RN JS network requests use NSURLSession as any other native request.

We capture failed requests in JS, captures from sentry-cocoa are duplicates of the JS capture.

The same does not apply to Android, as there the captured failed request are only enabled if the Sentry Android Gradle Plugin enables them.

💚 How did you test it?

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

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 426.48 ms 426.29 ms -0.19 ms
Size 17.74 MiB 20.08 MiB 2.34 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c639edf 466.48 ms 489.57 ms 23.09 ms
575f9da 415.26 ms 422.98 ms 7.72 ms
31fcca2 391.22 ms 414.78 ms 23.56 ms
baa882f 354.93 ms 356.18 ms 1.25 ms
3853f43 329.68 ms 346.32 ms 16.64 ms
4cc5c27 460.04 ms 496.32 ms 36.28 ms
c398f67 449.64 ms 461.38 ms 11.74 ms
9a3ca65+dirty 326.93 ms 330.14 ms 3.21 ms
e540498 436.26 ms 433.00 ms -3.26 ms
2ec71da 438.14 ms 460.46 ms 22.32 ms

App size

Revision Plain With Sentry Diff
c639edf 17.74 MiB 20.08 MiB 2.34 MiB
575f9da 17.73 MiB 19.83 MiB 2.10 MiB
31fcca2 17.73 MiB 19.90 MiB 2.17 MiB
baa882f 17.73 MiB 20.06 MiB 2.33 MiB
3853f43 17.73 MiB 19.81 MiB 2.08 MiB
4cc5c27 17.73 MiB 19.95 MiB 2.21 MiB
c398f67 17.73 MiB 19.94 MiB 2.21 MiB
9a3ca65+dirty 17.73 MiB 20.04 MiB 2.31 MiB
e540498 17.73 MiB 20.11 MiB 2.37 MiB
2ec71da 17.73 MiB 20.10 MiB 2.37 MiB

Previous results on branch: kw/fix/ios-duplicate-failed-requests

Startup times

Revision Plain With Sentry Diff
e47ca77 460.00 ms 441.04 ms -18.96 ms
a31b519 445.70 ms 443.53 ms -2.17 ms

App size

Revision Plain With Sentry Diff
e47ca77 17.74 MiB 20.08 MiB 2.34 MiB
a31b519 17.74 MiB 20.08 MiB 2.34 MiB

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Android (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 402.36 ms 430.08 ms 27.72 ms
Size 7.15 MiB 8.35 MiB 1.20 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
76d1baf+dirty 339.02 ms 408.65 ms 69.63 ms
86d6d2c+dirty 267.21 ms 325.24 ms 58.04 ms
f06c879+dirty 361.27 ms 407.88 ms 46.61 ms
8900e1a+dirty 371.40 ms 377.70 ms 6.31 ms
e540498+dirty 408.56 ms 480.00 ms 71.44 ms
62a750b+dirty 370.78 ms 376.73 ms 5.96 ms
2ec71da+dirty 375.64 ms 431.59 ms 55.95 ms
c639edf+dirty 363.39 ms 414.78 ms 51.39 ms
acadc0f+dirty 259.04 ms 304.67 ms 45.63 ms
27ef4ee+dirty 296.71 ms 351.00 ms 54.29 ms

App size

Revision Plain With Sentry Diff
76d1baf+dirty 7.15 MiB 8.09 MiB 964.41 KiB
86d6d2c+dirty 7.15 MiB 8.09 MiB 962.69 KiB
f06c879+dirty 7.15 MiB 8.12 MiB 997.78 KiB
8900e1a+dirty 7.15 MiB 8.03 MiB 901.79 KiB
e540498+dirty 7.15 MiB 8.38 MiB 1.23 MiB
62a750b+dirty 7.15 MiB 8.21 MiB 1.06 MiB
2ec71da+dirty 7.15 MiB 8.38 MiB 1.23 MiB
c639edf+dirty 7.15 MiB 8.35 MiB 1.20 MiB
acadc0f+dirty 7.15 MiB 8.03 MiB 903.20 KiB
27ef4ee+dirty 7.15 MiB 8.08 MiB 959.49 KiB

Previous results on branch: kw/fix/ios-duplicate-failed-requests

Startup times

Revision Plain With Sentry Diff
e47ca77+dirty 378.70 ms 420.57 ms 41.87 ms

App size

Revision Plain With Sentry Diff
e47ca77+dirty 7.15 MiB 8.35 MiB 1.20 MiB

Copy link
Contributor

github-actions bot commented Nov 8, 2024

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.12 ms 1223.16 ms -0.96 ms
Size 2.36 MiB 3.10 MiB 752.62 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0ebca77+dirty 1220.75 ms 1222.81 ms 2.06 ms
f06c879+dirty 1252.64 ms 1259.66 ms 7.02 ms
8ae23a7+dirty 1230.02 ms 1227.62 ms -2.40 ms
d197b5c+dirty 1217.61 ms 1242.66 ms 25.05 ms
3ffcddd+dirty 1244.47 ms 1264.14 ms 19.67 ms
457e29f+dirty 1253.94 ms 1269.18 ms 15.24 ms
ac41368+dirty 1226.65 ms 1237.90 ms 11.24 ms
cdf2f33+dirty 1227.71 ms 1233.94 ms 6.22 ms
c639edf+dirty 1236.18 ms 1235.04 ms -1.14 ms
1d86dd6+dirty 1249.71 ms 1279.16 ms 29.45 ms

App size

Revision Plain With Sentry Diff
0ebca77+dirty 2.36 MiB 3.04 MiB 698.33 KiB
f06c879+dirty 2.36 MiB 2.88 MiB 530.42 KiB
8ae23a7+dirty 2.36 MiB 3.10 MiB 752.42 KiB
d197b5c+dirty 2.36 MiB 2.82 MiB 462.86 KiB
3ffcddd+dirty 2.36 MiB 2.84 MiB 489.60 KiB
457e29f+dirty 2.36 MiB 2.87 MiB 520.67 KiB
ac41368+dirty 2.36 MiB 3.14 MiB 793.46 KiB
cdf2f33+dirty 2.36 MiB 3.10 MiB 751.38 KiB
c639edf+dirty 2.36 MiB 3.08 MiB 736.63 KiB
1d86dd6+dirty 2.36 MiB 2.89 MiB 535.43 KiB

Previous results on branch: kw/fix/ios-duplicate-failed-requests

Startup times

Revision Plain With Sentry Diff
e47ca77+dirty 1215.81 ms 1213.35 ms -2.47 ms
a31b519+dirty 1239.50 ms 1245.15 ms 5.65 ms

App size

Revision Plain With Sentry Diff
e47ca77+dirty 2.36 MiB 3.10 MiB 752.57 KiB
a31b519+dirty 2.36 MiB 3.10 MiB 752.61 KiB

Copy link
Contributor

github-actions bot commented Nov 8, 2024

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.78 ms 1229.18 ms -2.59 ms
Size 2.92 MiB 3.66 MiB 757.15 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0ebca77+dirty 1209.30 ms 1220.33 ms 11.03 ms
f06c879+dirty 1285.14 ms 1285.86 ms 0.72 ms
8ae23a7+dirty 1233.67 ms 1229.52 ms -4.15 ms
d197b5c+dirty 1234.80 ms 1249.20 ms 14.40 ms
3ffcddd+dirty 1272.22 ms 1273.98 ms 1.76 ms
457e29f+dirty 1256.71 ms 1258.50 ms 1.79 ms
ac41368+dirty 1226.69 ms 1229.96 ms 3.27 ms
cdf2f33+dirty 1210.00 ms 1218.50 ms 8.50 ms
c639edf+dirty 1223.63 ms 1227.98 ms 4.35 ms
1d86dd6+dirty 1289.25 ms 1293.36 ms 4.11 ms

App size

Revision Plain With Sentry Diff
0ebca77+dirty 2.92 MiB 3.61 MiB 705.12 KiB
f06c879+dirty 2.92 MiB 3.44 MiB 533.24 KiB
8ae23a7+dirty 2.92 MiB 3.66 MiB 757.67 KiB
d197b5c+dirty 2.92 MiB 3.37 MiB 464.41 KiB
3ffcddd+dirty 2.92 MiB 3.40 MiB 494.39 KiB
457e29f+dirty 2.92 MiB 3.43 MiB 524.75 KiB
ac41368+dirty 2.92 MiB 3.69 MiB 794.29 KiB
cdf2f33+dirty 2.92 MiB 3.66 MiB 755.50 KiB
c639edf+dirty 2.92 MiB 3.64 MiB 742.55 KiB
1d86dd6+dirty 2.92 MiB 3.44 MiB 538.27 KiB

Previous results on branch: kw/fix/ios-duplicate-failed-requests

Startup times

Revision Plain With Sentry Diff
e47ca77+dirty 1249.24 ms 1244.80 ms -4.44 ms
a31b519+dirty 1231.38 ms 1234.82 ms 3.43 ms

App size

Revision Plain With Sentry Diff
e47ca77+dirty 2.92 MiB 3.66 MiB 757.29 KiB
a31b519+dirty 2.92 MiB 3.66 MiB 757.37 KiB

@krystofwoldrich krystofwoldrich marked this pull request as ready for review November 11, 2024 10:04
@antonis antonis changed the title fix(ios): Disable failed requests capure on iOS fix(ios): Disable failed requests capture on iOS Nov 11, 2024
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 🚀

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.

LGTM!

CHANGELOG.md Outdated Show resolved Hide resolved
@lucas-zimerman lucas-zimerman merged commit 7bc4d75 into main Nov 11, 2024
61 of 62 checks passed
@lucas-zimerman lucas-zimerman deleted the kw/fix/ios-duplicate-failed-requests branch November 11, 2024 22:10
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.

3 participants