From da2ed96833893a955b8f090b13e537d15d87b7e8 Mon Sep 17 00:00:00 2001 From: Maciek Grzybowski Date: Tue, 28 May 2024 10:51:15 +0200 Subject: [PATCH] RUM-1660 Add tests for Session Ended Metric Controller --- Datadog/Datadog.xcodeproj/project.pbxproj | 6 + .../SDKMetrics/SessionEndedMetric.swift | 10 +- .../SessionEndedMetricController.swift | 26 ++-- .../SessionEndedMetricControllerTests.swift | 119 ++++++++++++++++++ .../SDKMetrics/SessionEndedMetricTests.swift | 11 +- 5 files changed, 155 insertions(+), 17 deletions(-) create mode 100644 DatadogRUM/Tests/SDKMetrics/SessionEndedMetricControllerTests.swift diff --git a/Datadog/Datadog.xcodeproj/project.pbxproj b/Datadog/Datadog.xcodeproj/project.pbxproj index 6284fc7cc3..6eb6137a52 100644 --- a/Datadog/Datadog.xcodeproj/project.pbxproj +++ b/Datadog/Datadog.xcodeproj/project.pbxproj @@ -592,6 +592,8 @@ 61DA8CB828647A500074A606 /* InternalLoggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61DA8CB728647A500074A606 /* InternalLoggerTests.swift */; }; 61DA8CB928647A500074A606 /* InternalLoggerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61DA8CB728647A500074A606 /* InternalLoggerTests.swift */; }; 61DB33B225DEDFC200F7EA71 /* CustomObjcViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 61DB33B125DEDFC200F7EA71 /* CustomObjcViewController.m */; }; + 61DCC8472C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61DCC8462C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift */; }; + 61DCC8482C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61DCC8462C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift */; }; 61E45BE724519A3700F2C652 /* JSONDataMatcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61E45BE624519A3700F2C652 /* JSONDataMatcher.swift */; }; 61E45ED12451A8730061DAC7 /* SpanMatcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61E45ED02451A8730061DAC7 /* SpanMatcher.swift */; }; 61E5333824B84EE2003D6C4E /* DebugRUMViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 61E5333724B84EE2003D6C4E /* DebugRUMViewController.swift */; }; @@ -2582,6 +2584,7 @@ 61DA8CB728647A500074A606 /* InternalLoggerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InternalLoggerTests.swift; sourceTree = ""; }; 61DB33B025DEDFC200F7EA71 /* CustomObjcViewController.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CustomObjcViewController.h; sourceTree = ""; }; 61DB33B125DEDFC200F7EA71 /* CustomObjcViewController.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = CustomObjcViewController.m; sourceTree = ""; }; + 61DCC8462C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionEndedMetricControllerTests.swift; sourceTree = ""; }; 61DE333525C8278A008E3EC2 /* CrashReportingPlugin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CrashReportingPlugin.swift; sourceTree = ""; }; 61E45BCE2450A6EC00F2C652 /* TraceIDTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TraceIDTests.swift; sourceTree = ""; }; 61E45BD12450F65B00F2C652 /* SpanEventBuilderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpanEventBuilderTests.swift; sourceTree = ""; }; @@ -4832,6 +4835,7 @@ isa = PBXGroup; children = ( 6174D6192BFE449300EC7469 /* SessionEndedMetricTests.swift */, + 61DCC8462C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift */, ); path = SDKMetrics; sourceTree = ""; @@ -8652,6 +8656,7 @@ buildActionMask = 2147483647; files = ( 6188697D2A4376F700E8996B /* RUMConfigurationTests.swift in Sources */, + 61DCC8482C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift in Sources */, D23F8EA029DDCD38001CFAE8 /* RUMOffViewEventsHandlingRuleTests.swift in Sources */, D23F8EA229DDCD38001CFAE8 /* RUMSessionScopeTests.swift in Sources */, D23F8EA329DDCD38001CFAE8 /* RUMUserActionScopeTests.swift in Sources */, @@ -8967,6 +8972,7 @@ buildActionMask = 2147483647; files = ( 6188697C2A4376F700E8996B /* RUMConfigurationTests.swift in Sources */, + 61DCC8472C05CD0000CB59E5 /* SessionEndedMetricControllerTests.swift in Sources */, D29A9FA629DDB483005C54A4 /* RUMOffViewEventsHandlingRuleTests.swift in Sources */, D29A9FBD29DDB483005C54A4 /* RUMSessionScopeTests.swift in Sources */, D29A9FAB29DDB483005C54A4 /* RUMUserActionScopeTests.swift in Sources */, diff --git a/DatadogRUM/Sources/SDKMetrics/SessionEndedMetric.swift b/DatadogRUM/Sources/SDKMetrics/SessionEndedMetric.swift index 66b276f950..aec4077bd4 100644 --- a/DatadogRUM/Sources/SDKMetrics/SessionEndedMetric.swift +++ b/DatadogRUM/Sources/SDKMetrics/SessionEndedMetric.swift @@ -18,12 +18,10 @@ internal final class SessionEndedMetric { static let typeValue = "rum session ended" /// Namespace for bundling metric attributes ("rse" = "RUM Session Ended"). static let rseKey = "rse" - /// Key referencing the session ID (`String`) that the metric refers to. - static let sessionIDKey = "sessionID" } /// An ID of the session being tracked through this metric object. - private let sessionID: String + let sessionID: String /// The type of OS component where the session was tracked. private let bundleType: BundleType @@ -128,7 +126,7 @@ internal final class SessionEndedMetric { /// /// Note: We don't expect it to ever become `nil`, but optionality is enforced in upstream code. let precondition: String? - /// The session's duration, calculated from view events. + /// The session's duration (in nanoseconds), calculated from view events. /// /// This calculation only includes view events that are written to disk, with no consideration if the I/O operation /// has succeeded or not. Views dropped through the mapper API are not included in this duration. @@ -196,12 +194,12 @@ internal final class SessionEndedMetric { let appLaunchViewsCount = trackedViews.values.filter({ $0.viewURL == RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL }).count // Compute SDK errors count - let totalSDKErrors = trackedSDKErrors.count + let totalSDKErrors = trackedSDKErrors.values.reduce(0, +) let top5SDKErrorsByKind = top5SDKErrorsByKind(from: trackedSDKErrors) return [ SDKMetricFields.typeKey: Constants.typeValue, - Constants.sessionIDKey: sessionID, + SDKMetricFields.sessionIDOverrideKey: sessionID, Constants.rseKey: Attributes( processType: { switch bundleType { diff --git a/DatadogRUM/Sources/SDKMetrics/SessionEndedMetricController.swift b/DatadogRUM/Sources/SDKMetrics/SessionEndedMetricController.swift index 0bdd7051ee..65bc7edde0 100644 --- a/DatadogRUM/Sources/SDKMetrics/SessionEndedMetricController.swift +++ b/DatadogRUM/Sources/SDKMetrics/SessionEndedMetricController.swift @@ -11,7 +11,10 @@ import DatadogInternal internal final class SessionEndedMetricController { /// Dictionary to keep track of pending metrics, keyed by session ID. @ReadWriteLock - private var pendingMetrics: [String: SessionEndedMetric] = [:] + private var metricsBySessionID: [String: SessionEndedMetric] = [:] + /// Array to keep track of pending metrics in their start order. + @ReadWriteLock + private var metrics: [SessionEndedMetric] = [] /// Telemetry endpoint for sending metrics. private let telemetry: Telemetry @@ -28,26 +31,35 @@ internal final class SessionEndedMetricController { /// - precondition: The precondition that led to starting this session. /// - context: The SDK context at the moment of starting this session. /// - Returns: The newly created `SessionEndedMetric` instance. - func startMetric(sessionID: String, precondition: RUMSessionPrecondition?, context: DatadogContext) -> SessionEndedMetric { + func startMetric(sessionID: String, precondition: RUMSessionPrecondition?, context: DatadogContext) { + guard sessionID != RUMUUID.nullUUID.toRUMDataFormat else { + return // do not track metric when session is not sampled + } let metric = SessionEndedMetric(sessionID: sessionID, precondition: precondition, context: context) - pendingMetrics[sessionID] = metric - return metric + metricsBySessionID[sessionID] = metric + metrics.append(metric) } /// Retrieves the metric for a given session ID. /// - Parameter sessionID: The ID of the session to retrieve the metric for. /// - Returns: The `SessionEndedMetric` instance if found, otherwise `nil`. func metric(for sessionID: String) -> SessionEndedMetric? { - return pendingMetrics[sessionID] + return metricsBySessionID[sessionID] + } + + /// Retrieves the last started metric. + var latestMetric: SessionEndedMetric? { + return metrics.last } /// Ends the metric for a given session, sending it to telemetry and removing it from pending metrics. /// - Parameter sessionID: The ID of the session to end the metric for. func endMetric(sessionID: String) { - guard let metric = pendingMetrics[sessionID] else { + guard let metric = metricsBySessionID[sessionID] else { return } telemetry.metric(name: SessionEndedMetric.Constants.name, attributes: metric.asMetricAttributes()) - pendingMetrics[sessionID] = nil + metricsBySessionID[sessionID] = nil + metrics.removeAll(where: { $0 === metric }) // O(n), but "ending the metric" is very rare event } } diff --git a/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricControllerTests.swift b/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricControllerTests.swift new file mode 100644 index 0000000000..57c7bb1952 --- /dev/null +++ b/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricControllerTests.swift @@ -0,0 +1,119 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2019-Present Datadog, Inc. + */ + +import XCTest +import TestUtilities +import DatadogInternal +@testable import DatadogRUM + +class SessionEndedMetricControllerTests: XCTestCase { + private let telemetry = TelemetryMock() + + func testWhenMetricIsStarted_itCanBeRetrievedByID() throws { + let controller = SessionEndedMetricController(telemetry: telemetry) + + // When + let sessionID1: String = .mockRandom() + let sessionID2: String = .mockRandom() + controller.startMetric(sessionID: sessionID1, precondition: .mockRandom(), context: .mockRandom()) + controller.startMetric(sessionID: sessionID2, precondition: .mockRandom(), context: .mockRandom()) + + // Then + let metric1 = try XCTUnwrap(controller.metric(for: sessionID1)) + let metric2 = try XCTUnwrap(controller.metric(for: sessionID2)) + XCTAssertEqual(metric1.sessionID, sessionID1) + XCTAssertEqual(metric2.sessionID, sessionID2) + } + + func testWhenMetricIsStarted_itCanBeRetrievedAsLatest() throws { + let controller = SessionEndedMetricController(telemetry: telemetry) + + // When + let sessionID1: String = .mockRandom() + let sessionID2: String = .mockRandom() + controller.startMetric(sessionID: sessionID1, precondition: .mockRandom(), context: .mockRandom()) + controller.startMetric(sessionID: sessionID2, precondition: .mockRandom(), context: .mockRandom()) + + // Then + XCTAssertEqual(controller.latestMetric?.sessionID, sessionID2) + controller.endMetric(sessionID: sessionID2) + XCTAssertEqual(controller.latestMetric?.sessionID, sessionID1) + controller.endMetric(sessionID: sessionID1) + XCTAssertNil(controller.latestMetric) + } + + func testWhenMetricIsEnded_itIsSentToTelemetry() throws { + let sessionID: String = .mockRandom() + let controller = SessionEndedMetricController(telemetry: telemetry) + controller.startMetric(sessionID: sessionID, precondition: .mockRandom(), context: .mockRandom()) + + // When + controller.endMetric(sessionID: sessionID) + + // Then + let metric = try XCTUnwrap(telemetry.messages.firstMetric(named: SessionEndedMetric.Constants.name)) + XCTAssertEqual(metric.attributes[SDKMetricFields.typeKey] as? String, SessionEndedMetric.Constants.typeValue) + XCTAssertEqual(metric.attributes[SDKMetricFields.sessionIDOverrideKey] as? String, sessionID) + } + + func testAfterMetricIsEnded_itCanNoLongerBeRetrieved() throws { + let sessionID: String = .mockRandom() + let controller = SessionEndedMetricController(telemetry: telemetry) + controller.startMetric(sessionID: sessionID, precondition: .mockRandom(), context: .mockRandom()) + + // When + XCTAssertNotNil(controller.metric(for: sessionID)) + controller.endMetric(sessionID: sessionID) + + // Then + XCTAssertNil(controller.metric(for: sessionID)) + XCTAssertNil(controller.latestMetric) + } + + func testWhenSessionIsSampled_itDoesNotTrackMetric() throws { + let controller = SessionEndedMetricController(telemetry: telemetry) + + // When + let rejectedSessionID = RUMUUID.nullUUID.toRUMDataFormat + controller.startMetric(sessionID: rejectedSessionID, precondition: .mockRandom(), context: .mockRandom()) + + // Then + XCTAssertNil(controller.metric(for: rejectedSessionID)) + XCTAssertNil(controller.latestMetric) + + controller.endMetric(sessionID: rejectedSessionID) + XCTAssertTrue(telemetry.messages.isEmpty) + } + + // MARK: - Thread Safety + + func testTrackingSessionEndedMetricIsThreadSafe() { + let sessionIDs: [String] = .mockRandom(count: 10) + let controller = SessionEndedMetricController(telemetry: telemetry) + + // swiftlint:disable opening_brace + callConcurrently( + closures: [ + { controller.startMetric( + sessionID: sessionIDs.randomElement()!, precondition: .mockRandom(), context: .mockRandom() + ) }, + { _ = controller.metric(for: sessionIDs.randomElement()!) }, + { + _ = controller.metric(for: sessionIDs.randomElement()!)?.track(view: .mockRandom()) + }, + { + _ = controller.metric(for: sessionIDs.randomElement()!)?.track(sdkErrorKind: .mockRandom()) + }, + { + _ = controller.metric(for: sessionIDs.randomElement()!)?.trackWasStopped() + }, + { controller.endMetric(sessionID: sessionIDs.randomElement()!) }, + ], + iterations: 100 + ) + // swiftlint:enable opening_brace + } +} diff --git a/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricTests.swift b/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricTests.swift index a719e0e199..1988097129 100644 --- a/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricTests.swift +++ b/DatadogRUM/Tests/SDKMetrics/SessionEndedMetricTests.swift @@ -54,7 +54,7 @@ class SessionEndedMetricTests: XCTestCase { let attributes = metric.asMetricAttributes() // Then - XCTAssertEqual(attributes[Constants.sessionIDKey] as? String, sessionID) + XCTAssertEqual(attributes[SDKMetricFields.sessionIDOverrideKey] as? String, sessionID) } // MARK: - Process Type @@ -286,18 +286,21 @@ class SessionEndedMetricTests: XCTestCase { // MARK: - SDK Errors Count func testReportingTotalSDKErrorsCount() throws { - let errorKinds: [String] = .mockRandom(count: .mockRandom(min: 0, max: 50)) + let errorKinds: [String] = .mockRandom(count: .mockRandom(min: 0, max: 10)) + let repetitions = 5 // Given let metric = SessionEndedMetric(sessionID: sessionID, precondition: .mockRandom(), context: .mockRandom()) // When - errorKinds.forEach { metric.track(sdkErrorKind: $0) } + (0..