RUM-17879 Merge session-activity reader into active-context reader and drop collector's session-scope dependency

Author: barboraplasovska

DatadogRUM/Sources/Feature/RUMFeature.swift

@@ -225,7 +225,6 @@ internal final class RUMFeature: DatadogRemoteFeature, RUMSessionSamplerProvider
         )
 
         timeseriesCollector?.activeContextReader = monitor
-        timeseriesCollector?.sessionActivityReader = monitor
         self.timeseriesCollector = timeseriesCollector
 
         if let refreshRateVital = dependencies.vitalsReaders?.refreshRate as? RenderLoopReader {

DatadogRUM/Sources/RUMMonitor/Monitor.swift

@@ -98,22 +98,20 @@ internal typealias RUMErrorCategory = RUMErrorEvent.Error.Category
 
 /// Exposes monitor state for readers that operate outside the `RUMCommand` pipeline
 /// (e.g. the timer-driven `TimeseriesSessionCollector`), which otherwise have no access to
-/// `command.globalAttributes` or the scope tree's active view.
+/// `command.globalAttributes`, the scope tree's active view, or `RUMSessionScope`'s own session
+/// lifetime rules.
 internal protocol RUMActiveContextReader: AnyObject {
     /// The current global attributes set through `addAttribute(forKey:value:)` / `addAttributes(_:)`.
-    /// Safe to read from any thread.
+    /// Conformers must guarantee this is safe to read from any thread.
     var globalAttributes: [AttributeKey: AttributeValue] { get }
-    /// The currently active view, if any. Safe to read from any thread.
+    /// The currently active view, if any. Conformers must guarantee this is safe to read from any thread.
     var activeView: (id: String?, path: String?, name: String?) { get }
-}
-
-/// Exposes the active session's lifetime state for readers that operate outside the `RUMCommand` pipeline
-/// (e.g. the timer-driven `TimeseriesSessionCollector`), so they can evaluate `RUMSessionScope`'s own
-/// expiry rules against a live, single source of truth instead of maintaining a shadow copy of that state.
-internal protocol RUMSessionActivityReader: AnyObject {
-    /// The active session's ID, start time, and time of last RUM interaction, or `nil` values if there is
-    /// no active session. Safe to read from any thread.
-    var sessionActivity: (sessionID: String?, sessionStartTime: Date?, lastInteractionTime: Date?) { get }
+    /// Whether the session identified by `sessionID` has expired (exceeded its max duration or inactivity
+    /// timeout) as of `date`, evaluated against a live, single source of truth instead of a shadow copy of
+    /// that state. Returns `false` if `sessionID` doesn't match the currently active session (e.g. a session
+    /// transition is still propagating), so callers should treat that as "skip the check for now", not
+    /// "not expired". Conformers must guarantee this is safe to call from any thread.
+    func isSessionExpired(sessionID: String, at date: Date) -> Bool
 }
 
 internal class Monitor: RUMCommandSubscriber {
@@ -239,10 +237,17 @@ internal class Monitor: RUMCommandSubscriber {
 extension Monitor: RUMActiveContextReader {
     var globalAttributes: [AttributeKey: AttributeValue] { attributes }
     var activeView: (id: String?, path: String?, name: String?) { activeViewSnapshot }
-}
 
-extension Monitor: RUMSessionActivityReader {
-    var sessionActivity: (sessionID: String?, sessionStartTime: Date?, lastInteractionTime: Date?) { sessionActivitySnapshot }
+    func isSessionExpired(sessionID: String, at date: Date) -> Bool {
+        let activity = sessionActivitySnapshot
+        guard activity.sessionID == sessionID,
+              let sessionStartTime = activity.sessionStartTime,
+              let lastInteractionTime = activity.lastInteractionTime else {
+            return false
+        }
+        return RUMSessionScope.hasExpired(sessionStartTime: sessionStartTime, currentTime: date)
+            || RUMSessionScope.hasTimedOut(lastInteractionTime: lastInteractionTime, currentTime: date)
+    }
 }
 
 /// Declares `Monitor` conformance to public `RUMMonitorProtocol`.

DatadogRUM/Sources/RUMMonitor/Scopes/RUMSessionScope.swift

@@ -16,15 +16,11 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
     }
 
     /// Whether a session is timed out due to inactivity, given the time of its last interaction.
-    /// Shared with `TimeseriesSessionCollector`, which self-enforces this same rule from a live
-    /// `RUMSessionActivityReader` snapshot instead of reacting to `RUMCommand`s.
     static func hasTimedOut(lastInteractionTime: Date, currentTime: Date) -> Bool {
         currentTime.timeIntervalSince(lastInteractionTime) >= Constants.sessionTimeoutDuration
     }
 
     /// Whether a session has exceeded its maximum duration, given its start time.
-    /// Shared with `TimeseriesSessionCollector`, which self-enforces this same rule from a live
-    /// `RUMSessionActivityReader` snapshot instead of reacting to `RUMCommand`s.
     static func hasExpired(sessionStartTime: Date, currentTime: Date) -> Bool {
         currentTime.timeIntervalSince(sessionStartTime) >= Constants.sessionMaxDuration
     }
@@ -109,10 +105,8 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider {
     /// If this is the very first session created in the current app process (`false` for session created upon expiration of a previous one).
     let isInitialSession: Bool
     /// The start time of this Session, measured in device date. In initial session this is the time of SDK init.
-    /// Exposed internally (not `private`) so `Monitor` can snapshot it for `RUMSessionActivityReader`.
     let sessionStartTime: Date
     /// Time of the last RUM interaction noticed by this Session.
-    /// Exposed internally (not `private`) so `Monitor` can snapshot it for `RUMSessionActivityReader`.
     private(set) var lastInteractionTime: Date
     /// Indicates whether the "ApplicationLaunch" view was active when the app entered the background.
     private var hadApplicationLaunchViewWhenEnteringBackground: Bool? = nil

DatadogRUM/Sources/Timeseries/TimeseriesSessionCollector.swift

@@ -9,13 +9,11 @@ import DatadogInternal
 
 /// Defines the interface for collecting timeseries data during a RUM session.
 internal protocol TimeseriesCollecting: AnyObject {
-    /// Provides global custom attributes and the active view at sample time. Set by `RUMFeature` once `Monitor`
-    /// is constructed, since the collector is created before it.
+    /// Provides global custom attributes, the active view, and the active session's expiry state at sample
+    /// time, so the collector can self-enforce `RUMSessionScope`'s own expiry rules without maintaining a
+    /// shadow copy of that state. Set by `RUMFeature` once `Monitor` is constructed, since the collector is
+    /// created before it.
     var activeContextReader: RUMActiveContextReader? { get set }
-    /// Provides the active session's start time and last-interaction time at sample time, so the collector can
-    /// self-enforce `RUMSessionScope`'s own expiry rules without maintaining a shadow copy of that state.
-    /// Set by `RUMFeature` once `Monitor` is constructed, since the collector is created before it.
-    var sessionActivityReader: RUMSessionActivityReader? { get set }
     func start(sessionID: String, applicationID: String, sessionType: RUMSessionType)
     func pause(sessionID: String)
     func resume(sessionID: String)
@@ -61,8 +59,6 @@ internal class TimeseriesSessionCollector: TimeseriesCollecting {
 
     /// `Monitor`'s conformance is safe to read from any thread.
     weak var activeContextReader: RUMActiveContextReader?
-    /// `Monitor`'s conformance is safe to read from any thread.
-    weak var sessionActivityReader: RUMSessionActivityReader?
 
     private var memoryBuffer: [MemorySample] = []
     private var cpuBuffer: [CPUSample] = []
@@ -258,15 +254,9 @@ internal class TimeseriesSessionCollector: TimeseriesCollecting {
         // idle with no user interaction). This is a safety net only — it does not affect RUM's own
         // session state, it just stops this collector from uploading data past session expiry.
         //
-        // Pulled fresh from `sessionActivityReader` on every tick, rather than from a locally pushed
+        // Pulled fresh from `activeContextReader` on every tick, rather than from a locally pushed
         // copy, so there's a single live source of truth and no race with how/when that state is updated.
-        // If the reader's session doesn't match (e.g. a session transition is still propagating), skip
-        // the check for this tick rather than guessing.
-        let activity = sessionActivityReader?.sessionActivity
-        if let activity = activity, activity.sessionID == sessionID,
-           let sessionStartTime = activity.sessionStartTime, let lastInteractionTime = activity.lastInteractionTime,
-           RUMSessionScope.hasExpired(sessionStartTime: sessionStartTime, currentTime: currentDate)
-            || RUMSessionScope.hasTimedOut(lastInteractionTime: lastInteractionTime, currentTime: currentDate) {
+        if activeContextReader?.isSessionExpired(sessionID: sessionID, at: currentDate) == true {
             timer?.cancel()
             timer = nil
             flushMemory()

DatadogRUM/Tests/RUMMonitor/Scopes/RUMSessionScopeTests.swift

@@ -867,7 +867,6 @@ class RUMSessionScopeTests: XCTestCase {
 
 private class TimeseriesCollectorSpy: TimeseriesCollecting {
     weak var activeContextReader: RUMActiveContextReader?
-    weak var sessionActivityReader: RUMSessionActivityReader?
     var startCallCount = 0
     var pauseCallCount = 0
     var resumeCallCount = 0

DatadogRUM/Tests/Timeseries/TimeseriesSessionCollectorTests.swift

@@ -820,12 +820,9 @@ class TimeseriesSessionCollectorTests: XCTestCase {
             totalRAM: 4_000_000_000,
             now: clock.now
         )
-        let contextReader = RUMActiveContextReaderMock()
-        collector.activeContextReader = contextReader
-
         let startTime = clock.date
-        let activityReader = RUMSessionActivityReaderMock(sessionID: "session-expired", sessionStartTime: startTime, lastInteractionTime: startTime)
-        collector.sessionActivityReader = activityReader
+        let contextReader = RUMActiveContextReaderMock(sessionID: "session-expired", sessionStartTime: startTime, lastInteractionTime: startTime)
+        collector.activeContextReader = contextReader
         collector.start(sessionID: "session-expired", applicationID: "app-1", sessionType: .user)
 
         let beforeExpiryExpectation = self.expectation(description: "samples collected before expiry")
@@ -838,7 +835,7 @@ class TimeseriesSessionCollectorTests: XCTestCase {
         // duration, mirroring how `Monitor` refreshes its snapshot on every processed command, without ever
         // calling stop() directly
         clock.date = startTime.addingTimeInterval(RUMSessionScope.Constants.sessionMaxDuration)
-        activityReader.sessionActivity.lastInteractionTime = clock.date
+        contextReader.sessionActivity.lastInteractionTime = clock.date
         let selfStopExpectation = self.expectation(description: "self-stop settled")
         DispatchQueue.global(qos: .utility).asyncAfter(deadline: .now() + 0.3) { selfStopExpectation.fulfill() }
         waitForExpectations(timeout: 2)
@@ -868,12 +865,9 @@ class TimeseriesSessionCollectorTests: XCTestCase {
             totalRAM: 4_000_000_000,
             now: clock.now
         )
-        let contextReader = RUMActiveContextReaderMock()
-        collector.activeContextReader = contextReader
-
         let startTime = clock.date
-        let activityReader = RUMSessionActivityReaderMock(sessionID: "session-idle", sessionStartTime: startTime, lastInteractionTime: startTime)
-        collector.sessionActivityReader = activityReader
+        let contextReader = RUMActiveContextReaderMock(sessionID: "session-idle", sessionStartTime: startTime, lastInteractionTime: startTime)
+        collector.activeContextReader = contextReader
         collector.start(sessionID: "session-idle", applicationID: "app-1", sessionType: .user)
 
         let beforeTimeoutExpectation = self.expectation(description: "samples collected before timeout")
@@ -906,18 +900,15 @@ class TimeseriesSessionCollectorTests: XCTestCase {
             totalRAM: 4_000_000_000,
             now: clock.now
         )
-        let contextReader = RUMActiveContextReaderMock()
-        collector.activeContextReader = contextReader
-
         let startTime = clock.date
-        let activityReader = RUMSessionActivityReaderMock(sessionID: "session-active", sessionStartTime: startTime, lastInteractionTime: startTime)
-        collector.sessionActivityReader = activityReader
+        let contextReader = RUMActiveContextReaderMock(sessionID: "session-active", sessionStartTime: startTime, lastInteractionTime: startTime)
+        collector.activeContextReader = contextReader
         collector.start(sessionID: "session-active", applicationID: "app-1", sessionType: .user)
 
         // When — a RUM interaction is reported right before what would have been the inactivity timeout,
         // resetting the clock the reader exposes to `sample()`
         clock.date = startTime.addingTimeInterval(RUMSessionScope.Constants.sessionTimeoutDuration - 1)
-        activityReader.sessionActivity.lastInteractionTime = clock.date
+        contextReader.sessionActivity.lastInteractionTime = clock.date
         clock.date = clock.date.addingTimeInterval(RUMSessionScope.Constants.sessionTimeoutDuration - 1)
 
         let expectation = self.expectation(description: "still sampling")
@@ -998,21 +989,28 @@ class TimeseriesSessionCollectorTests: XCTestCase {
 private class RUMActiveContextReaderMock: RUMActiveContextReader {
     var globalAttributes: [AttributeKey: AttributeValue]
     var activeView: (id: String?, path: String?, name: String?)
+    var sessionActivity: (sessionID: String?, sessionStartTime: Date?, lastInteractionTime: Date?)
 
     init(
         globalAttributes: [AttributeKey: AttributeValue] = [:],
-        activeView: (id: String?, path: String?, name: String?) = (.mockAny(), .mockAny(), nil)
+        activeView: (id: String?, path: String?, name: String?) = (.mockAny(), .mockAny(), nil),
+        sessionID: String? = nil,
+        sessionStartTime: Date? = nil,
+        lastInteractionTime: Date? = nil
     ) {
         self.globalAttributes = globalAttributes
         self.activeView = activeView
+        self.sessionActivity = (sessionID, sessionStartTime, lastInteractionTime)
     }
-}
 
-private class RUMSessionActivityReaderMock: RUMSessionActivityReader {
-    var sessionActivity: (sessionID: String?, sessionStartTime: Date?, lastInteractionTime: Date?)
-
-    init(sessionID: String? = nil, sessionStartTime: Date? = nil, lastInteractionTime: Date? = nil) {
-        self.sessionActivity = (sessionID, sessionStartTime, lastInteractionTime)
+    func isSessionExpired(sessionID: String, at date: Date) -> Bool {
+        guard sessionActivity.sessionID == sessionID,
+              let sessionStartTime = sessionActivity.sessionStartTime,
+              let lastInteractionTime = sessionActivity.lastInteractionTime else {
+            return false
+        }
+        return RUMSessionScope.hasExpired(sessionStartTime: sessionStartTime, currentTime: date)
+            || RUMSessionScope.hasTimedOut(lastInteractionTime: lastInteractionTime, currentTime: date)
     }
 }