Skip to content

Commit

Permalink
fix a couple bugs in launch profiling state mgmt, and get all tests p…
Browse files Browse the repository at this point in the history
…assing again
  • Loading branch information
armcknight committed May 15, 2024
1 parent a8c1f54 commit a24c529
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 29 deletions.
20 changes: 11 additions & 9 deletions Samples/iOS-Swift/iOS-Swift-UITests/ProfilingUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class ProfilingUITests: BaseUITest {
func testProfiledAppLaunches() throws {
if #available(iOS 16, *) {
app.launchArguments.append("--io.sentry.wipe-data")
setDefaultLaunchArgs()
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 Down Expand Up @@ -162,6 +163,15 @@ extension ProfilingUITests {
})
XCTAssert(try XCTUnwrap(sample["thread_id"] as? String) == "259") // the main thread is always ID 259
}

/**
* These cause traces to run the profiler, which can overwrite the launch profile file we need to retrieve to make assertions in the UI test.
*/
func setDefaultLaunchArgs() {
app.launchArguments.append("--disable-swizzling")
app.launchArguments.append("--disable-auto-performance-tracing")
app.launchArguments.append("--disable-uiviewcontroller-tracing")
}

/**
* Performs the various operations for the launch profiler test case:
Expand Down Expand Up @@ -216,16 +226,8 @@ extension ProfilingUITests {
if shouldDisableTracing {
app.launchArguments.append("--disable-tracing")
}
if shouldDisableSwizzling {
app.launchArguments.append("--disable-swizzling")
}
if shouldDisableAutoPerformanceTracking {
app.launchArguments.append("--disable-auto-performance-tracing")
}
if shouldDisableUIViewControllerTracing {
app.launchArguments.append("--disable-uiviewcontroller-tracing")
}

setDefaultLaunchArgs()
launchApp()

let sdkOptionsConfigurationAllowsLaunchProfiling = !(shouldDisableTracing || shouldDisableSwizzling || shouldDisableAutoPerformanceTracking || shouldDisableUIViewControllerTracing)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class ProfilingViewController: UIViewController, UITextFieldDelegate {

@IBAction func viewLaunchProfile(_ sender: Any) {
profilingUITestDataMarshalingTextField.text = "<fetching...>"
withProfile(fileName: "launchProfile") { file in
withProfile(fileName: "profile") { file in
handleContents(file: file)
}
}
Expand Down
12 changes: 6 additions & 6 deletions Sources/Sentry/Profiling/SentryLaunchProfiling.m
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

NS_ASSUME_NONNULL_BEGIN

BOOL isProfilingAppLaunch;
NSString *const kSentryLaunchProfileConfigKeyTracesSampleRate = @"traces";
NSString *const kSentryLaunchProfileConfigKeyProfilesSampleRate = @"profiles";
NSString *const kSentryLaunchProfileConfigKeyContinuousProfiling = @"continuous-profiling";
Expand Down Expand Up @@ -119,8 +118,7 @@
void
_sentry_nondeduplicated_startLaunchProfile(void)
{
sentry_isTracingAppLaunch = appLaunchProfileConfigFileExists();
if (!sentry_isTracingAppLaunch) {
if (!appLaunchProfileConfigFileExists()) {
return;
}

Expand All @@ -140,18 +138,19 @@
NSNumber *profilesRate = launchConfig[kSentryLaunchProfileConfigKeyProfilesSampleRate];
if (profilesRate == nil) {
SENTRY_LOG_DEBUG(@"Received a nil configured launch profile sample rate, will not "
@"start continuous profiler for launch.");
@"start trace profiler for launch.");
return;

Check warning on line 142 in Sources/Sentry/Profiling/SentryLaunchProfiling.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/Profiling/SentryLaunchProfiling.m#L141-L142

Added lines #L141 - L142 were not covered by tests
}

NSNumber *tracesRate = launchConfig[kSentryLaunchProfileConfigKeyTracesSampleRate];
if (tracesRate == nil) {
SENTRY_LOG_DEBUG(@"Received a nil configured launch trace sample rate, will not start "
@"a profiled launch trace.");
@"trace profiler for launch.");
return;

Check warning on line 149 in Sources/Sentry/Profiling/SentryLaunchProfiling.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/Profiling/SentryLaunchProfiling.m#L148-L149

Added lines #L148 - L149 were not covered by tests
}

SENTRY_LOG_INFO(@"Starting app launch profile at %llu.", getAbsoluteTime());
SENTRY_LOG_INFO(@"Starting app launch trace profile at %llu.", getAbsoluteTime());
sentry_isTracingAppLaunch = YES;
sentry_launchTracer =
[[SentryTracer alloc] initWithTransactionContext:sentry_context(tracesRate)
hub:nil
Expand Down Expand Up @@ -213,6 +212,7 @@
{
SENTRY_LOG_DEBUG(@"Finishing launch tracer.");
[sentry_launchTracer finish];
sentry_isTracingAppLaunch = NO;
}

NS_ASSUME_NONNULL_END
Expand Down
17 changes: 4 additions & 13 deletions Sources/Sentry/Profiling/SentryProfilerTestHelpers.m
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,15 @@
SENTRY_LOG_DEBUG(@"App support directory already exists.");
}

NSString *pathToWrite;
if (sentry_isTracingAppLaunch) {
SENTRY_LOG_DEBUG(@"Writing app launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"launchProfile"];
} else {
SENTRY_LOG_DEBUG(@"Overwriting last non-launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"];
}
NSString *pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"];

if ([fm fileExistsAtPath:pathToWrite]) {
SENTRY_LOG_DEBUG(@"Already a %@ profile file present; make sure to remove them right after "
SENTRY_LOG_DEBUG(@"Already a profile file present; make sure to remove them right after "
@"using them, and that tests clean state in between so there isn't "
@"leftover config producing one when it isn't expected.",
sentry_isTracingAppLaunch ? @" launch" : @"");
return;
@"leftover config producing one when it isn't expected.");
}

SENTRY_LOG_DEBUG(@"Writing%@ profile to file.", sentry_isTracingAppLaunch ? @" launch" : @"");
SENTRY_LOG_DEBUG(@"Writing profile to file.");

NSError *error;
if (![data writeToFile:pathToWrite options:NSDataWritingAtomic error:&error]) {
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ - (void)backgroundAbort

- (void)stopForReason:(SentryProfilerTruncationReason)reason
{
// the following line makes unit tests pass, but ui tests fail because sentry_writeProfileFile
// needs it to be true to write to the correct path
sentry_isTracingAppLaunch = NO;
[_timeoutTimer invalidate];
[self.metricProfiler stop];
Expand Down

0 comments on commit a24c529

Please sign in to comment.