Skip to content

Commit

Permalink
WIP: fixing bug
Browse files Browse the repository at this point in the history
  • Loading branch information
armcknight committed Nov 19, 2022
1 parent 506261f commit c93f841
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 36 deletions.
2 changes: 2 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1399,6 +1399,7 @@
844DA81F28246DE300E6B62E /* scripts */ = {isa = PBXFileReference; lastKnownFileType = folder; path = scripts; sourceTree = "<group>"; };
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>"; };
84732A6D2928BD4B00790372 /* SentryProfiler+SwiftTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryProfiler+SwiftTest.h"; sourceTree = "<group>"; };
84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = "SentryProfiler+Test.h"; path = "Sources/Sentry/include/SentryProfiler+Test.h"; sourceTree = SOURCE_ROOT; };
84A8891A28DBD28900C51DFD /* SentryDevice.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDevice.h; path = include/SentryDevice.h; sourceTree = "<group>"; };
84A8891B28DBD28900C51DFD /* SentryDevice.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryDevice.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2771,6 +2772,7 @@
03F84D1B27DD414C008FE43F /* SentryMachLogging.hpp */,
03F84D2C27DD4191008FE43F /* SentryMachLogging.cpp */,
03F84D1127DD414C008FE43F /* SentryProfiler.h */,
84732A6D2928BD4B00790372 /* SentryProfiler+SwiftTest.h */,
84A888FC28D9B11700C51DFD /* SentryProfiler+Test.h */,
03F84D2B27DD4191008FE43F /* SentryProfiler.mm */,
03BCC38D27E2A377003232C7 /* SentryProfilingConditionals.h */,
Expand Down
1 change: 1 addition & 0 deletions Sources/Sentry/Profiling/SentryProfiler+SwiftTest.h
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FOUNDATION_EXPORT NSTimeInterval kSentryProfilerTimeoutInterval;
32 changes: 13 additions & 19 deletions Sources/Sentry/SentryProfiler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

const int kSentryProfilerFrequencyHz = 101;
NSString *const kTestStringConst = @"test";
NSTimeInterval kSentryProfilerTimeoutInterval = 30;

using namespace sentry::profiling;

Expand Down Expand Up @@ -208,19 +209,6 @@ - (instancetype)init

+ (void)startForSpanID:(SentrySpanId *)spanID hub:(SentryHub *)hub
{
# if SENTRY_TARGET_PROFILING_SUPPORTED
NSTimeInterval timeoutInterval = 30;
# if defined(TEST) || defined(TESTCI)
timeoutInterval = 1;
# endif
[self startForSpanID:spanID hub:hub timeoutInterval:timeoutInterval];
# endif
}

+ (void)startForSpanID:(SentrySpanId *)spanID
hub:(SentryHub *)hub
timeoutInterval:(NSTimeInterval)timeoutInterval
{
# if SENTRY_TARGET_PROFILING_SUPPORTED
std::lock_guard<std::mutex> l(_gProfilerLock);

Expand All @@ -235,7 +223,7 @@ + (void)startForSpanID:(SentrySpanId *)spanID
# endif // SENTRY_HAS_UIKIT
[_gCurrentProfiler start];
_gCurrentProfiler->_timeoutTimer =
[NSTimer scheduledTimerWithTimeInterval:timeoutInterval
[NSTimer scheduledTimerWithTimeInterval:kSentryProfilerTimeoutInterval
target:self
selector:@selector(timeoutAbort)
userInfo:nil
Expand Down Expand Up @@ -276,19 +264,18 @@ + (void)stopProfilingSpan:(id<SentrySpan>)span
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
}

+ (void)dropTransaction:(SentryTransaction *)transaction
+ (void)dropTransactionWithID:(SentrySpanId *)transactionID
{
# if SENTRY_TARGET_PROFILING_SUPPORTED
std::lock_guard<std::mutex> l(_gProfilerLock);

const auto spanID = transaction.trace.context.spanId;
const auto profiler = _gProfilersPerSpanID[spanID];
const auto profiler = _gProfilersPerSpanID[transactionID];
if (profiler == nil) {
SENTRY_LOG_DEBUG(@"No profiler tracking span with id %@", spanID.sentrySpanIdString);
SENTRY_LOG_DEBUG(@"No profiler tracking span with id %@", transactionID.sentrySpanIdString);
return;
}

[self captureEnvelopeIfFinished:profiler spanID:spanID];
[self captureEnvelopeIfFinished:profiler spanID:transactionID];
# endif // SENTRY_TARGET_PROFILING_SUPPORTED
}

Expand Down Expand Up @@ -347,6 +334,7 @@ + (void)timeoutAbort

SENTRY_LOG_DEBUG(@"Stopping profiler %@ due to timeout.", _gCurrentProfiler);
[self stopProfilerForReason:SentryProfilerTruncationReasonTimeout];
[_gCurrentProfiler captureEnvelope];
}

+ (void)backgroundAbort
Expand All @@ -360,6 +348,7 @@ + (void)backgroundAbort

SENTRY_LOG_DEBUG(@"Stopping profiler %@ due to timeout.", _gCurrentProfiler);
[self stopProfilerForReason:SentryProfilerTruncationReasonAppMovedToBackground];
[_gCurrentProfiler captureEnvelope];
}

+ (void)stopProfilerForReason:(SentryProfilerTruncationReason)reason
Expand Down Expand Up @@ -499,6 +488,11 @@ - (void)stop

