Skip to content

Commit

Permalink
feat(profiling): change how continuous profiling is enabled (#4024)
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight authored Jun 11, 2024
1 parent 643853e commit e70a2e1
Show file tree
Hide file tree
Showing 22 changed files with 385 additions and 205 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
isEnabled = "NO">
</CommandLineArgument>
<CommandLineArgument
argument = "--io.sentry.enable-continuous-profiling"
argument = "--io.sentry.enableContinuousProfiling"
isEnabled = "YES">
</CommandLineArgument>
<CommandLineArgument
Expand Down
11 changes: 6 additions & 5 deletions Samples/iOS-Swift/iOS-Swift/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {

static let defaultDSN = "https://[email protected]/5428557"

//swiftlint:disable function_body_length
//swiftlint:disable function_body_length cyclomatic_complexity
static func startSentry() {

// For testing purposes, we want to be able to change the DSN and store it to disk. In a real app, you shouldn't need this behavior.
Expand Down Expand Up @@ -53,8 +53,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
}
}

var profilesSampleRate: NSNumber = 1
if let profilesSampleRateOverride = env["--io.sentry.profilesSampleRate"] {
var profilesSampleRate: NSNumber? = 1
if args.contains("--io.sentry.enableContinuousProfiling") {
profilesSampleRate = nil
} else if let profilesSampleRateOverride = env["--io.sentry.profilesSampleRate"] {
profilesSampleRate = NSNumber(value: (profilesSampleRateOverride as NSString).integerValue)
}
options.profilesSampleRate = profilesSampleRate
Expand All @@ -81,7 +83,6 @@ 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.add(inAppInclude: "iOS_External")

Expand Down Expand Up @@ -146,7 +147,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
SentrySDK.metrics.increment(key: "app.start", value: 1.0, tags: ["view": "app-delegate"])

}
//swiftlint:enable function_body_length
//swiftlint:enable function_body_length cyclomatic_complexity

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

Expand Down
4 changes: 4 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@
84A305572BC9EF8C00D84283 /* SentryTraceProfiler.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A305552BC9EF8C00D84283 /* SentryTraceProfiler.h */; };
84A305582BC9EF8C00D84283 /* SentryTraceProfiler.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84A305562BC9EF8C00D84283 /* SentryTraceProfiler.mm */; };
84A5D75B29D5170700388BFA /* TimeInterval+Sentry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84A5D75A29D5170700388BFA /* TimeInterval+Sentry.swift */; };
84A7890A2C0E9F6400FF0803 /* SentrySpan+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A789092C0E9F5800FF0803 /* SentrySpan+Private.h */; };
84A8891C28DBD28900C51DFD /* SentryDevice.h in Headers */ = {isa = PBXBuildFile; fileRef = 84A8891A28DBD28900C51DFD /* SentryDevice.h */; };
84A8891D28DBD28900C51DFD /* SentryDevice.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84A8891B28DBD28900C51DFD /* SentryDevice.mm */; };
84A8892128DBD8D600C51DFD /* SentryDeviceTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 84A8892028DBD8D600C51DFD /* SentryDeviceTests.mm */; };
Expand Down Expand Up @@ -1742,6 +1743,7 @@
84A305562BC9EF8C00D84283 /* SentryTraceProfiler.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryTraceProfiler.mm; sourceTree = "<group>"; };
84A305592BC9FD1600D84283 /* SentryTraceProfiler+Test.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryTraceProfiler+Test.h"; sourceTree = "<group>"; };
84A5D75A29D5170700388BFA /* TimeInterval+Sentry.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "TimeInterval+Sentry.swift"; sourceTree = "<group>"; };
84A789092C0E9F5800FF0803 /* SentrySpan+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentrySpan+Private.h"; path = "include/SentrySpan+Private.h"; sourceTree = "<group>"; };
84A8891A28DBD28900C51DFD /* SentryDevice.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDevice.h; path = include/SentryDevice.h; sourceTree = "<group>"; };
84A8891B28DBD28900C51DFD /* SentryDevice.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryDevice.mm; sourceTree = "<group>"; };
84A8892028DBD8D600C51DFD /* SentryDeviceTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryDeviceTests.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3524,6 +3526,7 @@
7B3B83712833832B0001FDEB /* SentrySpanOperations.h */,
622C08D729E546F4002571D4 /* SentryTraceOrigins.h */,
8E4E7C6C25DAAAFE006AB9E2 /* SentrySpan.h */,
84A789092C0E9F5800FF0803 /* SentrySpan+Private.h */,
8EC3AE7925CA23B600E7591A /* SentrySpan.m */,
7BE912AA272162AF00E49E62 /* SentryNoOpSpan.h */,
7BE912AC272162D900E49E62 /* SentryNoOpSpan.m */,
Expand Down Expand Up @@ -4179,6 +4182,7 @@
7BC852392458830A005A70F0 /* SentryEnvelopeItemType.h in Headers */,
63AA769D1EB9C57A00D153DE /* SentryError.h in Headers */,
63FE714F20DA4C1100CDBAE8 /* SentryCrashNSErrorUtil.h in Headers */,
84A7890A2C0E9F6400FF0803 /* SentrySpan+Private.h in Headers */,
7BC5B6FA290BCDE500D99477 /* SentryHttpStatusCodeRange+Private.h in Headers */,
7B04A9AF24EAC02C00E710B1 /* SentryRetryAfterHeaderParser.h in Headers */,
9286059529A5096600F96038 /* SentryGeo.h in Headers */,
Expand Down
8 changes: 4 additions & 4 deletions Sources/Sentry/Profiling/SentryLaunchProfiling.m
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
SentryLaunchProfileConfig
sentry_shouldProfileNextLaunch(SentryOptions *options)
{
if (options.enableAppLaunchProfiling && options.enableContinuousProfiling) {
if (options.enableAppLaunchProfiling && [options isContinuousProfilingEnabled]) {
return (SentryLaunchProfileConfig) { YES, nil, nil };
}

Expand Down Expand Up @@ -174,14 +174,14 @@

NSMutableDictionary<NSString *, NSNumber *> *configDict =
[NSMutableDictionary<NSString *, NSNumber *> dictionary];
if (options.enableContinuousProfiling) {
if ([options isContinuousProfilingEnabled]) {
configDict[kSentryLaunchProfileConfigKeyContinuousProfiling] = @YES;
} else {
configDict[kSentryLaunchProfileConfigKeyTracesSampleRate]
= config.tracesDecision.sampleRate;
configDict[kSentryLaunchProfileConfigKeyProfilesSampleRate]
= config.profilesDecision.sampleRate;
}
configDict[kSentryLaunchProfileConfigKeyProfilesSampleRate]
= config.profilesDecision.sampleRate;
writeAppLaunchProfilingConfigFile(configDict);
}];
}
Expand Down
19 changes: 16 additions & 3 deletions Sources/Sentry/Public/SentryOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -474,8 +484,11 @@ 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.
* @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.
*/
@property (nonatomic, assign, readonly) BOOL isProfilingEnabled;

