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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use recordHttpBreadcrumbs to set iOS enableNetworkBreadcrumbs #1884

Merged
merged 5 commits into from Mar 4, 2024

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Feb 19, 2024

馃摐 Description

We could not disable http breadcrumbs when running on iOS. This PR uses the recordHttpBreadcrumbs option to set enableNetworkBreadcrumbs option on the iOS side. This is the same mechanism as the captureFailedRequests to enableCaptureFailedRequests option pair.

馃挕 Motivation and Context

Closes #1873

馃挌 How did you test it?

Unit tests

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

Copy link
Contributor

github-actions bot commented Feb 19, 2024

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

Generated by 馃毇 dangerJS against df45c4b

@denrase denrase marked this pull request as ready for review February 19, 2024 15:08
Copy link
Contributor

github-actions bot commented Feb 19, 2024

Android Performance metrics 馃殌

Plain With Sentry Diff
Startup time 402.20 ms 476.65 ms 74.45 ms
Size 6.33 MiB 7.28 MiB 962.80 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
117d988 376.32 ms 450.85 ms 74.53 ms
48e79fd 354.22 ms 391.46 ms 37.24 ms
3f3ef0b 382.24 ms 459.26 ms 77.02 ms
1a93825 347.31 ms 424.54 ms 77.23 ms
6a40d32 292.09 ms 350.81 ms 58.73 ms
891efac 378.00 ms 461.20 ms 83.20 ms
bd37365 360.79 ms 440.74 ms 79.95 ms
732a7b4 371.98 ms 423.19 ms 51.21 ms
9d7e862 426.35 ms 510.88 ms 84.53 ms
754cdbe 325.08 ms 390.53 ms 65.45 ms

App size

Revision Plain With Sentry Diff
117d988 6.33 MiB 7.26 MiB 947.03 KiB
48e79fd 5.94 MiB 6.95 MiB 1.01 MiB
3f3ef0b 6.33 MiB 7.26 MiB 943.11 KiB
1a93825 6.27 MiB 7.20 MiB 956.36 KiB
6a40d32 6.16 MiB 7.14 MiB 1003.99 KiB
891efac 6.27 MiB 7.20 MiB 958.73 KiB
bd37365 6.27 MiB 7.20 MiB 957.75 KiB
732a7b4 6.33 MiB 7.27 MiB 954.02 KiB
9d7e862 6.33 MiB 7.26 MiB 943.41 KiB
754cdbe 6.16 MiB 7.14 MiB 1003.78 KiB

Previous results on branch: feat/enable-network-crumbs

Startup times

Revision Plain With Sentry Diff
aff1f23 362.31 ms 438.06 ms 75.76 ms
1f24cf2 396.73 ms 446.52 ms 49.79 ms

App size

Revision Plain With Sentry Diff
aff1f23 6.33 MiB 7.27 MiB 954.10 KiB
1f24cf2 6.33 MiB 7.27 MiB 954.11 KiB

Copy link
Contributor

github-actions bot commented Feb 19, 2024

iOS Performance metrics 馃殌

Plain With Sentry Diff
Startup time 1243.76 ms 1256.13 ms 12.36 ms
Size 8.32 MiB 9.38 MiB 1.06 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0118295 1211.31 ms 1227.02 ms 15.71 ms
fe4aa56 1248.82 ms 1261.35 ms 12.53 ms
ccc09e4 1254.74 ms 1277.08 ms 22.34 ms
d883d62 1221.39 ms 1230.18 ms 8.80 ms
62de927 1242.46 ms 1246.11 ms 3.65 ms
86d4841 1225.69 ms 1241.12 ms 15.43 ms
26e955b 1232.35 ms 1258.88 ms 26.52 ms
62dde43 1258.43 ms 1276.81 ms 18.38 ms
08a7b4f 1277.10 ms 1303.37 ms 26.27 ms
379d7a8 1267.65 ms 1288.39 ms 20.74 ms

App size

Revision Plain With Sentry Diff
0118295 8.32 MiB 9.38 MiB 1.05 MiB
fe4aa56 8.10 MiB 9.08 MiB 1004.36 KiB
ccc09e4 8.16 MiB 9.16 MiB 1.01 MiB
d883d62 8.29 MiB 9.36 MiB 1.07 MiB
62de927 8.29 MiB 9.37 MiB 1.08 MiB
86d4841 8.29 MiB 9.36 MiB 1.07 MiB
26e955b 8.28 MiB 9.34 MiB 1.05 MiB
62dde43 8.16 MiB 9.17 MiB 1.01 MiB
08a7b4f 8.16 MiB 9.16 MiB 1.01 MiB
379d7a8 8.16 MiB 9.16 MiB 1.00 MiB

Previous results on branch: feat/enable-network-crumbs

Startup times

Revision Plain With Sentry Diff
aff1f23 1253.14 ms 1272.19 ms 19.04 ms
1f24cf2 1241.92 ms 1251.72 ms 9.80 ms

App size

Revision Plain With Sentry Diff
aff1f23 8.32 MiB 9.38 MiB 1.06 MiB
1f24cf2 8.32 MiB 9.38 MiB 1.06 MiB

Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

馃憤

@denrase
Copy link
Collaborator Author

denrase commented Feb 20, 2024

@buenaflor Do you know why some screenshot tests are failing?

@buenaflor
Copy link
Contributor

buenaflor commented Feb 20, 2024

@denrase seems like something is up with the flutter beta channel with --platform chrome

@denrase
Copy link
Collaborator Author

denrase commented Feb 27, 2024

@buenaflor After debugging this (for a while -.-), I found that using canvasKit renderer in the Flutter beta does not work anymore. Because of this, no Screenshot works when running the test in chrome. --web-renderer=canvaskit

@denrase denrase merged commit e964e2b into main Mar 4, 2024
104 checks passed
@denrase denrase deleted the feat/enable-network-crumbs branch March 4, 2024 10:28
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.

Option "enableAutoNativeBreadcrumbs" doesn't work for iOS (http) breadcrumbs
2 participants