Merge pull request DataDog#1747 from DataDog/maciey/RUM-3569-store-an…

…d-filter-resource-ids

RUM-3569 Store and filter resource ids between sessions

CHANGELOG.md

@@ -1,16 +1,17 @@
 # Unreleased
 
-* [FEATURE] Add support for 128 bit trace IDs. See [#1721][]
-* [FEATURE] Fatal App Hangs are tracked in RUM. See [#1763][]
-* [FIX] Avoid name collision with Required Reason APIs. See [#1774][]
+- [IMPROVEMENT] Add image duplicate detection between sessions. See [#1747][]
+- [FEATURE] Add support for 128 bit trace IDs. See [#1721][]
+- [FEATURE] Fatal App Hangs are tracked in RUM. See [#1763][]
+- [FIX] Avoid name collision with Required Reason APIs. See [#1774][]
 
 # 2.9.0 / 11-04-2024
 
-* [FEATURE] Call RUM's `errorEventMapper` for crashes. See [#1742][]
-* [FEATURE] Support calling log event mapper for crashes. See [#1741][]
-* [FIX] Fix crash in `NetworkInstrumentationFeature`. See [#1767][]
-* [FIX] Remove modulemap. See [#1746][]
-* [FIX] Expose objc interfaces in Session Replay module. See [#1697][]
+- [FEATURE] Call RUM's `errorEventMapper` for crashes. See [#1742][]
+- [FEATURE] Support calling log event mapper for crashes. See [#1741][]
+- [FIX] Fix crash in `NetworkInstrumentationFeature`. See [#1767][]
+- [FIX] Remove modulemap. See [#1746][]
+- [FIX] Expose objc interfaces in Session Replay module. See [#1697][]
 
 # 2.8.1 / 20-03-2024
 
@@ -635,6 +636,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO
 [#1763]: https://github.com/DataDog/dd-sdk-ios/pull/1763
 [#1767]: https://github.com/DataDog/dd-sdk-ios/pull/1767
 [#1721]: https://github.com/DataDog/dd-sdk-ios/pull/1721
+[#1747]: https://github.com/DataDog/dd-sdk-ios/pull/1747
 [@00fa9a]: https://github.com/00FA9A
 [@britton-earnin]: https://github.com/Britton-Earnin
 [@hengyu]: https://github.com/Hengyu

DatadogSessionReplay/SRSnapshotTests/SRSnapshotTests/Utils/SnapshotTestCase.swift

@@ -49,7 +49,7 @@ internal class SnapshotTestCase: XCTestCase {
         )
         let resourceProcessor = ResourceProcessor(
             queue: NoQueue(),
-            resourcesWriter: ResourcesWriter(core: PassthroughCoreMock())
+            resourcesWriter: ResourcesWriter(scope: FeatureScopeMock())
         )
         let recorder = try Recorder(
             snapshotProcessor: snapshotProcessor,

DatadogSessionReplay/Sources/Feature/SessionReplayFeature.swift

@@ -35,7 +35,7 @@ internal class SessionReplayFeature: DatadogRemoteFeature {
         )
         let resourceProcessor = ResourceProcessor(
             queue: processorsQueue,
-            resourcesWriter: ResourcesWriter(core: core)
+            resourcesWriter: ResourcesWriter(scope: core.scope(for: ResourcesFeature.self))
         )
         let recorder = try Recorder(
             snapshotProcessor: snapshotProcessor,

DatadogSessionReplay/Sources/SessionReplay.swift

@@ -42,12 +42,11 @@ public enum SessionReplay {
         guard configuration.replaySampleRate > 0 else {
             return
         }
+        let resources = ResourcesFeature(core: core, configuration: configuration)
+        try core.register(feature: resources)
 
         let sessionReplay = try SessionReplayFeature(core: core, configuration: configuration)
         try core.register(feature: sessionReplay)
-
-        let resources = ResourcesFeature(core: core, configuration: configuration)
-        try core.register(feature: resources)
     }
 }
 #endif

DatadogSessionReplay/Sources/Writers/ResourcesWriter.swift

@@ -15,26 +15,111 @@ internal protocol ResourcesWriting {
 }
 
 internal class ResourcesWriter: ResourcesWriting {
-    /// An instance of SDK core the SR feature is registered to.
-    private weak var core: DatadogCoreProtocol?
+    private let scope: FeatureScope
+    private let encoder: JSONEncoder
+    private let decoder: JSONDecoder
+
+    @ReadWriteLock
+    private var knownIdentifiers = Set<String>() {
+        didSet {
+            if let knownIdentifiers = knownIdentifiers.asData(encoder) {
+                scope.dataStore.setValue(
+                    knownIdentifiers,
+                    forKey: Constants.knownResourcesKey
+                )
+            }
+        }
+    }
 
     init(
-        core: DatadogCoreProtocol
+        scope: FeatureScope,
+        dataStoreResetTime: TimeInterval = TimeInterval(30).days,
+        encoder: JSONEncoder = JSONEncoder(),
+        decoder: JSONDecoder = JSONDecoder()
     ) {
-        self.core = core
+        self.scope = scope
+        self.encoder = encoder
+        self.decoder = decoder
+        self.scope.dataStore.value(forKey: Constants.storeCreationKey) { [weak self]  result in
+            do {
+                if let storeCreation = try result.data()?.asTimeInterval(), Date().timeIntervalSince1970 - storeCreation < dataStoreResetTime {
+                    self?.scope.dataStore.value(forKey: Constants.knownResourcesKey) { result in
+                        switch result {
+                        case .value(let data, _):
+                            do {
+                                if let knownIdentifiers = try data.asKnownIdentifiers(decoder) {
+                                    self?.knownIdentifiers.formUnion(knownIdentifiers)
+                                }
+                            } catch let error {
+                                self?.scope.telemetry.error("Failed to decode known identifiers", error: error)
+                            }
+                        default:
+                            break
+                        }
+                    }
+                } else { // Reset if store was created more than 30 days ago
+                    self?.scope.dataStore.setValue(
+                        Date().timeIntervalSince1970.asData(),
+                        forKey: Constants.storeCreationKey
+                    )
+                    self?.scope.dataStore.removeValue(forKey: Constants.knownResourcesKey)
+                }
+            } catch let error {
+                self?.scope.telemetry.error("Failed to decode store creation", error: error)
+            }
+        }
     }
 
     // MARK: - Writing
 
     func write(resources: [EnrichedResource]) {
-        guard let scope = core?.scope(for: ResourcesFeature.self) else {
-            return
-        }
-        scope.eventWriteContext { _, recordWriter in
-            resources.forEach {
-                recordWriter.write(value: $0)
+        scope.eventWriteContext { [weak self] _, recordWriter in
+            let unknownResources = resources.filter { self?.knownIdentifiers.contains($0.identifier) == false }
+            for resource in unknownResources {
+                recordWriter.write(value: resource)
             }
+            self?.knownIdentifiers.formUnion(Set(unknownResources.map { $0.identifier }))
         }
     }
+
+    enum Constants {
+        static let knownResourcesKey = "known-resources"
+        static let storeCreationKey = "store-creation"
+    }
+}
+
+extension Data {
+    enum SerializationError: Error {
+        case invalidData
+    }
+
+    func asTimeInterval() throws -> TimeInterval {
+        var value: TimeInterval = 0
+        guard count >= MemoryLayout.size(ofValue: value) else {
+            throw SerializationError.invalidData
+        }
+        _ = Swift.withUnsafeMutableBytes(of: &value) {
+            copyBytes(to: $0)
+        }
+        return value
+    }
+
+    func asKnownIdentifiers(_ decoder: JSONDecoder) throws -> Set<String>? {
+        return try decoder.decode(Set<String>.self, from: self)
+    }
+}
+
+extension TimeInterval {
+    func asData() -> Data {
+        return Swift.withUnsafeBytes(of: self) {
+            Data($0)
+        }
+    }
+}
+
+extension Set<String> {
+    func asData(_ encoder: JSONEncoder) -> Data? {
+        return try? encoder.encode(self) // Never fails
+    }
 }
 #endif

DatadogSessionReplay/Tests/Mocks/ResourceMocks.swift

@@ -17,6 +17,18 @@ extension EnrichedResource: RandomMockable, AnyMockable {
         )
     }
 
+    public static func mockWith(
+        identifier: String = .mockAny(),
+        data: Data = .mockAny(),
+        context: Context = .mockAny()
+    ) -> EnrichedResource {
+        return .init(
+            identifier: identifier,
+            data: data,
+            context: context
+        )
+    }
+
     public static func mockRandom() -> Self {
         return .init(
             identifier: .mockRandom(),

DatadogSessionReplay/Tests/Writer/ResourcesWriterTests.swift

@@ -5,38 +5,117 @@
  */
 
 import XCTest
+import DatadogInternal
 
 @testable import DatadogSessionReplay
 @testable import TestUtilities
 
 class ResourcesWriterTests: XCTestCase {
-    func testWhenFeatureScopeIsConnected_itWritesResourcesToCore() {
-        // Given
-        let core = PassthroughCoreMock()
+    var scopeMock: FeatureScopeMock! // swiftlint:disable:this implicitly_unwrapped_optional
+    var writer: ResourcesWriter! // swiftlint:disable:this implicitly_unwrapped_optional
 
-        // When
-        let writer = ResourcesWriter(core: core)
+    override func setUp() {
+        scopeMock = FeatureScopeMock()
+        writer = ResourcesWriter(scope: scopeMock)
+    }
 
-        // Then
+    override func tearDown() {
+        writer = nil
+        scopeMock = nil
+    }
+
+    func testWhenInitialized_itSetsUpDataStore() {
+        XCTAssertNotNil(scopeMock.dataStoreMock.value(forKey: ResourcesWriter.Constants.storeCreationKey))
+        XCTAssertNil(scopeMock.dataStoreMock.value(forKey: ResourcesWriter.Constants.knownResourcesKey))
+        XCTAssertTrue(scopeMock.telemetryMock.messages.isEmpty)
+    }
+
+    func test_whenWritesResources_itDoesWriteRecordsToScope() {
+        // When
         writer.write(resources: [.mockRandom()])
         writer.write(resources: [.mockRandom()])
         writer.write(resources: [.mockRandom()])
 
-        XCTAssertEqual(core.events(ofType: EnrichedResource.self).count, 3)
+        // Then
+        XCTAssertEqual(scopeMock.eventsWritten(ofType: EnrichedResource.self).count, 3)
+        XCTAssertTrue(scopeMock.telemetryMock.messages.isEmpty)
+    }
+
+    func test_whenWritesSameResourcesToCore_itRemovesDuplicates() throws {
+        // Given
+        scopeMock.dataStoreMock.setValue(Date().timeIntervalSince1970.asData(), forKey: ResourcesWriter.Constants.storeCreationKey)
+
+        // When
+        writer.write(resources: [.mockWith(identifier: "1")])
+        writer.write(resources: [.mockWith(identifier: "1")])
+
+        // Then
+        XCTAssertEqual(scopeMock.eventsWritten(ofType: EnrichedResource.self).count, 1)
+        XCTAssertTrue(scopeMock.telemetryMock.messages.isEmpty)
+        let data = try XCTUnwrap(scopeMock.dataStoreMock.value(forKey: ResourcesWriter.Constants.knownResourcesKey)?.data())
+        XCTAssertGreaterThan(data.count, 0)
     }
 
-    func testWhenFeatureScopeIsNotConnected_itDoesNotWriteRecordsToCore() throws {
+    func test_whenReadsKnownDuplicates_itDoesNotWriteRecordsToScope() throws {
         // Given
-        let core = SingleFeatureCoreMock<MockFeature>()
-        let feature = MockFeature()
-        try core.register(feature: feature)
+        let knownIdentifiersData = Set(["1"]).asData(JSONEncoder())!
+        scopeMock.dataStoreMock.setValue(knownIdentifiersData, forKey: ResourcesWriter.Constants.knownResourcesKey)
+        scopeMock.dataStoreMock.setValue(Date().timeIntervalSince1970.asData(), forKey: ResourcesWriter.Constants.storeCreationKey)
+        let writer = ResourcesWriter(scope: scopeMock)
 
         // When
-        let writer = ResourcesWriter(core: core)
+        writer.write(resources: [.mockWith(identifier: "1")])
 
         // Then
-        writer.write(resources: [.mockRandom()])
+        XCTAssertEqual(scopeMock.eventsWritten(ofType: EnrichedResource.self).count, 0)
+        XCTAssertTrue(scopeMock.telemetryMock.messages.isEmpty)
+    }
 
-        XCTAssertEqual(core.events(ofType: EnrichedResource.self).count, 0)
+    func test_whenDataStoreIsOlderThan30Days_itClearsKnownDuplicates() throws {
+        // Given
+        let knownIdentifiersData = Set(["2", "1"]).asData(JSONEncoder())!
+        scopeMock.dataStoreMock.setValue(knownIdentifiersData, forKey: ResourcesWriter.Constants.knownResourcesKey)
+        scopeMock.dataStoreMock.setValue(
+            (Date().timeIntervalSince1970 - 31.days).asData(),
+            forKey: ResourcesWriter.Constants.storeCreationKey
+        )
+
+        let writer = ResourcesWriter(scope: scopeMock)
+
+        // When
+        XCTAssertNil(scopeMock.dataStoreMock.value(forKey: ResourcesWriter.Constants.knownResourcesKey))
+        writer.write(resources: [.mockWith(identifier: "1")])
+
+        // Then
+        XCTAssertEqual(scopeMock.eventsWritten(ofType: EnrichedResource.self).count, 1)
+        XCTAssertEqual(
+            scopeMock.dataStoreMock.value(forKey: ResourcesWriter.Constants.knownResourcesKey)?.data(),
+            Set(["1"]).asData(JSONEncoder())
+        )
+        XCTAssertTrue(scopeMock.telemetryMock.messages.isEmpty)
+    }
+
+    func test_whenKnownResourcesAreBroken_itLogsTelemetry() {
+        // Given
+        let brokenData = "broken".data(using: .utf8)!
+        scopeMock.dataStoreMock.setValue(brokenData, forKey: ResourcesWriter.Constants.knownResourcesKey)
+
+        // When
+        _ = ResourcesWriter(scope: scopeMock)
+
+        // Then
+        XCTAssertTrue(scopeMock.telemetryMock.messages[0].asError?.message.contains("Failed to decode known identifiers - ") ?? false)
+    }
+
+    func test_whenDataStoreCreationIsBroken_itLogsTelemetry() {
+        // Given
+        let brokenData = "broken".data(using: .utf8)!
+        scopeMock.dataStoreMock.setValue(brokenData, forKey: ResourcesWriter.Constants.storeCreationKey)
+
+        // When
+        _ = ResourcesWriter(scope: scopeMock)
+
+        // Then
+        XCTAssertEqual(scopeMock.telemetryMock.messages[0].asError?.message, "Failed to decode store creation - invalidData")
     }
 }

TestUtilities/Mocks/CoreMocks/PassthroughCoreMock.swift

@@ -62,11 +62,13 @@ open class PassthroughCoreMock: DatadogCoreProtocol, FeatureScope {
 
     public required init(
         context: DatadogContext = .mockAny(),
+        dataStore: DataStore = NOPDataStore(),
         expectation: XCTestExpectation? = nil,
         bypassConsentExpectation: XCTestExpectation? = nil,
         messageReceiver: FeatureMessageReceiver = NOPFeatureMessageReceiver()
     ) {
         self.context = context
+        self.dataStore = dataStore
         self.expectation = expectation
         self.bypassConsentExpectation = bypassConsentExpectation
         self.messageReceiver = messageReceiver
@@ -116,7 +118,7 @@ open class PassthroughCoreMock: DatadogCoreProtocol, FeatureScope {
         block(context)
     }
 
-    public var dataStore: DataStore { NOPDataStore() }
+    public var dataStore: DataStore
 
     /// Recorded events from feature scopes.
     ///

TestUtilities/Mocks/CoreMocks/SingleFeatureCoreMock.swift

@@ -42,6 +42,7 @@ public final class SingleFeatureCoreMock<Feature>: PassthroughCoreMock where Fea
     ///                  is invoked with `bypassConsent` parameter set to `true`.
     public required init(
         context: DatadogContext = .mockAny(),
+        dataStore: DataStore = NOPDataStore(),
         feature: Feature? = nil,
         expectation: XCTestExpectation? = nil,
         bypassConsentExpectation: XCTestExpectation? = nil,
@@ -51,6 +52,7 @@ public final class SingleFeatureCoreMock<Feature>: PassthroughCoreMock where Fea
 
         super.init(
             context: context,
+            dataStore: dataStore,
             expectation: expectation,
             bypassConsentExpectation: bypassConsentExpectation,
             messageReceiver: messageReceiver
@@ -67,6 +69,7 @@ public final class SingleFeatureCoreMock<Feature>: PassthroughCoreMock where Fea
     ///                  is invoked with `bypassConsent` parameter set to `true`.
     public required init(
         context: DatadogContext = .mockAny(),
+        dataStore: DataStore = NOPDataStore(),
         expectation: XCTestExpectation? = nil,
         bypassConsentExpectation: XCTestExpectation? = nil,
         messageReceiver: FeatureMessageReceiver = NOPFeatureMessageReceiver()
@@ -75,6 +78,7 @@ public final class SingleFeatureCoreMock<Feature>: PassthroughCoreMock where Fea
 
         super.init(
             context: context,
+            dataStore: dataStore,
             expectation: expectation,
             bypassConsentExpectation: bypassConsentExpectation,
             messageReceiver: messageReceiver