REPLAY-1965 fix: SR data uploads to AP1 site #1418
Conversation
by leveraging the base URL provided by Core SDK
| @@ -97,4 +77,8 @@ internal struct RequestBuilder: FeatureRequestBuilder { | |||
| // Data is already compressed, so request building request w/o compression: | |||
| return builder.uploadRequest(with: multipart.data, compress: false) | |||
| } | |||
|
|
|||
| private func url(with context: DatadogContext) -> URL { | |||
| customUploadURL ?? context.site.endpoint.appendingPathComponent("api/v2/replay") | |||
There was a problem hiding this comment.
a part of me preferred full paths, as in this one it's easy to make a typo
There was a problem hiding this comment.
Hardcoding the full path lead us to the problem we fix in this PR 🙂. What we do now is: trust the base URL provided by core SDK and specialise it with SR endpoint. Full URLs are asserted in tests, which gives us clarity and discoverability of expected values.
Note that context.site.endpoint depends on the Datadog site used.
| XCTAssertEqual(try url(for: .eu1), "https://session-replay.browser-intake-datadoghq.eu/api/v2/replay") | ||
| XCTAssertEqual(try url(for: .ap1), "https://session-replay.browser-intake-datadoghq.eu/api/v2/replay") | ||
| XCTAssertEqual(try url(for: .us1_fed), "https://session-replay.browser-intake-ddog-gov.com/api/v2/replay") | ||
| XCTAssertEqual(try url(for: .us1), "https://browser-intake-datadoghq.com/api/v2/replay") |
There was a problem hiding this comment.
Why do we drop session replay subdomain?
There was a problem hiding this comment.
Since we created first iteration of SR (using above URLs), there was a global change to the way we reference intakes, see: RUMM-2865. Here we migrate SR to the subdomain-less URLs. This is for uniformity with other products in the SDK 👌.
What and why?
🐞 AP1's site url for Session Replay was wrong, so data uploads would not be possible.
How?
Updating SR upload url to the correct one, by leveraging the base URL provided by SDK core (notably, switching SR to use subdomain-less URL the same way as other products do after RUMM-2865).
Sanity check in US1, works:
Review checklist
Custom CI job configuration (optional)