impr(profiling): always remove launch profile config after starting (#5318)

Author: armcknight

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Improvements
 
 - Removed `APPLICATION_EXTENSION_API_ONLY` requirement (#5524)
+- Improve launch profile configuration management (#5318)
 
 ## 8.53.1
 

Samples/iOS-Swift/iOS-Swift-UITests/BaseUITest.swift

@@ -7,7 +7,7 @@ class BaseUITest: XCTestCase {
     //swiftlint:disable implicit_getter
     var automaticallyLaunchAndTerminateApp: Bool { get { true } }
     //swiftlint:enable implicit_getter
-    
+
     override func setUp() {
         super.setUp()
         continueAfterFailure = false
@@ -37,7 +37,7 @@ extension BaseUITest {
         return app
     }
 
-    func launchApp(args: [String] = [], env: [String: String] = [:]) {
+    func launchApp(args: [String] = [], env: [String: String] = [:], activateBeforeLaunch: Bool = true) {
         app.launchArguments.append(contentsOf: args)
         for (k, v) in env {
             app.launchEnvironment[k] = v
@@ -46,10 +46,17 @@ extension BaseUITest {
         // Calling activate() and then launch() effectively launches the app twice, interfering with
         // local debugging. Only call activate if there isn't a debugger attached, which is a decent
         // proxy for whether this is running in CI.
-        if !isDebugging() {
+        if !isDebugging() && activateBeforeLaunch {
+            // activate() appears to drop launch args and environment variables, so save them beforehand and reset them before subsequent calls to launch()
+            let launchArguments = app.launchArguments
+            let launchEnvironment = app.launchEnvironment
+
             // App prewarming can sometimes cause simulators to get stuck in UI tests, activating them
             // before launching clears any prewarming state.
             app.activate()
+
+            app.launchArguments = launchArguments
+            app.launchEnvironment = launchEnvironment
         }
 
         app.launch()

Samples/iOS-Swift/iOS-Swift-UITests/ProfilingUITests.swift

@@ -1,56 +1,41 @@
+@testable import Sentry
 import XCTest
 
 //swiftlint:disable function_body_length todo
 
 class ProfilingUITests: BaseUITest {
     override var automaticallyLaunchAndTerminateApp: Bool { false }
-    
+
     func testAppLaunchesWithTraceProfiler() throws {
         guard #available(iOS 16, *) else {
             throw XCTSkip("Only run for latest iOS version we test; we've had issues with prior versions in SauceLabs")
         }
 
-        // by default, launch profiling is not enabled
-        try launchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false)
-        
-        // after configuring for launch profiling, check the marker file exists, and that the profile happens
-        try launchAndConfigureSubsequentLaunches(terminatePriorSession: true, shouldProfileThisLaunch: true)
+        try performTest(profileType: .trace)
     }
 
     func testAppLaunchesWithContinuousProfilerV1() throws {
         guard #available(iOS 16, *) else {
             throw XCTSkip("Only run for latest iOS version we test; we've had issues with prior versions in SauceLabs")
         }
 
-        // by default, launch profiling is not enabled
-        try launchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, continuousProfiling: true)
-        
-        // after configuring for launch profiling, check the marker file exists, and that the profile happens
-        try launchAndConfigureSubsequentLaunches(terminatePriorSession: true, shouldProfileThisLaunch: true, continuousProfiling: true)
+        try performTest(profileType: .continuous)
     }
 
     func testAppLaunchesWithContinuousProfilerV2TraceLifecycle() throws {
         guard #available(iOS 16, *) else {
             throw XCTSkip("Only run for latest iOS version we test; we've had issues with prior versions in SauceLabs")
         }
 
-        // by default, launch profiling is not enabled
-        try launchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, continuousProfiling: true, v2TraceLifecycle: true)
-
-        // after configuring for launch profiling, check the marker file exists, and that the profile happens
-        try launchAndConfigureSubsequentLaunches(terminatePriorSession: true, shouldProfileThisLaunch: true, continuousProfiling: true, v2TraceLifecycle: true)
+        try performTest(profileType: .ui, lifecycle: .trace)
     }
 
     func testAppLaunchesWithContinuousProfilerV2ManualLifeCycle() throws {
         guard #available(iOS 16, *) else {
             throw XCTSkip("Only run for latest iOS version we test; we've had issues with prior versions in SauceLabs")
         }
 
-        // by default, launch profiling is not enabled
-        try launchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, continuousProfiling: true, v2ManualLifecycle: true)
-
-        // after configuring for launch profiling, check the marker file exists, and that the profile happens
-        try launchAndConfigureSubsequentLaunches(terminatePriorSession: true, shouldProfileThisLaunch: true, continuousProfiling: true, v2ManualLifecycle: true)
+        try performTest(profileType: .ui, lifecycle: .manual)
     }
 
     /**
@@ -144,72 +129,104 @@ extension ProfilingUITests {
     func stopContinuousProfiler() {
         app.buttons["io.sentry.ios-swift.ui-test.button.stop-continuous-profiler"].afterWaitingForExistence("Couldn't find button to stop continuous profiler").tap()
     }
-     
+
+    enum ProfilingType {
+        case trace
+        case continuous // aka "continuous beta"
+        case ui // aka "continuous v2"
+    }
+
+    func performTest(profileType: ProfilingType, lifecycle: SentryProfileOptions.SentryProfileLifecycle? = nil) throws {
+        try launchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, shouldProfileNextLaunch: true, profileType: profileType, lifecycle: lifecycle)
+        try launchAndConfigureSubsequentLaunches(terminatePriorSession: true, shouldProfileThisLaunch: true, shouldProfileNextLaunch: false, profileType: profileType, lifecycle: lifecycle)
+    }
+
+    fileprivate func setAppLaunchParameters(_ profileType: ProfilingUITests.ProfilingType, _ lifecycle: SentryProfileOptions.SentryProfileLifecycle?, _ shouldProfileNextLaunch: Bool) {
+        app.launchArguments.append(contentsOf: [
+            // these help avoid other profiles that'd be taken automatically, that interfere with the checking we do for the assertions later in the tests
+            "--disable-swizzling",
+            "--disable-auto-performance-tracing",
+            "--disable-uiviewcontroller-tracing",
+            
+            // sets a marker function to run in a load command that the launch profile should detect
+            "--io.sentry.slow-load-method",
+            
+            // override full chunk completion before stoppage introduced in https://github.com/getsentry/sentry-cocoa/pull/4214
+            "--io.sentry.continuous-profiler-immediate-stop"
+        ])
+        
+        switch profileType {
+        case .ui:
+            app.launchEnvironment["--io.sentry.profile-session-sample-rate"] = "1"
+            switch lifecycle {
+            case .none:
+                fatalError("Misconfigured test case. Must provide a lifecycle for UI profiling.")
+            case .trace:
+                break
+            case .manual:
+                app.launchArguments.append("--io.sentry.profile-lifecycle-manual")
+            }
+        case .continuous:
+            app.launchArguments.append("--io.sentry.disable-ui-profiling")
+        case .trace:
+            app.launchEnvironment["--io.sentry.profilesSampleRate"] = "1"
+        }
+        
+        if !shouldProfileNextLaunch {
+            app.launchArguments.append("--io.sentry.disable-app-start-profiling")
+        }
+    }
+    
     /**
      * Performs the various operations for the launch profiler test case:
      * - terminates an existing app session
-     * - creates a new one
+     * - starts a new app session
      * - sets launch args and env vars to set the appropriate `SentryOption` values for the desired behavior
-     * - launches the new configured app session
+     * - launches the new configured app session, which will optionally start a launch profiler and then call SentrySDK.startWithOptions configured based on the launch args and env vars
      * - asserts the expected outcomes of the config file and launch profiler
      */
     func launchAndConfigureSubsequentLaunches(
         terminatePriorSession: Bool = false,
         shouldProfileThisLaunch: Bool,
-        continuousProfiling: Bool = false,
-        v2TraceLifecycle: Bool = false,
-        v2ManualLifecycle: Bool = false
+        shouldProfileNextLaunch: Bool,
+        profileType: ProfilingType,
+        lifecycle: SentryProfileOptions.SentryProfileLifecycle?
     ) throws {
         if terminatePriorSession {
             app.terminate()
             app = newAppSession()
         }
 
-        app.launchArguments.append(contentsOf: [
-            // these help avoid other profiles that'd be taken automatically, that interfere with the checking we do for the assertions later in the tests
-            "--disable-swizzling",
-            "--disable-auto-performance-tracing",
-            "--disable-uiviewcontroller-tracing",
+        setAppLaunchParameters(profileType, lifecycle, shouldProfileNextLaunch)
 
-            // sets a marker function to run in a load command that the launch profile should detect
-            "--io.sentry.slow-load-method",
+        launchApp(activateBeforeLaunch: false)
+        goToProfiling()
 
-            // override full chunk completion before stoppage introduced in https://github.com/getsentry/sentry-cocoa/pull/4214
-            "--io.sentry.continuous-profiler-immediate-stop"
-        ])
+        let configFileExists = try checkLaunchProfileMarkerFileExistence()
 
-        if continuousProfiling {
-            if v2TraceLifecycle {
-                app.launchEnvironment["--io.sentry.profile-session-sample-rate"] = "1"
-            } else if v2ManualLifecycle {
-                app.launchArguments.append(contentsOf: [
-                    "--io.sentry.profile-lifecycle-manual"
-                ])
-                app.launchEnvironment["--io.sentry.profile-session-sample-rate"] = "1"
-            } else {
-                app.launchArguments.append("--io.sentry.disable-ui-profiling")
-            }
+        if shouldProfileNextLaunch {
+            XCTAssert(configFileExists, "A launch profile config file should be present on disk if SentrySDK.startWithOptions configured launch profiling for the next launch.")
         } else {
-            app.launchEnvironment["--io.sentry.profilesSampleRate"] = "1"
+            XCTAssertFalse(configFileExists, "Launch profile config files should be removed upon starting launch profiles. If SentrySDK.startWithOptions doesn't reconfigure launch profiling, the config file should not be present.")
         }
 
-        launchApp()
-        goToProfiling()
-        XCTAssert(try checkLaunchProfileMarkerFileExistence())
-
         guard shouldProfileThisLaunch else {
             return
         }
-        
-        if continuousProfiling {
-            if !v2TraceLifecycle {
+
+        if profileType == .trace {
+            retrieveLastProfileData()
+        } else {
+            if profileType == .continuous || (profileType == .ui && lifecycle == .manual) {
                 stopContinuousProfiler()
             }
             retrieveFirstProfileChunkData()
-        } else {
-            retrieveLastProfileData()
         }
-        
+
+        try assertProfileContents()
+    }
+
+    func assertProfileContents() throws {
         let lastProfile = try marshalJSONDictionaryFromApp()
         let sampledProfile = try XCTUnwrap(lastProfile["profile"] as? [String: Any])
         let stacks = try XCTUnwrap(sampledProfile["stacks"] as? [[Int]])
@@ -219,7 +236,7 @@ extension ProfilingUITests {
                 frames[stackFrame]["function"]
             }
         })
-        
+
         // grab the first stack that contained frames from the fixture code that simulates a slow +[load] method
         var stackID: Int?
         let stack = try XCTUnwrap(stackFunctions.enumerated().first { nextStack in
@@ -238,12 +255,12 @@ extension ProfilingUITests {
             XCTFail("Didn't find the ID of the stack containing the target function")
             return
         }
-        
+
         // ensure that the stack doesn't contain any calls to main functions; this ensures we actually captured pre-main stacks
         XCTAssertFalse(stack.contains("main"))
         XCTAssertFalse(stack.contains("UIApplicationMain"))
         XCTAssertFalse(stack.contains("-[UIApplication _run]"))
-        
+
         // ensure that the stack happened on the main thread; this is a cross-check to make sure we didn't accidentally grab a stack from a different thread that wouldn't have had a call to main() anyways, thereby possibly missing the real stack that may have contained main() calls (but shouldn't for this test)
         let samples = try XCTUnwrap(sampledProfile["samples"] as? [[String: Any]])
         let sample = try XCTUnwrap(samples.first { nextSample in

Samples/iOS-Swift/iOS-Swift/AppDelegate.swift

@@ -7,12 +7,17 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
     var window: UIWindow?
 
     var args: [String] {
-        let args = ProcessInfo.processInfo.arguments
-        print("[iOS-Swift] [debug] launch arguments: \(args)")
-        return args
+        ProcessInfo.processInfo.arguments
+    }
+    
+    var env: [String: String] {
+        ProcessInfo.processInfo.environment
     }
 
     func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
+        print("[iOS-Swift] [debug] launch arguments: \(args)")
+        print("[iOS-Swift] [debug] launch environment: \(env)")
+
         if args.contains("--io.sentry.wipe-data") {
             removeAppData()
         }

Samples/iOS-Swift/iOS-Swift/Profiling/ProfilingViewController.swift

@@ -75,14 +75,14 @@ class ProfilingViewController: UIViewController, UITextFieldDelegate {
 
     @IBAction func defineProfilesSampleRateToggled(_ sender: UISwitch) {
         sampleRateField.isEnabled = sender.isOn
-        
+
         var sampleRate = SentrySDKOverrides.Profiling.sampleRate
         sampleRate.floatValue = getSampleRateOverride(field: sampleRateField)
     }
 
     @IBAction func defineTracesSampleRateToggled(_ sender: UISwitch) {
         tracesSampleRateField.isEnabled = sender.isOn
-        
+
         var sampleRate = SentrySDKOverrides.Tracing.sampleRate
         sampleRate.floatValue = getSampleRateOverride(field: tracesSampleRateField)
     }
@@ -109,7 +109,16 @@ private extension ProfilingViewController {
         let cachesDirectory = NSSearchPathForDirectoriesInDomains(.cachesDirectory, .userDomainMask, true).first!
         let fm = FileManager.default
         let dir = "\(cachesDirectory)/io.sentry/" + (continuous ? "continuous-profiles" : "trace-profiles")
-        let count = try! fm.contentsOfDirectory(atPath: dir).count
+
+        let count: Int
+        do {
+            count = try fm.contentsOfDirectory(atPath: dir).count
+        } catch {
+            print("[iOS-Swift] [debug] [ProfilingViewController] error reading directory \(dir): \(error)")
+            profilingUITestDataMarshalingStatus.text = "<error>"
+            return
+        }
+
         //swiftlint:disable empty_count
         guard continuous || count > 0 else {
             //swiftlint:enable empty_count
@@ -118,7 +127,7 @@ private extension ProfilingViewController {
         }
         let fileName = "profile\(continuous ? 0 : count - 1)"
         let fullPath = "\(dir)/\(fileName)"
-        
+
         if fm.fileExists(atPath: fullPath) {
             let url = NSURL.fileURL(withPath: fullPath)
             block(url)
@@ -129,17 +138,17 @@ private extension ProfilingViewController {
             }
             return
         }
-        
+
         block(nil)
     }
-    
+
     func handleContents(file: URL?) {
         guard let file = file else {
             profilingUITestDataMarshalingTextField.text = "<missing>"
             profilingUITestDataMarshalingStatus.text = "❌"
             return
         }
-        
+
         do {
             let data = try Data(contentsOf: file)
             let contents = data.base64EncodedString()