Skip to content

Commit

Permalink
test(app launch profiling): integration test fixes and additions (#3605)
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight authored Feb 8, 2024
1 parent 97de0c4 commit f3c04cd
Show file tree
Hide file tree
Showing 11 changed files with 377 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ProfilingViewController: UIViewController, UITextFieldDelegate {
let value = SentryBenchmarking.stopBenchmark()!
valueTextField.isHidden = false
valueTextField.text = value
print("[iOS-Swift] [Profiling] benchmarking results:\n\(value)")
print("[iOS-Swift] [ProfilingViewController] benchmarking results:\n\(value)")
}

@IBAction func startCPUWork(_ sender: UIButton) {
Expand Down Expand Up @@ -130,6 +130,7 @@ extension ProfilingViewController {
let url = file as! URL
if url.absoluteString.contains(fileName) {
block(url)
print("[iOS-Swift] [ProfilingViewController] removing file at \(url)")
try! FileManager.default.removeItem(at: url)
return
}
Expand All @@ -149,7 +150,7 @@ extension ProfilingViewController {
return
}
let contents = data.base64EncodedString()
print("[iOS-Swift] [Profiling] contents of file at \(file): \(String(data: data, encoding: .utf8))")
print("[iOS-Swift] [ProfilingViewController] contents of file at \(file): \(String(describing: String(data: data, encoding: .utf8)))")
profilingUITestDataMarshalingTextField.text = contents
profilingUITestDataMarshalingStatus.text = ""
}
Expand Down
34 changes: 25 additions & 9 deletions Samples/iOS-Swift/iOS-SwiftUITests/ProfilingUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class ProfilingUITests: BaseUITest {
}

func testProfiledAppLaunches() throws {
app.launchArguments.append("--io.sentry.wipe-data")
launchApp()

// First launch enables in-app profiling by setting traces/profiles sample rates to 1 (which is the default configuration in the sample app), but not launch profiling; assert that we did not write a config to allow the next launch to be profiled
Expand All @@ -22,12 +23,12 @@ class ProfilingUITests: BaseUITest {
try relaunchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, shouldEnableLaunchProfilingOptionForNextLaunch: true)

// this launch should run the profiler, then set the option to allow launch profiling to true, but set the numerical sample rates to 0 so that the next launch should not profile
try relaunchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, shouldEnableLaunchProfilingOptionForNextLaunch: true, profilesSampleRate: 0, tracesSampleRate: 0)
try relaunchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: true, shouldEnableLaunchProfilingOptionForNextLaunch: true, profilesSampleRate: 0, tracesSampleRate: 0)

Check failure on line 26 in Samples/iOS-Swift/iOS-SwiftUITests/ProfilingUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer

testProfiledAppLaunches, failed: caught error: "missingFile"

Check failure on line 26 in Samples/iOS-Swift/iOS-SwiftUITests/ProfilingUITests.swift

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer

testProfiledAppLaunches, failed: caught error: "missingFile"

// this launch should not run the profiler; configure sampler functions returning 1 and numerical rates set to 0, which should result in a profile being taken as samplers override numerical rates
try relaunchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, shouldEnableLaunchProfilingOptionForNextLaunch: true, profilesSampleRate: 0, tracesSampleRate: 0, profilesSamplerValue: 1, tracesSamplerValue: 1)

// this launch has the configuration to run the profiler, but because swizzling is disabled, it will not run due to the ui.load transaction not being allowed to be created but configure it not to run the next launch due to disabling swizzling, which would override the option to enable launch profiling
// this launch has the configuration to run the profiler, but because swizzling is disabled, it will not be saved due to the ui.load transaction not being allowed to be created. it will also configure it to not run a profile for the next launch due to disabling swizzling, which would override the option to enable launch profiling. this specific scenario, where a previous launch configures a profile, but then something prevents the associated tx from running, is not automatically avoidable. in the future we will create a dummy transaction to attach the profile to.
try relaunchAndConfigureSubsequentLaunches(shouldProfileThisLaunch: false, shouldEnableLaunchProfilingOptionForNextLaunch: true, shouldDisableSwizzling: true)

