RUM-3175 fix: use context queue to force file IO happens serially

DataDog · Sep 30, 2024 · c765a57 · c765a57
1 parent dec751b
commit c765a57
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,9 +1,12 @@
 # Unreleased
 
+- [FIX] Force events flushing by performing consent change on context queue. See [#2063][]
+
 # 2.18.0 / 25-09-2024
 - [IMPROVEMENT] Add overwrite required (breaking) param to addViewLoadingTime & usage telemetry. See [#2040][]
 - [FEATURE] Prevent "show password" features from revealing sensitive texts in Session Replay. See [#2050][]
 - [FEATURE] Add Fine-Grained Masking configuration options to Session Replay. See [#2043][]
+- [FEATURE] Prevent "show password" features from revealing sensitive texts. See [#2050][]
 
 # 2.17.0 / 11-09-2024
 
@@ -774,6 +777,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
 [#2043]: https://github.com/DataDog/dd-sdk-ios/pull/2043
 [#2040]: https://github.com/DataDog/dd-sdk-ios/pull/2040
 [#2050]: https://github.com/DataDog/dd-sdk-ios/pull/2050
+[#2063]: https://github.com/DataDog/dd-sdk-ios/pull/2063
 [@00fa9a]: https://github.com/00FA9A
 [@britton-earnin]: https://github.com/Britton-Earnin
 [@hengyu]: https://github.com/Hengyu

diff --git a/DatadogCore/Sources/Core/DatadogCore.swift b/DatadogCore/Sources/Core/DatadogCore.swift
@@ -159,9 +159,17 @@ internal final class DatadogCore {
     ///
     /// - Parameter trackingConsent: new consent value, which will be applied for all data collected from now on
     func set(trackingConsent: TrackingConsent) {
-        if trackingConsent != consentPublisher.consent {
-            allStorages.forEach { $0.migrateUnauthorizedData(toConsent: trackingConsent) }
-            consentPublisher.consent = trackingConsent
+        // There could be events in the queue that were collected before the consent change
+        // but not written to the disk yet, which must be migrated to the new consent.
+        // We synchronize this by reading the context because to obtain a file writer, we need to read the context.
+        // which is performed via CoreFeatureScope.eventWriteContext(bypassConsent:,block:) API.
+        contextProvider.read { _ in
+            if trackingConsent != self.consentPublisher.consent {
+                self.allStorages.forEach {
+                    $0.migrateUnauthorizedData(toConsent: trackingConsent)
+                }
+                self.consentPublisher.consent = trackingConsent
+            }
         }
     }
 

diff --git a/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift b/DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift
@@ -132,6 +132,61 @@ class DatadogCoreTests: XCTestCase {
         XCTAssertEqual(requestBuilderSpy.requestParameters.count, 1, "It should send only one request")
     }
 
+    func testWhenWritingEventsWithPendingConsentThenGranted_itUploadsAllEvents() throws {
+        // Given
+        let core = DatadogCore(
+            directory: temporaryCoreDirectory,
+            dateProvider: SystemDateProvider(),
+            initialConsent: .mockRandom(),
+            performance: .init(batchSize: .large, uploadFrequency: .frequent, bundleType: .iOSApp),
+            httpClient: HTTPClientMock(),
+            encryption: nil,
+            contextProvider: .mockAny(),
+            applicationVersion: .mockAny(),
+            maxBatchesPerUpload: .mockRandom(min: 1, max: 100),
+            backgroundTasksEnabled: .mockAny()
+        )
+
+        let requestBuilderSpy = FeatureRequestBuilderSpy()
+        try core.register(feature: FeatureMock(requestBuilder: requestBuilderSpy))
+        let scope = core.scope(for: FeatureMock.self)
+
+        // When
+        core.set(trackingConsent: .notGranted)
+        scope.eventWriteContext(bypassConsent: true) { context, writer in
+            writer.write(value: FeatureMock.Event(event: "not granted"))
+        }
+
+        core.set(trackingConsent: .granted)
+        scope.eventWriteContext(bypassConsent: true) { context, writer in
+            writer.write(value: FeatureMock.Event(event: "granted"))
+        }
+
+        // wait for 10 seconds asynchronously to make sure events are flushed and sent
+        // we don't want to manually flush because that's not how the SDK is used in production
+        waitForEvents(requestBuilder: requestBuilderSpy)
+
+        let uploadedEvents = requestBuilderSpy.requestParameters
+            .flatMap { $0.events }
+            .map { $0.data.utf8String }
+
+        XCTAssertEqual(
+            uploadedEvents,
+            [
+                #"{"event":"not granted"}"#,
+                #"{"event":"granted"}"#
+            ],
+            "It should upload all events"
+        )
+        XCTAssertEqual(requestBuilderSpy.requestParameters.count, 1, "It should send only one request")
+    }
+
+    private func waitForEvents(requestBuilder: FeatureRequestBuilderSpy) {
+       while requestBuilder.requestParameters.isEmpty {
+           Thread.sleep(forTimeInterval: .fromMilliseconds(1))
+       }
+    }
+
     func testWhenFeatureBaggageIsUpdated_thenNewValueIsImmediatellyAvailable() throws {
         // Given
         let core = DatadogCore(

diff --git a/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift b/DatadogCore/Tests/Datadog/Mocks/DatadogInternal/UploadMock.swift
@@ -30,6 +30,7 @@ internal class FeatureRequestBuilderMock: FeatureRequestBuilder {
 
 internal class FeatureRequestBuilderSpy: FeatureRequestBuilder {
     /// Records parameters passed to `requet(for:with:)`
+    @ReadWriteLock
     private(set) var requestParameters: [(events: [Event], context: DatadogContext)] = []
 
     func request(