feat: Add an accessor for the current session ID

[fuzzybinary]

What and why?

This adds a currentSessionID accessor onto the RUM Monitor. There are cases (usually support cases) where users want to be able to access the current session ID more easily than with the currently available "sessionStarted" notifications.

refs: RUM-1961

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: jward/RUM-1961-session-id
Commit report: 1abd42e
Test service: dd-sdk-ios

✅ 0 Failed, 2483 Passed, 0 Skipped, 4m 34.95s Wall Time
⬇️ Test Sessions change in coverage: 7 decreased, 6 increased

⬇️ Code Coverage Decreases vs Default Branch (7)

This report shows up to 5 code coverage decreases.

• test DatadogCrashReportingTests tvOS (-0.03%) - Details
• test DatadogInternalTests tvOS (-0.01%) - Details
• test DatadogTraceTests iOS (-0.01%) - Details
• test DatadogLogsTests tvOS (-0.01%) - Details
• test DatadogLogsTests iOS (-0.01%) - Details
• test DatadogWebViewTrackingTests iOS (-0.00%) - Details
• test DatadogCrashReportingTests iOS (-0.00%) - Details

[ncreated]

blocker/ This isn't thread safe and suffers from race condition.

• thread safety: scopes are mutated on monitor's queue so we need to sync the getter on that queue as well.
• race condition: scopes mutation is dispatched asynchronously on core's context queue (with eventWriteContext { ... }), so session ID is not yet resolved before process(command:) returns.

Both problems are not visible in tests this PR adds in MonitorTests, because it is using PassthroughCoreMock which is single-threaded and we end these tests by calling monitor.flush() to force-syncs monitor state. To surface these problems, try adding tests in RUMMonitorTests.swift where we use a real instance of the SDK core.

Solving thread-safety is simple as it may only require using queue.sync {}. The price would be on performance - it will block the caller thread for the duration of current RUM event processing.

Solving race condition would be more difficult, as it may require sync-ing on core context queue as well. This might be not a good idea, as it will block the caller thread for additional duration of pending core operations. For that reason there is no sync {} API provided by core and we may need to use semaphores to workaround it.

Compromise might be guaranteeing thread-safety but explicitly not promising sessionID integrity (in API comment). It would mean that following sequence might print the same sessionID twice:

print(monitor.currentSessionID)
monitor.stopSession()
monitor.startView(key: "Foo")
print(monitor.currentSessionID)

Alternatively, we can make this API asynchronous:

monitor.currentSessionID { sessionID in
   print(sessionID) // called asynchronously
}

and dispatch sessionID in the same model as we are processing RUM commands. This would be the correct solution, but it might be less convenient for users.

[fuzzybinary]

@ncreated Switched to the async callback. Let me know what you think.

[ncreated]

The implementation looks fine 👍, although we need a change in tests. Also, as we're adding user-facing API:

• please add CHANGELOG.md entry

[ncreated]

blocking/ This test is false positive. There is no such behaviour as "current session starts as nil" after #1594

One problem is that RUMMonitor.shared() is missing the core:

RUMMonitor.shared(in: core)

However, if we pass the core reference this test will fail because we use FeatureRegistrationCoreMock in this file and that only offers feature registration capability, see:

dd-sdk-ios/TestUtilities/Mocks/CoreMocks/FeatureRegistrationCoreMock.swift

Lines 9 to 23 in a43c63d

	/// Core mock that only allows registering and retrieving features.
	///
	/// Usage:
	///
	/// let core = FeatureRegistrationCoreMock()
	/// let feature = MyCustomFeature()
	///
	/// try core.register(feature: feature)
	///
	/// core.get(feature: MyCustomFeature.self) === feature // true
	/// core.get(feature: OtherFeature.self) // returns nil
	///
	/// **Note:** If you need different capabilities, check other available core mocks,
	/// before you consider adding it here.
	public class FeatureRegistrationCoreMock: DatadogCoreProtocol {

If we change it to PassthroughCoreMock as suggested, it will keep failing because that mock doesn't support get<T>(feature:) required in `RUMMonitor.shared(in:).

After all, I think the problem is because we try to test RUMMonitor in RUMTests.swift, which is only configured to test RUM.* configuration and no behaviours. My recommendation is to remove this test from here and cover this flow in RUMMonitorTests.swift, which depends on real instance of the core.

[fuzzybinary]

I think I'll just delete this test then, and add a test to RUMMonitorTests that checks that the session is nil after a call to stopSession.

Thoughts?

[ncreated]

Looks good 💪