diff --git a/CHANGELOG.md b/CHANGELOG.md index fd1f7c57a1..56e5874c87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased - [FEATURE] Add wildcard host pattern support to WebView tracking via `WebViewTracking.enable(webView:hostPatterns:)`. See [#2963][] +- [FIX] Fix `DatadogFlags` exposure deduplication so assignment changes for the same subject and flag emit new exposure events. See [#2987][] - [IMPROVEMENT] DatadogTrace now leverages Swift 6 compile time checking against data races. Types like `DDSpan` and `OTSpan` are now marked as Sendable and can be used safely across isolation barriers. External dependency `opentelemetry-swift-core` version was updated to 2.5.0. See [#2876][] - [FIX] Fix watchOS uploads blocked by NWPathMonitor always reporting no reachability. See [#2975][] - [FIX] Fix several instances where misaligned memory could be loaded without proper checking. See [#2995][] @@ -1179,6 +1180,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO [#2968]: https://github.com/DataDog/dd-sdk-ios/pull/2968 [#2969]: https://github.com/DataDog/dd-sdk-ios/pull/2969 [#2963]: https://github.com/DataDog/dd-sdk-ios/pull/2963 +[#2987]: https://github.com/DataDog/dd-sdk-ios/pull/2987 [#2955]: https://github.com/DataDog/dd-sdk-ios/pull/2955 [#2975]: https://github.com/DataDog/dd-sdk-ios/pull/2975 [#2995]: https://github.com/DataDog/dd-sdk-ios/pull/2995 diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj index ecdb03138a..8d4e53bac8 100644 --- a/Datadog/Datadog.xcodeproj/project.pbxproj +++ b/Datadog/Datadog.xcodeproj/project.pbxproj @@ -320,6 +320,7 @@ 5B4F37692E93C5C90093778F /* FlagsRUMIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B4F37682E93C5B80093778F /* FlagsRUMIntegrationTests.swift */; }; 5B4F376C2E93C5E90093778F /* FlagsEvaluationIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B4F376B2E93C5D90093778F /* FlagsEvaluationIntegrationTests.swift */; }; 5B4F552D2E853B3B00A241C3 /* ExposureLoggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B4F552B2E853B2E00A241C3 /* ExposureLoggerTests.swift */; }; + 5B4F55312E85400000A241C3 /* ExposureTrackerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B4F55302E85400000A241C3 /* ExposureTrackerTests.swift */; }; 5B4F552F2E853D7700A241C3 /* FlagsClientMocks.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B4F552E2E853D7200A241C3 /* FlagsClientMocks.swift */; }; 5B5B8CB62E8BCA6A00A6740E /* FlagsRepositoryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B5B8CB52E8BCA5F00A6740E /* FlagsRepositoryTests.swift */; }; 5B5B8CB92E8BD44700A6740E /* FlagsDataStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5B5B8CB82E8BD44000A6740E /* FlagsDataStore.swift */; }; @@ -2091,6 +2092,7 @@ 5B4F37682E93C5B80093778F /* FlagsRUMIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlagsRUMIntegrationTests.swift; sourceTree = ""; }; 5B4F376B2E93C5D90093778F /* FlagsEvaluationIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlagsEvaluationIntegrationTests.swift; sourceTree = ""; }; 5B4F552B2E853B2E00A241C3 /* ExposureLoggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExposureLoggerTests.swift; sourceTree = ""; }; + 5B4F55302E85400000A241C3 /* ExposureTrackerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExposureTrackerTests.swift; sourceTree = ""; }; 5B4F552E2E853D7200A241C3 /* FlagsClientMocks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlagsClientMocks.swift; sourceTree = ""; }; 5B5B8CB52E8BCA5F00A6740E /* FlagsRepositoryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlagsRepositoryTests.swift; sourceTree = ""; }; 5B5B8CB82E8BD44000A6740E /* FlagsDataStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlagsDataStore.swift; sourceTree = ""; }; @@ -3843,6 +3845,7 @@ 5B4F552E2E853D7200A241C3 /* FlagsClientMocks.swift */, 5B16A3722E8434140046A863 /* ExposureMocks.swift */, 5B4F552B2E853B2E00A241C3 /* ExposureLoggerTests.swift */, + 5B4F55302E85400000A241C3 /* ExposureTrackerTests.swift */, 5BF802742E9CD28500097B0E /* RUMFlagEvaluationReporterTests.swift */, 5BB294AF2E82CCA100CAA44B /* ExposureRequestBuilderTests.swift */, 8DADC3C02E7CCF190060F558 /* FlagsClientTests.swift */, @@ -8054,6 +8057,7 @@ 5BF228702E795DF800D79E8A /* FlagsTests.swift in Sources */, 5BB294B02E82CCA400CAA44B /* ExposureRequestBuilderTests.swift in Sources */, 5B4F552D2E853B3B00A241C3 /* ExposureLoggerTests.swift in Sources */, + 5B4F55312E85400000A241C3 /* ExposureTrackerTests.swift in Sources */, 5B4F552F2E853D7700A241C3 /* FlagsClientMocks.swift in Sources */, 8DADC3C92E7CCF190060F558 /* FlagsClientTests.swift in Sources */, 5B1D028E2E8ED10200AB2391 /* FlagAssignmentsResponseTests.swift in Sources */, diff --git a/DatadogFlags/Sources/Client/ExposureLogger.swift b/DatadogFlags/Sources/Client/ExposureLogger.swift index 60d2323395..00ef6cb0f4 100644 --- a/DatadogFlags/Sources/Client/ExposureLogger.swift +++ b/DatadogFlags/Sources/Client/ExposureLogger.swift @@ -46,10 +46,9 @@ internal final class ExposureLogger: ExposureLogging { variationKey: assignment.variationKey ) - guard !loggedExposures.contains(exposure) else { + guard loggedExposures.track(exposure) else { return } - loggedExposures.insert(exposure) let date = dateProvider.now.addingTimeInterval(context.serverTimeOffset) let exposureEvent = ExposureEvent( diff --git a/DatadogFlags/Sources/Client/ExposureTracker.swift b/DatadogFlags/Sources/Client/ExposureTracker.swift index 5f4fb2d9c1..8b78372deb 100644 --- a/DatadogFlags/Sources/Client/ExposureTracker.swift +++ b/DatadogFlags/Sources/Client/ExposureTracker.swift @@ -14,39 +14,72 @@ internal final class ExposureTracker { let variationKey: String } - private class ExposureBox: NSObject { - let value: Exposure + private final class ExposureKey: NSObject { + let targetingKey: String + let flagKey: String - init(_ value: Exposure) { - self.value = value + init(_ exposure: Exposure) { + self.targetingKey = exposure.targetingKey + self.flagKey = exposure.flagKey } override var hash: Int { - value.hashValue + var hasher = Hasher() + hasher.combine(targetingKey) + hasher.combine(flagKey) + return hasher.finalize() } override func isEqual(_ object: Any?) -> Bool { - guard let other = object as? ExposureBox else { + guard let other = object as? ExposureKey else { return false } - return value == other.value + + return targetingKey == other.targetingKey && flagKey == other.flagKey } } - static let defaultCountLimit: Int = 1_000 + private final class Assignment { + let allocationKey: String + let variationKey: String - private let cache = NSCache() - private let sentinel = NSNumber(value: true) + init(_ exposure: Exposure) { + self.allocationKey = exposure.allocationKey + self.variationKey = exposure.variationKey + } - init(countLimit: Int = defaultCountLimit) { - self.cache.countLimit = countLimit + func isEqual(to other: Assignment) -> Bool { + allocationKey == other.allocationKey && variationKey == other.variationKey + } } - func contains(_ exposure: Exposure) -> Bool { - cache.object(forKey: .init(exposure)) != nil + // Keep enough latest assignments for the expected mobile case of 2 subjects x 2,500 flags. + // Normal flag keys are typically tens of characters, so entry count is easier to reason about + // than object-size estimates. + static let defaultCountLimit: Int = 5_000 + + private let assignmentsByExposureKey = NSCache() + private let lock: NSLocking + + init( + countLimit: Int = defaultCountLimit, + lock: NSLocking = NSLock() + ) { + self.lock = lock + self.assignmentsByExposureKey.countLimit = countLimit } - func insert(_ exposure: Exposure) { - cache.setObject(sentinel, forKey: .init(exposure)) + func track(_ exposure: Exposure) -> Bool { + lock.lock() + defer { lock.unlock() } + + let key = ExposureKey(exposure) + let assignment = Assignment(exposure) + guard assignmentsByExposureKey.object(forKey: key)?.isEqual(to: assignment) != true else { + return false + } + + assignmentsByExposureKey.setObject(assignment, forKey: key) + return true } } diff --git a/DatadogFlags/Tests/Client/ExposureLoggerTests.swift b/DatadogFlags/Tests/Client/ExposureLoggerTests.swift index 6176973840..4c3628809d 100644 --- a/DatadogFlags/Tests/Client/ExposureLoggerTests.swift +++ b/DatadogFlags/Tests/Client/ExposureLoggerTests.swift @@ -148,6 +148,48 @@ final class ExposureLoggerTests: XCTestCase { XCTAssertEqual(featureScope.eventsWritten.count, 2, "Same flag with different allocation should be not deduplicated") } + func testLogExposureAssignmentCycle() { + // Given + let logger = ExposureLogger( + dateProvider: DateProviderMock(), + featureScope: featureScope + ) + let evaluationContext = FlagsEvaluationContext( + targetingKey: "target-1", + attributes: .mockAny() + ) + var assignmentA = FlagAssignment.mockAnyBoolean() + assignmentA.allocationKey = "allocation-a" + assignmentA.variationKey = "variation-a" + var assignmentB = assignmentA + assignmentB.allocationKey = "allocation-b" + assignmentB.variationKey = "variation-b" + + // When + logger.logExposure( + for: "flag-1", + assignment: assignmentA, + evaluationContext: evaluationContext + ) + logger.logExposure( + for: "flag-1", + assignment: assignmentB, + evaluationContext: evaluationContext + ) + logger.logExposure( + for: "flag-1", + assignment: assignmentA, + evaluationContext: evaluationContext + ) + + // Then + XCTAssertEqual( + featureScope.eventsWritten.count, + 3, + "Assignment changes should emit exposures, including cycles back to a previous assignment" + ) + } + func testLogExposureDifferentFlag() { // Given let logger = ExposureLogger( diff --git a/DatadogFlags/Tests/Client/ExposureTrackerTests.swift b/DatadogFlags/Tests/Client/ExposureTrackerTests.swift new file mode 100644 index 0000000000..4ff8a234b3 --- /dev/null +++ b/DatadogFlags/Tests/Client/ExposureTrackerTests.swift @@ -0,0 +1,96 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-Present Datadog, Inc. + */ + +import XCTest + +@_spi(Internal) +@testable import DatadogFlags + +final class ExposureTrackerTests: XCTestCase { + func testItSuppressesRepeatedAssignment() { + // Given + let tracker = ExposureTracker() + let exposure = ExposureTracker.Exposure( + targetingKey: "subject-1", + flagKey: "flag-1", + allocationKey: "allocation-a", + variationKey: "variation-a" + ) + + // When + let firstTrack = tracker.track(exposure) + let secondTrack = tracker.track(exposure) + + // Then + XCTAssertTrue(firstTrack) + XCTAssertFalse(secondTrack) + } + + func testItTracksAssignmentCycle() { + // Given + let tracker = ExposureTracker() + let exposureA = ExposureTracker.Exposure( + targetingKey: "subject-1", + flagKey: "flag-1", + allocationKey: "allocation-a", + variationKey: "variation-a" + ) + let exposureB = ExposureTracker.Exposure( + targetingKey: "subject-1", + flagKey: "flag-1", + allocationKey: "allocation-b", + variationKey: "variation-b" + ) + + // When + let firstTrack = tracker.track(exposureA) + let secondTrack = tracker.track(exposureB) + let thirdTrack = tracker.track(exposureA) + + // Then + XCTAssertTrue(firstTrack) + XCTAssertTrue(secondTrack) + XCTAssertTrue(thirdTrack) + } + + func testItTracksTwoSubjectsAcrossManyFlags() { + // Given + let tracker = ExposureTracker() + let targetingKeys = ["subject-1", "subject-2"] + let flagKeys = (0..<2_500).map { "flag-\($0)" } + + // Then + for targetingKey in targetingKeys { + for flagKey in flagKeys { + XCTAssertTrue( + tracker.track( + .init( + targetingKey: targetingKey, + flagKey: flagKey, + allocationKey: "allocation-a", + variationKey: "variation-a" + ) + ) + ) + } + } + + for targetingKey in targetingKeys { + for flagKey in flagKeys { + XCTAssertFalse( + tracker.track( + .init( + targetingKey: targetingKey, + flagKey: flagKey, + allocationKey: "allocation-a", + variationKey: "variation-a" + ) + ) + ) + } + } + } +}