use correct timestamps, use teimestamp internally to handle completio…

…n before start
getsentry · Oct 2, 2024 · 3bd5c5d · 3bd5c5d
1 parent 736001f
commit 3bd5c5d
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 10 deletions.
diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart
@@ -164,7 +164,8 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
     _finishAndStartTTIDTracking(route, timestamp);
   }
 
-  Future<void> _finishAndStartTTIDTracking(Route<dynamic>? route, DateTime timestamp) async {
+  Future<void> _finishAndStartTTIDTracking(
+      Route<dynamic>? route, DateTime timestamp) async {
     await _finishTimeToDisplayTracking(endTimestamp: timestamp);
     await _startTimeToDisplayTracking(route, timestamp);
   }
@@ -302,7 +303,8 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
     await _native?.beginNativeFrames();
   }
 
-  Future<void> _finishTimeToDisplayTracking({required DateTime endTimestamp, bool clearAfter = false}) async {
+  Future<void> _finishTimeToDisplayTracking(
+      {required DateTime endTimestamp, bool clearAfter = false}) async {
     final transaction = _transaction;
     _transaction = null;
     try {
@@ -324,7 +326,10 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
         final isTTFDSpan =
             child.context.operation == SentrySpanOperations.uiTimeToFullDisplay;
         if (!child.finished && (isTTIDSpan || isTTFDSpan)) {
-          await child.finish(endTimestamp: endTimestamp, status: SpanStatus.deadlineExceeded());
+          await child.finish(
+            endTimestamp: endTimestamp,
+            status: SpanStatus.deadlineExceeded(),
+          );
         }
       }
     } catch (exception, stacktrace) {
@@ -345,7 +350,8 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
     }
   }
 
-  Future<void> _startTimeToDisplayTracking(Route<dynamic>? route, DateTime startTimestamp) async {
+  Future<void> _startTimeToDisplayTracking(
+      Route<dynamic>? route, DateTime startTimestamp) async {
     try {
       final routeName = _getRouteName(route) ?? _currentRouteName;
       if (!_enableAutoTransactions || routeName == null) {

diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart
@@ -29,6 +29,8 @@ class TimeToInitialDisplayTracker {
   bool _isManual = false;
   Completer<DateTime?>? _trackingCompleter;
   DateTime? _endTimestamp;
+  DateTime? _completeTrackingTimeStamp;
+
   final Duration _determineEndtimeTimeout = Duration(seconds: 5);
 
   /// This endTimestamp is needed in the [TimeToFullDisplayTracker] class
@@ -87,6 +89,13 @@ class TimeToInitialDisplayTracker {
 
     // If we already know it's manual we can return the future immediately
     if (_isManual) {
+      final completeTrackingTimeStamp = _completeTrackingTimeStamp;
+      if (completeTrackingTimeStamp != null) {
+        // If complete was called before we could call start, complete it here.
+        _endTimestamp = completeTrackingTimeStamp;
+        _trackingCompleter?.complete(completeTrackingTimeStamp);
+        _completeTrackingTimeStamp = null;
+      }
       return future;
     }
 
@@ -102,16 +111,17 @@ class TimeToInitialDisplayTracker {
   }
 
   void markAsManual() {
-    if (_trackingCompleter != null && !_trackingCompleter!.isCompleted) {
-      _isManual = true;
-    }
+    _isManual = true;
   }
 
   void completeTracking() {
+    final timestamp = DateTime.now();
+
     if (_trackingCompleter != null && !_trackingCompleter!.isCompleted) {
-      final endTimestamp = DateTime.now();
-      _endTimestamp = endTimestamp;
-      _trackingCompleter?.complete(endTimestamp);
+      _endTimestamp = timestamp;
+      _trackingCompleter?.complete(timestamp);
+    } else {
+      _completeTrackingTimeStamp = timestamp;
     }
   }
 

diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart
@@ -84,6 +84,37 @@ void main() {
               .difference(ttidSpan.startTimestamp)
               .inMilliseconds);
     });
+
+    test('starting after completing still finished correctly', () async {
+      await Future.delayed(fixture.finishFrameDuration, () {
+        sut.markAsManual();
+        sut.completeTracking();
+      });
+
+      final transaction = fixture.getTransaction() as SentryTracer;
+      await sut.trackRegularRoute(transaction, fixture.startTimestamp);
+
+      final children = transaction.children;
+      expect(children, hasLength(1));
+
+      final ttidSpan = children.first;
+      expect(ttidSpan.context.operation,
+          SentrySpanOperations.uiTimeToInitialDisplay);
+      expect(ttidSpan.finished, isTrue);
+      expect(ttidSpan.context.description, 'Regular route initial display');
+      expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay);
+      final ttidMeasurement =
+          transaction.measurements['time_to_initial_display'];
+      expect(ttidMeasurement, isNotNull);
+      expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond);
+      expect(ttidMeasurement?.value,
+          greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds));
+      expect(
+          ttidMeasurement?.value,
+          ttidSpan.endTimestamp!
+              .difference(ttidSpan.startTimestamp)
+              .inMilliseconds);
+    });
   });
 
   group('determineEndtime', () {