// this launch should not run the profiler and configure it not to run the next launch due to disabling automatic performance tracking, which would override the option to enable launch profiling
Expand Down Expand Up @@ -132,31 +133,46 @@ extension ProfilingUITests {
func assertLaunchProfile() throws {
retrieveLaunchProfileData()

var lastProfile = try marshalJSONDictionaryFromApp()
let lastProfile = try marshalJSONDictionaryFromApp()
let sampledProfile = try XCTUnwrap(lastProfile["profile"] as? [String: Any])
let stacks = try XCTUnwrap(sampledProfile["stacks"] as? [[Int]])
let frames = try XCTUnwrap(sampledProfile["frames"] as? [[String: Any]])
let functions = stacks.map({ stack in
let stackFunctions = stacks.map({ stack in
stack.map { stackFrame in
frames[stackFrame]["function"]
}
})
let stack = try XCTUnwrap(functions.first { nextStack in
let result = nextStack.contains { frame in

// 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
let result = nextStack.element.contains { frame in
let result = (frame as! String).contains("+[SentryProfiler(SlowLoad) load]")
if result {
stackID = nextStack.offset
}
return result
}
return result
}).map { any in
}).element.map { any in
try XCTUnwrap(any as? String)
}
guard stackID != nil else {
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]"))

// ???: can we assert that this stack was on the main thread?
// TODO: yes! need to correlate the samples.[].stack_id with samples.[].thread_id
// 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
try XCTUnwrap(nextSample["stack_id"] as? NSNumber).intValue == stackID
})
XCTAssert(try XCTUnwrap(sample["thread_id"] as? String) == "259") // the main thread is always ID 259
}

/**
Expand Down
2 changes: 0 additions & 2 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,6 @@
8431EFE229B27BAD00D8DC56 /* SentryNSTimerFactoryTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 849472842971C41A002603DE /* SentryNSTimerFactoryTest.swift */; };
8431EFE529B27BAD00D8DC56 /* SentryNSProcessInfoWrapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 849472822971C2CD002603DE /* SentryNSProcessInfoWrapperTests.swift */; };
8431EFE829B27BAD00D8DC56 /* SentrySystemWrapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 849472802971C107002603DE /* SentrySystemWrapperTests.swift */; };
8431F00529B2849A00D8DC56 /* (null) in Sources */ = {isa = PBXBuildFile; };
8431F01529B2851500D8DC56 /* TestSentryNSTimerFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 844EDCE72947DCD700C86F34 /* TestSentryNSTimerFactory.swift */; };
8431F01629B2851500D8DC56 /* TestSentryNSProcessInfoWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 844EDC712941442200C86F34 /* TestSentryNSProcessInfoWrapper.swift */; };
8431F01729B2851500D8DC56 /* TestSentrySystemWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 844EDC7829415AB300C86F34 /* TestSentrySystemWrapper.swift */; };
Expand Down Expand Up @@ -4519,7 +4518,6 @@
0A2D7BBA29152CBF008727AF /* SentryWatchdogTerminationsScopeObserverTests.swift in Sources */,
7BD4BD4B27EB2DC20071F4FF /* SentryDiscardedEventTests.swift in Sources */,
63FE721A20DA66EC00CDBAE8 /* SentryCrashSysCtl_Tests.m in Sources */,
8431F00529B2849A00D8DC56 /* (null) in Sources */,
7B88F30424BC8E6500ADF90A /* SentrySerializationTests.swift in Sources */,
7B34721728086A9D0041F047 /* SentrySwizzleWrapperTests.swift in Sources */,
8EC4CF5025C3A0070093DEE9 /* SentrySpanContextTests.swift in Sources */,
Expand Down
11 changes: 8 additions & 3 deletions Sources/Sentry/Profiling/SentryLaunchProfiling.m
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
SentryId *_Nullable appLaunchTraceId;
NSObject *appLaunchTraceLock;
uint64_t appLaunchSystemTime;
static NSString *const kSentryLaunchProfileConfigKeyTracesSampleRate = @"traces";
static NSString *const kSentryLaunchProfileConfigKeyProfilesSampleRate = @"profiles";
NSString *const kSentryLaunchProfileConfigKeyTracesSampleRate = @"traces";
NSString *const kSentryLaunchProfileConfigKeyProfilesSampleRate = @"profiles";

