Skip to content

feat(V9): Make performance V2 the default in V9 #6008

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+15 −1
Open

feat(V9): Make performance V2 the default in V9 #6008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-v9.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Removes deprecated `setExtraValue` from SentrySpan (#5864)
Removes `integrations` property from `SentryOptions` (#5749)
Makes `SentryEventDecodable` internal (#5808)
The `span` property on `SentryScope` is now readonly (#5866)
Removes `enablePerformanceV2` option and make this the default (#6008)

### Fixes

Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/Public/SentryOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ NS_SWIFT_NAME(Options)
*/
@property (nonatomic, assign) BOOL enableAutoPerformanceTracing;

#if !SDK_V9
Copy link
Member

Choose a reason for hiding this comment

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

h: Not sure if I understand this PR's intention. Do you want to make enablePerformanceV2 be enabled by default or do you want to remove the options? If the latter one why would we want to remove the option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philprime I assumed if we had a V2 of something we'd eventually want to remove the V1 since V2 is better/newer, but maybe the intent here was to have two definitions of app start, and then I'd recommend having something like an enum for "AppStartType" so it's clear what is changing rather than a v1/v2

Copy link
Member

Choose a reason for hiding this comment

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

We should remove the option and enable everything by default that depends on performanceV2 in the next major. performanceV2. Currently that's only

The app start duration will now finish when the first frame is drawn instead of when the OS posts the

We didn't change this in a minor, because it impacts the app start duration, and it could cause alerts.

I thought we would make more changes with this feature flag, but then our performance work improvements got deprioritized, and we never made further improvements. This option adds complexity, and we should ditch it.

Copy link
Member

Choose a reason for hiding this comment

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

I understand, in that case yes let's change this for V9.

We need to start a v8 -> v9 migration guide!

/**
* We're working to update our Performance product offering in order to be able to provide better
* insights and highlight specific actions you can take to improve your mobile app's overall
Expand All @@ -295,6 +296,7 @@ NS_SWIFT_NAME(Options)
* UIWindowDidBecomeVisibleNotification. This change will be the default in the next major version.
*/
@property (nonatomic, assign) BOOL enablePerformanceV2;
#endif // !SDK_V9

/**
* @warning This is an experimental feature and may still have bugs.
Expand Down
7 changes: 6 additions & 1 deletion Sources/Sentry/SentryAppStartTrackingIntegration.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,17 @@ - (BOOL)installWithOptions:(SentryOptions *)options
SentryAppStateManager *appStateManager =
[SentryDependencyContainer sharedInstance].appStateManager;

# if SDK_V9
BOOL usePerformanceV2 = YES;
# else
BOOL usePerformanceV2 = options.enablePerformanceV2;
# endif // SDK_V9
self.tracker = [[SentryAppStartTracker alloc]
initWithDispatchQueueWrapper:[[SentryDispatchQueueWrapper alloc] init]
appStateManager:appStateManager
framesTracker:SentryDependencyContainer.sharedInstance.framesTracker
enablePreWarmedAppStartTracing:options.enablePreWarmedAppStartTracing
enablePerformanceV2:options.enablePerformanceV2];
enablePerformanceV2:usePerformanceV2];
[self.tracker start];

return YES;
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/SentryOptions.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ - (instancetype)init
self.maxAttachmentSize = 20 * 1024 * 1024;
self.sendDefaultPii = NO;
self.enableAutoPerformanceTracing = YES;
#if !SDK_V9
self.enablePerformanceV2 = NO;
#endif // !SDK_V9
self.enablePersistingTracesWhenCrashing = NO;
self.enableCaptureFailedRequests = YES;
self.environment = kSentryDefaultEnvironment;
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/SentyOptionsInternal.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,10 @@ + (BOOL)validateOptions:(NSDictionary<NSString *, id> *)options
[self setBool:options[@"enableAutoPerformanceTracing"]
block:^(BOOL value) { sentryOptions.enableAutoPerformanceTracing = value; }];

#if !SDK_V9
[self setBool:options[@"enablePerformanceV2"]
block:^(BOOL value) { sentryOptions.enablePerformanceV2 = value; }];
#endif // !SDK_V9

[self setBool:options[@"enablePersistingTracesWhenCrashing"]
block:^(BOOL value) { sentryOptions.enablePersistingTracesWhenCrashing = value; }];
Expand Down
2 changes: 2 additions & 0 deletions Sources/Swift/Helper/SentryEnabledFeaturesBuilder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import Foundation
features.append("captureFailedRequests")
}

#if !SDK_V9
if options.enablePerformanceV2 {
features.append("performanceV2")
}
#endif // !SDK_V9

if options.enableTimeToFullDisplayTracing {
features.append("timeToFullDisplayTracing")
Expand Down
Loading