Expand Down
20 changes: 15 additions & 5 deletions Sources/Sentry/SentryOptions.m
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ - (instancetype)init
#if SENTRY_TARGET_PROFILING_SUPPORTED
_enableProfiling = NO;
self.profilesSampleRate = nil;
_enableContinuousProfiling = NO;
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
self.enableCoreDataTracing = YES;
_enableSwizzling = YES;
Expand Down Expand Up @@ -496,9 +495,6 @@ - (BOOL)validateOptions:(NSDictionary<NSString *, id> *)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"]
Expand Down Expand Up @@ -650,18 +646,32 @@ - (BOOL)isProfilingEnabled
|| _profilesSampler != nil || _enableProfiling;
}

- (BOOL)isContinuousProfilingEnabled
{
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wdeprecated-declarations"
// this looks a little weird with the `!self.enableProfiling` but that actually is the
// deprecated way to say "enable trace-based profiling", which necessarily disables continuous
// profiling as they are mutually exclusive modes
return _profilesSampleRate == nil && _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

/**
Expand Down
8 changes: 7 additions & 1 deletion Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}

Expand Down
14 changes: 8 additions & 6 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,10 @@ + (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 "
@"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;
}

Expand All @@ -539,9 +540,10 @@ + (void)startProfiler

+ (void)stopProfiler
{
if (!SENTRY_ASSERT_RETURN(currentHub.client.options.enableContinuousProfiling,
@"You must set SentryOptions.enableContinuousProfiling 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;
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentrySampling.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
* returned a valid value, @c nil otherwise.
*/
NSNumber *_Nullable _sentry_samplerCallbackRate(SentryTracesSamplerCallback _Nullable callback,
SentrySamplingContext *context, NSNumber *defaultSampleRate)
SentrySamplingContext *context, NSNumber *_Nullable defaultSampleRate)
{
if (callback == nil) {
return nil;
Expand Down
7 changes: 3 additions & 4 deletions Sources/Sentry/SentrySpan.m
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#import "SentrySpan.h"
#import "SentryCrashThread.h"
#import "SentryDependencyContainer.h"
#import "SentryFrame.h"
Expand All @@ -8,6 +7,7 @@
#import "SentryNSDictionarySanitize.h"
#import "SentryNoOpSpan.h"
#import "SentrySampleDecision+Private.h"
#import "SentrySpan+Private.h"
#import "SentrySpanContext.h"
#import "SentrySpanId.h"
#import "SentrySwift.h"
Expand Down Expand Up @@ -50,7 +50,6 @@ @implementation SentrySpan {
#endif // SENTRY_HAS_UIKIT

#if SENTRY_TARGET_PROFILING_SUPPORTED
NSString *_Nullable _profileSessionID;
BOOL _isContinuousProfiling;
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
}
Expand Down Expand Up @@ -99,7 +98,7 @@ - (instancetype)initWithContext:(SentrySpanContext *)context
_origin = context.origin;

#if SENTRY_TARGET_PROFILING_SUPPORTED
_isContinuousProfiling = SentrySDK.options.enableContinuousProfiling;
_isContinuousProfiling = [SentrySDK.options isContinuousProfilingEnabled];
if (_isContinuousProfiling) {
_profileSessionID = SentryContinuousProfiler.currentProfilerID.sentryIdString;
if (_profileSessionID == nil) {
Expand Down Expand Up @@ -376,7 +375,7 @@ - (NSDictionary *)serialize

#if SENTRY_TARGET_PROFILING_SUPPORTED
if (_profileSessionID != nil) {
mutableDictionary[@"profile_id"] = _profileSessionID;
mutableDictionary[@"profiler_id"] = _profileSessionID;
}
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

Expand Down
4 changes: 2 additions & 2 deletions Sources/Sentry/SentryTimeToDisplayTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -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 isContinuousProfilingEnabled]) {
sentry_stopAndDiscardLaunchProfileTracer();
}
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
Expand All @@ -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 isContinuousProfilingEnabled]) {
sentry_stopAndDiscardLaunchProfileTracer();
}
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
Expand Down
8 changes: 5 additions & 3 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,11 @@ - (instancetype)initWithTransactionContext:(SentryTransactionContext *)transacti
#endif // SENTRY_HAS_UIKIT

