Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@armcknight
armcknight committed May 15, 2024
1 parent ccf77d8 commit 8b6a180
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 23 deletions.
3 changes: 2 additions & 1 deletion Sources/Sentry/Profiling/SentryLaunchProfiling.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
SentryLaunchProfileConfig
sentry_shouldProfileNextLaunch(SentryOptions *options)
{
if (options.enableContinuousProfiling) {
if (options.enableAppLaunchProfiling && options.enableContinuousProfiling) {
return (SentryLaunchProfileConfig) { YES, nil, nil };
}

Expand Down Expand Up @@ -213,6 +213,7 @@
SENTRY_LOG_DEBUG(@"Finishing launch tracer.");
[sentry_launchTracer finish];
sentry_isTracingAppLaunch = NO;
sentry_launchTracer = nil;
}

NS_ASSUME_NONNULL_END
Expand Down
10 changes: 4 additions & 6 deletions Sources/Sentry/SentryTimeToDisplayTracker.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,9 @@ - (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate
if (!_waitForFullDisplay) {
[SentryDependencyContainer.sharedInstance.framesTracker removeListener:self];
# if SENTRY_TARGET_PROFILING_SUPPORTED
if (SentrySDK.options.enableContinuousProfiling) {
return;
if (!SentrySDK.options.enableContinuousProfiling) {

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 15 (17.2) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 15 (17.2) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 8 (15.5) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 8 (15.5) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / Release Build of iOS Swift 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 14 (16.4) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 14 (16.4) 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / Build app and test runner 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (15.2) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 143 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (15.2) Simulator 

use of undeclared identifier 'SentrySDK'
sentry_stopAndDiscardLaunchProfileTracer();
}
sentry_stopAndDiscardLaunchProfileTracer();
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
}
}
Expand All @@ -153,10 +152,9 @@ - (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate
self.fullDisplaySpan.timestamp = newFrameDate;
[self.fullDisplaySpan finish];
# if SENTRY_TARGET_PROFILING_SUPPORTED
if (SentrySDK.options.enableContinuousProfiling) {
return;
if (!SentrySDK.options.enableContinuousProfiling) {

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 15 (17.2) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 15 (17.2) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 8 (15.5) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 8 (15.5) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / Release Build of iOS Swift 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 14 (16.4) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / iOS-Swift UI Tests iPhone 14 (16.4) 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for ios_objc on Simulators 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (16.1) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / Build app and test runner 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests with Address Sanitizer 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (15.2) Simulator 

use of undeclared identifier 'SentrySDK'

Check failure on line 155 in Sources/Sentry/SentryTimeToDisplayTracker.m

View workflow job for this annotation

GitHub Actions / UI Tests for SwiftUI on iPhone 8 (15.2) Simulator 

use of undeclared identifier 'SentrySDK'
sentry_stopAndDiscardLaunchProfileTracer();
}
sentry_stopAndDiscardLaunchProfileTracer();
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
}

Expand Down
33 changes: 17 additions & 16 deletions Tests/SentryProfilerTests/SentryAppLaunchProfilingTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase {
// test continuous launch profiling configuration
func testContinuousLaunchProfileConfiguration() throws {
let options = Options()
options.enableAppLaunchProfiling = true
options.enableContinuousProfiling = true

// sample rates are not considered for continuous profiling
Expand Down Expand Up @@ -110,19 +111,19 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase {
(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: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: 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),
// 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: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 0, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 0, shouldProfileLaunch: true),
(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),
// 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),
Expand All @@ -145,18 +146,18 @@ final class SentryAppLaunchProfilingSwiftTests: XCTestCase {
(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: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: false, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(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: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 0, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 0, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(enableAppLaunchProfiling: false, enableTracing: true, enableContinuousProfiling: true, tracesSampleRate: 1, profilesSampleRate: 1, profilesSamplerReturnValue: 1, shouldProfileLaunch: true),
(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),
Expand Down

0 comments on commit 8b6a180

Please sign in to comment.