# pragma mark - Private

Expand Down Expand Up @@ -101,7 +101,11 @@
= config.profilesDecision.sampleRate;
writeAppLaunchProfilingConfigFile(configDict);
} else {
removeAppLaunchProfilingConfigFile();
if (isTracingAppLaunch) {
backupAppLaunchProfilingConfigFile();
} else {
removeAppLaunchProfilingConfigFile();
}
}
}];
}
Expand Down Expand Up @@ -142,6 +146,7 @@
SentryTransactionContext *transactionContext, SentryTracerConfiguration *configuration)
{
NSDictionary<NSString *, NSNumber *> *rates = appLaunchProfileConfiguration();
removeAppLaunchProfilingConfigBackupFile();
NSNumber *profilesRate = rates[kSentryLaunchProfileConfigKeyProfilesSampleRate];
NSNumber *tracesRate = rates[kSentryLaunchProfileConfigKeyTracesSampleRate];
if (profilesRate == nil || tracesRate == nil) {
Expand Down
81 changes: 64 additions & 17 deletions Sources/Sentry/SentryFileManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,28 @@
return YES;
}

/**
* @warning This is called from a `@synchronized` context in instance methods, but doesn't require
* that when calling from other static functions. Make sure you pay attention to where this is used
* from.
*/
void
_non_thread_safe_removeFileAtPath(NSString *path)
{
NSError *error = nil;
NSFileManager *fileManager = [NSFileManager defaultManager];
if (![fileManager removeItemAtPath:path error:&error]) {
if (error.code == NSFileNoSuchFileError) {
SENTRY_LOG_DEBUG(@"No file to delete at %@", path);
} else {
SENTRY_LOG_ERROR(
@"Error occurred while deleting file at %@ because of %@", path, error);
}
} else {
SENTRY_LOG_DEBUG(@"Successfully deleted file at %@", path);
}
}

@interface
SentryFileManager ()

Expand Down Expand Up @@ -284,19 +306,8 @@ - (void)deleteAllEnvelopes

- (void)removeFileAtPath:(NSString *)path
{
NSFileManager *fileManager = [NSFileManager defaultManager];
NSError *error = nil;
@synchronized(self) {
if (![fileManager removeItemAtPath:path error:&error]) {
if (error.code == NSFileNoSuchFileError) {
SENTRY_LOG_DEBUG(@"No file to delete at %@", path);
} else {
SENTRY_LOG_ERROR(
@"Error occurred while deleting file at %@ because of %@", path, error);
}
} else {
SENTRY_LOG_DEBUG(@"Successfully deleted file at %@", path);
}
_non_thread_safe_removeFileAtPath(path);
}
}

Expand Down Expand Up @@ -751,11 +762,30 @@ - (void)createPathsWithOptions:(SentryOptions *)options
return sentryLaunchConfigFileURL;
}

NSURL *
launchProfileConfigBackupFileURL(void)
{
static NSURL *sentryLaunchConfigFileURL;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
sentryLaunchConfigFileURL =
[[NSURL fileURLWithPath:[sentryApplicationSupportPath()
stringByAppendingPathComponent:@"profileLaunch"]]
URLByAppendingPathExtension:@"bak"];
});
return sentryLaunchConfigFileURL;
}

