Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Datadog/Datadog.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
09A936A12F0EB989000B6379 /* SamplingPriority.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09A9369B2F0EB989000B6379 /* SamplingPriority.swift */; };
09A936A22F0EB989000B6379 /* SpanContext.swift in Sources */ = {isa = PBXBuildFile; fileRef = 09A9369C2F0EB989000B6379 /* SpanContext.swift */; };
1019DFD92FB1BB2E006599B4 /* DatadogSiteTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1019DFD82FB1BB23006599B4 /* DatadogSiteTests.swift */; };
10E9062A2FBDB81A002D5F45 /* RemoteConfigurationSynchronizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 10E906292FBDB818002D5F45 /* RemoteConfigurationSynchronizer.swift */; };
10E9062A2FBDB81A002D5F45 /* RemoteConfigurationProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 10E906292FBDB818002D5F45 /* RemoteConfigurationProvider.swift */; };
10E9062C2FBDBA4B002D5F45 /* RemoteConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 10E9062B2FBDBA3C002D5F45 /* RemoteConfigurationTests.swift */; };
11030D5F2D959EAD00732D5F /* DatadogLogs.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D207317C29A5226A00ECBF94 /* DatadogLogs.framework */; };
11030D6D2D95A48B00732D5F /* DatadogCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 61133B82242393DE00786299 /* DatadogCore.framework */; };
Expand Down Expand Up @@ -1775,7 +1775,7 @@
09A9369C2F0EB989000B6379 /* SpanContext.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpanContext.swift; sourceTree = "<group>"; };
0D2A20EFB1D82D0B8AC781F9 /* HeatmapMocks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HeatmapMocks.swift; sourceTree = "<group>"; };
1019DFD82FB1BB23006599B4 /* DatadogSiteTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DatadogSiteTests.swift; sourceTree = "<group>"; };
10E906292FBDB818002D5F45 /* RemoteConfigurationSynchronizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RemoteConfigurationSynchronizer.swift; sourceTree = "<group>"; };
10E906292FBDB818002D5F45 /* RemoteConfigurationProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RemoteConfigurationProvider.swift; sourceTree = "<group>"; };
10E9062B2FBDBA3C002D5F45 /* RemoteConfigurationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RemoteConfigurationTests.swift; sourceTree = "<group>"; };
11014EACF1FB9927DAD57822 /* EvaluationMocks.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = EvaluationMocks.swift; sourceTree = "<group>"; };
11030D752D96EC5300732D5F /* ViewHitchesMetric.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewHitchesMetric.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4464,7 +4464,7 @@
61133B9E2423979B00786299 /* Core */ = {
isa = PBXGroup;
children = (
10E906292FBDB818002D5F45 /* RemoteConfigurationSynchronizer.swift */,
10E906292FBDB818002D5F45 /* RemoteConfigurationProvider.swift */,
D2B3F04C282A85FD00C2B5EE /* DatadogCore.swift */,
D214DAA429E072D7004D0AE8 /* MessageBus.swift */,
D2EFA866286DA82700F1FAA6 /* Context */,
Expand Down Expand Up @@ -7922,7 +7922,7 @@
617699182A860D9D0030022B /* HTTPClient.swift in Sources */,
D21C26C528A3B49C005DD405 /* FeatureStorage.swift in Sources */,
61133BD42423979B00786299 /* FileReader.swift in Sources */,
10E9062A2FBDB81A002D5F45 /* RemoteConfigurationSynchronizer.swift in Sources */,
10E9062A2FBDB81A002D5F45 /* RemoteConfigurationProvider.swift in Sources */,
D29294E0291D5ED100F8EFF9 /* ApplicationVersionPublisher.swift in Sources */,
61D3E0D9277B23F1008BE766 /* KronosNTPProtocol.swift in Sources */,
61D3E0DA277B23F1008BE766 /* KronosTimeFreeze.swift in Sources */,
Expand Down
22 changes: 10 additions & 12 deletions DatadogCore/Sources/Core/DatadogCore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ internal final class DatadogCore {
/// The message-bus instance.
let bus = MessageBus()

/// Owns the remote configuration cache and fetch lifecycle.
/// `nil` when `remoteConfiguration` was not set at init.
internal let synchronizer: RemoteConfigurationSynchronizer?
/// The remote configuration provider, if configured.
let remoteConfigurationProvider: RemoteConfigurationProvider?

/// The last successfully fetched remote configuration, if any.
var remoteConfiguration: RemoteConfiguration? {
remoteConfigurationProvider?.remoteConfiguration

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Do not expose post-init remote config fetches

With remoteConfigurationID set, this computed property reads the provider's live cache, which sync() mutates after initialization. If the CDN request finishes before a feature is enabled (for example an app delays RUM.enable or a test uses a local protocol), a first-ever launch can receive a freshly fetched configuration instead of nil, despite the core protocol requiring the document cached at initialization time; this makes remote configuration application depend on network timing, so snapshot the initial cache separately from the async refresh.

Useful? React with 👍 / 👎.

}

/// Registry for Features.
@ReadWriteLock
Expand All @@ -79,10 +83,6 @@ internal final class DatadogCore {
/// Maximum number of batches per upload.
internal let maxBatchesPerUpload: Int

/// The last successfully fetched and cached remote configuration, if any.
@ReadWriteLock
var remoteConfiguration: RemoteConfiguration? = nil

/// Creates a core instance.
///
/// - Parameters:
Expand All @@ -94,7 +94,7 @@ internal final class DatadogCore {
/// - encryption: The on-disk data encryption.
/// - contextProvider: The core context provider.
/// - applicationVersion: The application version.
/// - remoteConfiguration: The remote configuration ID and site; `nil` when not configured.
/// - remoteConfigurationProvider: The remote configuration provider, if configured.
init(
directory: CoreDirectory,
dateProvider: DateProvider,
Expand All @@ -107,10 +107,11 @@ internal final class DatadogCore {
maxBatchesPerUpload: Int,
backgroundTasksEnabled: Bool,
isRunFromExtension: Bool = false,
remoteConfiguration: (id: String, site: DatadogSite)? = nil
remoteConfigurationProvider: RemoteConfigurationProvider? = nil
) {
self.directory = directory
self.dateProvider = dateProvider
self.remoteConfigurationProvider = remoteConfigurationProvider
self.performance = performance
self.httpClient = httpClient
self.encryption = encryption
Expand All @@ -124,9 +125,6 @@ internal final class DatadogCore {
self.contextProvider.subscribe(\.accountInfo, to: accountInfoPublisher)
self.contextProvider.subscribe(\.version, to: applicationVersionPublisher)
self.contextProvider.subscribe(\.trackingConsent, to: consentPublisher)
self.synchronizer = remoteConfiguration.map {
RemoteConfigurationSynchronizer(id: $0.id, site: $0.site, directory: directory.coreDirectory, httpClient: httpClient)
}
// connect the core to the message bus.
// the bus will keep a weak ref to the core.
bus.connect(core: self)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,25 @@
import Foundation
import DatadogInternal

/// Owns the remote configuration cache and fetch lifecycle.
///
/// Created by `DatadogCore` during initialization when a `remoteConfigurationID` is provided.
/// Call `sync(_:)` to fire a CDN fetch and update the cache.
/// Provides the last successfully fetched remote configuration and refreshes its cache.
///
/// TODO: Also trigger `sync()` on every app foreground transition so remote config
/// is refreshed while the app is in use, not only at SDK init (RFC §Caching Strategy).
internal final class RemoteConfigurationSynchronizer {
internal final class RemoteConfigurationProvider {
let id: String
let site: DatadogSite
let directory: Directory
let httpClient: HTTPClient

/// The result of the last cache read or CDN fetch.
/// `.success(data)` — data is available.
/// `.success(remoteConfiguration)` — remote configuration is available.
/// `.failure` — no cache yet, or a read/write error.
@ReadWriteLock
private(set) var cache: Result<Data, Error>
private(set) var cache: Result<RemoteConfiguration, RemoteConfigurationError>

var remoteConfiguration: RemoteConfiguration? {
try? cache.get()
}

init(id: String, site: DatadogSite, directory: Directory, httpClient: HTTPClient) {
self.id = id
Expand All @@ -39,11 +40,22 @@ internal final class RemoteConfigurationSynchronizer {

// MARK: - Private

private static func readCache(id: String, from directory: Directory) -> Result<Data, Error> {
private static func readCache(id: String, from directory: Directory) -> Result<RemoteConfiguration, RemoteConfigurationError> {
let data: Data
do {
data = try directory.file(named: "\(id).json").read()
} catch {
return .failure(.diskError)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

suggestion/ Lets report a failure to telemetry

}

return decode(data)
}

private static func decode(_ data: Data) -> Result<RemoteConfiguration, RemoteConfigurationError> {
do {
return .success(try directory.file(named: "\(id).json").read())
return .success(try JSONDecoder().decode(RemoteConfiguration.self, from: data))
} catch {
return .failure(error)
return .failure(.decodingError(error))
}
}

Expand All @@ -52,7 +64,7 @@ internal final class RemoteConfigurationSynchronizer {
/// Fires an async CDN fetch and updates the cache on success.
///
/// - Parameter completionHandler: Called with the fetch result when the operation (and any cache write) is done.
func sync(_ completionHandler: @escaping (Result<Data, Error>) -> Void) {
func sync(_ completionHandler: @escaping (Result<RemoteConfiguration, RemoteConfigurationError>) -> Void) {
// Build request with conditional ETag header if a previous ETag is stored.
// Only send If-None-Match when the cache is usable — if cache is .failure,
// a 304 response would leave us with no data to serve.
Expand All @@ -67,10 +79,10 @@ internal final class RemoteConfigurationSynchronizer {
request.setValue(etag, forHTTPHeaderField: "If-None-Match")
}

httpClient.fetch(request: request) { result in
httpClient.send(request: request, delegate: nil) { result in
switch result {
case .failure(let error):
completionHandler(.failure(error))
completionHandler(.failure(.networkError(error)))

case .success(let (http, data)):
// 1. Not Modified — existing cache is still valid
Expand All @@ -81,24 +93,32 @@ internal final class RemoteConfigurationSynchronizer {

// 2. Non-2xx HTTP status
guard (200..<300).contains(http.statusCode) else {
completionHandler(.failure(RemoteConfigurationError.httpError(http.statusCode)))
let error = RemoteConfigurationError.httpError(http.statusCode)
completionHandler(.failure(error))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit/

Suggested change
let error = RemoteConfigurationError.httpError(http.statusCode)
completionHandler(.failure(error))
completionHandler(.failure(httpError(http.statusCode)))

return
}

// 3. Empty body
guard !data.isEmpty else {
completionHandler(.failure(RemoteConfigurationError.emptyBody))
guard let data = data, !data.isEmpty else {
completionHandler(.failure(.emptyBody))
return
}

// TODO RUM-16387: Validate the schema before saving
let remoteConfiguration: RemoteConfiguration
switch Self.decode(data) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid rejecting remote config before caching

When the CDN returns a syntactically valid remote-config JSON that this SDK cannot fully decode yet (for example a new enum value in a server-controlled field), this eager decode returns before the disk write, so the raw document is never cached. That regresses the documented remoteConfigurationID behavior of fetching raw JSON for subsequent launches and makes older SDKs stop updating whenever the remote schema evolves; persist the non-empty body first and decode only when exposing the typed value.

Useful? React with 👍 / 👎.

case .success(let decodedRemoteConfiguration):
remoteConfiguration = decodedRemoteConfiguration
case .failure(let error):
completionHandler(.failure(error))
return
}

// All checks passed — persist to disk and update in-memory cache.
// File.write uses .atomic (write to temp, then rename), so the update is
// all-or-nothing: the existing file is never left in a truncated state.
do {
try File(url: self.directory.url.appendingPathComponent("\(self.id).json")).write(data: data)
self.cache = .success(data)
self.cache = .success(remoteConfiguration)

// Store ETag for conditional requests on the next sync.
// If the response has no ETag, delete any stale validator so we
Expand All @@ -114,23 +134,29 @@ internal final class RemoteConfigurationSynchronizer {
try? self.directory.file(named: etagFileName).delete()
}

completionHandler(.success(data))
completionHandler(.success(remoteConfiguration))
} catch {
completionHandler(.failure(error))
completionHandler(.failure(.diskError))
}
}
}
}
}

private enum RemoteConfigurationError: Error, LocalizedError {
internal enum RemoteConfigurationError: Error, LocalizedError {
case networkError(Error)
case httpError(Int)
case emptyBody
case decodingError(Error)
case diskError

var errorDescription: String? {
switch self {
case .networkError(let error): return "Network error: \(error.localizedDescription)"
case .httpError(let code): return "Non-2xx response: HTTP \(code)"
case .emptyBody: return "Empty response body"
case .decodingError(let error): return "Remote configuration decoding failed: \(error.localizedDescription)"
case .diskError: return "Remote configuration disk read/write failed"
}
}
}
2 changes: 1 addition & 1 deletion DatadogCore/Sources/Core/Upload/DataUploader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal final class DataUploader: DataUploaderType {

httpClient.send(request: request, delegate: delegate) { result in
switch result {
case .success(let httpResponse):
case .success(let (httpResponse, _)):
uploadStatus = DataUploadStatus(
httpResponse: httpResponse,
ddRequestID: requestID,
Expand Down
23 changes: 8 additions & 15 deletions DatadogCore/Sources/Core/Upload/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,27 +12,20 @@ internal protocol HTTPClient {
/// - Parameters:
/// - request: The request to be sent.
/// - delegate: The task-specific delegate.
/// - completion: A closure that receives a Result containing either an HTTPURLResponse or an Error.
func send(request: URLRequest, delegate: URLSessionTaskDelegate?, completion: @escaping (Result<HTTPURLResponse, Error>) -> Void)

/// Fetches the provided request using HTTP and returns both the response and the response body.
/// - Parameters:
/// - request: The request to be sent.
/// - completion: A closure that receives a Result containing either an (HTTPURLResponse, Data) pair or an Error.
func fetch(request: URLRequest, completion: @escaping (Result<(HTTPURLResponse, Data), Error>) -> Void)
/// - completion: A closure that receives a Result containing either the HTTP response and body or an Error.
func send(
request: URLRequest,
delegate: URLSessionTaskDelegate?,
completion: @escaping (Result<(HTTPURLResponse, Data?), Error>) -> Void
)
}

extension HTTPClient {
/// Sends the provided request using HTTP.
/// - Parameters:
/// - request: The request to be sent.
/// - completion: A closure that receives a Result containing either an HTTPURLResponse or an Error.
func send(request: URLRequest, completion: @escaping (Result<HTTPURLResponse, Error>) -> Void) {
/// - completion: A closure that receives a Result containing either the HTTP response and body or an Error.
func send(request: URLRequest, completion: @escaping (Result<(HTTPURLResponse, Data?), Error>) -> Void) {
self.send(request: request, delegate: nil, completion: completion)
}

/// Default no-op — concrete types that support response body data must provide their own implementation.
func fetch(request: URLRequest, completion: @escaping (Result<(HTTPURLResponse, Data), Error>) -> Void) {
completion(.failure(NSError(domain: "HTTPClient", code: -1, userInfo: [NSLocalizedDescriptionKey: "fetch(_:) not implemented"])))
}
}
30 changes: 9 additions & 21 deletions DatadogCore/Sources/Core/Upload/URLSessionClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,11 @@ internal class URLSessionClient: HTTPClient {
self.session = session
}

func fetch(request: URLRequest, completion: @escaping (Result<(HTTPURLResponse, Data), Error>) -> Void) {
let task = session.dataTask(with: request) { data, response, error in
if let error = error {
completion(.failure(error))
return
}
guard let http = response as? HTTPURLResponse, let data = data else {
completion(.failure(URLSessionTransportInconsistencyException()))
return
}
completion(.success((http, data)))
}
task.resume()
}

func send(request: URLRequest, delegate: URLSessionTaskDelegate?, completion: @escaping (Result<HTTPURLResponse, Error>) -> Void) {
func send(
request: URLRequest,
delegate: URLSessionTaskDelegate?,
completion: @escaping (Result<(HTTPURLResponse, Data?), Error>) -> Void
) {
let task = session.dataTask(with: request) { data, response, error in
completion(httpClientResult(for: (data, response, error)))
}
Expand All @@ -76,17 +65,16 @@ private func basicHTTPAuthentication(username: String, password: String) -> Stri
return "Basic \(credential)"
}

/// As `URLSession` returns 3-values-tuple for request execution, this function applies consistency constraints and turns
/// it into only two possible states of `HTTPTransportResult`.
private func httpClientResult(for urlSessionTaskCompletion: (Data?, URLResponse?, Error?)) -> Result<HTTPURLResponse, Error> {
let (_, response, error) = urlSessionTaskCompletion
/// Returns a typed HTTP result from `URLSession`'s completion values.
private func httpClientResult(for urlSessionTaskCompletion: (Data?, URLResponse?, Error?)) -> Result<(HTTPURLResponse, Data?), Error> {
let (data, response, error) = urlSessionTaskCompletion

if let error = error {
return .failure(error)
}

if let httpResponse = response as? HTTPURLResponse {
return .success(httpResponse)
return .success((httpResponse, data))
}

return .failure(URLSessionTransportInconsistencyException())
Expand Down
18 changes: 14 additions & 4 deletions DatadogCore/Sources/Datadog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -423,12 +423,22 @@ extension DatadogCore {
site: configuration.site
)

let httpClient = configuration.httpClientFactory(configuration.proxyConfiguration)
let remoteConfigurationProvider = configuration.remoteConfigurationID.map {
RemoteConfigurationProvider(
id: $0,
site: configuration.site,
directory: directory.coreDirectory,
httpClient: httpClient
)
}

self.init(
directory: directory,
dateProvider: configuration.dateProvider,
initialConsent: trackingConsent,
performance: performance,
httpClient: configuration.httpClientFactory(configuration.proxyConfiguration),
httpClient: httpClient,
encryption: configuration.encryption,
contextProvider: DatadogContextProvider(
site: configuration.site,
Expand Down Expand Up @@ -462,12 +472,12 @@ extension DatadogCore {
maxBatchesPerUpload: configuration.batchProcessingLevel.maxBatchesPerUpload,
backgroundTasksEnabled: configuration.backgroundTasksEnabled,
isRunFromExtension: isRunFromExtension,
remoteConfiguration: configuration.remoteConfigurationID.map { ($0, configuration.site) }
remoteConfigurationProvider: remoteConfigurationProvider
)

synchronizer?.sync { [weak self] result in
remoteConfigurationProvider?.sync { result in
if case .failure(let error) = result {
self?.telemetry.error("[RemoteConfig] Sync failed", error: error)
self.telemetry.error("[RemoteConfig] Sync failed", error: error)
}
}

Expand Down
Loading