feat(network-details): Extend SentryReplayOptions API with Session Replay Network Details configuration#7580
feat(network-details): Extend SentryReplayOptions API with Session Replay Network Details configuration#7580
Conversation
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. |
|
Implement SDK fields, setters and validation for network details capture: https://docs.sentry.io/platforms/javascript/session-replay/configuration/#network-details - String patterns for prefix matching (e.g., "https://api.example.com" matches subpaths) - NSRegularExpression patterns for complex regex matching - Deny list precedence over allow list - Empty string filtering to handle invalid input gracefully
43jay
force-pushed
the
mobile-935/sdk-options
branch
from
1d2a165 to
8e08354
Compare
43jay
requested review from
itaybre,
noahsmartin,
philipphofmann and
philprime
as code owners
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7580 +/- ##
=============================================
+ Coverage 85.328% 85.393% +0.064%
=============================================
Files 483 485 +2
Lines 28750 28823 +73
Branches 12489 12529 +40
=============================================
+ Hits 24532 24613 +81
+ Misses 4170 4163 -7
+ Partials 48 47 -1
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: URL pattern validation runs twice on dictionary init
- I removed pre-validation in the dictionary initializer and routed raw values to the private initializer so URL patterns are validated exactly once in a single source of truth.
Or push these changes by commenting:
@cursor push 95c595cf60
Preview (95c595cf60)
diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
--- a/Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
+++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
@@ -601,8 +601,8 @@
maximumDuration: (dictionary["maximumDuration"] as? NSNumber)?.doubleValue,
excludedViewClasses: (dictionary["excludedViewClasses"] as? [String]).map { Set($0) },
includedViewClasses: (dictionary["includedViewClasses"] as? [String]).map { Set($0) },
- networkDetailAllowUrls: Self.validateNetworkDetailUrlPatterns(from: dictionary["networkDetailAllowUrls"]),
- networkDetailDenyUrls: Self.validateNetworkDetailUrlPatterns(from: dictionary["networkDetailDenyUrls"]),
+ networkDetailAllowUrls: dictionary["networkDetailAllowUrls"],
+ networkDetailDenyUrls: dictionary["networkDetailDenyUrls"],
networkCaptureBodies: (dictionary["networkCaptureBodies"] as? NSNumber)?.boolValue,
networkRequestHeaders: Self.parseStringArray(from: dictionary["networkRequestHeaders"]),
networkResponseHeaders: Self.parseStringArray(from: dictionary["networkResponseHeaders"])
@@ -745,8 +745,8 @@
maximumDuration: TimeInterval?,
excludedViewClasses: Set<String>? = nil,
includedViewClasses: Set<String>? = nil,
- networkDetailAllowUrls: [Any]? = nil,
- networkDetailDenyUrls: [Any]? = nil,
+ networkDetailAllowUrls: Any? = nil,
+ networkDetailDenyUrls: Any? = nil,
networkCaptureBodies: Bool? = nil,
networkRequestHeaders: [String]? = nil,
networkResponseHeaders: [String]? = nil
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
| * - Note: Request and response bodies are truncated to 150KB maximum. | ||
| * - Note: See ``SentryReplayOptions.DefaultValues.networkDetailAllowUrls`` for the default value. | ||
| */ | ||
| public var networkDetailAllowUrls: [Any] |
There was a problem hiding this comment.
m: This might be a great opportunity to use protocol-based such as we did in SentryMetricsApiProtocol where the attributes are actually just the protocol [SentryAttributeValue]https://github.com/getsentry/sentry-cocoa/blob/main/Sources/Swift/Protocol/SentryAttributeValue.swift#L46)
If not we could also consider using enums with associated values instead of type-erasing it. See these two blog posts for context:
- https://sentry.engineering/blog/building-type-safe-metrics-api-in-swift-part-i
- https://sentry.engineering/blog/building-type-safe-metrics-api-in-swift-part-ii
Might not work with Objective-C though
There was a problem hiding this comment.
Great suggestion. i took a crack at it. PTAL
Might not work with Objective-C though
The only oddity i needed was introducing an objC getter for networkDetailAllowUrls and networkDetailDenyUrls ... an oddity b/c the getters are currently only called from SentryReplayOptionsObjcTests so it's odd to have them hanging around on the SentryReplayOptions.swift API (and i couldn't see a way to hide them).
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
sentry[bot] flagged it.
…URL filtering Replace [Any] with [SentryUrlMatchable] for networkDetailAllowUrls/DenyUrls. Provides compile-time type safety in Swift while maintaining Objective-C compatibility through bridge properties. Follows the API pattern of SentryAttributeValue/Content.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 93ef486 | 1220.22 ms | 1244.96 ms | 24.74 ms |
| 4883c74 | 1224.80 ms | 1258.65 ms | 33.85 ms |
| 19f3e6b | 1231.17 ms | 1257.02 ms | 25.85 ms |
| 21cd5ba | 1218.68 ms | 1255.54 ms | 36.86 ms |
| 2f4ddaa | 1227.26 ms | 1260.04 ms | 32.78 ms |
| 37bc095 | 1210.00 ms | 1242.69 ms | 32.69 ms |
| 41cc629 | 1222.58 ms | 1253.09 ms | 30.51 ms |
| b984142 | 1222.18 ms | 1257.77 ms | 35.59 ms |
| d29a425 | 1209.96 ms | 1239.00 ms | 29.04 ms |
| 18c94be | 1218.26 ms | 1258.80 ms | 40.54 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 93ef486 | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| 4883c74 | 24.14 KiB | 1.12 MiB | 1.10 MiB |
| 19f3e6b | 24.14 KiB | 1.10 MiB | 1.08 MiB |
| 21cd5ba | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| 2f4ddaa | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| 37bc095 | 24.14 KiB | 1.06 MiB | 1.04 MiB |
| 41cc629 | 24.14 KiB | 1.12 MiB | 1.10 MiB |
| b984142 | 24.14 KiB | 1.11 MiB | 1.09 MiB |
| d29a425 | 24.14 KiB | 1.04 MiB | 1.02 MiB |
| 18c94be | 24.14 KiB | 1.10 MiB | 1.08 MiB |
itaybre
left a comment
itaybre
left a comment
There was a problem hiding this comment.
Almost LGTM, just some small comments
And you will need to run make generate-public-api due to the new public APIs
| * - Note: This setting only applies when ``networkDetailAllowUrls`` is non-empty. | ||
| * - Note: Header names preserve the case seen on the request, not the case specified here. | ||
| */ | ||
| public var networkRequestHeaders: [String] { |
There was a problem hiding this comment.
m" This API won't be available for ObjC?
| * - Note: This setting only applies when ``networkDetailAllowUrls`` is non-empty. | ||
| * - Note: Header names preserve the case seen on the response, not the case specified here. | ||
| */ | ||
| public var networkResponseHeaders: [String] { |
There was a problem hiding this comment.
m: Same question as above, not available for ObjC?
3 participants
Implement SDK fields, setters and validation for network details capture: https://docs.sentry.io/platforms/javascript/session-replay/configuration/#network-details
networkDetailAllowUrls(string|RegExp)[][]networkDetailDenyUrls(string|RegExp)[][]networkDetailAllowUrls.networkCaptureBodiesbooleantruenetworkDetailAllowUrls.networkRequestHeadersstring[][]networkDetailAllowUrls.networkResponseHeadersstring[][]networkDetailAllowUrls.networkDetail[Allow|Deny]Urlsnetwork[Request|Response]HeadersnetworkCaptureBodies📜 Description
PR 1/N. Extend SentryReplayOptions API with Session Replay Network Details configuration
PR 2/N. Adds test app UI to configure and test network details collection.
PR 3/N. Adds data holder classes to define structure of data being extracted
PR 4/N. Adds new swizzling introduced to capture response bodies.
PR 5/N. Implements extraction logic for headers & bodies.
PR 6/N. Hook into existing SentryNetworkTracker.m|h
PR 7/N. Implement conversion from breadcrumb data -> Session Replay compatible RRWebEvent
💡 Motivation and Context
Parent issue (android, cocoa, RN) - getsentry/sentry#84596
Cocoa sub-issue - #4944
This PR adds the SDK fields required for developers to enable network details extraction for network requests made during a session replay capture, by following the impl in sentry-javascript and sentry-java (android).
💚 How did you test it?
Unit tests
SentryReplayOptionsTests
SentryReplayOptionsNetworkTests
Additional unit tests for network details (I didn't put them in SentryReplayOptionsTests to avoid over-crowding that file / I assumed SentryReplayOptionsTests should be for high level testing of SentryReplayOptions).
SentryReplayOptionsObjcTests
ObjC <> Swift compatibility tests. Mostly for Regex matching
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled. Nothing extracted in this PR - SDKOptions available but no backing implementation.