Skip to content

Commit

Permalink
fix: Finish TTID span when transaction finishes (#3610)
Browse files Browse the repository at this point in the history
The tracer finishes when the screen is fully displayed. Therefore, we
must also finish the TTID span.
  • Loading branch information
philipphofmann authored Feb 5, 2024
1 parent 24c8a0f commit 340fb46
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
### Fixes

- Don't use main thread for app hang screenshot and view hierarchy (#3547)
- Finish TTID span when transaction finishes (#3610)


## 8.20.0

Expand Down
16 changes: 16 additions & 0 deletions Sources/Sentry/SentryTimeToDisplayTracker.m
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,23 @@ - (void)startForTracer:(SentryTracer *)tracer
self.initialDisplaySpan.startTimestamp = tracer.startTimestamp;

[SentryDependencyContainer.sharedInstance.framesTracker addListener:self];

[tracer setShouldIgnoreWaitForChildrenCallback:^(id<SentrySpan> span) {
if (span.origin == SentryTraceOriginAutoUITimeToDisplay) {
return YES;
} else {
return NO;
}
}];
[tracer setFinishCallback:^(SentryTracer *_tracer) {
[SentryDependencyContainer.sharedInstance.framesTracker removeListener:self];

// The tracer finishes when the screen is fully displayed. Therefore, we must also finish
// the TTID span.
if (self.initialDisplaySpan.isFinished == NO) {
[self.initialDisplaySpan finish];
}

// If the start time of the tracer changes, which is the case for app start transactions, we
// also need to adapt the start time of our spans.
self.initialDisplaySpan.startTimestamp = _tracer.startTimestamp;
Expand Down
8 changes: 8 additions & 0 deletions Sources/Sentry/SentryTracer.m
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,10 @@ - (BOOL)hasUnfinishedChildSpansToWaitFor

@synchronized(_children) {
for (id<SentrySpan> span in _children) {
if (self.shouldIgnoreWaitForChildrenCallback != nil
&& self.shouldIgnoreWaitForChildrenCallback(span)) {
continue;
}
if (![span isFinished])
return YES;
}
Expand Down Expand Up @@ -519,6 +523,10 @@ - (void)finishInternal
self.finishCallback = nil;
}

if (self.shouldIgnoreWaitForChildrenCallback) {
self.shouldIgnoreWaitForChildrenCallback = nil;
}

// 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 even hours.
// Therefore, we drop transactions lasting longer than SENTRY_AUTO_TRANSACTION_MAX_DURATION.
Expand Down
2 changes: 2 additions & 0 deletions Sources/Sentry/include/SentryTracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ static const NSTimeInterval SENTRY_AUTO_TRANSACTION_MAX_DURATION = 500.0;

@property (nullable, nonatomic, copy) void (^finishCallback)(SentryTracer *);

@property (nullable, nonatomic, copy) BOOL (^shouldIgnoreWaitForChildrenCallback)(id<SentrySpan>);

/**
* Retrieves a trace context from this tracer.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,33 @@ class SentryTimeToDisplayTrackerTest: XCTestCase {

expect(Dynamic(self.fixture.framesTracker).listeners.count) == 0
}

func testTracerFinishesBeforeReportInitialDisplay_FinishesInitialDisplaySpan() throws {
let sut = fixture.getSut(for: UIViewController(), waitForFullDisplay: false)

fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 7))
let tracer = try fixture.getTracer()

sut.start(for: tracer)
expect(tracer.children.count) == 1
expect(Dynamic(self.fixture.framesTracker).listeners.count) == 1

let ttidSpan = try XCTUnwrap(tracer.children.first, "Expected a TTID span")
expect(ttidSpan.startTimestamp) == fixture.dateProvider.date()

fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 9))

tracer.finish()

expect(ttidSpan.timestamp) == fixture.dateProvider.date()
expect(ttidSpan.isFinished) == true
expect(ttidSpan.spanDescription) == "UIViewController initial display"
expect(ttidSpan.status) == .ok

assertMeasurement(tracer: tracer, name: "time_to_initial_display", duration: 2_000)

expect(Dynamic(self.fixture.framesTracker).listeners.count) == 0
}

func testCheckInitialTime() throws {
fixture.dateProvider.setDate(date: Date(timeIntervalSince1970: 9))
Expand Down
25 changes: 25 additions & 0 deletions Tests/SentryTests/Transaction/SentryTracerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,31 @@ class SentryTracerTests: XCTestCase {
XCTAssertEqual(tracerTimestamp.timeIntervalSince1970, span["timestamp"] as? TimeInterval)
}
}

func testFinish_ShouldIgnoreWaitForChildrenCallback() throws {
let sut = fixture.getSut()

sut.shouldIgnoreWaitForChildrenCallback = { _ in
return true
}
let child = sut.startChild(operation: fixture.transactionOperation)
sut.finish()

expect(child.status) == .deadlineExceeded

assertOneTransactionCaptured(sut)

let serialization = try getSerializedTransaction()
let spans = serialization["spans"]! as! [[String: Any]]

let tracerTimestamp: NSDate = sut.timestamp! as NSDate

expect(spans.count) == 1
let span = try XCTUnwrap(spans.first, "Expected first span not to be nil")
expect(span["timestamp"] as? TimeInterval) == tracerTimestamp.timeIntervalSince1970

expect(sut.shouldIgnoreWaitForChildrenCallback) == nil
}

func testDeadlineTimer_FinishesTransactionAndChildren() {
fixture.dispatchQueue.blockBeforeMainBlock = { true }
Expand Down

0 comments on commit 340fb46

Please sign in to comment.