From 92efe83559c388b020b207713e450fbe8ab5d1ec Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Wed, 29 May 2024 16:29:16 -0800 Subject: [PATCH 01/13] wip removing SentryOptions.enableContinuousProfiling and using profilesSampleRate=nil to enable --- Samples/iOS-Swift/iOS-Swift/AppDelegate.swift | 2 +- .../Sentry/Profiling/SentryLaunchProfiling.m | 4 +- Sources/Sentry/Public/SentryOptions.h | 16 ++- Sources/Sentry/SentryOptions.m | 8 +- Sources/Sentry/SentryProfiler.mm | 2 +- Sources/Sentry/SentrySDK.m | 8 +- Sources/Sentry/SentrySpan.m | 2 +- Sources/Sentry/SentryTimeToDisplayTracker.m | 4 +- Sources/Sentry/SentryTracer.m | 2 +- .../SentryAppLaunchProfilingTests.swift | 136 ++++++++---------- .../SentryContinuousProfilerTests.swift | 2 +- Tests/SentryTests/SentryOptionsTest.m | 28 ++-- 12 files changed, 103 insertions(+), 111 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift b/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift index b1e43b65198..ba44c9bacfb 100644 --- a/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift +++ b/Samples/iOS-Swift/iOS-Swift/AppDelegate.swift @@ -79,7 +79,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { options.enableTimeToFullDisplayTracing = true options.enablePerformanceV2 = true options.enableMetrics = true - options.enableContinuousProfiling = ProcessInfo.processInfo.arguments.contains("--io.sentry.enable-continuous-profiling") + options.profilesSampleRate = ProcessInfo.processInfo.arguments.contains("--io.sentry.enable-continuous-profiling") ? nil : 1 options.add(inAppInclude: "iOS_External") diff --git a/Sources/Sentry/Profiling/SentryLaunchProfiling.m b/Sources/Sentry/Profiling/SentryLaunchProfiling.m index a017b969918..b6c1d271b2e 100644 --- a/Sources/Sentry/Profiling/SentryLaunchProfiling.m +++ b/Sources/Sentry/Profiling/SentryLaunchProfiling.m @@ -56,7 +56,7 @@ SentryLaunchProfileConfig sentry_shouldProfileNextLaunch(SentryOptions *options) { - if (options.enableAppLaunchProfiling && options.enableContinuousProfiling) { + if (options.enableAppLaunchProfiling && options.profilesSampleRate == nil) { return (SentryLaunchProfileConfig) { YES, nil, nil }; } @@ -174,7 +174,7 @@ NSMutableDictionary *configDict = [NSMutableDictionary dictionary]; - if (options.enableContinuousProfiling) { + if (options.profilesSampleRate == nil) { configDict[kSentryLaunchProfileConfigKeyContinuousProfiling] = @YES; } else { configDict[kSentryLaunchProfileConfigKeyTracesSampleRate] diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index 7b903b77365..f8560639c23 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -456,7 +456,17 @@ NS_SWIFT_NAME(Options) * the SDK sets it to the default of @c 0. * This property is dependent on @c tracesSampleRate -- if @c tracesSampleRate is @c 0 (default), * no profiles will be collected no matter what this property is set to. This property is - * used to undersample profiles *relative to* @c tracesSampleRate + * used to undersample profiles *relative to* @c tracesSampleRate . + * @note Setting this value to @c nil enables an experimental new profiling mode, called continuous + * profiling. This allows you to start and stop a profiler any time with @c SentrySDK.startProfiler + * and @c SentrySDK.stopProfiler, which can run with no time limit, periodically uploading profiling + * data. You can also set @c SentryOptions.enableAppLaunchProfiling to have the profiler start on + * app launch; there is no automatic stop, you must stop it manually at some later time if you + * choose to do so. Sampling rates do not apply to continuous profiles, including those + * automatically started for app launches. If you wish to sample them, you must do so at the + * callsites where you use the API or configure launch profiling. Continuous profiling is not + * automatically started for performance transactions as was the previous version of profiling. + * @warning The new continuous profiling mode is experimental and may still contain bugs. */ @property (nullable, nonatomic, strong) NSNumber *profilesSampleRate; @@ -474,8 +484,8 @@ NS_SWIFT_NAME(Options) /** * If profiling should be enabled or not. * @note Profiling is not supported on watchOS or tvOS. - * @returns @c YES if either a profilesSampleRate > @c 0 and \<= @c 1 or a profilesSampler is set, - * otherwise @c NO. + * @returns @c YES if either @c profilesSampleRate > @c 0 and \<= @c 1 , or @c profilesSampler is + * set, otherwise @c NO. */ @property (nonatomic, assign, readonly) BOOL isProfilingEnabled; diff --git a/Sources/Sentry/SentryOptions.m b/Sources/Sentry/SentryOptions.m index 3e18d5dfeaa..7e363e24b67 100644 --- a/Sources/Sentry/SentryOptions.m +++ b/Sources/Sentry/SentryOptions.m @@ -130,8 +130,7 @@ - (instancetype)init self.tracesSampleRate = nil; #if SENTRY_TARGET_PROFILING_SUPPORTED _enableProfiling = NO; - self.profilesSampleRate = nil; - _enableContinuousProfiling = NO; + self.profilesSampleRate = SENTRY_DEFAULT_PROFILES_SAMPLE_RATE; #endif // SENTRY_TARGET_PROFILING_SUPPORTED self.enableCoreDataTracing = YES; _enableSwizzling = YES; @@ -495,9 +494,6 @@ - (BOOL)validateOptions:(NSDictionary *)options [self setBool:options[NSStringFromSelector(@selector(enableAppLaunchProfiling))] block:^(BOOL value) { self->_enableAppLaunchProfiling = value; }]; - - [self setBool:options[@"enableContinuousProfiling"] - block:^(BOOL value) { self->_enableContinuousProfiling = value; }]; #endif // SENTRY_TARGET_PROFILING_SUPPORTED [self setBool:options[@"sendClientReports"] @@ -643,7 +639,7 @@ - (void)setProfilesSampleRate:(NSNumber *)profilesSampleRate - (BOOL)isProfilingEnabled { - return (_profilesSampleRate != nil && [_profilesSampleRate doubleValue] > 0) + return (_profilesSampleRate == nil || [_profilesSampleRate doubleValue] > 0) || _profilesSampler != nil || _enableProfiling; } diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 07c8c46826d..fe48f3ab549 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -40,7 +40,7 @@ sentry_manageTraceProfilerOnStartSDK(SentryOptions *options, SentryHub *hub) { [SentryDependencyContainer.sharedInstance.dispatchQueueWrapper dispatchAsyncWithBlock:^{ - BOOL shouldStopAndTransmitLaunchProfile = !options.enableContinuousProfiling; + BOOL shouldStopAndTransmitLaunchProfile = options.profilesSampleRate != nil; # if SENTRY_HAS_UIKIT if (SentryUIViewControllerPerformanceTracker.shared.enableWaitForFullDisplay) { shouldStopAndTransmitLaunchProfile = NO; diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 71d6b6d7a15..243b1181b78 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -528,8 +528,8 @@ + (void)crash #if SENTRY_TARGET_PROFILING_SUPPORTED + (void)startProfiler { - if (!SENTRY_ASSERT_RETURN(currentHub.client.options.enableContinuousProfiling, - @"You must set SentryOptions.enableContinuousProfiling to true before starting a " + if (!SENTRY_ASSERT_RETURN(currentHub.client.options.profilesSampleRate == nil, + @"You must set Sentryoptions.profilesSampleRate == nil to true before starting a " @"continuous profiler.")) { return; } @@ -539,8 +539,8 @@ + (void)startProfiler + (void)stopProfiler { - if (!SENTRY_ASSERT_RETURN(currentHub.client.options.enableContinuousProfiling, - @"You must set SentryOptions.enableContinuousProfiling to true before using continuous " + if (!SENTRY_ASSERT_RETURN(currentHub.client.options.profilesSampleRate == nil, + @"You must set Sentryoptions.profilesSampleRate == nil to true before using continuous " @"profiling API.")) { return; } diff --git a/Sources/Sentry/SentrySpan.m b/Sources/Sentry/SentrySpan.m index b43ec27b058..8347c22d6f2 100644 --- a/Sources/Sentry/SentrySpan.m +++ b/Sources/Sentry/SentrySpan.m @@ -99,7 +99,7 @@ - (instancetype)initWithContext:(SentrySpanContext *)context _origin = context.origin; #if SENTRY_TARGET_PROFILING_SUPPORTED - _isContinuousProfiling = SentrySDK.options.enableContinuousProfiling; + _isContinuousProfiling = SentrySDK.options.profilesSampleRate == nil; if (_isContinuousProfiling) { _profileSessionID = SentryContinuousProfiler.currentProfilerID.sentryIdString; if (_profileSessionID == nil) { diff --git a/Sources/Sentry/SentryTimeToDisplayTracker.m b/Sources/Sentry/SentryTimeToDisplayTracker.m index dbc68ef9936..a32951395d7 100644 --- a/Sources/Sentry/SentryTimeToDisplayTracker.m +++ b/Sources/Sentry/SentryTimeToDisplayTracker.m @@ -142,7 +142,7 @@ - (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate if (!_waitForFullDisplay) { [SentryDependencyContainer.sharedInstance.framesTracker removeListener:self]; # if SENTRY_TARGET_PROFILING_SUPPORTED - if (!SentrySDK.options.enableContinuousProfiling) { + if (SentrySDK.options.profilesSampleRate != nil) { sentry_stopAndDiscardLaunchProfileTracer(); } # endif // SENTRY_TARGET_PROFILING_SUPPORTED @@ -154,7 +154,7 @@ - (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate self.fullDisplaySpan.timestamp = newFrameDate; [self.fullDisplaySpan finish]; # if SENTRY_TARGET_PROFILING_SUPPORTED - if (!SentrySDK.options.enableContinuousProfiling) { + if (SentrySDK.options.profilesSampleRate != nil) { sentry_stopAndDiscardLaunchProfileTracer(); } # endif // SENTRY_TARGET_PROFILING_SUPPORTED diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index ca971984c69..2bc3c656de7 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -179,7 +179,7 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti #endif // SENTRY_HAS_UIKIT #if SENTRY_TARGET_PROFILING_SUPPORTED - if (!hub.client.options.enableContinuousProfiling + if (hub.client.options.profilesSampleRate != nil && (_configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes || sentry_isTracingAppLaunch)) { _internalID = [[SentryId alloc] init]; diff --git a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift index e6ca9931cb0..adc03289f8f 100644 --- a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift +++ b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift @@ -27,7 +27,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { func testLaunchContinuousProfileNotStoppedOnFullyDisplayed() throws { // start a launch profile fixture.options.enableAppLaunchProfiling = true - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil sentry_configureLaunchProfiling(fixture.options) _sentry_nondeduplicated_startLaunchProfile() XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) @@ -66,7 +66,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { func testLaunchContinuousProfileNotStoppedOnInitialDisplayWithoutWaitingForFullDisplay() throws { // start a launch profile fixture.options.enableAppLaunchProfiling = true - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil sentry_configureLaunchProfiling(fixture.options) _sentry_nondeduplicated_startLaunchProfile() XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) @@ -155,7 +155,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { func testContinuousLaunchProfileConfiguration() throws { let options = Options() options.enableAppLaunchProfiling = true - options.enableContinuousProfiling = true + options.profilesSampleRate = nil // sample rates are not considered for continuous profiling options.profilesSampleRate = 0 @@ -187,95 +187,81 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { * Test how combinations of the following options interact to ultimately decide whether or not to start the profiler on the next app launch.. * - `enableLaunchProfiling` * - `enableTracing` - * - `enableContinuousProfiling` (always profiles regardless of sample rate or trace options) * - `tracesSampleRate` - * - `profilesSampleRate` - * - `profilesSampler` + * - `profilesSampleRate` (set to `nil` to enable continuous profiling, which ignores sample rates) + * - `profilesSampler` (return `nil` to enable continuous profiling, which ignores sample rates) */ func testShouldProfileLaunchBasedOnOptionsCombinations() { - for testCase: (enableAppLaunchProfiling: Bool, enableTracing: Bool, enableContinuousProfiling: Bool, tracesSampleRate: Int, profilesSampleRate: Int, profilesSamplerReturnValue: Int, shouldProfileLaunch: Bool) in [ + for testCase: (enableAppLaunchProfiling: Bool, enableTracing: Bool, tracesSampleRate: Int, profilesSampleRate: Int?, profilesSamplerReturnValue: Int, shouldProfileLaunch: Bool) in [ // everything false/0 - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change profilesSampleRate to 1 - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change tracesSampleRate to 1 - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - // change enableContinuousProfiling to true - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + // enable continuous profiling by setting profilesSampleRate to nil + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change enableTracing to true - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change enableAppLaunchProfiling to true - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), // change profilesSamplerReturnValue to 1 - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true) + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true) ] { let options = Options() options.enableAppLaunchProfiling = testCase.enableAppLaunchProfiling options.enableTracing = testCase.enableTracing - options.enableContinuousProfiling = testCase.enableContinuousProfiling options.tracesSampleRate = NSNumber(value: testCase.tracesSampleRate) - options.profilesSampleRate = NSNumber(value: testCase.profilesSampleRate) + if let profilesSampleRate = testCase.profilesSampleRate { + options.profilesSampleRate = NSNumber(value: profilesSampleRate) + } else { + options.profilesSampleRate = nil + } options.profilesSampler = { _ in NSNumber(value: testCase.profilesSamplerReturnValue) } - XCTAssertEqual(sentry_willProfileNextLaunch(options), testCase.shouldProfileLaunch, "Expected \(testCase.shouldProfileLaunch ? "" : "not ")to enable app launch profiling with options: { enableAppLaunchProfiling: \(testCase.enableAppLaunchProfiling), enableTracing: \(testCase.enableTracing), enableContinuousProfiling: \(testCase.enableContinuousProfiling), tracesSampleRate: \(testCase.tracesSampleRate), profilesSampleRate: \(testCase.profilesSampleRate), profilesSamplerReturnValue: \(testCase.profilesSamplerReturnValue) }") + XCTAssertEqual(sentry_willProfileNextLaunch(options), testCase.shouldProfileLaunch, "Expected \(testCase.shouldProfileLaunch ? "" : "not ")to enable app launch profiling with options: { enableAppLaunchProfiling: \(testCase.enableAppLaunchProfiling), enableTracing: \(testCase.enableTracing), tracesSampleRate: \(testCase.tracesSampleRate), profilesSampleRate: \(String(describing: testCase.profilesSampleRate)), profilesSamplerReturnValue: \(testCase.profilesSamplerReturnValue) }") } } } diff --git a/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift b/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift index 685dbe7be10..d45bc80dd7c 100644 --- a/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift +++ b/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift @@ -16,7 +16,7 @@ final class SentryContinuousProfilerTests: XCTestCase { override func setUp() { super.setUp() fixture = SentryProfileTestFixture() - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil } override func tearDown() { diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index ff7c1f17408..bd3ce6fbc43 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -584,6 +584,10 @@ - (void)testNSNull_SetsDefaultValue @"enableAutoBreadcrumbTracking" : [NSNull null], @"tracesSampleRate" : [NSNull null], @"tracesSampler" : [NSNull null], +#if SENTRY_TARGET_PROFILING_SUPPORTED + @"profilesSampleRate" : [NSNull null], + @"profilesSampler" : [NSNull null], +#endif @"inAppIncludes" : [NSNull null], @"inAppExcludes" : [NSNull null], @"urlSessionDelegate" : [NSNull null], @@ -681,9 +685,9 @@ - (void)assertDefaultValues:(SentryOptions *)options # pragma clang diagnostic ignored "-Wdeprecated-declarations" XCTAssertEqual(NO, options.enableProfiling); # pragma clang diagnostic pop - XCTAssertNil(options.profilesSampleRate); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); XCTAssertNil(options.profilesSampler); - XCTAssertFalse(options.enableContinuousProfiling); #endif XCTAssertTrue([options.spotlightUrl isEqualToString:@"http://localhost:8969/stream"]); @@ -1063,11 +1067,6 @@ - (void)testEnableProfiling [self testBooleanField:@"enableProfiling" defaultValue:NO]; } -- (void)testEnableContinuousProfiling -{ - [self testBooleanField:@"enableContinuousProfiling" defaultValue:NO]; -} - - (void)testProfilesSampleRate { SentryOptions *options = [self getValidOptions:@{ @"profilesSampleRate" : @0.1 }]; @@ -1078,6 +1077,7 @@ - (void)testDefaultProfilesSampleRate { SentryOptions *options = [self getValidOptions:@{}]; XCTAssertEqual(options.profilesSampleRate.doubleValue, 0); + XCTAssertFalse(options.isProfilingEnabled); } - (void)testProfilesSampleRate_SetToNil @@ -1085,7 +1085,7 @@ - (void)testProfilesSampleRate_SetToNil SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = nil; XCTAssertNil(options.profilesSampleRate); - XCTAssertEqual(options.profilesSampleRate.doubleValue, 0); + XCTAssert(options.isProfilingEnabled); } - (void)testProfilesSampleRateLowerBound @@ -1124,7 +1124,7 @@ - (void)testIsProfilingEnabled_NothingSet_IsDisabled { SentryOptions *options = [[SentryOptions alloc] init]; XCTAssertFalse(options.isProfilingEnabled); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled @@ -1132,7 +1132,7 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.00; XCTAssertFalse(options.isProfilingEnabled); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testIsProfilingEnabled_ProfilesSampleRateSet_IsEnabled @@ -1140,7 +1140,7 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSet_IsEnabled SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.01; XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled @@ -1151,7 +1151,7 @@ - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled return @0.0; }; XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled @@ -1162,7 +1162,7 @@ - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled options.enableProfiling = YES; # pragma clang diagnostic pop XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testProfilesSampler @@ -1176,7 +1176,7 @@ - (void)testProfilesSampler SentrySamplingContext *context = [[SentrySamplingContext alloc] init]; XCTAssertEqual(options.profilesSampler(context), @1.0); - XCTAssertFalse(options.enableContinuousProfiling); + XCTAssertFalse(options.profilesSampleRate == nil); } - (void)testDefaultProfilesSampler From 5f472433ecb3669261f6a69a9cd4b575a928142a Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 30 May 2024 10:07:28 -0800 Subject: [PATCH 02/13] wip fixing tests --- .../Sentry/Profiling/SentryLaunchProfiling.m | 4 ++-- Sources/Sentry/SentryProfiler.mm | 6 ++++++ Sources/Sentry/SentryTracer.m | 8 +++++--- .../SentryAppLaunchProfilingTests.swift | 7 ++++--- .../Transaction/SentrySpanTests.swift | 18 ++++++++---------- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/Sources/Sentry/Profiling/SentryLaunchProfiling.m b/Sources/Sentry/Profiling/SentryLaunchProfiling.m index b6c1d271b2e..bdc299883f4 100644 --- a/Sources/Sentry/Profiling/SentryLaunchProfiling.m +++ b/Sources/Sentry/Profiling/SentryLaunchProfiling.m @@ -179,9 +179,9 @@ } else { configDict[kSentryLaunchProfileConfigKeyTracesSampleRate] = config.tracesDecision.sampleRate; + configDict[kSentryLaunchProfileConfigKeyProfilesSampleRate] + = config.profilesDecision.sampleRate; } - configDict[kSentryLaunchProfileConfigKeyProfilesSampleRate] - = config.profilesDecision.sampleRate; writeAppLaunchProfilingConfigFile(configDict); }]; } diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index fe48f3ab549..ba2a90a7a41 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -61,6 +61,12 @@ @implementation SentryProfiler { + (void)load { +# if !defined(TEST) && !defined(TESTCI) + // we want to allow starting a launch profile from here for UI tests, but not unit tests + if (NSProcessInfo.processInfo.environment[@"io.sentry.ui-test.test-name"] == nil) { + return; + } +# endif // !defined(TEST) && !defined(TESTCI) sentry_startLaunchProfile(); } diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index 2bc3c656de7..8903a14a2b3 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -179,9 +179,11 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti #endif // SENTRY_HAS_UIKIT #if SENTRY_TARGET_PROFILING_SUPPORTED - if (hub.client.options.profilesSampleRate != nil - && (_configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes - || sentry_isTracingAppLaunch)) { + BOOL profileShouldBeSampled + = _configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes; + BOOL isContinuousProfiling = hub.client.options.profilesSampleRate == nil; + BOOL shouldStartNormalTraceProfile = !isContinuousProfiling && profileShouldBeSampled; + if (sentry_isTracingAppLaunch || shouldStartNormalTraceProfile) { _internalID = [[SentryId alloc] init]; if ((_isProfiling = [SentryTraceProfiler startWithTracer:_internalID])) { SENTRY_LOG_DEBUG(@"Started profiler for trace %@ with internal id %@", diff --git a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift index adc03289f8f..72767430b39 100644 --- a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift +++ b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift @@ -54,6 +54,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { _sentry_nondeduplicated_startLaunchProfile() XCTAssert(try XCTUnwrap(SentryTraceProfiler.getCurrentProfiler()).isRunning()) + SentrySDK.setStart(fixture.options) let ttd = SentryTimeToDisplayTracker(for: UIViewController(nibName: nil, bundle: nil), waitForFullDisplay: true, dispatchQueueWrapper: fixture.dispatchQueueWrapper) ttd.start(for: try XCTUnwrap(sentry_launchTracer)) ttd.reportInitialDisplay() @@ -92,6 +93,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { _sentry_nondeduplicated_startLaunchProfile() XCTAssert(try XCTUnwrap(SentryTraceProfiler.getCurrentProfiler()).isRunning()) + SentrySDK.setStart(fixture.options) let ttd = SentryTimeToDisplayTracker(for: UIViewController(nibName: nil, bundle: nil), waitForFullDisplay: false, dispatchQueueWrapper: fixture.dispatchQueueWrapper) ttd.start(for: try XCTUnwrap(sentry_launchTracer)) ttd.reportInitialDisplay() @@ -157,8 +159,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { options.enableAppLaunchProfiling = true options.profilesSampleRate = nil - // sample rates are not considered for continuous profiling - options.profilesSampleRate = 0 + // sample rates are not considered for continuous profiling (can't test this with a profilesSampleRate of 0 though, because it must be nil to enable continuous profiling) options.tracesSampleRate = 0 XCTAssertFalse(appLaunchProfileConfigFileExists()) @@ -171,7 +172,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) } - func testTraceProfilerStartsWhenBothSampleRatesAreSet() { + func testTraceProfilerStartsWhenBothSampleRatesAreSetAboveZero() { let options = Options() options.enableAppLaunchProfiling = true options.profilesSampleRate = 0.567 diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift index 7c20d41618d..ccff5fc99cc 100644 --- a/Tests/SentryTests/Transaction/SentrySpanTests.swift +++ b/Tests/SentryTests/Transaction/SentrySpanTests.swift @@ -69,7 +69,6 @@ class SentrySpanTests: XCTestCase { #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst) func testSpanDoesNotIncludeTraceProfilerID() throws { fixture.options.profilesSampleRate = 1 - fixture.options.enableContinuousProfiling = false SentrySDK.setStart(fixture.options) let span = fixture.getSut() let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter { @@ -84,7 +83,7 @@ class SentrySpanTests: XCTestCase { } func testSpanDoesNotSubscribeToNotificationsIfAlreadyCapturedContinuousProfileID() { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentryContinuousProfiler.start() SentrySDK.setStart(fixture.options) let _ = fixture.getSut() @@ -96,7 +95,6 @@ class SentrySpanTests: XCTestCase { func testSpanDoesNotSubscribeToNotificationsIfContinuousProfilingDisabled() { fixture.options.profilesSampleRate = 1 - fixture.options.enableContinuousProfiling = false SentrySDK.setStart(fixture.options) let _ = fixture.getSut() let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter { @@ -106,7 +104,7 @@ class SentrySpanTests: XCTestCase { } func testSpanDoesSubscribeToNotificationsIfNotAlreadyCapturedContinuousProfileID() { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) let _ = fixture.getSut() let continuousProfileObservations = fixture.notificationCenter.addObserverInvocations.invocations.filter { @@ -122,7 +120,7 @@ class SentrySpanTests: XCTestCase { /// +----profile----+ /// ``` func test_spanStart_profileStart_spanEnd_profileEnd_spanIncludesProfileID() throws { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) let span = fixture.getSut() XCTAssertEqual(fixture.notificationCenter.addObserverInvocations.invocations.filter { @@ -144,7 +142,7 @@ class SentrySpanTests: XCTestCase { /// +----profile----+ /// ``` func test_spanStart_profileStart_profileEnd_spanEnd_spanIncludesProfileID() throws { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) let span = fixture.getSut() SentryContinuousProfiler.start() @@ -164,7 +162,7 @@ class SentrySpanTests: XCTestCase { /// +-------span-------+ /// ``` func test_profileStart_spanStart_profileEnd_spanEnd_spanIncludesProfileID() throws { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) SentryContinuousProfiler.start() let profileId = try XCTUnwrap(SentryContinuousProfiler.profiler()?.profilerId.sentryIdString) @@ -184,7 +182,7 @@ class SentrySpanTests: XCTestCase { /// +-------span-------+ /// ``` func test_profileStart_spanStart_spanEnd_profileEnd_spanIncludesProfileID() throws { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) SentryContinuousProfiler.start() let profileId = try XCTUnwrap(SentryContinuousProfiler.profiler()?.profilerId.sentryIdString) @@ -203,7 +201,7 @@ class SentrySpanTests: XCTestCase { /// +--profile1--+ +--profile2--+ /// ``` func test_spanStart_profileStart_profileEnd_profileStart_profileEnd_spanEnd_spanIncludesSameProfileID() throws { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) let span = fixture.getSut() SentryContinuousProfiler.start() @@ -226,7 +224,7 @@ class SentrySpanTests: XCTestCase { /// +----profile----+ /// ``` func test_spanStart_spanEnd_profileStart_profileEnd_spanDoesNotIncludeProfileID() { - fixture.options.enableContinuousProfiling = true + fixture.options.profilesSampleRate = nil SentrySDK.setStart(fixture.options) SentryContinuousProfiler.start() SentryContinuousProfiler.stop() From fc7f58b3b13eecfa3d313bdee20050c9333350c6 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 3 Jun 2024 15:59:31 -0800 Subject: [PATCH 03/13] tweak options reqs for continuous profiling; add more launch profiling tests; add new private SentryOptions.isContinuousProfiling method, and note to isProfilingEnabled prop --- .../Sentry/Profiling/SentryLaunchProfiling.m | 4 +- Sources/Sentry/Public/SentryOptions.h | 3 + Sources/Sentry/SentryOptions.m | 15 +++- Sources/Sentry/SentrySDK.m | 14 ++-- Sources/Sentry/SentrySpan.m | 2 +- Sources/Sentry/SentryTracer.m | 2 +- .../Sentry/include/SentryOptions+Private.h | 2 +- .../SentryAppLaunchProfilingTests.swift | 73 +++++++++++++++---- .../SentryContinuousProfilerTests.swift | 10 +++ Tests/SentryTests/SentryOptionsTest.m | 49 +++++++++---- Tests/SentryTests/SentrySDKTests.swift | 37 ++++++++++ 11 files changed, 169 insertions(+), 42 deletions(-) diff --git a/Sources/Sentry/Profiling/SentryLaunchProfiling.m b/Sources/Sentry/Profiling/SentryLaunchProfiling.m index bdc299883f4..2a8754c37ff 100644 --- a/Sources/Sentry/Profiling/SentryLaunchProfiling.m +++ b/Sources/Sentry/Profiling/SentryLaunchProfiling.m @@ -56,7 +56,7 @@ SentryLaunchProfileConfig sentry_shouldProfileNextLaunch(SentryOptions *options) { - if (options.enableAppLaunchProfiling && options.profilesSampleRate == nil) { + if (options.enableAppLaunchProfiling && [options isContinuousProfilingEnabled]) { return (SentryLaunchProfileConfig) { YES, nil, nil }; } @@ -174,7 +174,7 @@ NSMutableDictionary *configDict = [NSMutableDictionary dictionary]; - if (options.profilesSampleRate == nil) { + if ([options isContinuousProfilingEnabled]) { configDict[kSentryLaunchProfileConfigKeyContinuousProfiling] = @YES; } else { configDict[kSentryLaunchProfileConfigKeyTracesSampleRate] diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index f8560639c23..bd646019f75 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -484,6 +484,9 @@ NS_SWIFT_NAME(Options) /** * If profiling should be enabled or not. * @note Profiling is not supported on watchOS or tvOS. + * @note This only returns whether or not trace-based profiling is enabled. If it is not, then + * continuous profiling is effectively enabled, and calling SentrySDK.startProfiler will + * successfully start a continuous profile. * @returns @c YES if either @c profilesSampleRate > @c 0 and \<= @c 1 , or @c profilesSampler is * set, otherwise @c NO. */ diff --git a/Sources/Sentry/SentryOptions.m b/Sources/Sentry/SentryOptions.m index 7e363e24b67..539a8a97a28 100644 --- a/Sources/Sentry/SentryOptions.m +++ b/Sources/Sentry/SentryOptions.m @@ -639,22 +639,33 @@ - (void)setProfilesSampleRate:(NSNumber *)profilesSampleRate - (BOOL)isProfilingEnabled { - return (_profilesSampleRate == nil || [_profilesSampleRate doubleValue] > 0) + return (_profilesSampleRate != nil && [_profilesSampleRate doubleValue] > 0) || _profilesSampler != nil || _enableProfiling; } +- (BOOL)isContinuousProfilingEnabled +{ # pragma clang diagnostic push # pragma clang diagnostic ignored "-Wdeprecated-declarations" + return _profilesSampleRate.doubleValue == 0 && _profilesSampler == nil && !self.enableProfiling; +# pragma clang diagnostic pop +} + - (void)setEnableProfiling_DEPRECATED_TEST_ONLY:(BOOL)enableProfiling_DEPRECATED_TEST_ONLY { +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wdeprecated-declarations" self.enableProfiling = enableProfiling_DEPRECATED_TEST_ONLY; +# pragma clang diagnostic pop } - (BOOL)enableProfiling_DEPRECATED_TEST_ONLY { +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wdeprecated-declarations" return self.enableProfiling; -} # pragma clang diagnostic pop +} #endif // SENTRY_TARGET_PROFILING_SUPPORTED /** diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 243b1181b78..348e2556c73 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -528,9 +528,10 @@ + (void)crash #if SENTRY_TARGET_PROFILING_SUPPORTED + (void)startProfiler { - if (!SENTRY_ASSERT_RETURN(currentHub.client.options.profilesSampleRate == nil, - @"You must set Sentryoptions.profilesSampleRate == nil to true before starting a " - @"continuous profiler.")) { + if (![currentHub.client.options isContinuousProfilingEnabled]) { + SENTRY_LOG_WARN( + @"You must disable trace profiling by setting SentryOptions.profilesSampleRate to nil " + @"or 0 before using continuous profiling features."); return; } @@ -539,9 +540,10 @@ + (void)startProfiler + (void)stopProfiler { - if (!SENTRY_ASSERT_RETURN(currentHub.client.options.profilesSampleRate == nil, - @"You must set Sentryoptions.profilesSampleRate == nil to true before using continuous " - @"profiling API.")) { + if (![currentHub.client.options isContinuousProfilingEnabled]) { + SENTRY_LOG_WARN( + @"You must disable trace profiling by setting SentryOptions.profilesSampleRate to nil " + @"or 0 before using continuous profiling features."); return; } diff --git a/Sources/Sentry/SentrySpan.m b/Sources/Sentry/SentrySpan.m index 8347c22d6f2..d6818060564 100644 --- a/Sources/Sentry/SentrySpan.m +++ b/Sources/Sentry/SentrySpan.m @@ -99,7 +99,7 @@ - (instancetype)initWithContext:(SentrySpanContext *)context _origin = context.origin; #if SENTRY_TARGET_PROFILING_SUPPORTED - _isContinuousProfiling = SentrySDK.options.profilesSampleRate == nil; + _isContinuousProfiling = [SentrySDK.options isContinuousProfilingEnabled]; if (_isContinuousProfiling) { _profileSessionID = SentryContinuousProfiler.currentProfilerID.sentryIdString; if (_profileSessionID == nil) { diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m index 8903a14a2b3..e758f08bd56 100644 --- a/Sources/Sentry/SentryTracer.m +++ b/Sources/Sentry/SentryTracer.m @@ -181,7 +181,7 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti #if SENTRY_TARGET_PROFILING_SUPPORTED BOOL profileShouldBeSampled = _configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes; - BOOL isContinuousProfiling = hub.client.options.profilesSampleRate == nil; + BOOL isContinuousProfiling = [hub.client.options isContinuousProfilingEnabled]; BOOL shouldStartNormalTraceProfile = !isContinuousProfiling && profileShouldBeSampled; if (sentry_isTracingAppLaunch || shouldStartNormalTraceProfile) { _internalID = [[SentryId alloc] init]; diff --git a/Sources/Sentry/include/SentryOptions+Private.h b/Sources/Sentry/include/SentryOptions+Private.h index 57fe707685c..80570f9dcb5 100644 --- a/Sources/Sentry/include/SentryOptions+Private.h +++ b/Sources/Sentry/include/SentryOptions+Private.h @@ -12,7 +12,7 @@ FOUNDATION_EXPORT NSString *const kSentryDefaultEnvironment; SentryOptions () #if SENTRY_TARGET_PROFILING_SUPPORTED @property (nonatomic, assign) BOOL enableProfiling_DEPRECATED_TEST_ONLY; -@property (nonatomic, assign) BOOL enableContinuousProfiling; +- (BOOL)isContinuousProfilingEnabled; #endif // SENTRY_TARGET_PROFILING_SUPPORTED SENTRY_EXTERN BOOL sentry_isValidSampleRate(NSNumber *sampleRate); diff --git a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift index 72767430b39..2cc70766ed3 100644 --- a/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift +++ b/Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift @@ -141,7 +141,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { XCTAssertFalse(appLaunchProfileConfigFileExists()) sentry_manageTraceProfilerOnStartSDK(options, TestHub(client: nil, andScope: nil)) XCTAssert(appLaunchProfileConfigFileExists()) - options.profilesSampleRate = 0 + options.profilesSampleRate = 0.1 // less than the fixture's "random" value of 0.5 sentry_manageTraceProfilerOnStartSDK(options, TestHub(client: nil, andScope: nil)) XCTAssertFalse(appLaunchProfileConfigFileExists()) // ensure we get another config written, to test removal again @@ -171,7 +171,30 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { _sentry_nondeduplicated_startLaunchProfile() XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) } - + + // test that after configuring trace based app launch profiling, then on + // the next launch, configuring profiling for continuous mode, that the + // configuration file switches from trace-based to continuous-style config + func testSwitchFromTraceBasedToContinuousLaunchProfileConfiguration() throws { + let options = Options() + options.enableAppLaunchProfiling = true + options.profilesSampleRate = 0.567 + options.tracesSampleRate = 0.789 + XCTAssertFalse(appLaunchProfileConfigFileExists()) + sentry_manageTraceProfilerOnStartSDK(options, TestHub(client: nil, andScope: nil)) + XCTAssert(appLaunchProfileConfigFileExists()) + let dict = try XCTUnwrap(appLaunchProfileConfiguration()) + XCTAssertEqual(dict[kSentryLaunchProfileConfigKeyTracesSampleRate], options.tracesSampleRate) + XCTAssertEqual(dict[kSentryLaunchProfileConfigKeyProfilesSampleRate], options.profilesSampleRate) + + options.profilesSampleRate = nil + sentry_manageTraceProfilerOnStartSDK(options, TestHub(client: nil, andScope: nil)) + let newDict = try XCTUnwrap(appLaunchProfileConfiguration()) + XCTAssertEqual(newDict[kSentryLaunchProfileConfigKeyContinuousProfiling], true) + XCTAssertNil(newDict[kSentryLaunchProfileConfigKeyTracesSampleRate]) + XCTAssertNil(newDict[kSentryLaunchProfileConfigKeyProfilesSampleRate]) + } + func testTraceProfilerStartsWhenBothSampleRatesAreSetAboveZero() { let options = Options() options.enableAppLaunchProfiling = true @@ -193,7 +216,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { * - `profilesSampler` (return `nil` to enable continuous profiling, which ignores sample rates) */ func testShouldProfileLaunchBasedOnOptionsCombinations() { - for testCase: (enableAppLaunchProfiling: Bool, enableTracing: Bool, tracesSampleRate: Int, profilesSampleRate: Int?, profilesSamplerReturnValue: Int, shouldProfileLaunch: Bool) in [ + for testCase: (enableAppLaunchProfiling: Bool, enableTracing: Bool, tracesSampleRate: Int, profilesSampleRate: Int?, profilesSamplerReturnValue: Int?, shouldProfileLaunch: Bool) in [ // everything false/0 (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change profilesSampleRate to 1 @@ -216,14 +239,14 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 0, shouldProfileLaunch: false), // change profilesSamplerReturnValue to 1 (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), @@ -241,14 +264,32 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), - (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), - (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true) + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: 1, shouldProfileLaunch: true), + + // just those cases that had nil profilesSampleRate but nonnil profilesSamplerReturnValue, now with both as nil, which would enable launch profiling with continuous mode + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: false, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: false), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: false, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 0, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true), + (enableAppLaunchProfiling: true, enableTracing: true, tracesSampleRate: 1, profilesSampleRate: nil, profilesSamplerReturnValue: nil, shouldProfileLaunch: true) ] { let options = Options() options.enableAppLaunchProfiling = testCase.enableAppLaunchProfiling @@ -259,10 +300,14 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase { } else { options.profilesSampleRate = nil } - options.profilesSampler = { _ in - NSNumber(value: testCase.profilesSamplerReturnValue) + if let profilesSamplerReturnValue = testCase.profilesSamplerReturnValue { + options.profilesSampler = { _ in + NSNumber(value: profilesSamplerReturnValue) + } + } else { + options.profilesSampler = nil } - XCTAssertEqual(sentry_willProfileNextLaunch(options), testCase.shouldProfileLaunch, "Expected \(testCase.shouldProfileLaunch ? "" : "not ")to enable app launch profiling with options: { enableAppLaunchProfiling: \(testCase.enableAppLaunchProfiling), enableTracing: \(testCase.enableTracing), tracesSampleRate: \(testCase.tracesSampleRate), profilesSampleRate: \(String(describing: testCase.profilesSampleRate)), profilesSamplerReturnValue: \(testCase.profilesSamplerReturnValue) }") + XCTAssertEqual(sentry_willProfileNextLaunch(options), testCase.shouldProfileLaunch, "Expected \(testCase.shouldProfileLaunch ? "" : "not ")to enable app launch profiling with options: { enableAppLaunchProfiling: \(testCase.enableAppLaunchProfiling), enableTracing: \(testCase.enableTracing), tracesSampleRate: \(testCase.tracesSampleRate), profilesSampleRate: \(String(describing: testCase.profilesSampleRate)), profilesSamplerReturnValue: \(String(describing: testCase.profilesSamplerReturnValue)) }") } } } diff --git a/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift b/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift index d45bc80dd7c..89a1180c40e 100644 --- a/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift +++ b/Tests/SentryProfilerTests/SentryContinuousProfilerTests.swift @@ -63,6 +63,16 @@ final class SentryContinuousProfilerTests: XCTestCase { try performContinuousProfilingTest(expectedEnvironment: expectedEnvironment) } + func testStartingContinuousProfilerWithSampleRateOne() throws { + fixture.options.profilesSampleRate = 1 + try performContinuousProfilingTest() + } + + func testStartingContinuousProfilerWithZeroSampleRate() throws { + fixture.options.profilesSampleRate = 0 + try performContinuousProfilingTest() + } + #if !os(macOS) // test that receiving a background notification stops the continuous // profiler after it has been started manually diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index bd3ce6fbc43..a39e38d0a9a 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -577,7 +577,7 @@ - (void)testNSNull_SetsDefaultValue @"enableUIViewControllerTracing" : [NSNull null], @"attachScreenshot" : [NSNull null], @"sessionReplayOptions" : [NSNull null], -#endif +#endif // SENTRY_HAS_UIKIT @"enableAppHangTracking" : [NSNull null], @"appHangTimeoutInterval" : [NSNull null], @"enableNetworkTracking" : [NSNull null], @@ -587,7 +587,7 @@ - (void)testNSNull_SetsDefaultValue #if SENTRY_TARGET_PROFILING_SUPPORTED @"profilesSampleRate" : [NSNull null], @"profilesSampler" : [NSNull null], -#endif +#endif // SENTRY_TARGET_PROFILING_SUPPORTED @"inAppIncludes" : [NSNull null], @"inAppExcludes" : [NSNull null], @"urlSessionDelegate" : [NSNull null], @@ -643,7 +643,7 @@ - (void)assertDefaultValues:(SentryOptions *)options XCTAssertEqual(options.attachViewHierarchy, NO); XCTAssertEqual(options.experimental.sessionReplay.errorSampleRate, 0); XCTAssertEqual(options.experimental.sessionReplay.sessionSampleRate, 0); -#endif +#endif // SENTRY_HAS_UIKIT XCTAssertFalse(options.enableTracing); XCTAssertTrue(options.enableAppHangTracking); XCTAssertEqual(options.appHangTimeoutInterval, 2); @@ -664,7 +664,7 @@ - (void)assertDefaultValues:(SentryOptions *)options if (@available(iOS 15.0, macOS 12.0, macCatalyst 15.0, *)) { XCTAssertEqual(NO, options.enableMetricKit); } -#endif +#endif // SENTRY_HAS_METRIC_KIT NSRegularExpression *regexTrace = options.tracePropagationTargets[0]; XCTAssertTrue([regexTrace.pattern isEqualToString:@".*"]); @@ -688,7 +688,7 @@ - (void)assertDefaultValues:(SentryOptions *)options XCTAssertNotNil(options.profilesSampleRate); XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); XCTAssertNil(options.profilesSampler); -#endif +#endif // SENTRY_TARGET_PROFILING_SUPPORTED XCTAssertTrue([options.spotlightUrl isEqualToString:@"http://localhost:8969/stream"]); } @@ -746,7 +746,7 @@ - (void)testInvalidDsnViaEnvironment XCTAssertEqual(options.enabled, YES); setenv("SENTRY_DSN", "", 1); } -#endif +#endif // TARGET_OS_OSX - (void)testMaxAttachmentSize { @@ -842,7 +842,7 @@ - (void)testSessionReplaySettingsDefaults } } -#endif +#endif // SENTRY_HAS_UIKIT #if SENTRY_HAS_METRIC_KIT @@ -852,7 +852,7 @@ - (void)testEnableMetricKit [self testBooleanField:@"enableMetricKit" defaultValue:NO]; } } -#endif +#endif // SENTRY_HAS_METRIC_KIT - (void)testEnableAppHangTracking { @@ -1071,6 +1071,7 @@ - (void)testProfilesSampleRate { SentryOptions *options = [self getValidOptions:@{ @"profilesSampleRate" : @0.1 }]; XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.1); + XCTAssertFalse([options isContinuousProfilingEnabled]); } - (void)testDefaultProfilesSampleRate @@ -1078,6 +1079,7 @@ - (void)testDefaultProfilesSampleRate SentryOptions *options = [self getValidOptions:@{}]; XCTAssertEqual(options.profilesSampleRate.doubleValue, 0); XCTAssertFalse(options.isProfilingEnabled); + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testProfilesSampleRate_SetToNil @@ -1085,7 +1087,8 @@ - (void)testProfilesSampleRate_SetToNil SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = nil; XCTAssertNil(options.profilesSampleRate); - XCTAssert(options.isProfilingEnabled); + XCTAssertFalse(options.isProfilingEnabled); + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testProfilesSampleRateLowerBound @@ -1102,6 +1105,10 @@ - (void)testProfilesSampleRateLowerBound NSNumber *tooLow = @-0.01; options.profilesSampleRate = tooLow; XCTAssertEqual(options.profilesSampleRate.doubleValue, 0); + + // setting an invalid sample rate effectively now enables continuous profiling, since it can let + // the backing variable remain nil + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testProfilesSampleRateUpperBound @@ -1118,13 +1125,18 @@ - (void)testProfilesSampleRateUpperBound NSNumber *tooLow = @1.01; options.profilesSampleRate = tooLow; XCTAssertEqual(options.profilesSampleRate.doubleValue, 0); + + // setting an invalid sample rate effectively now enables continuous profiling, since it can let + // the backing variable remain nil + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_NothingSet_IsDisabled { SentryOptions *options = [[SentryOptions alloc] init]; XCTAssertFalse(options.isProfilingEnabled); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled @@ -1132,7 +1144,8 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.00; XCTAssertFalse(options.isProfilingEnabled); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_ProfilesSampleRateSet_IsEnabled @@ -1140,7 +1153,8 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSet_IsEnabled SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.01; XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssertFalse([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled @@ -1151,7 +1165,8 @@ - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled return @0.0; }; XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssertFalse([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled @@ -1162,7 +1177,8 @@ - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled options.enableProfiling = YES; # pragma clang diagnostic pop XCTAssertTrue(options.isProfilingEnabled); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssertFalse([options isContinuousProfilingEnabled]); } - (void)testProfilesSampler @@ -1176,19 +1192,22 @@ - (void)testProfilesSampler SentrySamplingContext *context = [[SentrySamplingContext alloc] init]; XCTAssertEqual(options.profilesSampler(context), @1.0); - XCTAssertFalse(options.profilesSampleRate == nil); + XCTAssertNotNil(options.profilesSampleRate); + XCTAssertFalse([options isContinuousProfilingEnabled]); } - (void)testDefaultProfilesSampler { SentryOptions *options = [self getValidOptions:@{}]; XCTAssertNil(options.profilesSampler); + XCTAssert([options isContinuousProfilingEnabled]); } - (void)testGarbageProfilesSampler_ReturnsNil { SentryOptions *options = [self getValidOptions:@{ @"profilesSampler" : @"fault" }]; XCTAssertNil(options.profilesSampler); + XCTAssert([options isContinuousProfilingEnabled]); } #endif // SENTRY_TARGET_PROFILING_SUPPORTED diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index d9f5a6a68c1..2cb2cb452bb 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -415,6 +415,43 @@ class SentrySDKTests: XCTestCase { XCTAssert(transaction === newSpan) } + func testStartingContinuousProfilerWithSampleRateZero() throws { + givenSdkWithHub() + + // 0 this is the default value for profilesSampleRate, so we don't have to explicitly set it on the fixture + XCTAssertEqual(try XCTUnwrap(fixture.options.profilesSampleRate).doubleValue, 0) + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + SentrySDK.startProfiler() + XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) + } + + func testStartingContinuousProfilerWithSampleRateNil() throws { + givenSdkWithHub() + + fixture.options.profilesSampleRate = nil + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + SentrySDK.startProfiler() + XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) + } + + func testNotStartingContinuousProfilerWithSampleRateBlock() throws { + givenSdkWithHub() + + fixture.options.profilesSampler = { _ in 0 } + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + SentrySDK.startProfiler() + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + } + + func testNotStartingContinuousProfilerWithSampleRateNonZero() throws { + givenSdkWithHub() + + fixture.options.profilesSampleRate = 1 + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + SentrySDK.startProfiler() + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) + } + func testInstallIntegrations() throws { let options = Options() options.dsn = "mine" From adc3fbd3b2af9c8c3ede3bd6e6302f175efb077e Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 23 May 2024 00:43:03 -0800 Subject: [PATCH 04/13] expose the API for continuous profiling --- Sources/Sentry/Public/SentryOptions.h | 13 +++++++++++++ Sources/Sentry/Public/SentrySDK.h | 14 ++++++++++++++ Sources/Sentry/include/SentrySDK+Private.h | 14 -------------- .../Sentry/include/SentrySampleDecision+Private.h | 2 ++ 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index bd646019f75..f054155272d 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -437,6 +437,19 @@ NS_SWIFT_NAME(Options) @property (nonatomic, assign) BOOL enableCoreDataTracing; #if SENTRY_TARGET_PROFILING_SUPPORTED +/* + * @warning This is an experimental feature and may still have bugs. + * Enable continuous profiling. This allows you to start and stop a profiler any time with + * @c SentrySDK.startProfiler and @c SentrySDK.stopProfiler, which can run with no time limit, + * periodically uploading profiling data. You can also set @c SentryOptions.enableAppLaunchProfiling + * to have the profiler start on app launch; there is no automatic stop, you must stop it manually + * at some later time if you choose to do so. + * @note Sampling rates do not apply to continuous profiles, including those automatically started + * for app launches. If you wish to sample them, you must do so at the callsites where you use the + * API or configure launch profiling.. + */ +@property (nonatomic, assign) BOOL enableContinuousProfiling; + /** * @warning This is an experimental feature and may still have bugs. * Set to @c YES to run the profiler as early as possible in an app launch, before you would diff --git a/Sources/Sentry/Public/SentrySDK.h b/Sources/Sentry/Public/SentrySDK.h index 9d8884be97e..c9de2a0d05b 100644 --- a/Sources/Sentry/Public/SentrySDK.h +++ b/Sources/Sentry/Public/SentrySDK.h @@ -320,6 +320,20 @@ SENTRY_NO_INIT */ + (void)close; +#if SENTRY_TARGET_PROFILING_SUPPORTED +/** + * Start a new continuous profiling session if one is not already running. + * @seealso https://docs.sentry.io/platforms/apple/profiling/ + */ ++ (void)startProfiler; + +/** + * Stop a continuous profiling session if there is one ongoing. + * @seealso https://docs.sentry.io/platforms/apple/profiling/ + */ ++ (void)stopProfiler; +#endif // SENTRY_TARGET_PROFILING_SUPPORTED + @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentrySDK+Private.h b/Sources/Sentry/include/SentrySDK+Private.h index ae1344d16b7..9e502178c53 100644 --- a/Sources/Sentry/include/SentrySDK+Private.h +++ b/Sources/Sentry/include/SentrySDK+Private.h @@ -52,20 +52,6 @@ SentrySDK () */ + (void)captureEnvelope:(SentryEnvelope *)envelope; -#if SENTRY_TARGET_PROFILING_SUPPORTED -/** - * Start a new continuous profiling session if one is not already running. - * @seealso https://docs.sentry.io/platforms/apple/profiling/ - */ -+ (void)startProfiler; - -/** - * Stop a continuous profiling session if there is one ongoing. - * @seealso https://docs.sentry.io/platforms/apple/profiling/ - */ -+ (void)stopProfiler; -#endif // SENTRY_TARGET_PROFILING_SUPPORTED - @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentrySampleDecision+Private.h b/Sources/Sentry/include/SentrySampleDecision+Private.h index 93e781ec8e0..fe904ff95b2 100644 --- a/Sources/Sentry/include/SentrySampleDecision+Private.h +++ b/Sources/Sentry/include/SentrySampleDecision+Private.h @@ -1,5 +1,7 @@ #import +#import "SentrySampleDecision.h" + /** Returns the value to use when serializing a SentrySampleDecision. */ From 07f9030cf96faf533be01a051ae29b9d3a0fb257 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Thu, 23 May 2024 00:44:33 -0800 Subject: [PATCH 05/13] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51719c90afe..d53281d412d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ for SIGTERM reporting in the last release and enabled it by default. For some users, SIGTERM events were verbose and not actionable. Therefore, we disable it per default in this release. If you'd like to receive SIGTERM events, set the option `enableSigtermReporting = true`. +- Add continuous profiling (#4010) ### Improvements From 2e20c59daacf6f00ba38de9511dff6001e14eaeb Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 29 Jul 2024 14:36:35 -0800 Subject: [PATCH 06/13] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b6e3d32a00..e466afa30fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Replay for crashes (#4171) +- Add continuous profiling mode (#4010) ## 8.32.0 @@ -99,7 +100,6 @@ for SIGTERM reporting in the last release and enabled it by default. For some users, SIGTERM events were verbose and not actionable. Therefore, we disable it per default in this release. If you'd like to receive SIGTERM events, set the option `enableSigtermReporting = true`. -- Add continuous profiling (#4010) ### Improvements From 68eef590d877f32db3ce4c91e8dc8650012e479f Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 29 Jul 2024 14:37:53 -0800 Subject: [PATCH 07/13] fix merge conflict resolution mistake --- Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme index dd5d3586eff..efc3f0e8db1 100644 --- a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme +++ b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme @@ -103,6 +103,9 @@ + + From 48db2b091f2f9e5d326130d436799c9c7d1a9e8f Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 29 Jul 2024 14:40:26 -0800 Subject: [PATCH 08/13] remove the explicit option, add experimental warning to start/stop api methods --- Sources/Sentry/Public/SentryOptions.h | 13 ------------- Sources/Sentry/Public/SentrySDK.h | 2 ++ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index 5d008f4d8ad..97032857c4b 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -455,19 +455,6 @@ NS_SWIFT_NAME(Options) @property (nonatomic, assign) BOOL enableCoreDataTracing; #if SENTRY_TARGET_PROFILING_SUPPORTED -/* - * @warning This is an experimental feature and may still have bugs. - * Enable continuous profiling. This allows you to start and stop a profiler any time with - * @c SentrySDK.startProfiler and @c SentrySDK.stopProfiler, which can run with no time limit, - * periodically uploading profiling data. You can also set @c SentryOptions.enableAppLaunchProfiling - * to have the profiler start on app launch; there is no automatic stop, you must stop it manually - * at some later time if you choose to do so. - * @note Sampling rates do not apply to continuous profiles, including those automatically started - * for app launches. If you wish to sample them, you must do so at the callsites where you use the - * API or configure launch profiling.. - */ -@property (nonatomic, assign) BOOL enableContinuousProfiling; - /** * @warning This is an experimental feature and may still have bugs. * Set to @c YES to run the profiler as early as possible in an app launch, before you would diff --git a/Sources/Sentry/Public/SentrySDK.h b/Sources/Sentry/Public/SentrySDK.h index a2e9bb6e113..0efd03c565b 100644 --- a/Sources/Sentry/Public/SentrySDK.h +++ b/Sources/Sentry/Public/SentrySDK.h @@ -336,12 +336,14 @@ SENTRY_NO_INIT #if SENTRY_TARGET_PROFILING_SUPPORTED /** * Start a new continuous profiling session if one is not already running. + * @warning Continuous profiling mode is experimental and may still contain bugs. * @seealso https://docs.sentry.io/platforms/apple/profiling/ */ + (void)startProfiler; /** * Stop a continuous profiling session if there is one ongoing. + * @warning Continuous profiling mode is experimental and may still contain bugs. * @seealso https://docs.sentry.io/platforms/apple/profiling/ */ + (void)stopProfiler; From b23ab4b220174d7eb6e667959c3ee45bfef77e5e Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 26 Aug 2024 12:13:54 -0800 Subject: [PATCH 09/13] update changelog and improve some headerdocs --- CHANGELOG.md | 5 ++++- Sources/Sentry/Public/SentryOptions.h | 23 +++++++++++++---------- Sources/Sentry/Public/SentrySDK.h | 8 ++++++-- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1f4de7b3e6..d96a7c24a93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog ## Unreleased +### Features + +- Continuous mode profiling (see `SentrySDK.startProfiler` and `SentryOptions.profilesSampleRate`) (#4010) + ### Fixes - Proper redact SR during animation (#4289) @@ -46,7 +50,6 @@ This bug caused unhandled/crash events to have the unhandled property and mach i - Support orientation change for session replay (#4194) - Replay for crashes (#4171) -- Add continuous profiling mode (#4010) - Redact web view from replay (#4203) - Add beforeCaptureViewHierarchy callback (#4210) - Rename session replay `errorSampleRate` property to `onErrorSampleRate` (#4218) diff --git a/Sources/Sentry/Public/SentryOptions.h b/Sources/Sentry/Public/SentryOptions.h index 465c31a9eb2..9b549a3266f 100644 --- a/Sources/Sentry/Public/SentryOptions.h +++ b/Sources/Sentry/Public/SentryOptions.h @@ -466,11 +466,12 @@ NS_SWIFT_NAME(Options) /** * @warning This is an experimental feature and may still have bugs. * Set to @c YES to run the profiler as early as possible in an app launch, before you would - * normally have the opportunity to call @c SentrySDK.start . If enabled, the @c tracesSampleRate - * and @c profilesSampleRate are persisted to disk and read on the next app launch to decide whether - * to profile that launch. - * @see @c tracesSampler and @c profilesSampler for more information on how they work for this - * feature. + * normally have the opportunity to call @c SentrySDK.start . If @c profilesSampleRate is nonnull, + * the @c tracesSampleRate and @c profilesSampleRate are persisted to disk and read on the next app + * launch to decide whether to profile that launch. + * @warning If @c profilesSampleRate is @c nil then a continuous profile will be started on every + * launch; if you desire sampling profiled launches, you must compute your own sample rate to decide + * whether to set this property to @c YES or @c NO . * @note Profiling is automatically disabled if a thread sanitizer is attached. */ @property (nonatomic, assign) BOOL enableAppLaunchProfiling; @@ -478,12 +479,11 @@ NS_SWIFT_NAME(Options) /** * @note Profiling is not supported on watchOS or tvOS. * Indicates the percentage profiles being sampled out of the sampled transactions. - * @note The default is @c 0. * @note The value needs to be >= @c 0.0 and \<= @c 1.0. When setting a value out of range - * the SDK sets it to the default of @c 0. - * This property is dependent on @c tracesSampleRate -- if @c tracesSampleRate is @c 0 (default), - * no profiles will be collected no matter what this property is set to. This property is - * used to undersample profiles *relative to* @c tracesSampleRate . + * the SDK sets it to @c 0. When set to a valid nonnull value, this property is dependent on + * @c tracesSampleRate -- if @c tracesSampleRate is @c 0 (default), no profiles will be collected no + * matter what this property is set to. This property is used to undersample profiles *relative to* + * @c tracesSampleRate . * @note Setting this value to @c nil enables an experimental new profiling mode, called continuous * profiling. This allows you to start and stop a profiler any time with @c SentrySDK.startProfiler * and @c SentrySDK.stopProfiler, which can run with no time limit, periodically uploading profiling @@ -493,6 +493,9 @@ NS_SWIFT_NAME(Options) * automatically started for app launches. If you wish to sample them, you must do so at the * callsites where you use the API or configure launch profiling. Continuous profiling is not * automatically started for performance transactions as was the previous version of profiling. + * @seealso https://docs.sentry.io/platforms/apple/profiling/ for more information about the + * different profiling modes. + * @note The default is @c nil (which implies continuous profiling mode). * @warning The new continuous profiling mode is experimental and may still contain bugs. * @note Profiling is automatically disabled if a thread sanitizer is attached. */ diff --git a/Sources/Sentry/Public/SentrySDK.h b/Sources/Sentry/Public/SentrySDK.h index 6c788ee7586..9f728c26380 100644 --- a/Sources/Sentry/Public/SentrySDK.h +++ b/Sources/Sentry/Public/SentrySDK.h @@ -356,15 +356,19 @@ SENTRY_NO_INIT #if SENTRY_TARGET_PROFILING_SUPPORTED /** * Start a new continuous profiling session if one is not already running. + * @note Unlike trace-based profiling, continuous profiling does not take into account @c + * SentryOptions.profilesSampleRate ; a call to this method will always start a profile if one is + * not already running. This includes app launch profiles configured with @c + * SentryOptions.enableAppLaunchProfiling . * @warning Continuous profiling mode is experimental and may still contain bugs. - * @seealso https://docs.sentry.io/platforms/apple/profiling/ + * @seealso https://docs.sentry.io/platforms/apple/guides/ios/profiling/#continuous-profiling */ + (void)startProfiler; /** * Stop a continuous profiling session if there is one ongoing. * @warning Continuous profiling mode is experimental and may still contain bugs. - * @seealso https://docs.sentry.io/platforms/apple/profiling/ + * @seealso https://docs.sentry.io/platforms/apple/guides/ios/profiling/#continuous-profiling */ + (void)stopProfiler; #endif // SENTRY_TARGET_PROFILING_SUPPORTED From 4edc5dbe515783a9051d47a361046854d95ee79c Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 26 Aug 2024 12:36:02 -0800 Subject: [PATCH 10/13] fix tests that werent expecting continuous profiling mode by default --- Tests/SentryTests/SentryOptionsTest.m | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index 84748e4cb73..8e77819320f 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -730,10 +730,10 @@ - (void)assertDefaultValues:(SentryOptions *)options # pragma clang diagnostic ignored "-Wdeprecated-declarations" XCTAssertEqual(NO, options.enableProfiling); # pragma clang diagnostic pop - XCTAssertNotNil(options.profilesSampleRate); + XCTAssertNil(options.profilesSampleRate); XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); XCTAssertNil(options.profilesSampler); - XCTAssertFalse([options isContinuousProfilingEnabled]); + XCTAssertTrue([options isContinuousProfilingEnabled]); #endif // SENTRY_TARGET_PROFILING_SUPPORTED XCTAssertTrue([options.spotlightUrl isEqualToString:@"http://localhost:8969/stream"]); @@ -1146,14 +1146,14 @@ - (void)testProfilesSampleRate - (void)testDefaultProfilesSampleRate { SentryOptions *options = [self getValidOptions:@{}]; - XCTAssertNotNil(options.profilesSampleRate); + XCTAssertNil(options.profilesSampleRate); XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); // This property now only refers to trace-based profiling, but renaming it would require a major // rev XCTAssertFalse(options.isProfilingEnabled); - XCTAssertFalse([options isContinuousProfilingEnabled]); + XCTAssertTrue([options isContinuousProfilingEnabled]); } - (void)testProfilesSampleRate_SetToNil @@ -1217,8 +1217,8 @@ - (void)testIsProfilingEnabled_NothingSet_IsDisabled // rev XCTAssertFalse(options.isProfilingEnabled); - XCTAssertNotNil(options.profilesSampleRate); - XCTAssertFalse([options isContinuousProfilingEnabled]); + XCTAssertNil(options.profilesSampleRate); + XCTAssertTrue([options isContinuousProfilingEnabled]); } - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled @@ -1259,7 +1259,7 @@ - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled // rev XCTAssertTrue(options.isProfilingEnabled); - XCTAssertNotNil(options.profilesSampleRate); + XCTAssertNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1275,7 +1275,7 @@ - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled // rev XCTAssertTrue(options.isProfilingEnabled); - XCTAssertNotNil(options.profilesSampleRate); + XCTAssertNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1309,7 +1309,7 @@ - (void)testProfilesSamplerReturnsNil_ContinuousProfilingNotEnabled SentrySamplingContext *context = [[SentrySamplingContext alloc] init]; XCTAssertNil(options.profilesSampler(context)); - XCTAssertNotNil(options.profilesSampleRate); + XCTAssertNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1317,14 +1317,14 @@ - (void)testDefaultProfilesSampler { SentryOptions *options = [self getValidOptions:@{}]; XCTAssertNil(options.profilesSampler); - XCTAssertFalse([options isContinuousProfilingEnabled]); + XCTAssertTrue([options isContinuousProfilingEnabled]); } - (void)testGarbageProfilesSampler_ReturnsNil { SentryOptions *options = [self getValidOptions:@{ @"profilesSampler" : @"fault" }]; XCTAssertNil(options.profilesSampler); - XCTAssertFalse([options isContinuousProfilingEnabled]); + XCTAssertTrue([options isContinuousProfilingEnabled]); } #endif // SENTRY_TARGET_PROFILING_SUPPORTED From 87860b87622ee9d452b3cbc47b34da6c5110ee86 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 26 Aug 2024 12:41:42 -0800 Subject: [PATCH 11/13] remove redundant assertions --- Tests/SentryTests/SentryOptionsTest.m | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index 8e77819320f..8757799eab6 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -731,7 +731,6 @@ - (void)assertDefaultValues:(SentryOptions *)options XCTAssertEqual(NO, options.enableProfiling); # pragma clang diagnostic pop XCTAssertNil(options.profilesSampleRate); - XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); XCTAssertNil(options.profilesSampler); XCTAssertTrue([options isContinuousProfilingEnabled]); #endif // SENTRY_TARGET_PROFILING_SUPPORTED @@ -1148,7 +1147,6 @@ - (void)testDefaultProfilesSampleRate SentryOptions *options = [self getValidOptions:@{}]; XCTAssertNil(options.profilesSampleRate); - XCTAssertEqual(options.profilesSampleRate.doubleValue, 0.0); // This property now only refers to trace-based profiling, but renaming it would require a major // rev XCTAssertFalse(options.isProfilingEnabled); From 4d7b4d914203d30e3fbe377a87c3a72cbc4f0feb Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 26 Aug 2024 12:53:32 -0800 Subject: [PATCH 12/13] fixup! fix tests that werent expecting continuous profiling mode by default --- Tests/SentryTests/SentrySDKTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 7103c2d1fb0..3a4d82fb845 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -423,8 +423,9 @@ class SentrySDKTests: XCTestCase { func testStartingContinuousProfilerWithSampleRateZero() throws { givenSdkWithHub() - // 0 is the default value for profilesSampleRate, so we don't have to explicitly set it on the fixture + fixture.options.profilesSampleRate = 0 XCTAssertEqual(try XCTUnwrap(fixture.options.profilesSampleRate).doubleValue, 0) + XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) SentrySDK.startProfiler() XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) @@ -433,7 +434,7 @@ class SentrySDKTests: XCTestCase { func testStartingContinuousProfilerWithSampleRateNil() throws { givenSdkWithHub() - fixture.options.profilesSampleRate = nil + // nil is the default initial value for profilesSampleRate, so we don't have to explicitly set it on the fixture XCTAssertFalse(SentryContinuousProfiler.isCurrentlyProfiling()) SentrySDK.startProfiler() XCTAssert(SentryContinuousProfiler.isCurrentlyProfiling()) From 5870dcce4fccb67b53b70b9778f062ec88aeaa82 Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Mon, 26 Aug 2024 18:45:43 -0800 Subject: [PATCH 13/13] remove redundant option set --- Tests/SentryTests/Transaction/SentryTransactionTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/SentryTests/Transaction/SentryTransactionTests.swift b/Tests/SentryTests/Transaction/SentryTransactionTests.swift index 8cf7f3ac254..e501c1df96d 100644 --- a/Tests/SentryTests/Transaction/SentryTransactionTests.swift +++ b/Tests/SentryTests/Transaction/SentryTransactionTests.swift @@ -229,7 +229,6 @@ class SentryTransactionTests: XCTestCase { #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst) func testTransactionWithContinuousProfile() throws { let options = Options() - options.profilesSampleRate = nil // enables continuous profiling SentrySDK.setStart(options) let transaction = fixture.getTransaction() SentryContinuousProfiler.start()