diff --git a/Sources/Sentry/SentryOptions.m b/Sources/Sentry/SentryOptions.m index 45400de4c56..2a1a995b971 100644 --- a/Sources/Sentry/SentryOptions.m +++ b/Sources/Sentry/SentryOptions.m @@ -129,7 +129,7 @@ - (instancetype)init self.tracesSampleRate = nil; #if SENTRY_TARGET_PROFILING_SUPPORTED _enableProfiling = NO; - self.profilesSampleRate = nil; + self.profilesSampleRate = SENTRY_INITIAL_PROFILES_SAMPLE_RATE; #endif // SENTRY_TARGET_PROFILING_SUPPORTED self.enableCoreDataTracing = YES; _enableSwizzling = YES; diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index 71266f3117b..a788d649260 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -546,8 +546,10 @@ + (void)startProfiler { 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."); + @"You must disable trace profiling by setting SentryOptions.profilesSampleRate and " + @"SentryOptions.profilesSampler to nil (which is the default initial value for both " + @"properties, so you can also just remove those lines from your configuration " + @"altogether) before attempting to start a continuous profiling session."); return; } @@ -558,8 +560,10 @@ + (void)stopProfiler { 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."); + @"You must disable trace profiling by setting SentryOptions.profilesSampleRate and " + @"SentryOptions.profilesSampler to nil (which is the default initial value for both " + @"properties, so you can also just remove those lines from your configuration " + @"altogether) before attempting to stop a continuous profiling session."); return; } diff --git a/Sources/Sentry/include/SentryInternalDefines.h b/Sources/Sentry/include/SentryInternalDefines.h index 7a528138ae0..716da264274 100644 --- a/Sources/Sentry/include/SentryInternalDefines.h +++ b/Sources/Sentry/include/SentryInternalDefines.h @@ -6,6 +6,17 @@ static NSString *const SentryPlatformName = @"cocoa"; #define SENTRY_DEFAULT_SAMPLE_RATE @1 #define SENTRY_DEFAULT_TRACES_SAMPLE_RATE @0 + +/** + * The value we give when initializing the options object, and what it will be if a consumer never + * modifies it in their SDK config. + * */ +#define SENTRY_INITIAL_PROFILES_SAMPLE_RATE nil + +/** + * The default value we will give for profiles sample rate if an invalid value is supplied for the + * options property in config or returned from the sampler function. + */ #define SENTRY_DEFAULT_PROFILES_SAMPLE_RATE @0 /** diff --git a/Tests/SentryTests/SentryOptionsTest.m b/Tests/SentryTests/SentryOptionsTest.m index 1f0b9500a43..1b722f428d8 100644 --- a/Tests/SentryTests/SentryOptionsTest.m +++ b/Tests/SentryTests/SentryOptionsTest.m @@ -1141,7 +1141,11 @@ - (void)testDefaultProfilesSampleRate { SentryOptions *options = [self getValidOptions:@{}]; XCTAssertNil(options.profilesSampleRate); + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertFalse(options.isProfilingEnabled); + XCTAssert([options isContinuousProfilingEnabled]); } @@ -1150,7 +1154,11 @@ - (void)testProfilesSampleRate_SetToNil SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = nil; XCTAssertNil(options.profilesSampleRate); + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertFalse(options.isProfilingEnabled); + XCTAssert([options isContinuousProfilingEnabled]); } @@ -1193,7 +1201,11 @@ - (void)testProfilesSampleRateUpperBound - (void)testIsProfilingEnabled_NothingSet_IsDisabled { SentryOptions *options = [[SentryOptions alloc] init]; + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertFalse(options.isProfilingEnabled); + XCTAssertNil(options.profilesSampleRate); XCTAssert([options isContinuousProfilingEnabled]); } @@ -1202,7 +1214,11 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSetToZero_IsDisabled { SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.00; + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertFalse(options.isProfilingEnabled); + XCTAssertNotNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1211,7 +1227,11 @@ - (void)testIsProfilingEnabled_ProfilesSampleRateSet_IsEnabled { SentryOptions *options = [[SentryOptions alloc] init]; options.profilesSampleRate = @0.01; + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertTrue(options.isProfilingEnabled); + XCTAssertNotNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1223,7 +1243,11 @@ - (void)testIsProfilingEnabled_ProfilesSamplerSet_IsEnabled XCTAssertNotNil(context); return @0.0; }; + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertTrue(options.isProfilingEnabled); + XCTAssertNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1235,7 +1259,11 @@ - (void)testIsProfilingEnabled_EnableProfilingSet_IsEnabled # pragma clang diagnostic ignored "-Wdeprecated-declarations" options.enableProfiling = YES; # pragma clang diagnostic pop + + // This property now only refers to trace-based profiling, but renaming it would require a major + // rev XCTAssertTrue(options.isProfilingEnabled); + XCTAssertNil(options.profilesSampleRate); XCTAssertFalse([options isContinuousProfilingEnabled]); } @@ -1255,6 +1283,25 @@ - (void)testProfilesSampler XCTAssertFalse([options isContinuousProfilingEnabled]); } +// this is a tricky part of the API, because while a profilesSampleRate of nil enables continuous +// profiling, just having the profilesSampler set at all disables it, even if the sampler function +// would return nil +- (void)testProfilesSamplerReturnsNil_ContinuousProfilingNotEnabled +{ + SentryTracesSamplerCallback sampler = ^(SentrySamplingContext *context) { + XCTAssertNotNil(context); + NSNumber *result = nil; + return result; + }; + + SentryOptions *options = [self getValidOptions:@{ @"profilesSampler" : sampler }]; + + SentrySamplingContext *context = [[SentrySamplingContext alloc] init]; + XCTAssertNil(options.profilesSampler(context)); + XCTAssertNil(options.profilesSampleRate); + XCTAssertFalse([options isContinuousProfilingEnabled]); +} + - (void)testDefaultProfilesSampler { SentryOptions *options = [self getValidOptions:@{}];