Skip to content
New issue

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

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

change(profiling): deprecate old trace-and continuous profiling API #4854

Draft
wants to merge 3 commits into
base: armcknight/profiling/new-continuous-apis/0-topic-branch
Choose a base branch
from
Draft

change(profiling): deprecate old trace-and continuous profiling API #4854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bee4d74
change(profiling): deprecate old trace-and continuous profiling API
armcknight Feb 14, 2025
c277fa0
NOMAIN: temporarily disable warnings as errors until we figure out ho…
armcknight Feb 14, 2025
3b04d2d
add doc comment entries
armcknight Feb 18, 2025
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
5 changes: 2 additions & 3 deletions Sources/Configuration/SDK.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@ ENABLE_NS_ASSERTIONS = $(ENABLE_NS_ASSERTIONS_$(CONFIGURATION))

ENABLE_STRICT_OBJC_MSGSEND = YES

GCC_TREAT_WARNINGS_AS_ERRORS = YES

CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES
CLANG_WARN_COMMA = YES
CLANG_WARN_DOCUMENTATION_COMMENTS = YES
Expand Down Expand Up @@ -163,7 +161,8 @@ SWIFT_OPTIMIZATION_LEVEL_Test = -Onone
SWIFT_OPTIMIZATION_LEVEL_TestCI = -Onone
SWIFT_OPTIMIZATION_LEVEL = $(SWIFT_OPTIMIZATION_LEVEL_$(CONFIGURATION))

SWIFT_TREAT_WARNINGS_AS_ERRORS = YES
GCC_TREAT_WARNINGS_AS_ERRORS = NO
SWIFT_TREAT_WARNINGS_AS_ERRORS = NO
Comment on lines +164 to +165
Copy link
Member

Choose a reason for hiding this comment

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

h: Ah 😡 , this is again a point where Swift sucks pretty much compared to ObjC. I didn't find a way to only treat deprecations as warnings or properly ignore deprecation warnings. I thought it's possible but I think it's not. The problem is that we open the door to ignore new compiler warnings, which could cause bugs. I enabled treat warnings as errors some time ago because a warning led to a severe bug, but I can't recall which one.

If we disable warnings as errors, we need to ensure we're not introducing new warnings by accident. We could have an allowed warning list in a file and then compare it with the Xcode output or something. This should run in CI and fail if there's a new warning that's not on that list, in my opinion. We shouldn't do this in this PR thought. Maybe using the weird deprecated workaround for once more is acdeptable.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, i'll try the deprecation workaround you showed me. this change turning off warnings as errors isn't meant to be merged.


MTL_ENABLE_DEBUG_INFO_Debug = YES
MTL_ENABLE_DEBUG_INFO_DebugWithoutUIKit = YES
Expand Down
22 changes: 18 additions & 4 deletions Sources/Sentry/Public/SentryOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,13 @@ NS_SWIFT_NAME(Options)
* 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.
* @deprecated See @c SentryProfilingOptions.startOnAppStart and
* @c SentryProfilingOptions.lifecycle .
*/
@property (nonatomic, assign) BOOL enableAppLaunchProfiling;
@property (nonatomic, assign) BOOL enableAppLaunchProfiling DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK. See "
"SentryProfilingOptions.startOnAppStart and "
"SentryProfilingOptions.lifecycle.");

/**
* @note Profiling is not supported on watchOS or tvOS.
Expand All @@ -549,8 +554,11 @@ NS_SWIFT_NAME(Options)
* @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.
* @deprecated See @c SentryProfilingOptions.sessionSampleRate .
*/
@property (nullable, nonatomic, strong) NSNumber *profilesSampleRate;
@property (nullable, nonatomic, strong) NSNumber *profilesSampleRate DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK. See "
"SentryProfilingOptions.sessionSampleRate.");

/**
* @note Profiling is not supported on watchOS or tvOS.
Expand All @@ -561,8 +569,12 @@ NS_SWIFT_NAME(Options)
* with @c SentrySamplingContext.forNextAppLaunch set to @c YES, and the result will be persisted to
* disk for use on the next app launch.
* @note Profiling is automatically disabled if a thread sanitizer is attached.
* @deprecated See @c SentryProfilingOptions.sessionSampleRate .
*/
@property (nullable, nonatomic) SentryTracesSamplerCallback profilesSampler NS_SWIFT_SENDABLE;
@property (nullable, nonatomic)
SentryTracesSamplerCallback profilesSampler NS_SWIFT_SENDABLE DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK. See "
"SentryProfilingOptions.sessionSampleRate.");

/**
* If profiling should be enabled or not.
Expand All @@ -573,8 +585,10 @@ NS_SWIFT_NAME(Options)
* @returns @c YES if either @c profilesSampleRate > @c 0 and \<= @c 1 , or @c profilesSampler is
* set, otherwise @c NO.
* @note Profiling is automatically disabled if a thread sanitizer is attached.
* @deprecated This property will be removed in a future release.
*/
@property (nonatomic, assign, readonly) BOOL isProfilingEnabled;
@property (nonatomic, assign, readonly) BOOL isProfilingEnabled DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK.");

/**
* @brief Whether to enable the sampling profiler.
Expand Down
6 changes: 4 additions & 2 deletions Sources/Sentry/Public/SentrySDK.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,16 @@ SENTRY_NO_INIT
* @warning Continuous profiling mode is experimental and may still contain bugs.
* @seealso https://docs.sentry.io/platforms/apple/guides/ios/profiling/#continuous-profiling
*/
+ (void)startProfiler;
+ (void)startProfiler DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK. See startProfileSession.");

/**
* 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/guides/ios/profiling/#continuous-profiling
*/
+ (void)stopProfiler;
+ (void)stopProfiler DEPRECATED_MSG_ATTRIBUTE(
"This property will be removed in a future version of the SDK. See stopProfileSession.");
#endif // SENTRY_TARGET_PROFILING_SUPPORTED

@end
Expand Down
3 changes: 3 additions & 0 deletions Tests/Configuration/SentryTests.xcconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ CLANG_ENABLE_OBJC_ARC = YES
CLANG_ENABLE_MODULES = YES
OTHER_CFLAGS = -Wall -Wextra
APPLICATION_EXTENSION_API_ONLY = NO

GCC_TREAT_WARNINGS_AS_ERRORS = NO
SWIFT_TREAT_WARNINGS_AS_ERRORS = NO
Loading