Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-3175 fix: Fix race condition during consent change (pending β†’ granted) #2063

Merged
merged 3 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

- [FIX] Fix race condition during consent change, preventing loss of events recorded on the current thread. See [#2063][]
- [IMPROVEMENT] Support mutation of events' attributes. See [#2099][]

# 2.19.0 / 28-10-2024
Expand Down Expand Up @@ -786,6 +787,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
[#2088]: https://github.com/DataDog/dd-sdk-ios/pull/2088
[#2083]: https://github.com/DataDog/dd-sdk-ios/pull/2083
[#2099]: https://github.com/DataDog/dd-sdk-ios/pull/2099
[#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
Expand Down
7 changes: 6 additions & 1 deletion DatadogCore/Sources/Core/DatadogCore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,12 @@ 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) }
contextProvider.queue.async { [allStorages] in
// RUM-3175: To prevent race conditions with ongoing "event write" operations,
// data migration must be synchronized on the context queue. This guarantees that
// all latest events have been written before migration occurs.
allStorages.forEach { $0.migrateUnauthorizedData(toConsent: trackingConsent) }
}
consentPublisher.consent = trackingConsent
}
}
Expand Down
59 changes: 59 additions & 0 deletions DatadogCore/Tests/Datadog/DatadogCore/DatadogCoreTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,65 @@ 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: .combining(
storagePerformance: StoragePerformanceMock.readAllFiles,
uploadPerformance: UploadPerformanceMock.veryQuick
),
httpClient: HTTPClientMock(),
encryption: nil,
contextProvider: .mockAny(),
applicationVersion: .mockAny(),
maxBatchesPerUpload: 1,
backgroundTasksEnabled: .mockAny()
)
defer { core.flushAndTearDown() }

let send2RequestsExpectation = expectation(description: "send 2 requests")
send2RequestsExpectation.expectedFulfillmentCount = 2

let requestBuilderSpy = FeatureRequestBuilderSpy()
requestBuilderSpy.onRequest = { _, _ in send2RequestsExpectation.fulfill() }

try core.register(feature: FeatureMock(requestBuilder: requestBuilderSpy))

// When
let scope = core.scope(for: FeatureMock.self)
core.set(trackingConsent: .pending)
scope.eventWriteContext { context, writer in
XCTAssertEqual(context.trackingConsent, .pending)
writer.write(value: FeatureMock.Event(event: "pending"))
}

core.set(trackingConsent: .granted)
scope.eventWriteContext { context, writer in
XCTAssertEqual(context.trackingConsent, .granted)
writer.write(value: FeatureMock.Event(event: "granted"))
}

// Then
waitForExpectations(timeout: 2)

let uploadedEvents = requestBuilderSpy.requestParameters
.flatMap { $0.events }
.map { $0.data.utf8String }

XCTAssertEqual(
uploadedEvents,
[
#"{"event":"pending"}"#,
#"{"event":"granted"}"#
],
"It should upload all events"
)
XCTAssertEqual(requestBuilderSpy.requestParameters.count, 2, "It should send 2 requests")
}

func testWhenWritingEventsWithBypassingConsent_itUploadsAllEvents() throws {
// Given
let core = DatadogCore(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,21 @@ internal class FeatureRequestBuilderMock: FeatureRequestBuilder {
}

internal class FeatureRequestBuilderSpy: FeatureRequestBuilder {
/// Records parameters passed to `requet(for:with:)`
/// Stores the parameters passed to the `request(for:with:)` method.
@ReadWriteLock
private(set) var requestParameters: [(events: [Event], context: DatadogContext)] = []

/// A closure that is called when a request is about to be created in the `request(for:with:)` method.
@ReadWriteLock
var onRequest: ((_ events: [Event], _ context: DatadogContext) -> Void)?

func request(
for events: [Event],
with context: DatadogContext,
execution: ExecutionContext
) throws -> URLRequest {
requestParameters.append((events: events, context: context))
onRequest?(events, context)
return .mockAny()
}
}
Expand Down