Skip to content

Commit

Permalink
ref(profiling): move test helpers to separate code unit (#3873)
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight committed Apr 23, 2024
1 parent 6e31b7c commit 42f4107
Show file tree
Hide file tree
Showing 10 changed files with 125 additions and 79 deletions.
8 changes: 8 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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 */; };
Expand Down Expand Up @@ -1681,6 +1683,8 @@
8453421128BE855D00C22EEC /* SentrySampleDecision.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySampleDecision.m; sourceTree = "<group>"; };
8453421528BE8A9500C22EEC /* SentrySpanStatus.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySpanStatus.m; sourceTree = "<group>"; };
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 = "<group>"; };
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 = "<group>"; };
8489B8872A5F7905009A055A /* SentryThreadWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = SentryThreadWrapperTests.swift; path = Helper/SentryThreadWrapperTests.swift; sourceTree = "<group>"; };
848A45172BBF8D33006AAAEC /* SentryContinuousProfiler.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryContinuousProfiler.h; path = ../include/SentryContinuousProfiler.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down Expand Up @@ -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 */,
Expand Down
74 changes: 74 additions & 0 deletions Sources/Sentry/Profiling/SentryProfilerTestHelpers.m
Original file line number Diff line number Diff line change
@@ -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<NSString *, id> *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
71 changes: 4 additions & 67 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -362,54 +347,6 @@ + (nullable SentryEnvelopeItem *)createEnvelopeItemForProfilePayload:
return [[SentryEnvelopeItem alloc] initWithHeader:header data:JSONData];
}

# if defined(TEST) || defined(TESTCI) || defined(DEBUG)
void
writeProfileFile(NSDictionary<NSString *, id> *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<NSString *, id> *)collectProfileBetween:(uint64_t)startSystemTime
and:(uint64_t)endSystemTime
forTrace:(SentryId *)traceId
Expand All @@ -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;
Expand Down Expand Up @@ -511,7 +448,7 @@ - (void)startMetricProfiler

- (void)start
{
if (threadSanitizerIsPresent()) {
if (sentry_threadSanitizerIsPresent()) {
SENTRY_LOG_DEBUG(@"Disabling profiling when running with TSAN");
return;
}
Expand Down
6 changes: 0 additions & 6 deletions Sources/Sentry/include/SentryProfiler+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions Sources/Sentry/include/SentryProfilerTestHelpers.h
Original file line number Diff line number Diff line change
@@ -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 <Foundation/Foundation.h>

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<NSString *, id> *payload);

# endif // defined(TEST) || defined(TESTCI) || defined(DEBUG)

NS_ASSUME_NONNULL_END

#endif // SENTRY_TARGET_PROFILING_SUPPORTED
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/SentryTests/PrivateSentrySDKOnlyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Tests/SentryTests/SentryTests-Bridging-Header.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions Tests/SentryTests/Transaction/SentryTracerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 42f4107

Please sign in to comment.