#if SENTRY_TARGET_PROFILING_SUPPORTED
if (!hub.client.options.enableContinuousProfiling
&& (_configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes
|| sentry_isTracingAppLaunch)) {
BOOL profileShouldBeSampled
= _configuration.profilesSamplerDecision.decision == kSentrySampleDecisionYes;
BOOL isContinuousProfiling = [hub.client.options isContinuousProfilingEnabled];
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 %@",
Expand Down
8 changes: 8 additions & 0 deletions Sources/Sentry/SentryTransaction.m
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
#import "SentryEnvelopeItemType.h"
#import "SentryMeasurementValue.h"
#import "SentryNSDictionarySanitize.h"
#import "SentryProfilingConditionals.h"
#import "SentrySpan+Private.h"
#import "SentrySwift.h"
#import "SentryTransactionContext.h"

Expand Down Expand Up @@ -54,6 +56,12 @@ - (instancetype)initWithTrace:(SentryTracer *)trace children:(NSArray<id<SentryS
serializedData[@"_metrics_summary"] = metricsSummary;
[serializedTrace removeObjectForKey:@"_metrics_summary"];
}
#if SENTRY_TARGET_PROFILING_SUPPORTED
NSMutableDictionary *traceDataEntry =
[serializedTrace[@"data"] mutableCopy] ?: [NSMutableDictionary dictionary];
traceDataEntry[@"profiler_id"] = self.trace.profileSessionID;
serializedTrace[@"data"] = traceDataEntry;
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
mutableContext[@"trace"] = serializedTrace;

[serializedData setValue:mutableContext forKey:@"contexts"];
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/include/SentryOptions+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 12 additions & 0 deletions Sources/Sentry/include/SentrySpan+Private.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#import "SentrySpan.h"

#import "SentryProfilingConditionals.h"

@interface
SentrySpan ()

#if SENTRY_TARGET_PROFILING_SUPPORTED
@property (copy, nonatomic) NSString *_Nullable profileSessionID;
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

@end
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include <stdlib.h>
#include <string.h>
#include <typeinfo>
#include <exception>

#define STACKTRACE_BUFFER_LENGTH 30
#define DESCRIPTION_BUFFER_LENGTH 1000
Expand Down
Loading

0 comments on commit e70a2e1

Please sign in to comment.