Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use correct rendered frames timestamp #3531

Merged
merged 5 commits into from Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,8 @@
- Crash when adding a crumb for a timezone change (#3524)
- Fix a race condition in SentryTracer (#3523)
- App start ends when first frame is drawn when performanceV2 is enabled (#3530)
- Use correct rendered frames timestamp for TTID/TTFD and app start (#3531)

- Missing transactions when not calling `reportFullyDisplayed` (#3477)

## 8.17.2
Expand Down
3 changes: 2 additions & 1 deletion SentryTestUtils/TestCurrentDateProvider.swift
Expand Up @@ -6,12 +6,13 @@ public class TestCurrentDateProvider: CurrentDateProvider {
private var internalDate = defaultStartingDate
private var internalSystemTime: UInt64 = 0
public var driftTimeForEveryRead = false
public var driftTimeInterval = 0.1

public override func date() -> Date {

defer {
if driftTimeForEveryRead {
internalDate = internalDate.addingTimeInterval(0.1)
internalDate = internalDate.addingTimeInterval(driftTimeInterval)
}
}

Expand Down
19 changes: 10 additions & 9 deletions Sources/Sentry/SentryAppStartTracker.m
Expand Up @@ -120,7 +120,8 @@
object:nil];

if (PrivateSentrySDKOnly.appStartMeasurementHybridSDKMode) {
[self buildAppStartMeasurement];
[self
buildAppStartMeasurement:[SentryDependencyContainer.sharedInstance.dateProvider date]];
}

# if SENTRY_HAS_UIKIT
Expand All @@ -130,7 +131,7 @@
self.isRunning = YES;
}

- (void)buildAppStartMeasurement
- (void)buildAppStartMeasurement:(NSDate *)appStartEnd
{
void (^block)(void) = ^(void) {
[self stop];
Expand Down Expand Up @@ -181,12 +182,11 @@
NSDate *appStartTimestamp;
SentrySysctl *sysctl = SentryDependencyContainer.sharedInstance.sysctlWrapper;
if (isPreWarmed) {
appStartDuration = [[SentryDependencyContainer.sharedInstance.dateProvider date]
timeIntervalSinceDate:sysctl.moduleInitializationTimestamp];
appStartDuration =
[appStartEnd timeIntervalSinceDate:sysctl.moduleInitializationTimestamp];

Check warning on line 186 in Sources/Sentry/SentryAppStartTracker.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryAppStartTracker.m#L185-L186

Added lines #L185 - L186 were not covered by tests
appStartTimestamp = sysctl.moduleInitializationTimestamp;
} else {
appStartDuration = [[SentryDependencyContainer.sharedInstance.dateProvider date]
timeIntervalSinceDate:sysctl.processStartTimestamp];
appStartDuration = [appStartEnd timeIntervalSinceDate:sysctl.processStartTimestamp];
appStartTimestamp = sysctl.processStartTimestamp;
}

Expand Down Expand Up @@ -240,16 +240,17 @@
- (void)didBecomeVisible
{
if (!_enablePerformanceV2) {
[self buildAppStartMeasurement];
[self
buildAppStartMeasurement:[SentryDependencyContainer.sharedInstance.dateProvider date]];
}
}

/**
* This is when the first frame is drawn.
*/
- (void)framesTrackerHasNewFrame
- (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate
{
[self buildAppStartMeasurement];
[self buildAppStartMeasurement:newFrameDate];
}

- (SentryAppStartType)getStartType
Expand Down
4 changes: 3 additions & 1 deletion Sources/Sentry/SentryFramesTracker.m
Expand Up @@ -200,8 +200,10 @@ - (void)reportNewFrame
localListeners = [self.listeners allObjects];
}

NSDate *newFrameDate = [self.dateProvider date];

for (id<SentryFramesTrackerListener> listener in localListeners) {
[listener framesTrackerHasNewFrame];
[listener framesTrackerHasNewFrame:newFrameDate];
}
}

Expand Down
9 changes: 3 additions & 6 deletions Sources/Sentry/SentryTimeToDisplayTracker.m
Expand Up @@ -2,7 +2,6 @@

#if SENTRY_HAS_UIKIT

# import "SentryCurrentDateProvider.h"
# import "SentryDependencyContainer.h"
# import "SentryFramesTracker.h"
# import "SentryMeasurementValue.h"
Expand Down Expand Up @@ -99,15 +98,13 @@ - (void)reportFullyDisplayed
_fullyDisplayedReported = YES;
}

- (void)framesTrackerHasNewFrame
- (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate
{
NSDate *finishTime = [SentryDependencyContainer.sharedInstance.dateProvider date];

// The purpose of TTID and TTFD is to measure how long
// takes to the content of the screen to change.
// Thats why we need to wait for the next frame to be drawn.
if (_initialDisplayReported && self.initialDisplaySpan.isFinished == NO) {
self.initialDisplaySpan.timestamp = finishTime;
self.initialDisplaySpan.timestamp = newFrameDate;

[self.initialDisplaySpan finish];

Expand All @@ -117,7 +114,7 @@ - (void)framesTrackerHasNewFrame
}
if (_waitForFullDisplay && _fullyDisplayedReported && self.fullDisplaySpan.isFinished == NO
&& self.initialDisplaySpan.isFinished == YES) {
self.fullDisplaySpan.timestamp = finishTime;
self.fullDisplaySpan.timestamp = newFrameDate;

[self.fullDisplaySpan finish];
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/include/SentryFramesTracker.h
Expand Up @@ -13,7 +13,7 @@ NS_ASSUME_NONNULL_BEGIN

@protocol SentryFramesTrackerListener

- (void)framesTrackerHasNewFrame;
- (void)framesTrackerHasNewFrame:(NSDate *)newFrameDate;

@end

Expand Down
Expand Up @@ -16,7 +16,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
let crashWrapper = TestSentryCrashWrapper.sharedInstance()
let appStateManager: SentryAppStateManager
var displayLinkWrapper = TestDisplayLinkWrapper()
var framesTracker: SentryFramesTracker
private let framesTracker: SentryFramesTracker
let dispatchQueue = TestSentryDispatchQueueWrapper()
var enablePreWarmedAppStartTracing = true
var enablePerformanceV2 = false
Expand Down Expand Up @@ -46,7 +46,6 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
)

framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: currentDate, keepDelayedFramesDuration: 0)
SentryDependencyContainer.sharedInstance().framesTracker = framesTracker
framesTracker.start()

runtimeInitTimestamp = SentryDependencyContainer.sharedInstance().dateProvider.date().addingTimeInterval(0.2)
Expand Down Expand Up @@ -93,15 +92,15 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
func testPerformanceV2_UsesRenderedFrameAsEndTimeStamp() {
fixture.enablePerformanceV2 = true

startApp()
startApp(callDisplayLink: true)

assertValidStart(type: .cold, expectedDuration: 0.45)
}

func testPerformanceV2_RemovesFramesTrackerListener() {
fixture.enablePerformanceV2 = true

startApp()
startApp(callDisplayLink: true)

advanceTime(bySeconds: 0.05)
fixture.displayLinkWrapper.normalFrame()
Expand Down Expand Up @@ -358,7 +357,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
advanceTime(bySeconds: 0.3)
}

private func startApp(processStartTimeStamp: Date? = nil, runtimeInitTimestamp: Date? = nil, moduleInitializationTimestamp: Date? = nil) {
private func startApp(processStartTimeStamp: Date? = nil, runtimeInitTimestamp: Date? = nil, moduleInitializationTimestamp: Date? = nil, callDisplayLink: Bool = false) {
givenProcessStartTimestamp(processStartTimestamp: processStartTimeStamp)

sut = fixture.sut
Expand All @@ -375,8 +374,12 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase {
uiWindowDidBecomeVisible()
didBecomeActive()

advanceTime(bySeconds: 0.05)
fixture.displayLinkWrapper.normalFrame()
if callDisplayLink {
advanceTime(bySeconds: 0.05)
fixture.currentDate.driftTimeForEveryRead = true
fixture.displayLinkWrapper.normalFrame()
fixture.currentDate.driftTimeForEveryRead = false
}
}

private func hybridAppStart() {
Expand Down
Expand Up @@ -517,16 +517,26 @@ class SentryFramesTrackerTests: XCTestCase {

wait(for: [expectation], timeout: 3.0)
}

func testAddListener() {
func testAddMultipleListeners_AllCalledWithSameDate() {
let sut = fixture.sut
let listener = FrameTrackerListener()
let listener1 = FrameTrackerListener()
let listener2 = FrameTrackerListener()
sut.start()
sut.add(listener)
sut.add(listener1)
sut.add(listener2)

fixture.dateProvider.driftTimeForEveryRead = true

fixture.displayLinkWrapper.normalFrame()
var expectedFrameDate = fixture.dateProvider.date()
expectedFrameDate.addTimeInterval(-fixture.dateProvider.driftTimeInterval)

XCTAssertTrue(listener.newFrameReported)
expect(listener1.newFrameInvocations.count) == 1
expect(listener1.newFrameInvocations.first?.timeIntervalSince1970) == expectedFrameDate.timeIntervalSince1970

expect(listener2.newFrameInvocations.count) == 1
expect(listener2.newFrameInvocations.first?.timeIntervalSince1970) == expectedFrameDate.timeIntervalSince1970
}

func testRemoveListener() {
Expand All @@ -538,7 +548,7 @@ class SentryFramesTrackerTests: XCTestCase {

fixture.displayLinkWrapper.normalFrame()

XCTAssertFalse(listener.newFrameReported)
expect(listener.newFrameInvocations.count) == 0
}

func testReleasedListener() {
Expand Down Expand Up @@ -594,10 +604,11 @@ class SentryFramesTrackerTests: XCTestCase {
}

private class FrameTrackerListener: NSObject, SentryFramesTrackerListener {
var newFrameReported = false

var newFrameInvocations = Invocations<Date>()
var callback: (() -> Void)?
func framesTrackerHasNewFrame() {
newFrameReported = true
func framesTrackerHasNewFrame(_ newFrameDate: Date) {
newFrameInvocations.record(newFrameDate)
callback?()
}
}
Expand Down