NSDictionary<NSString *, NSNumber *> *_Nullable appLaunchProfileConfiguration(void)
{
NSURL *url = launchProfileConfigBackupFileURL();
if (![[NSFileManager defaultManager] fileExistsAtPath:url.path]) {
return nil;
}

NSError *error;
NSDictionary<NSString *, NSNumber *> *config = [NSDictionary<NSString *, NSNumber *>
dictionaryWithContentsOfURL:launchProfileConfigFileURL()
dictionaryWithContentsOfURL:launchProfileConfigBackupFileURL()
error:&error];
SENTRY_CASSERT(
error == nil, @"Encountered error trying to retrieve app launch profile config: %@", error);
Expand All @@ -778,16 +808,33 @@ - (void)createPathsWithOptions:(SentryOptions *)options

void
removeAppLaunchProfilingConfigFile(void)
{
_non_thread_safe_removeFileAtPath(launchProfileConfigFileURL().path);
}

void
removeAppLaunchProfilingConfigBackupFile(void)
{
_non_thread_safe_removeFileAtPath(launchProfileConfigBackupFileURL().path);
}

void
backupAppLaunchProfilingConfigFile(void)
{
NSFileManager *fm = [NSFileManager defaultManager];
NSString *path = launchProfileConfigFileURL().path;
if (![fm fileExistsAtPath:path]) {
NSString *fromPath = launchProfileConfigFileURL().path;
if (!SENTRY_CASSERT_RETURN([fm fileExistsAtPath:fromPath],
@"Expect to have a current launch profile config to use for subsequent transaction.")) {
return;
}

NSString *toPath = launchProfileConfigBackupFileURL().path;
_non_thread_safe_removeFileAtPath(toPath);

NSError *error;
SENTRY_CASSERT([fm removeItemAtPath:path error:&error],
@"Failed to remove launch profile marker file: %@", error);
SENTRY_CASSERT([fm moveItemAtPath:fromPath toPath:toPath error:&error],
@"Failed to backup launch profile config file for use to set up associated transaction: %@",
error);
}
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

Expand Down
15 changes: 15 additions & 0 deletions Sources/Sentry/include/SentryFileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ SENTRY_EXTERN void writeAppLaunchProfilingConfigFile(
* start the profiler.
*/
SENTRY_EXTERN void removeAppLaunchProfilingConfigFile(void);

/**
* Save a current launch profile config file for use when the associated transaction needs to be
* started. This is when checking @c SentryOptions for whether to write a config for the next
* launch; if there's no current app launch profile going, then we'd simply remove whatever config
* file may exist, but if there is a launch profile going, we need to save the config that it used.
*/
SENTRY_EXTERN void backupAppLaunchProfilingConfigFile(void);

/**
* Clean up the backup file saved off in the case that an app launch is in-flight when the SDK
* configures subsequent launches not to profile, requiring the current config to be saved for the
* associated transaction. This can be cleaned up when that transaction is started.
*/
SENTRY_EXTERN void removeAppLaunchProfilingConfigBackupFile(void);
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

@end
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/include/SentryLaunchProfiling.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ SENTRY_EXTERN NSObject *appLaunchTraceLock;

void startLaunchProfile(void);

/*
/**
* Write a file to disk containing sample rates for profiles and traces. The presence of this file
* will let the profiler know to start on the app launch, and the sample rates contained will help
* thread sampling decisions through to SentryHub later when it needs to start a transaction for the
Expand Down
3 changes: 3 additions & 0 deletions Tests/SentryTests/Helper/SentryFileManager+Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

NS_ASSUME_NONNULL_BEGIN

SENTRY_EXTERN NSURL *launchProfileConfigFileURL(void);
SENTRY_EXTERN NSURL *launchProfileConfigBackupFileURL(void);

@interface
SentryFileManager ()

Expand Down
Loading

0 comments on commit f3c04cd

Please sign in to comment.