Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(launch profiling): fix data wipe between integration test runs #3633

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
84B527BD28DD25E400475E8D /* SentryDevice.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84B527BC28DD25E400475E8D /* SentryDevice.mm */; };
84BE546F287503F100ACC735 /* SentrySDKPerformanceBenchmarkTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 84BE546E287503F100ACC735 /* SentrySDKPerformanceBenchmarkTests.m */; };
84BE547E287645B900ACC735 /* SentryProcessInfo.m in Sources */ = {isa = PBXBuildFile; fileRef = 84BE54792876451D00ACC735 /* SentryProcessInfo.m */; };
84DEE88E2B6A4D1200A7BC17 /* AppStartup.m in Sources */ = {isa = PBXBuildFile; fileRef = 84DEE88D2B6A4D1200A7BC17 /* AppStartup.m */; };
84FB812A284001B800F3A94A /* SentryBenchmarking.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84FB8129284001B800F3A94A /* SentryBenchmarking.mm */; };
84FB812B284001B800F3A94A /* SentryBenchmarking.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84FB8129284001B800F3A94A /* SentryBenchmarking.mm */; };
8E8C57AF25EF16E6001CEEFA /* TraceTestViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8E8C57AE25EF16E6001CEEFA /* TraceTestViewController.swift */; };
Expand Down Expand Up @@ -315,8 +314,6 @@
84BE546E287503F100ACC735 /* SentrySDKPerformanceBenchmarkTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySDKPerformanceBenchmarkTests.m; sourceTree = "<group>"; };
84BE54782876451D00ACC735 /* SentryProcessInfo.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SentryProcessInfo.h; sourceTree = "<group>"; };
84BE54792876451D00ACC735 /* SentryProcessInfo.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryProcessInfo.m; sourceTree = "<group>"; };
84DEE88C2B6A4D1200A7BC17 /* AppStartup.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AppStartup.h; sourceTree = "<group>"; };
84DEE88D2B6A4D1200A7BC17 /* AppStartup.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = AppStartup.m; sourceTree = "<group>"; };
84FB8125284001B800F3A94A /* SentryBenchmarking.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SentryBenchmarking.h; sourceTree = "<group>"; };
84FB8129284001B800F3A94A /* SentryBenchmarking.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryBenchmarking.mm; sourceTree = "<group>"; };
84FB812C2840021B00F3A94A /* iOS-Swift-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "iOS-Swift-Bridging-Header.h"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -478,8 +475,6 @@
D8DBDA73274D4DF900007380 /* ViewControllers */,
63F93AA9245AC91600A500DB /* iOS-Swift.entitlements */,
637AFDA9243B02760034958B /* AppDelegate.swift */,
84DEE88C2B6A4D1200A7BC17 /* AppStartup.h */,
84DEE88D2B6A4D1200A7BC17 /* AppStartup.m */,
637AFDAD243B02760034958B /* TransactionsViewController.swift */,
84AB90782A50031B0054C99A /* Profiling */,
D80D021229EE93630084393D /* ErrorsViewController.swift */,
Expand Down Expand Up @@ -958,7 +953,6 @@
7B79000429028C7300A7F467 /* MetricKitManager.swift in Sources */,
D8D7BB4A2750067900044146 /* UIAssert.swift in Sources */,
D8F3D057274E574200B56F8C /* LoremIpsumViewController.swift in Sources */,
84DEE88E2B6A4D1200A7BC17 /* AppStartup.m in Sources */,
629EC8AD2B0B537400858855 /* ANRs.swift in Sources */,
D8DBDA78274D5FC400007380 /* SplitViewController.swift in Sources */,
84ACC43C2A73CB5900932A18 /* ProfilingNetworkScanner.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,14 @@
argument = "--disable-file-io-tracing"
isEnabled = "NO">
</CommandLineArgument>
<CommandLineArgument
argument = "--io.sentry.wipe-data"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "--io.sentry.slow-load-method"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "--disable-watchdog-tracking"
isEnabled = "NO">
Expand All @@ -82,8 +90,8 @@
isEnabled = "NO">
</CommandLineArgument>
<CommandLineArgument
argument = "--io.sentry.test.profilesAppLaunches"
isEnabled = "NO">
argument = "--profile-app-launches"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
argument = "--disable-crash-handler"
Expand Down
21 changes: 19 additions & 2 deletions Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {

print("[iOS-Swift] launch arguments: \(ProcessInfo.processInfo.arguments)")
print("[iOS-Swift] environment: \(ProcessInfo.processInfo.environment)")
print("[iOS-Swift] [debug] launch arguments: \(ProcessInfo.processInfo.arguments)")
print("[iOS-Swift] [debug] environment: \(ProcessInfo.processInfo.environment)")

maybeWipeData()
AppDelegate.startSentry()

if #available(iOS 15.0, *) {
Expand Down Expand Up @@ -162,3 +163,19 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
// swiftlint:enable force_cast
}
}

private extension AppDelegate {
// previously tried putting this in an AppDelegate.load override in ObjC, but it wouldn't run until after a launch profiler would have an opportunity to run, since SentryProfiler.load would always run first due to being dynamically linked in a framework module. it is sufficient to do it before calling SentrySDK.startWithOptions to clear state for testProfiledAppLaunches because we don't make any assertions on a launch profile the first launch of the app in that test
func maybeWipeData() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: We could rename this to wipeAppData and call it above when ProcessInfo.processInfo.arguments.contains("--io.sentry.wipe-data") is true. The name maybeWipeData is a bit confusing to me.

if ProcessInfo.processInfo.arguments.contains("--io.sentry.wipe-data") {
print("[iOS-Swift] [debug] removing app data")
let appSupport = NSSearchPathForDirectoriesInDomains(.applicationSupportDirectory, .userDomainMask, true).first!
let cache = NSSearchPathForDirectoriesInDomains(.cachesDirectory, .userDomainMask, true).first!
for path in [appSupport, cache] {
for item in FileManager.default.enumerator(atPath: path)! {
try! FileManager.default.removeItem(atPath: (path as NSString).appendingPathComponent((item as! String)))
}
}
}
}
}
17 changes: 0 additions & 17 deletions Samples/iOS-Swift/iOS-Swift/AppStartup.h

This file was deleted.

26 changes: 0 additions & 26 deletions Samples/iOS-Swift/iOS-Swift/AppStartup.m

This file was deleted.

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] [ProfilingViewController] benchmarking results:\n\(value)")
print("[iOS-Swift] [debug] [ProfilingViewController] benchmarking results:\n\(value)")
}

@IBAction func startCPUWork(_ sender: UIButton) {
Expand Down Expand Up @@ -130,7 +130,7 @@ extension ProfilingViewController {
let url = file as! URL
if url.absoluteString.contains(fileName) {
block(url)
print("[iOS-Swift] [ProfilingViewController] removing file at \(url)")
print("[iOS-Swift] [debug] [ProfilingViewController] removing file at \(url)")
try! FileManager.default.removeItem(at: url)
return
}
Expand All @@ -150,7 +150,7 @@ extension ProfilingViewController {
return
}
let contents = data.base64EncodedString()
print("[iOS-Swift] [ProfilingViewController] contents of file at \(file): \(String(describing: String(data: data, encoding: .utf8)))")
print("[iOS-Swift] [debug] [ProfilingViewController] contents of file at \(file): \(String(describing: String(data: data, encoding: .utf8)))")
profilingUITestDataMarshalingTextField.text = contents
profilingUITestDataMarshalingStatus.text = "✅"
}
Expand Down
66 changes: 43 additions & 23 deletions Sources/Sentry/Profiling/SentryLaunchProfiling.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@
# import "SentrySamplerDecision.h"
# import "SentrySampling.h"
# import "SentrySamplingContext.h"
# import "SentryTracer.h"
# import "SentryTraceOrigins.h"
# import "SentryTracer+Private.h"
# import "SentryTracerConfiguration.h"
# import "SentryTransactionContext.h"
# import "SentryTransactionContext+Private.h"

NS_ASSUME_NONNULL_BEGIN

BOOL isTracingAppLaunch;
SentryId *_Nullable appLaunchTraceId;
NSObject *appLaunchTraceLock;
uint64_t appLaunchSystemTime;
NSString *const kSentryLaunchProfileConfigKeyTracesSampleRate = @"traces";
NSString *const kSentryLaunchProfileConfigKeyProfilesSampleRate = @"profiles";
SentryTracer *_Nullable launchTracer;
static SentryTracer *_Nullable launchTracer;

# pragma mark - Private

Expand Down Expand Up @@ -91,6 +91,29 @@
}];
}

SentryTransactionContext *
context(NSNumber *tracesRate)
{
SentryTransactionContext *context =
[[SentryTransactionContext alloc] initWithName:@"launch"
nameSource:kSentryTransactionNameSourceCustom
operation:@"app.lifecycle"
origin:SentryTraceOriginAutoAppStartProfile
sampled:kSentrySampleDecisionYes];
context.sampleRate = tracesRate;
return context;
}

SentryTracerConfiguration *
config(NSNumber *profilesRate)
{
SentryTracerConfiguration *config = [SentryTracerConfiguration defaultConfiguration];
config.profilesSamplerDecision =
[[SentrySamplerDecision alloc] initWithDecision:kSentrySampleDecisionYes
forSampleRate:profilesRate];
return config;
}

void
startLaunchProfile(void)
{
Expand All @@ -111,39 +134,36 @@
[SentryLog configure:YES diagnosticLevel:kSentryLevelDebug];
# endif // defined(DEBUG)

appLaunchSystemTime = SentryDependencyContainer.sharedInstance.dateProvider.systemTime;
appLaunchTraceLock = [[NSObject alloc] init];
appLaunchTraceId = [[SentryId alloc] init];

SENTRY_LOG_INFO(@"Starting app launch profile at %llu", appLaunchSystemTime);

SentryTransactionContext *context =
[[SentryTransactionContext alloc] initWithName:@"launch" operation:@"app.lifecycle"];
SentryTracerConfiguration *config = [SentryTracerConfiguration defaultConfiguration];
NSDictionary<NSString *, NSNumber *> *rates = appLaunchProfileConfiguration();
NSNumber *profilesRate = rates[kSentryLaunchProfileConfigKeyProfilesSampleRate];
NSNumber *tracesRate = rates[kSentryLaunchProfileConfigKeyTracesSampleRate];
if (profilesRate != nil && tracesRate != nil) {
config.profilesSamplerDecision =
[[SentrySamplerDecision alloc] initWithDecision:kSentrySampleDecisionYes
forSampleRate:profilesRate];
context.sampleRate = tracesRate;
if (profilesRate == nil || tracesRate == nil) {
SENTRY_LOG_DEBUG(
@"Received a nil configured launch sample rate, will not trace or profile.");
return;
}
launchTracer = [[SentryTracer alloc] initWithTransactionContext:context

SENTRY_LOG_INFO(@"Starting app launch profile.");
launchTracer = [[SentryTracer alloc] initWithTransactionContext:context(tracesRate)
hub:nil
configuration:config];
configuration:config(profilesRate)];
});
}

void
stopLaunchProfile(void)
stopLaunchProfile(SentryHub *hub)
{
if (launchTracer == nil) {
SENTRY_LOG_DEBUG(@"No launch tracer present to stop.");
return;
}

SENTRY_LOG_DEBUG(@"Finishing launch tracer.");

launchTracer.hub = hub;
[launchTracer finish];
}

NS_ASSUME_NONNULL_END

#endif // SENTRY_TARGET_PROFILING_SUPPORTED
24 changes: 15 additions & 9 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@

# if defined(TEST) || defined(TESTCI) || defined(DEBUG)
# import "SentryFileManager.h"
# import "SentryInternalDefines.h"
# import "SentryLaunchProfiling.h"

/**
Expand Down Expand Up @@ -436,7 +437,7 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload:
return [[SentryEnvelopeItem alloc] initWithHeader:header data:JSONData];
}

# if defined(TEST) || defined(TESTCI)
# if defined(TEST) || defined(TESTCI) || defined(DEBUG)
void
writeProfileFile(NSDictionary<NSString *, id> *payload)
{
Expand All @@ -460,20 +461,25 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload:

NSString *pathToWrite;
if (isTracingAppLaunch) {
SENTRY_LOG_DEBUG(@"Writing app launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"launchProfile"];
if ([fm fileExistsAtPath:pathToWrite]) {
SENTRY_LOG_DEBUG(@"Already a launch profile file present.");
return;
}
} else {
SENTRY_LOG_DEBUG(@"Overwriting last non-launch profile.");
pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"];
}

SENTRY_LOG_DEBUG(@"Writing app launch profile to file.");
if ([fm fileExistsAtPath:pathToWrite]) {
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.",
isTracingAppLaunch ? @" launch" : @"");
}

SENTRY_LOG_DEBUG(@"Writing%@ profile to file.", isTracingAppLaunch ? @" launch" : @"");
SENTRY_CASSERT(
[data writeToFile:pathToWrite atomically:YES], @"Failed to write profile to test file");
}
# endif // defined(TEST) || defined(TESTCI)
# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG)

+ (nullable NSMutableDictionary<NSString *, id> *)collectProfileBetween:(uint64_t)startSystemTime
and:(uint64_t)endSystemTime
Expand All @@ -487,9 +493,9 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload:

const auto payload = [profiler serializeBetween:startSystemTime and:endSystemTime onHub:hub];

# if defined(TEST) || defined(TESTCI)
# if defined(TEST) || defined(TESTCI) || defined(DEBUG)
writeProfileFile(payload);
# endif // defined(TEST) || defined(TESTCI)
# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG)

return payload;
}
Expand Down
14 changes: 7 additions & 7 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,6 @@ + (void)setStartTimestamp:(NSDate *)value

+ (void)startWithOptions:(SentryOptions *)options
{
#if SENTRY_TARGET_PROFILING_SUPPORTED
stopLaunchProfile();
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

[SentryLog configure:options.debug diagnosticLevel:options.diagnosticLevel];

// We accept the tradeoff that the SDK might not be fully initialized directly after
Expand All @@ -190,7 +186,8 @@ + (void)startWithOptions:(SentryOptions *)options
= options.initialScope([[SentryScope alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs]);
// The Hub needs to be initialized with a client so that closing a session
// can happen.
[SentrySDK setCurrentHub:[[SentryHub alloc] initWithClient:newClient andScope:scope]];
SentryHub *hub = [[SentryHub alloc] initWithClient:newClient andScope:scope];
[SentrySDK setCurrentHub:hub];
SENTRY_LOG_DEBUG(@"SDK initialized! Version: %@", SentryMeta.versionString);

SENTRY_LOG_DEBUG(@"Dispatching init work required to run on main thread.");
Expand All @@ -204,11 +201,14 @@ + (void)startWithOptions:(SentryOptions *)options
#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
[SentryDependencyContainer.sharedInstance.uiDeviceWrapper start];
#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT
}];

#if SENTRY_TARGET_PROFILING_SUPPORTED
configureLaunchProfiling(options);
[SentryDependencyContainer.sharedInstance.dispatchQueueWrapper dispatchAsyncWithBlock:^{
stopLaunchProfile(hub);
configureLaunchProfiling(options);
}];
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
}];
}

+ (void)startWithConfigureOptions:(void (^)(SentryOptions *options))configureOptions
Expand Down
9 changes: 0 additions & 9 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -609,15 +609,6 @@ - (void)finishInternal
#if SENTRY_TARGET_PROFILING_SUPPORTED
if (self.isProfiling) {
[self captureTransactionWithProfile:transaction];

// as long as this isn't used for any conditional branching logic, and is just being set to
// NO, we don't need to synchronize the read here
if (isTracingAppLaunch) {
@synchronized(appLaunchTraceLock) {
isTracingAppLaunch = NO;
}
}

return;
}
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
Expand Down
Loading
Loading