From 42f4107997def601a9f8782c44a51d150f2dbcbf Mon Sep 17 00:00:00 2001 From: Andrew McKnight Date: Tue, 23 Apr 2024 10:15:56 -0800 Subject: [PATCH] ref(profiling): move test helpers to separate code unit (#3873) --- Sentry.xcodeproj/project.pbxproj | 8 ++ .../Profiling/SentryProfilerTestHelpers.m | 74 +++++++++++++++++++ Sources/Sentry/SentryProfiler.mm | 71 +----------------- .../Sentry/include/SentryProfiler+Private.h | 6 -- .../include/SentryProfilerTestHelpers.h | 32 ++++++++ .../SentryFramesTrackerTests.swift | 2 +- .../PrivateSentrySDKOnlyTests.swift | 2 +- .../SentryStacktraceBuilderTests.swift | 4 +- .../SentryTests/SentryTests-Bridging-Header.h | 1 + .../Transaction/SentryTracerTests.swift | 4 +- 10 files changed, 125 insertions(+), 79 deletions(-) create mode 100644 Sources/Sentry/Profiling/SentryProfilerTestHelpers.m create mode 100644 Sources/Sentry/include/SentryProfilerTestHelpers.h diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 714ff6a32ab..b67eabf35e9 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -667,6 +667,8 @@ 8453421228BE855D00C22EEC /* SentrySampleDecision.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421128BE855D00C22EEC /* SentrySampleDecision.m */; }; 8453421628BE8A9500C22EEC /* SentrySpanStatus.m in Sources */ = {isa = PBXBuildFile; fileRef = 8453421528BE8A9500C22EEC /* SentrySpanStatus.m */; }; 8454CF8D293EAF9A006AC140 /* SentryMetricProfiler.mm in Sources */ = {isa = PBXBuildFile; fileRef = 8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */; }; + 8459FCB72BD737AC0038E9C9 /* SentryProfilerTestHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = 8459FCB62BD737AC0038E9C9 /* SentryProfilerTestHelpers.h */; }; + 8459FCB82BD738CE0038E9C9 /* SentryProfilerTestHelpers.m in Sources */ = {isa = PBXBuildFile; fileRef = 8459FCB32BD737250038E9C9 /* SentryProfilerTestHelpers.m */; }; 845C16D52A622A5B00EC9519 /* SentryTracer+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 845C16D42A622A5B00EC9519 /* SentryTracer+Private.h */; }; 8489B8882A5F7905009A055A /* SentryThreadWrapperTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8489B8872A5F7905009A055A /* SentryThreadWrapperTests.swift */; }; 848A45192BBF8D33006AAAEC /* SentryContinuousProfiler.m in Sources */ = {isa = PBXBuildFile; fileRef = 848A45182BBF8D33006AAAEC /* SentryContinuousProfiler.m */; }; @@ -1681,6 +1683,8 @@ 8453421128BE855D00C22EEC /* SentrySampleDecision.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySampleDecision.m; sourceTree = ""; }; 8453421528BE8A9500C22EEC /* SentrySpanStatus.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySpanStatus.m; sourceTree = ""; }; 8454CF8B293EAF9A006AC140 /* SentryMetricProfiler.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; name = SentryMetricProfiler.mm; path = Sources/Sentry/SentryMetricProfiler.mm; sourceTree = SOURCE_ROOT; }; + 8459FCB32BD737250038E9C9 /* SentryProfilerTestHelpers.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryProfilerTestHelpers.m; sourceTree = ""; }; + 8459FCB62BD737AC0038E9C9 /* SentryProfilerTestHelpers.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryProfilerTestHelpers.h; path = Sources/Sentry/include/SentryProfilerTestHelpers.h; sourceTree = SOURCE_ROOT; }; 845C16D42A622A5B00EC9519 /* SentryTracer+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentryTracer+Private.h"; path = "include/SentryTracer+Private.h"; sourceTree = ""; }; 8489B8872A5F7905009A055A /* SentryThreadWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = SentryThreadWrapperTests.swift; path = Helper/SentryThreadWrapperTests.swift; sourceTree = ""; }; 848A45172BBF8D33006AAAEC /* SentryContinuousProfiler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryContinuousProfiler.h; path = ../include/SentryContinuousProfiler.h; sourceTree = ""; }; @@ -3318,6 +3322,8 @@ 84AF45A529A7FFA500FBB177 /* SentryProfiledTracerConcurrency.mm */, 840B7EF22BBF83DF008B8120 /* SentryProfiler+Private.h */, 03F84D2B27DD4191008FE43F /* SentryProfiler.mm */, + 8459FCB62BD737AC0038E9C9 /* SentryProfilerTestHelpers.h */, + 8459FCB32BD737250038E9C9 /* SentryProfilerTestHelpers.m */, 848A45172BBF8D33006AAAEC /* SentryContinuousProfiler.h */, 848A45182BBF8D33006AAAEC /* SentryContinuousProfiler.m */, 0354A22A2A134D9C003C3A04 /* SentryProfilerState.h */, @@ -4016,6 +4022,7 @@ 7B3B83722833832B0001FDEB /* SentrySpanOperations.h in Headers */, 7BF9EF722722A84800B5BBEF /* SentryClassRegistrator.h in Headers */, D86B7B5C2B7A529C0017E8D9 /* SentryReplayEvent.h in Headers */, + 8459FCB72BD737AC0038E9C9 /* SentryProfilerTestHelpers.h in Headers */, 63FE715520DA4C1100CDBAE8 /* SentryCrashStackCursor_MachineContext.h in Headers */, 62E081A929ED4260000F69FC /* SentryBreadcrumbDelegate.h in Headers */, 15360CF02433A16D00112302 /* SentryInstallation.h in Headers */, @@ -4590,6 +4597,7 @@ 620379DD2AFE1432005AC0C1 /* SentryBuildAppStartSpans.m in Sources */, 62991A8D2BAC1B4A0078A8B8 /* SentryMetricsAPI.swift in Sources */, 7B77BE3727EC8460003C9020 /* SentryDiscardReasonMapper.m in Sources */, + 8459FCB82BD738CE0038E9C9 /* SentryProfilerTestHelpers.m in Sources */, 63FE712520DA4C1000CDBAE8 /* SentryCrashSignalInfo.c in Sources */, 63FE70F320DA4C1000CDBAE8 /* SentryCrashMonitor_Signal.c in Sources */, D859696F27BECDA20036A46E /* SentryCoreDataTracker.m in Sources */, diff --git a/Sources/Sentry/Profiling/SentryProfilerTestHelpers.m b/Sources/Sentry/Profiling/SentryProfilerTestHelpers.m new file mode 100644 index 00000000000..f1eb44fef1f --- /dev/null +++ b/Sources/Sentry/Profiling/SentryProfilerTestHelpers.m @@ -0,0 +1,74 @@ +#import "SentryProfilerTestHelpers.h" + +#if SENTRY_TARGET_PROFILING_SUPPORTED + +# import "SentryFileManager.h" +# import "SentryInternalDefines.h" +# import "SentryLaunchProfiling.h" +# import "SentrySerialization.h" + +BOOL +sentry_threadSanitizerIsPresent(void) +{ +# if defined(__has_feature) +# if __has_feature(thread_sanitizer) + return YES; +# pragma clang diagnostic push +# pragma clang diagnostic ignored "-Wunreachable-code" +# endif // __has_feature(thread_sanitizer) +# endif // defined(__has_feature) + + return NO; +} + +# if defined(TEST) || defined(TESTCI) || defined(DEBUG) + +void +sentry_writeProfileFile(NSDictionary *payload) +{ + NSData *data = [SentrySerialization dataWithJSONObject:payload]; + NSFileManager *fm = [NSFileManager defaultManager]; + NSString *appSupportDirPath = sentryApplicationSupportPath(); + + if (![fm fileExistsAtPath:appSupportDirPath]) { + SENTRY_LOG_DEBUG(@"Creating app support directory."); + NSError *error; + if (!SENTRY_CASSERT_RETURN([fm createDirectoryAtPath:appSupportDirPath + withIntermediateDirectories:NO + attributes:nil + error:&error], + @"Failed to create sentry app support directory")) { + return; + } + } else { + SENTRY_LOG_DEBUG(@"App support directory already exists."); + } + + NSString *pathToWrite; + if (sentry_isTracingAppLaunch) { + SENTRY_LOG_DEBUG(@"Writing app launch profile."); + pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"launchProfile"]; + } else { + SENTRY_LOG_DEBUG(@"Overwriting last non-launch profile."); + pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"]; + } + + if ([fm fileExistsAtPath:pathToWrite]) { + SENTRY_LOG_DEBUG(@"Already a %@ profile file present; make sure to remove them right after " + @"using them, and that tests clean state in between so there isn't " + @"leftover config producing one when it isn't expected.", + sentry_isTracingAppLaunch ? @" launch" : @""); + return; + } + + SENTRY_LOG_DEBUG(@"Writing%@ profile to file.", sentry_isTracingAppLaunch ? @" launch" : @""); + + NSError *error; + if (![data writeToFile:pathToWrite options:NSDataWritingAtomic error:&error]) { + SENTRY_LOG_ERROR(@"Failed to write data to path %@: %@", pathToWrite, error); + } +} + +# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG) + +#endif // SENTRY_TARGET_PROFILING_SUPPORTED diff --git a/Sources/Sentry/SentryProfiler.mm b/Sources/Sentry/SentryProfiler.mm index 5d428129268..f4a0af72abf 100644 --- a/Sources/Sentry/SentryProfiler.mm +++ b/Sources/Sentry/SentryProfiler.mm @@ -29,6 +29,7 @@ # import "SentryProfileTimeseries.h" # import "SentryProfiledTracerConcurrency.h" # import "SentryProfilerState+ObjCpp.h" +# import "SentryProfilerTestHelpers.h" # import "SentrySDK+Private.h" # import "SentrySample.h" # import "SentrySamplingProfiler.hpp" @@ -53,9 +54,7 @@ # endif // SENTRY_HAS_UIKIT # if defined(TEST) || defined(TESTCI) || defined(DEBUG) -# import "SentryFileManager.h" -# import "SentryInternalDefines.h" -# import "SentryLaunchProfiling.h" +# import "SentryProfilerTestHelpers.h" # endif // defined(TEST) || defined(TESTCI) || defined(DEBUG) const int kSentryProfilerFrequencyHz = 101; @@ -70,20 +69,6 @@ std::mutex _gProfilerLock; SentryProfiler *_Nullable _gCurrentProfiler; -BOOL -threadSanitizerIsPresent(void) -{ -# if defined(__has_feature) -# if __has_feature(thread_sanitizer) - return YES; -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wunreachable-code" -# endif // __has_feature(thread_sanitizer) -# endif // defined(__has_feature) - - return NO; -} - NSString * profilerTruncationReasonName(SentryProfilerTruncationReason reason) { @@ -362,54 +347,6 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload: return [[SentryEnvelopeItem alloc] initWithHeader:header data:JSONData]; } -# if defined(TEST) || defined(TESTCI) || defined(DEBUG) -void -writeProfileFile(NSDictionary *payload) -{ - NSData *data = [SentrySerialization dataWithJSONObject:payload]; - NSFileManager *fm = [NSFileManager defaultManager]; - NSString *appSupportDirPath = sentryApplicationSupportPath(); - - if (![fm fileExistsAtPath:appSupportDirPath]) { - SENTRY_LOG_DEBUG(@"Creating app support directory."); - NSError *error; - if (!SENTRY_CASSERT_RETURN([fm createDirectoryAtPath:appSupportDirPath - withIntermediateDirectories:NO - attributes:nil - error:&error], - @"Failed to create sentry app support directory")) { - return; - } - } else { - SENTRY_LOG_DEBUG(@"App support directory already exists."); - } - - NSString *pathToWrite; - if (sentry_isTracingAppLaunch) { - SENTRY_LOG_DEBUG(@"Writing app launch profile."); - pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"launchProfile"]; - } else { - SENTRY_LOG_DEBUG(@"Overwriting last non-launch profile."); - pathToWrite = [appSupportDirPath stringByAppendingPathComponent:@"profile"]; - } - - if ([fm fileExistsAtPath:pathToWrite]) { - SENTRY_LOG_DEBUG(@"Already a %@ profile file present; make sure to remove them right after " - @"using them, and that tests clean state in between so there isn't " - @"leftover config producing one when it isn't expected.", - sentry_isTracingAppLaunch ? @" launch" : @""); - return; - } - - SENTRY_LOG_DEBUG(@"Writing%@ profile to file.", sentry_isTracingAppLaunch ? @" launch" : @""); - - NSError *error; - if (![data writeToFile:pathToWrite options:NSDataWritingAtomic error:&error]) { - SENTRY_LOG_ERROR(@"Failed to write data to path %@: %@", pathToWrite, error); - } -} -# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG) - + (nullable NSMutableDictionary *)collectProfileBetween:(uint64_t)startSystemTime and:(uint64_t)endSystemTime forTrace:(SentryId *)traceId @@ -423,7 +360,7 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload: const auto payload = [profiler serializeBetween:startSystemTime and:endSystemTime onHub:hub]; # if defined(TEST) || defined(TESTCI) || defined(DEBUG) - writeProfileFile(payload); + sentry_writeProfileFile(payload); # endif // defined(TEST) || defined(TESTCI) || defined(DEBUG) return payload; @@ -511,7 +448,7 @@ - (void)startMetricProfiler - (void)start { - if (threadSanitizerIsPresent()) { + if (sentry_threadSanitizerIsPresent()) { SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN"); return; } diff --git a/Sources/Sentry/include/SentryProfiler+Private.h b/Sources/Sentry/include/SentryProfiler+Private.h index 41235225837..aa1fd06af55 100644 --- a/Sources/Sentry/include/SentryProfiler+Private.h +++ b/Sources/Sentry/include/SentryProfiler+Private.h @@ -32,12 +32,6 @@ SENTRY_EXTERN NSString *const kSentryProfilerSerializationKeyFrameRates; SENTRY_EXTERN_C_BEGIN -/** - * Disable profiling when running with TSAN because it produces a TSAN false positive, similar to - * the situation described here: https://github.com/envoyproxy/envoy/issues/2561 - */ -BOOL threadSanitizerIsPresent(void); - NSString *profilerTruncationReasonName(SentryProfilerTruncationReason reason); SENTRY_EXTERN_C_END diff --git a/Sources/Sentry/include/SentryProfilerTestHelpers.h b/Sources/Sentry/include/SentryProfilerTestHelpers.h new file mode 100644 index 00000000000..4cb9d4e340c --- /dev/null +++ b/Sources/Sentry/include/SentryProfilerTestHelpers.h @@ -0,0 +1,32 @@ +/** + * These declarations are needed in both SDK and test code, for use with various testing scenarios. + */ + +#import "SentryProfilingConditionals.h" + +#if SENTRY_TARGET_PROFILING_SUPPORTED + +# import "SentryDefines.h" +# import + +NS_ASSUME_NONNULL_BEGIN + +/** + * Disable profiling when running with TSAN because it produces a TSAN false positive, similar to + * the situation described here: https://github.com/envoyproxy/envoy/issues/2561 + */ +SENTRY_EXTERN BOOL sentry_threadSanitizerIsPresent(void); + +# if defined(TEST) || defined(TESTCI) || defined(DEBUG) + +/** + * Write a file to application support containing the profile data. This is an affordance for UI + * tests to be able to validate the contents of a profile. + */ +SENTRY_EXTERN void sentry_writeProfileFile(NSDictionary *payload); + +# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG) + +NS_ASSUME_NONNULL_END + +#endif // SENTRY_TARGET_PROFILING_SUPPORTED diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift index 001a0fb6164..11a974dacc9 100644 --- a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift @@ -684,7 +684,7 @@ private extension SentryFramesTrackerTests { #if os(iOS) || os(macOS) || targetEnvironment(macCatalyst) func assertProfilingData(slow: UInt? = nil, frozen: UInt? = nil, frameRates: UInt? = nil) throws { - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { // profiling data will not have been gathered with TSAN running return } diff --git a/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift b/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift index 388e6ede39e..50be9bd9e98 100644 --- a/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift +++ b/Tests/SentryTests/PrivateSentrySDKOnlyTests.swift @@ -125,7 +125,7 @@ class PrivateSentrySDKOnlyTests: XCTestCase { * Smoke Tests profiling via PrivateSentrySDKOnly. Actual profiling unit tests are done elsewhere. */ func testProfilingStartAndCollect() throws { - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { throw XCTSkip("Profiler does not run if thread sanitizer is attached.") } let options = Options() diff --git a/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift b/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift index 7a7d9242deb..c732370956f 100644 --- a/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryStacktraceBuilderTests.swift @@ -98,7 +98,7 @@ class SentryStacktraceBuilderTests: XCTestCase { var timeout: TimeInterval = 1 #if !os(watchOS) && !os(tvOS) // observed the async task taking a long time to finish if TSAN is attached - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { timeout = 10 } #endif // !os(watchOS) || !os(tvOS) @@ -128,7 +128,7 @@ class SentryStacktraceBuilderTests: XCTestCase { var timeout: TimeInterval = 1 #if !os(watchOS) && !os(tvOS) // observed the async task taking a long time to finish if TSAN is attached - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { timeout = 10 } #endif // !os(watchOS) || !os(tvOS) diff --git a/Tests/SentryTests/SentryTests-Bridging-Header.h b/Tests/SentryTests/SentryTests-Bridging-Header.h index dd54ed6b6b0..257d7c69bdc 100644 --- a/Tests/SentryTests/SentryTests-Bridging-Header.h +++ b/Tests/SentryTests/SentryTests-Bridging-Header.h @@ -32,6 +32,7 @@ # import "SentryProfiler+Test.h" # import "SentryProfilerMocksSwiftCompatible.h" # import "SentryProfilerState.h" +# import "SentryProfilerTestHelpers.h" #endif // SENTRY_TARGET_PROFILING_SUPPORTED #import "NSLocale+Sentry.h" diff --git a/Tests/SentryTests/Transaction/SentryTracerTests.swift b/Tests/SentryTests/Transaction/SentryTracerTests.swift index 8a8237e1d89..11e87d8ebd9 100644 --- a/Tests/SentryTests/Transaction/SentryTracerTests.swift +++ b/Tests/SentryTests/Transaction/SentryTracerTests.swift @@ -256,7 +256,7 @@ class SentryTracerTests: XCTestCase { func testCancelDeadlineTimer_TracerDeallocated() throws { #if !os(tvOS) && !os(watchOS) - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { throw XCTSkip("doesn't currently work with TSAN enabled. the tracer instance remains retained by something in the TSAN dylib, and we cannot debug the memory graph with TSAN attached to see what is retaining it. it's likely out of our control.") } #endif // !os(tvOS) && !os(watchOS) @@ -624,7 +624,7 @@ class SentryTracerTests: XCTestCase { func testIdleTimeout_TracerDeallocated() throws { #if !os(tvOS) && !os(watchOS) - if threadSanitizerIsPresent() { + if sentry_threadSanitizerIsPresent() { throw XCTSkip("doesn't currently work with TSAN enabled. the tracer instance remains retained by something in the TSAN dylib, and we cannot debug the memory graph with TSAN attached to see what is retaining it. it's likely out of our control.") } #endif // !os(tvOS) && !os(watchOS)