RUM-3462 feat: Introduce DataStore

[ncreated]

What and why?

📦 This PR introduces a DataStore interface designed to store key-value pairs for a given feature. The primary goal is to provide a mechanism for persisting data between app launches.

For the full list of use cases and requirements considered in this implementation, see RFC - Local Datastore (internal).

How?

New APIs

DataStore interface:

public protocol DataStore {
    func setValue(_ value: Data, forKey key: String, version: DataStoreKeyVersion = 0) {}
    func value(forKey key: String, callback: @escaping (DataStoreValueResult) -> Void) {}
    func removeValue(forKey key: String) {}
}

public enum DataStoreValueResult {
    case value(Data, DataStoreKeyVersion)
    case noValue
    case error(Error)
}

public typealias DataStoreKeyVersion = UInt16

DataStoreKeyVersion (an UInt16): Values are persisted in files and SDK upgrades may occur between app launches. To ensure proper data reading, the caller can specify the version of expected data on read. This way it can handle the situation when data was written in old format, so it must skip decoding it in the new format.

The DataStore is specific for a feature. It can be obtained through FeatureScope interface:

public extension FeatureScope {
   func dataStore() -> DataStore
   func dataStoreContext(_ block: @escaping (DatadogContext, DataStore) -> Void) {
}

This comes in-line with Event Write Context API that already exists on the FeatureScope:

public protocol FeatureScope {
   func context(_ block: @escaping (DatadogContext) -> Void)
   func eventWriteContext(_ block: @escaping (DatadogContext, Writer) -> Void) {
}

TLV format

Data is encoded in TLV format, leveraging prior work on batch files and recent refinements in #1725.

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration)
• Make sure each commit and the PR mention the Issue number or JIRA reference
• Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

• Run unit tests for Core, RUM, Trace, Logs, CR and WVT
• Run unit tests for Session Replay
• Run integration tests
• Run smoke tests
• Run tests for tools/

[datadog-datadog-prod-us1]

Datadog Report

Branch report: ncreated/RUM-3462/introduce-core-data-store
Commit report: b7948b2
Test service: dd-sdk-ios

✅ 0 Failed, 2903 Passed, 0 Skipped, 12m 29.43s Wall Time
🔻 Test Sessions change in coverage: 8 decreased, 5 increased, 1 no change

🔻 Code Coverage Decreases vs Default Branch (8)

This report shows up to 5 code coverage decreases.

• test DatadogTraceTests tvOS 49.35% (-0.39%) - Details
• test DatadogRUMTests iOS 79.43% (-0.38%) - Details
• test DatadogTraceTests iOS 49.32% (-0.33%) - Details
• test DatadogRUMTests tvOS 79.64% (-0.27%) - Details
• test DatadogCrashReportingTests tvOS 28.69% (-0.22%) - Details

[maciejburda]

Wow! Really nice piece 💪

Great tests, nice docs. I like how it's exposed to features 🎖️

One thing I would consider around naming is dropping Core to avoid having CoreData. It might confuse some developers.

[maciejburda]

nice!

[ncreated]

Alternatively we can change the semantic to make it always return Data (if found) with performing an optional check on expectedVersion (if provided) as in following code. I have no strong opinion.

public func data(expectedVersion: DataStoreKeyVersion? = nil) -> Data? {
    guard case .value(let data, let storedVersion) = self else {
        return nil
    }
    if let expectedVersion == expectedVersion, storedVersion != expectedVersion {
        return nil
    }

    return data
}

[maciejburda]

not sure if version expectation should be built in.

I would assume that the consumer of the api will handle different model version migration

[ncreated]

The version is now made a pure convenience. It is fully hidden in the basic usage:

store.setValue(data, forKey: "key")
store.value(forKey: "key") { result in
   _ = result.data()
}

If only versioning problem occurs, we can leverage it:

store.setValue(data_in_new_version, forKey: "key", version: 2)
store.value(forKey: "key") { result in
   _ = result.data(expectedVersion: 2)
}

I think this is the best balance between having this support and keeping a clean API. Requiring the caller to handle versioning will cause unnecessary repetitive code infection across different places.

[ncreated]

Requiring the caller to handle versioning will cause unnecessary repetitive code infection across different places.

Also, if version is not encoded for every data upfront, we won't be able to ever support migration from "no version" to "version: 2". The decoding failure will become ambiguous and (if not filtered somehow) will flood telemetry with false-positives.

[ncreated]

The presence of DataStore in FeatureScope interface limits its usage to only DatadogRemoteFeatures (the ones that can write and read batch files). In consequence, it won't be available in following features:

• CrashReportingFeature
• NetworkInstrumentationFeature
• BacktraceReportingFeature

While last two are rather fine, there might be use cases for DataStore in Crash Reporting. I don't foresee any at this point, hence I don't consider it blocking.

Even if DataStore is required for CrashReportingFeature at some point, I rather thing we should work on the FeatureScope interface rather than re-design the DataStore management. I'm open for discussion 🙂.

[maxep]

I rather thing we should work on the FeatureScope interface rather than re-design the DataStore management.

Totally 👍

/suggestion can we use a var instead?

[maxep]

It looks great, well done!!

I have left minor suggestions but it's good to go 👍

[maxep]

I rather thing we should work on the FeatureScope interface rather than re-design the DataStore management.

Totally 👍

/suggestion can we use a var instead?

[maciejburda]

Nice! Thanks for taking the extra effort and improving the API 💪

[maxep]

Nice!