feat(network-details): Hook up request/response capture in SentryNetworkTracker#7588
Open
feat(network-details): Hook up request/response capture in SentryNetworkTracker#7588
Conversation
Contributor
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
Contributor
|
43jay
requested review from
itaybre,
noahsmartin,
philipphofmann and
philprime
as code owners
7 tasks
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
43jay
changed the title
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7588 +/- ##
=============================================
+ Coverage 85.186% 85.248% +0.062%
=============================================
Files 490 490
Lines 29520 29678 +158
Branches 12761 12841 +80
=============================================
+ Hits 25147 25300 +153
- Misses 4322 4328 +6
+ Partials 51 50 -1
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
5f57b05 to
a4f221a
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
0d2b4c1 to
3547546
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
a4f221a to
fde4a5e
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
3547546 to
e7c5abb
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
fde4a5e to
8b6853c
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
e7c5abb to
2e9607e
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
2e9607e to
6e5c5bb
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
8b6853c to
624e1f5
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
6e5c5bb to
41cf944
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
2 times, most recently
from
d80d538 to
d08a33e
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
41cf944 to
68fbe88
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
d08a33e to
d5a8320
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
68fbe88 to
6a06365
Compare
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ef5b57a | 1226.96 ms | 1259.79 ms | 32.83 ms |
| 0390710 | 1214.67 ms | 1245.27 ms | 30.60 ms |
| ba67433 | 1228.73 ms | 1248.05 ms | 19.31 ms |
| 00b9497 | 1212.87 ms | 1240.63 ms | 27.76 ms |
| 4a5e66b | 1228.77 ms | 1254.27 ms | 25.50 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ef5b57a | 24.14 KiB | 1.15 MiB | 1.12 MiB |
| 0390710 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| ba67433 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 00b9497 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 4a5e66b | 24.14 KiB | 1.15 MiB | 1.13 MiB |
Previous results on branch: mobile-935/hook-up-sentry-network-tracker
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 251c343 | 1238.62 ms | 1263.79 ms | 25.17 ms |
| 13c3140 | 1231.37 ms | 1266.27 ms | 34.90 ms |
| 31cc0e7 | 1222.19 ms | 1252.21 ms | 30.02 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 251c343 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 13c3140 | 24.14 KiB | 1.16 MiB | 1.13 MiB |
| 31cc0e7 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
dc3a7af to
5372649
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
2 times, most recently
from
251b0a8 to
f0b8955
Compare
43jay
force-pushed
the
mobile-935/extract-network-details
branch
2 times, most recently
from
c1488f7 to
98d01fc
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
f0b8955 to
2ab86d5
Compare
1) Extracts bodies that are JSON, formurlencoded, or text. Uses UTType to accurately classify content types as JSON or text without maintaining a manual list. Falls back to a string match for application/x-www-form-urlencoded which has no UTType representation. !Relies on having a valid `contentType` 2) Populates NetworkBodyWarning's for "MAYBE_JSON_TRUNCATED" "TEXT_TRUNCATED" "BODY_PARSE_ERROR" ^when encountered, these show custom dashboard UI.
Uses UTType to classify content types: only content positively identified as text is decoded. Everything else gets a descriptive placeholder: Example - "[Body not captured: contentType=image/png (8 bytes)]" Known text types (where UTType conforms to .text) are reliably classified by UTType's type hierarchy. If a content type header is incorrect (e.g. claims text but contains binary), the resulting decode failure is caught by the existing bodyParseError warning.
UTType (UniformTypeIdentifiers) requires macOS 11+, but the SDK targets macOS 10.14. Extract UTType-based MIME detection into a separate method gated with @available(macOS 11, *) so the code compiles on all macOS targets. Session Replay is not available on macOS, so the fallback placeholder is fine.
Casts to lower-case before comparing headers. ObjC setters now accept raw allHeaders and configuredHeaders instead of pre-filtered headers, keeping the filtering logic in Swift.
#7585 (comment) Parse MIME type and charset from Content-Type using CFStringConvertIANACharSetNameToEncoding instead of hardcoding UTF-8/ISO-Latin-1 fallbacks.
Replace force-unwraps and if-let/XCTFail patterns in SentryReplayNetworkDetailsBodyTests with XCTUnwrap for optional casts and Data(_:) for String-to-Data conversions.
We only support dataTasks, for whcih we will always have a bodyData #7588 (comment)
Synchronize objc_get/setAssociatedObject and SentryNetwokDetailsData locking on the NSURLSessionTask instance. `setState` - iOS internal API, undocumented threading model - calls captureRequestDetails `completionHandler` - callback queue (thread) configured by app. - calls captureResponseDetails captureRequestDetails To avoid introducing blocking on apple's thread, we use the lock to objc_getAssociated(task,..) but drop the lock before processing the request (SentryNetworkRequestDetails:setRequest). captureResponseDetails However we need the lock while processing response data (SentryNetworkRequestDetails:setResponse) b/c (later) it will race with calls to `SentryNetworkRequestDetails:serialize`. The completionHandler is on the app's network response path, so adding some thread contention here (worst-case) is less noticable (an i/o request just finished).
43jay
force-pushed
the
mobile-935/extract-network-details
branch
from
98d01fc to
d0b9491
Compare
43jay
force-pushed
the
mobile-935/hook-up-sentry-network-tracker
branch
from
2ab86d5 to
c78bb2d
Compare
Contributor
🚨 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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
Hook the network details implementation into SentryNetworkTracker by implementing the 2 entrypoints
💡 Motivation and Context
This PR updates SentryNetworkTracker to call into extraction logic introduced in previous PRs.
💚 How did you test it?
See other PRs for tests.
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled. requires SDKOptions config (networkDetailAllowUrls)Closes #7589