REPLAY-1963 Simplify implementation

DataDog · Aug 17, 2023 · 168cb7f · 168cb7f
1 parent db346af
commit 168cb7f
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 249 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - [BUGFIX] Do not propagate attributes from Errors and LongTasks to Views.
 - [IMPROVEMENT] Upgrade to PLCrashReporter 1.11.1.
+- [IMPROVEMENT] Add UIBackgroundTask for uploading jobs. See [#1412][]
 
 # 2.0.0 / 31-07-2023
 
@@ -491,6 +492,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
 [#1331]: https://github.com/DataDog/dd-sdk-ios/pull/1331
 [#1328]: https://github.com/DataDog/dd-sdk-ios/pull/1328
 [#1355]: https://github.com/DataDog/dd-sdk-ios/pull/1355
+[#1412]: https://github.com/DataDog/dd-sdk-ios/pull/1412
 [@00fa9a]: https://github.com/00FA9A
 [@britton-earnin]: https://github.com/Britton-Earnin
 [@hengyu]: https://github.com/Hengyu

diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj
@@ -447,8 +447,6 @@
 		9E68FB55244707FD0013A8AA /* ObjcExceptionHandler.m in Sources */ = {isa = PBXBuildFile; fileRef = 9E68FB53244707FD0013A8AA /* ObjcExceptionHandler.m */; };
 		9E68FB56244707FD0013A8AA /* ObjcExceptionHandler.h in Headers */ = {isa = PBXBuildFile; fileRef = 9E68FB54244707FD0013A8AA /* ObjcExceptionHandler.h */; settings = {ATTRIBUTES = (Public, ); }; };
 		9EE5AD8226205B82001E699E /* DDNSURLSessionDelegateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9EE5AD8126205B82001E699E /* DDNSURLSessionDelegateTests.swift */; };
-		A716C8312A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A716C8302A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift */; };
-		A716C8322A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A716C8302A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift */; };
 		A728ADAB2934EA2100397996 /* W3CHTTPHeadersWriter+objc.swift in Sources */ = {isa = PBXBuildFile; fileRef = A728ADAA2934EA2100397996 /* W3CHTTPHeadersWriter+objc.swift */; };
 		A728ADAC2934EA2100397996 /* W3CHTTPHeadersWriter+objc.swift in Sources */ = {isa = PBXBuildFile; fileRef = A728ADAA2934EA2100397996 /* W3CHTTPHeadersWriter+objc.swift */; };
 		A728ADB02934EB0900397996 /* DDW3CHTTPHeadersWriter+apiTests.m in Sources */ = {isa = PBXBuildFile; fileRef = A728ADAD2934EB0300397996 /* DDW3CHTTPHeadersWriter+apiTests.m */; };
@@ -2355,7 +2353,6 @@
 		9EC8B5D92668197B000F7529 /* VitalCPUReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalCPUReader.swift; sourceTree = "<group>"; };
 		9EC8B5ED2668E4DB000F7529 /* VitalCPUReaderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VitalCPUReaderTests.swift; sourceTree = "<group>"; };
 		9EE5AD8126205B82001E699E /* DDNSURLSessionDelegateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DDNSURLSessionDelegateTests.swift; sourceTree = "<group>"; };
-		A716C8302A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIKitBackgroundTaskCoordinatorTests.swift; sourceTree = "<group>"; };
 		A728AD9C2934CE4400397996 /* W3CHTTPHeaders.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = W3CHTTPHeaders.swift; sourceTree = "<group>"; };
 		A728AD9E2934CE5000397996 /* W3CHTTPHeadersWriter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = W3CHTTPHeadersWriter.swift; sourceTree = "<group>"; };
 		A728ADA02934CE5D00397996 /* W3CHTTPHeadersReader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = W3CHTTPHeadersReader.swift; sourceTree = "<group>"; };
@@ -3919,7 +3916,6 @@
 				61133C322423990D00786299 /* DataUploaderTests.swift */,
 				61133C342423990D00786299 /* HTTPClientTests.swift */,
 				61133C332423990D00786299 /* RequestBuilderTests.swift */,
-				A716C8302A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift */,
 			);
 			path = Upload;
 			sourceTree = "<group>";
@@ -7382,7 +7378,6 @@
 				61DA20F026C40121004AFE6D /* DataUploadStatusTests.swift in Sources */,
 				61112F8E2A4417D6006FFCA6 /* DDRUM+apiTests.m in Sources */,
 				6139CD772589FEE3007E8BB7 /* RetryingTests.swift in Sources */,
-				A716C8312A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift in Sources */,
 				D29A9FC429DDB710005C54A4 /* RUMInternalProxyTests.swift in Sources */,
 				61133C482423990D00786299 /* DDDatadogTests.swift in Sources */,
 				D2B3F0442823EE8400C2B5EE /* DataBlockTests.swift in Sources */,
@@ -8419,7 +8414,6 @@
 				D2DA23CE298D5EDF00C6C7E6 /* _InternalProxyTests.swift in Sources */,
 				D2CB6F1827C520D400A62B57 /* DatadogTestsObserver.swift in Sources */,
 				D2CB6F1927C520D400A62B57 /* RequestBuilderTests.swift in Sources */,
-				A716C8322A8CFF1E00086ECF /* UIKitBackgroundTaskCoordinatorTests.swift in Sources */,
 				D2CB6F1A27C520D400A62B57 /* FileReaderTests.swift in Sources */,
 				D2777D9E29F6A75800FFBB40 /* TelemetryReceiverTests.swift in Sources */,
 				D2A1EE39287EEB7600D28DFB /* NetworkConnectionInfoPublisherTests.swift in Sources */,

diff --git a/DatadogCore/Sources/Core/Upload/BackgroundTaskCoordinator.swift b/DatadogCore/Sources/Core/Upload/BackgroundTaskCoordinator.swift
@@ -7,35 +7,24 @@
 import Foundation
 
 internal protocol BackgroundTaskCoordinator {
-    func registerBackgroundTask() -> UUID
-    @discardableResult
-    func endBackgroundTaskIfActive(_ uuid: UUID) -> Bool
+    func registerBackgroundTask() -> BackgroundTaskIdentifier?
+    func endBackgroundTaskIfActive(_ backgroundTaskIdentifier: BackgroundTaskIdentifier)
 }
 
+internal typealias BackgroundTaskIdentifier = Int
+
 #if canImport(UIKit)
 import UIKit
 
 /// The `BackgroundTaskCoordinator` class provides an abstraction for managing background tasks and includes methods for registering and ending background tasks.
 /// It also serves as a useful abstraction for testing purposes.
 internal class UIKitBackgroundTaskCoordinator: BackgroundTaskCoordinator {
-    internal var tasks: [UUID: UIBackgroundTaskIdentifier] = [:]
-
-    internal func registerBackgroundTask() -> UUID {
-        let uuid = UUID()
-        tasks[uuid] = UIApplication.dd.managedShared?.beginBackgroundTask { [weak self] in
-            self?.endBackgroundTaskIfActive(uuid)
-        }
-        return uuid
+    internal func registerBackgroundTask() -> BackgroundTaskIdentifier? {
+        return UIApplication.dd.managedShared?.beginBackgroundTask().rawValue
     }
 
-    @discardableResult
-    internal func endBackgroundTaskIfActive(_ uuid: UUID) -> Bool {
-        if let backgroundTask = tasks[uuid], backgroundTask != .invalid {
-            UIApplication.dd.managedShared?.endBackgroundTask(backgroundTask)
-            tasks.removeValue(forKey: uuid)
-            return true
-        }
-        return false
+    func endBackgroundTaskIfActive(_ backgroundTaskIdentifier: BackgroundTaskIdentifier) {
+        UIApplication.dd.managedShared?.endBackgroundTask(.init(rawValue: backgroundTaskIdentifier))
     }
 }
 #endif
diff --git a/DatadogCore/Sources/Core/Upload/DataUploadDelay.swift b/DatadogCore/Sources/Core/Upload/DataUploadDelay.swift
@@ -9,12 +9,12 @@ import DatadogInternal
 
 internal protocol Delay {
     var current: TimeInterval { get }
-    mutating func decrease()
-    mutating func increase()
+    func decrease()
+    func increase()
 }
 
 /// Mutable interval used for periodic data uploads.
-internal struct DataUploadDelay: Delay {
+internal class DataUploadDelay: Delay {
     private let minDelay: TimeInterval
     private let maxDelay: TimeInterval
     private let changeRate: Double
@@ -29,11 +29,11 @@ internal struct DataUploadDelay: Delay {
 
     var current: TimeInterval { delay }
 
-    mutating func decrease() {
+    func decrease() {
         delay = max(minDelay, delay * (1.0 - changeRate))
     }
 
-    mutating func increase() {
+    func increase() {
         delay = min(delay * (1.0 + changeRate), maxDelay)
     }
 }
diff --git a/DatadogCore/Sources/Core/Upload/DataUploadWorker.swift b/DatadogCore/Sources/Core/Upload/DataUploadWorker.swift
@@ -29,7 +29,7 @@ internal class DataUploadWorker: DataUploadWorkerType {
     private let contextProvider: DatadogContextProvider
 
     /// Delay used to schedule consecutive uploads.
-    private var delay: Delay
+    private let delay: Delay
 
     /// Upload work scheduled by this worker.
     private var uploadWork: DispatchWorkItem?
@@ -59,7 +59,56 @@ internal class DataUploadWorker: DataUploadWorkerType {
             guard let self = self else {
                 return
             }
-            self.tryToSendBatch()
+            let taskID = self.backgroundTaskCoordinator?.registerBackgroundTask()
+            let context = contextProvider.read()
+            let blockersForUpload = self.uploadConditions.blockersForUpload(with: context)
+            let isSystemReady = blockersForUpload.isEmpty
+            let nextBatch = isSystemReady ? self.fileReader.readNextBatch() : nil
+            if let batch = nextBatch {
+                DD.logger.debug("⏳ (\(self.featureName)) Uploading batch...")
+
+                do {
+                    // Upload batch
+                    let uploadStatus = try self.dataUploader.upload(
+                        events: batch.events,
+                        context: context
+                    )
+
+                    // Delete or keep batch depending on the upload status
+                    if uploadStatus.needsRetry {
+                        self.delay.increase()
+
+                        DD.logger.debug("   → (\(self.featureName)) not delivered, will be retransmitted: \(uploadStatus.userDebugDescription)")
+                    } else {
+                        self.fileReader.markBatchAsRead(batch, reason: .intakeCode(responseCode: uploadStatus.responseCode ?? -1)) // -1 is unexpected here
+                        self.delay.decrease()
+
+                        DD.logger.debug("   → (\(self.featureName)) accepted, won't be retransmitted: \(uploadStatus.userDebugDescription)")
+                    }
+
+                    switch uploadStatus.error {
+                    case .unauthorized:
+                        DD.logger.error("⚠️ Make sure that the provided token still exists and you're targeting the relevant Datadog site.")
+                    case let .httpError(statusCode: statusCode):
+                        DD.telemetry.error("Data upload finished with status code: \(statusCode)")
+                    case let .networkError(error: error):
+                        DD.telemetry.error("Data upload finished with error", error: error)
+                    case .none: break
+                    }
+                } catch let error {
+                    // If upload can't be initiated do not retry, so drop the batch:
+                    self.fileReader.markBatchAsRead(batch, reason: .invalid)
+                    DD.telemetry.error("Failed to initiate '\(self.featureName)' data upload", error: error)
+                }
+            } else {
+                let batchLabel = nextBatch != nil ? "YES" : (isSystemReady ? "NO" : "NOT CHECKED")
+                DD.logger.debug("💡 (\(self.featureName)) No upload. Batch to upload: \(batchLabel), System conditions: \(blockersForUpload.description)")
+
+                self.delay.increase()
+            }
+            if let taskID = taskID {
+                self.backgroundTaskCoordinator?.endBackgroundTaskIfActive(taskID)
+            }
             self.scheduleNextUpload(after: self.delay.current)
         }
 
@@ -72,72 +121,27 @@ internal class DataUploadWorker: DataUploadWorkerType {
         guard let work = uploadWork else {
             return
         }
-        queue.asyncAfter(deadline: .now() + delay, execute: work)
-    }
-
-    private func tryToSendBatch(_ batch: Batch? = nil, retryOnce: Bool = false) {
-        let taskID = backgroundTaskCoordinator?.registerBackgroundTask()
-        let context = contextProvider.read()
-        let blockersForUpload = uploadConditions.blockersForUpload(with: context)
-        let isSystemReady = blockersForUpload.isEmpty
-        let nextBatch = isSystemReady
-            ? (batch ?? fileReader.readNextBatch())
-            : nil
-        if let batch = nextBatch {
-            DD.logger.debug("⏳ (\(featureName)) Uploading batch...")
-
-            do {
-                // Upload batch
-                let uploadStatus = try dataUploader.upload(
-                    events: batch.events,
-                    context: context
-                )
-
-                // Delete or keep batch depending on the upload status
-                if uploadStatus.needsRetry {
-                    delay.increase()
-                    DD.logger.debug("   → (\(featureName)) not delivered, will be retransmitted: \(uploadStatus.userDebugDescription)")
-
-                    tryToSendBatch(batch, retryOnce: false)
-                }
-                else {
-                    fileReader.markBatchAsRead(batch, reason: .intakeCode(responseCode: uploadStatus.responseCode ?? -1)) // -1 is unexpected here
-                    delay.decrease()
-
-                    DD.logger.debug("   → (\(featureName)) accepted, won't be retransmitted: \(uploadStatus.userDebugDescription)")
-                }
 
-                switch uploadStatus.error {
-                case .unauthorized:
-                    DD.logger.error("⚠️ Make sure that the provided token still exists and you're targeting the relevant Datadog site.")
-                case let .httpError(statusCode: statusCode):
-                    DD.telemetry.error("Data upload finished with status code: \(statusCode)")
-                case let .networkError(error: error):
-                    DD.telemetry.error("Data upload finished with error", error: error)
-                case .none: break
-                }
-            } catch let error {
-                // If upload can't be initiated do not retry, so drop the batch:
-                self.fileReader.markBatchAsRead(batch, reason: .invalid)
-                DD.telemetry.error("Failed to initiate '\(featureName)' data upload", error: error)
-            }
-        } else {
-            let batchLabel = nextBatch != nil ? "YES" : (isSystemReady ? "NO" : "NOT CHECKED")
-            DD.logger.debug("💡 (\(featureName)) No upload. Batch to upload: \(batchLabel), System conditions: \(blockersForUpload.description)")
-
-            delay.increase()
-        }
-        if let taskID = taskID {
-            backgroundTaskCoordinator?.endBackgroundTaskIfActive(taskID)
-        }
+        queue.asyncAfter(deadline: .now() + delay, execute: work)
     }
 
     /// Sends all unsent data synchronously.
     /// - It performs arbitrary upload (without checking upload condition and without re-transmitting failed uploads).
     internal func flushSynchronously() {
-        queue.sync { [weak self] in
-            while let nextBatch = self?.fileReader.readNextBatch() {
-                self?.tryToSendBatch(nextBatch, retry: false)
+        queue.sync {
+            while let nextBatch = self.fileReader.readNextBatch() {
+                defer {
+                    // RUMM-3459 Delete the underlying batch with `.flushed` reason that will be ignored in reported
+                    // metrics or telemetry. This is legitimate as long as `flush()` routine is only available for testing
+                    // purposes and never run in production apps.
+                    self.fileReader.markBatchAsRead(nextBatch, reason: .flushed)
+                }
+                do {
+                    // Try uploading the batch and do one more retry on failure.
+                    _ = try self.dataUploader.upload(events: nextBatch.events, context: contextProvider.read())
+                } catch {
+                    _ = try? self.dataUploader.upload(events: nextBatch.events, context: contextProvider.read())
+                }
             }
         }
     }