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

feat: HTTP Crumb level based on response status #4779

Merged
merged 5 commits into from
Jan 31, 2025

Conversation

philipphofmann
Copy link
Member

📜 Description

Set the HTTP crumb level based on the response status code. 4xx is warning and 5xx is error.

💡 Motivation and Context

Fixes GH-4776

💚 How did you test it?

Unit tests.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the 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.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Set the HTTP crumb level based on the response status code. 4xx is
warning and 5xx is error.

Fixes GH-4776
Copy link

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Copy link

github-actions bot commented Jan 30, 2025

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

Generated by 🚫 dangerJS against 4780d45

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.317%. Comparing base (ee7b41e) to head (4780d45).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4779       +/-   ##
=============================================
- Coverage   91.351%   91.317%   -0.034%     
=============================================
  Files          626       627        +1     
  Lines        74288     74568      +280     
  Branches     26628     26183      -445     
=============================================
+ Hits         67863     68094      +231     
- Misses        6327      6380       +53     
+ Partials        98        94        -4     
Files with missing lines Coverage Δ
Sources/Sentry/SentryNetworkTracker.m 96.626% <100.000%> (+0.083%) ⬆️
...erformance/Network/SentryNetworkTrackerTests.swift 99.026% <100.000%> (+0.133%) ⬆️

... and 37 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee7b41e...4780d45. Read the comment docs.

Copy link

github-actions bot commented Jan 30, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1221.13 ms 1249.88 ms 28.75 ms
Size 22.31 KiB 780.90 KiB 758.58 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b9d59f7 1250.71 ms 1257.78 ms 7.06 ms
9ce54cc 1230.31 ms 1237.20 ms 6.90 ms
72c8d84 1238.96 ms 1247.34 ms 8.38 ms
f8833c4 1236.45 ms 1252.82 ms 16.37 ms
7cd187e 1243.04 ms 1244.79 ms 1.75 ms
253bb71 1221.62 ms 1250.82 ms 29.20 ms
3eb2a52 1241.48 ms 1250.50 ms 9.02 ms
21fd61f 1226.42 ms 1247.52 ms 21.10 ms
1bbcb9c 1189.61 ms 1223.50 ms 33.89 ms
be51b56 1220.84 ms 1249.36 ms 28.52 ms

App size

Revision Plain With Sentry Diff
b9d59f7 22.85 KiB 405.77 KiB 382.93 KiB
9ce54cc 21.58 KiB 573.71 KiB 552.13 KiB
72c8d84 22.85 KiB 408.88 KiB 386.03 KiB
f8833c4 21.58 KiB 422.66 KiB 401.08 KiB
7cd187e 20.76 KiB 401.65 KiB 380.89 KiB
253bb71 20.76 KiB 393.37 KiB 372.60 KiB
3eb2a52 20.76 KiB 393.38 KiB 372.62 KiB
21fd61f 20.76 KiB 435.50 KiB 414.74 KiB
1bbcb9c 20.76 KiB 426.10 KiB 405.34 KiB
be51b56 20.76 KiB 432.20 KiB 411.44 KiB

Previous results on branch: feat/crumb-level-respone-code

Startup times

Revision Plain With Sentry Diff
04a5b92 1224.41 ms 1243.40 ms 18.99 ms
1ac9eac 1214.81 ms 1239.70 ms 24.89 ms
f0284e3 1225.21 ms 1247.32 ms 22.11 ms

App size

Revision Plain With Sentry Diff
04a5b92 22.31 KiB 775.41 KiB 753.09 KiB
1ac9eac 22.31 KiB 780.58 KiB 758.27 KiB
f0284e3 22.31 KiB 778.85 KiB 756.54 KiB

Copy link

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Copy link

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

Copy link

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryNetworkTracker.m

@philipphofmann philipphofmann merged commit 3dcba20 into main Jan 31, 2025
48 checks passed
@philipphofmann philipphofmann deleted the feat/crumb-level-respone-code branch January 31, 2025 12:55
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.

Set HTTP client breadcrumbs log level based on response status code
2 participants