RUM-3462 refactor: Make FeatureScope always available

[ncreated]

What and why?

📦 This PR straightens the FeatureScope interface, proposing it to become the main (and in most cases the only) dependency for features implementation. Instead of spreading the core reference across features and managing it in a weak manner, the implementation should take a strong dependency on FeatureScope.

There are 3 category of motivations behind this change:

• Short-term goal: Necessary for fatal App Hangs monitoring. This change is to ensure DataStore can be available in AppHangsObserver before RUMFeature is initialized.
• Mid-term goal: We want features implementation to take dependency on FeatureScope rather than managing the DatadogCoreProtocol reference by themselves. As this PR proves, manual management of the core reference is error-prone and may sometimes lead to memory leaks due to capturing strong ref that surpasses core's lifecycle. With FeatureScope abstracting the weak reference we will enforce proper and safe implementation in all features 📈.
• Long-term goal: By iteratively replacing DatadogCoreProtocol dependency with FeatureScope, feature unit tests will be vastly simplified and become properly segregated. Features will no longer be able to make side-effects on core in unit tests, hence tests of one feature expecting side-effects in another feature will have to be moved to the integration-unit target.

How?

Changed the feature scope retrieval API:

- func scope(for feature: String) -> FeatureScope?
+ func scope<Feature>(for featureType: Feature.Type) -> FeatureScope

• Only the scope of a known feature type can be requested. It means RUM can request its own scope, but it has no access to Logs or Session Replay scope.
• The scope is available right away, even before the feature's registration completed in core. It means the FeatureScope interface can be injected all across the feature implementation.
• The FeatureScope manages weak var core: DatadogCoreProtocol? reference. It is no longer required for feature implementation to deal with core's ref safety.

The FeatureScope interface was extended with all capabilities that feature needs to interact with the core. That includes baggage sharing, message sending (over MB) and an access to the telemetry endpoint:

For backward-compatibility and to avoid large refactoring, all FeatureScope capabilities are still present in DatadogCoreProtocol in this PR:

This is done by adding two composition protocols: MessageSending and BaggageSharing in DatadogInternal for sharing these aspects between core and scope:

protocol DatadogCoreProtocol: MessageSending, BaggageSharing
protocol FeatureScope: MessageSending, BaggageSharing

After all features are migrated to depend on FeatureScope the conformance to both protocols will be removed from DatadogCoreProtocol.

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/

[maxep]

Is it possible to provide a new instance each time?

[ncreated]

FeatureDataStore has class semantic, so it sounds reasonable to cache it. Also, I assume we will use it this way:

scope.dataStore.setValue(...)
scope.dataStore.value(...)
scope.dataStore.remove(...)

so the instance will be crated each time.

Alternatively we can change FeatureDataStore to be struct, but that would mean creating unnecessary copies each time, no?

[ncreated]

⚠️ Removing "strong core ref" antipattern

[ncreated]

⚠️ Removing "strong core ref" antipattern

[datadog-datadog-prod-us1]

Datadog Report

Branch report: ncreated/RUM-3462/refactor-feature-scope-availability
Commit report: 831c88b
Test service: dd-sdk-ios

✅ 0 Failed, 2944 Passed, 0 Skipped, 27m 44.74s Wall Time
🔻 Test Sessions change in coverage: 13 decreased, 2 increased

🔻 Code Coverage Decreases vs Default Branch (13)

This report shows up to 5 code coverage decreases.

• test DatadogInternalTests iOS 79.13% (-0.62%) - Details
• test DatadogInternalTests tvOS 79.1% (-0.61%) - Details
• test DatadogTraceTests tvOS 49.3% (-0.44%) - Details
• test DatadogTraceTests iOS 49.21% (-0.42%) - Details
• test DatadogLogsTests iOS 45.52% (-0.41%) - Details

[maciejburda]

Looks good!

I love how the introduction of the data store led to this refactor 💪

[maxep]

Looks great, very nice improvement!

But I don't see the need for MessageSending and BaggageSharing 🤔 backward-compatible apis only need to stay in DatadogCoreProtocol, unless I'm missing something?

[maciejburda]

LGTM

[ncreated]

Looks great, very nice improvement!

But I don't see the need for MessageSending and BaggageSharing 🤔 backward-compatible apis only need to stay in DatadogCoreProtocol, unless I'm missing something?

@maxep I only introduced MessageSending and BaggageSharing to avoid duplicating this and that code in FeatureScope interface. Instead, I solved it by protocols composition to keep the change small. Ultimately we want these APIs being only part of FeatureScope interface and not DatadogCoreProtocol, as shown on the "After" picture in PR description.

Alternatively, I can copy & paste the code into FeatureScope interface without introducing MessageSending and BaggageSharing, but I don't see a reason for doing so. The migration may take a while, so any code or API comment change would have to be maintained twice. WDYT?

[maxep]

@ncreated Ah yes, protocol extension was the piece I missed 👍 If we have a ticket to tackle the debt, lets add a note to remove the protocols. Thanks for the clarification!

[maxep]

👍