- (void)captureEnvelope
{
if (_transactions.count == 0) {
SENTRY_LOG_DEBUG(@"No linked transactions, won't send profile.");
return;
}

NSMutableDictionary<NSString *, id> *profile = nil;
@synchronized(self) {
profile = [_profile mutableCopy];
Expand Down
16 changes: 12 additions & 4 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,11 @@ - (void)finishInternal
if (self.idleTimeout > 0.0 && _children.count == 0) {
SENTRY_LOG_DEBUG(@"Was waiting for timeout for UI event trace but it had no children, "
@"will not keep transaction.");

#if SENTRY_TARGET_PROFILING_SUPPORTED
[SentryProfiler stopProfilingSpan:self.rootSpan];
[SentryProfiler dropTransactionWithID:self.transactionContext.spanId];
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
return;
}

Expand All @@ -517,17 +522,20 @@ - (void)finishInternal

SentryTransaction *transaction = [self toTransaction];

// Prewarming can execute code up to viewDidLoad of a UIViewController, and keep the app in the
// background. This can lead to auto-generated transactions lasting for minutes or event hours.
// Therefore, we drop transactions lasting longer than SENTRY_AUTO_TRANSACTION_MAX_DURATION.
/**
* Prewarming can execute code up to viewDidLoad of a UIViewController, and keep the app in the
* background, leading to auto-generated transactions lasting for minutes or event hours. It
* could be that other circumstances could lead to long-lasting transactions as well. Therefore,
* as a safety measure for all auto-generated transactions, we drop transactions lasting longer
* than SENTRY_AUTO_TRANSACTION_MAX_DURATION. */
NSTimeInterval transactionDuration = [self.timestamp timeIntervalSinceDate:self.startTimestamp];
if ([self isAutoGeneratedTransaction]
&& transactionDuration >= SENTRY_AUTO_TRANSACTION_MAX_DURATION) {
SENTRY_LOG_INFO(@"Auto generated transaction exceeded the max duration of %f seconds. Not "
@"capturing transaction.",
SENTRY_AUTO_TRANSACTION_MAX_DURATION);
#if SENTRY_TARGET_PROFILING_SUPPORTED
[SentryProfiler dropTransaction:transaction];
[SentryProfiler dropTransactionWithID:self.transactionContext.spanId];
#endif // SENTRY_TARGET_PROFILING_SUPPORTED
return;
}
Expand Down
3 changes: 1 addition & 2 deletions Sources/Sentry/SentryUIEventTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,8 @@ - (void)start
return;
}

[currentActiveTransaction finish];

if (currentActiveTransaction) {
[currentActiveTransaction finish];
SENTRY_LOG_DEBUG(@"SentryUIEventTracker finished transaction %@ (span ID %@)",
currentActiveTransaction.transactionContext.name,
currentActiveTransaction.context.spanId.sentrySpanIdString);
Expand Down
3 changes: 3 additions & 0 deletions Sources/Sentry/include/SentryProfiler+Test.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// This header extension contains C++ symbols so cannot be included in a Swift bridging header. To
// export things to Swift, see SwntryProfiler+SwiftTest.h.

#include "SentryBacktrace.hpp"
#import "SentryProfiler.h"
#import "SentryProfilingConditionals.h"
Expand Down
5 changes: 2 additions & 3 deletions Sources/Sentry/include/SentryProfiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ SENTRY_EXTERN_C_END
/**
* Certain transactions may be dropped by the SDK at the time they are ended, when we've already
* been tracking them for profiling. This allows them to be removed from bookkeeping and finish
* profile if necessary.
* the profile if necessary.
*/
+ (void)dropTransaction:(SentryTransaction *)transaction;
;
+ (void)dropTransactionWithID:(SentrySpanId *)transactionID;

/**
* After the SDK creates a transaction for a span, link it to this profile. If it was the last
Expand Down
1 change: 1 addition & 0 deletions Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ class SentryFileManagerTests: XCTestCase {
}

func testDeleteOldEnvelopes() throws {
fixture.dispatchQueueWrapper.dispatchAfterExecutesBlock = true
let envelope = TestConstants.envelope
let path = sut.store(envelope)

Expand Down
6 changes: 5 additions & 1 deletion Tests/SentryTests/Networking/SentryHttpTransportTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ class SentryHttpTransportTests: XCTestCase {
let requestManager: TestRequestManager
let requestBuilder = TestNSURLRequestBuilder()
let rateLimits: DefaultRateLimits
let dispatchQueueWrapper = TestSentryDispatchQueueWrapper()
var dispatchQueueWrapper: TestSentryDispatchQueueWrapper = {
let dqw = TestSentryDispatchQueueWrapper()
dqw.dispatchAfterExecutesBlock = true
return dqw
}()
let reachability = TestSentryReachability()
let flushTimeout: TimeInterval = 0.5

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import Foundation
class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {

var dispatchAsyncCalled = 0
var dispatchAfterExecutesBlock = false
var delayDispatches = false

override func dispatchAsync(_ block: @escaping () -> Void) {
dispatchAsyncCalled += 1
Expand Down Expand Up @@ -30,7 +32,13 @@ class TestSentryDispatchQueueWrapper: SentryDispatchQueueWrapper {
override func dispatch(after interval: TimeInterval, block: @escaping () -> Void) {
dispatchAfterInvocations.record((interval, block))
if blockBeforeMainBlock() {
block()
if dispatchAfterExecutesBlock {
if delayDispatches {
DispatchQueue.main.asyncAfter(deadline: .now() + interval, execute: .init(block: block))
} else {
block()
}
}
}
}

Expand Down
Loading

0 comments on commit c93f841

Please sign in to comment.