Start missing TTFD for root screen transaction (#2332)

* Handle TTFD for root screen transaction * add chengelog entry * cleanup * use options clock for app start end * use timeToDisplayTracker from options, remove timeToDisplayTracker from navigator ctor * fix test * fix analyze --------- Co-authored-by: GIancarlo Buenaflor <[email protected]>
getsentry · Oct 11, 2024 · f3a18f2 · f3a18f2
1 parent ca9f398
commit f3a18f2
Show file tree

Hide file tree

Showing 15 changed files with 341 additions and 313 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -63,6 +63,7 @@ Navigator.push(
 - Rounding error used on frames.total and reject frame measurements if frames.total is less than frames.slow or frames.frozen ([#2308](https://github.com/getsentry/sentry-dart/pull/2308))
 - iOS replay integration when only `onErrorSampleRate` is specified ([#2306](https://github.com/getsentry/sentry-dart/pull/2306))
 - Fix TTID timing issue ([#2326](https://github.com/getsentry/sentry-dart/pull/2326))
+- Start missing TTFD for root screen transaction ([#2332](https://github.com/getsentry/sentry-dart/pull/2332))
 - Accessing invalid json fields from `fetchNativeAppStart` should return null ([#2340](https://github.com/getsentry/sentry-dart/pull/2340))
 - Error when calling `SentryFlutter.reportFullyDisplayed()` twice ([#2339](https://github.com/getsentry/sentry-dart/pull/2339))
 

diff --git a/flutter/lib/src/integrations/native_app_start_handler.dart b/flutter/lib/src/integrations/native_app_start_handler.dart
@@ -21,7 +21,7 @@ class NativeAppStartHandler {
   static const _maxAppStartMillis = 60000;
 
   Future<void> call(Hub hub, SentryFlutterOptions options,
-      {required DateTime? appStartEnd}) async {
+      {required DateTime appStartEnd}) async {
     _hub = hub;
     _options = options;
 
@@ -42,43 +42,33 @@ class NativeAppStartHandler {
       SentrySpanOperations.uiLoad,
       startTimestamp: appStartInfo.start,
     );
-    final ttidSpan = transaction.startChild(
-      SentrySpanOperations.uiTimeToInitialDisplay,
-      description: '$screenName initial display',
+
+    await options.timeToDisplayTracker.track(
+      transaction,
       startTimestamp: appStartInfo.start,
+      endTimestamp: appStartInfo.end,
+      origin: SentryTraceOrigins.autoUiTimeToDisplay,
     );
 
-    // Enrich Transaction
-
     SentryTracer sentryTracer;
     if (transaction is SentryTracer) {
       sentryTracer = transaction;
     } else {
       return;
     }
 
-    SentryMeasurement? measurement;
-    if (options.autoAppStart) {
-      measurement = appStartInfo.toMeasurement();
-    } else if (appStartEnd != null) {
-      appStartInfo.end = appStartEnd;
-      measurement = appStartInfo.toMeasurement();
-    }
-
-    if (measurement != null) {
-      sentryTracer.measurements[measurement.name] = measurement;
-      await _attachAppStartSpans(appStartInfo, sentryTracer);
-    }
-
-    // Finish Transaction & Span
+    // Enrich Transaction
+    SentryMeasurement? measurement = appStartInfo.toMeasurement();
+    sentryTracer.measurements[measurement.name] = appStartInfo.toMeasurement();
+    await _attachAppStartSpans(appStartInfo, sentryTracer);
 
-    await ttidSpan.finish(endTimestamp: appStartInfo.end);
+    // Finish Transaction
     await transaction.finish(endTimestamp: appStartInfo.end);
   }
 
   _AppStartInfo? _infoNativeAppStart(
     NativeAppStart nativeAppStart,
-    DateTime? appStartEnd,
+    DateTime appStartEnd,
   ) {
     final sentrySetupStartDateTime = SentryFlutter.sentrySetupStartTime;
     if (sentrySetupStartDateTime == null) {
@@ -90,23 +80,18 @@ class NativeAppStartHandler {
     final pluginRegistrationDateTime = DateTime.fromMillisecondsSinceEpoch(
         nativeAppStart.pluginRegistrationTime);
 
-    if (_options.autoAppStart) {
-      // We only assign the current time if it's not already set - this is useful in tests
-      appStartEnd ??= _options.clock();
-
-      final duration = appStartEnd.difference(appStartDateTime);
-
-      // We filter out app start more than 60s.
-      // This could be due to many different reasons.
-      // If you do the manual init and init the SDK too late and it does not
-      // compute the app start end in the very first Screen.
-      // If the process starts but the App isn't in the foreground.
-      // If the system forked the process earlier to accelerate the app start.
-      // And some unknown reasons that could not be reproduced.
-      // We've seen app starts with hours, days and even months.
-      if (duration.inMilliseconds > _maxAppStartMillis) {
-        return null;
-      }
+    final duration = appStartEnd.difference(appStartDateTime);
+
+    // We filter out app start more than 60s.
+    // This could be due to many different reasons.
+    // If you do the manual init and init the SDK too late and it does not
+    // compute the app start end in the very first Screen.
+    // If the process starts but the App isn't in the foreground.
+    // If the system forked the process earlier to accelerate the app start.
+    // And some unknown reasons that could not be reproduced.
+    // We've seen app starts with hours, days and even months.
+    if (duration.inMilliseconds > _maxAppStartMillis) {
+      return null;
     }
 
     List<_TimeSpan> nativeSpanTimes = [];
@@ -145,9 +130,6 @@ class NativeAppStartHandler {
       _AppStartInfo appStartInfo, SentryTracer transaction) async {
     final transactionTraceId = transaction.context.traceId;
     final appStartEnd = appStartInfo.end;
-    if (appStartEnd == null) {
-      return;
-    }
 
     final appStartSpan = await _createAndFinishSpan(
       tracer: transaction,
@@ -256,30 +238,24 @@ class _AppStartInfo {
   _AppStartInfo(
     this.type, {
     required this.start,
+    required this.end,
     required this.pluginRegistration,
     required this.sentrySetupStart,
     required this.nativeSpanTimes,
-    this.end,
   });
 
   final _AppStartType type;
   final DateTime start;
+  final DateTime end;
   final List<_TimeSpan> nativeSpanTimes;
 
-  // We allow the end to be null, since it might be set at a later time
-  // with setAppStartEnd when autoAppStart is disabled
-  DateTime? end;
-
   final DateTime pluginRegistration;
   final DateTime sentrySetupStart;
 
-  Duration? get duration => end?.difference(start);
+  Duration get duration => end.difference(start);
 
-  SentryMeasurement? toMeasurement() {
+  SentryMeasurement toMeasurement() {
     final duration = this.duration;
-    if (duration == null) {
-      return null;
-    }
     return type == _AppStartType.cold
         ? SentryMeasurement.coldAppStart(duration)
         : SentryMeasurement.warmAppStart(duration);

diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart
@@ -33,15 +33,25 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
   void call(Hub hub, SentryFlutterOptions options) async {
     _frameCallbackHandler.addPostFrameCallback((timeStamp) async {
       try {
-        if (!options.autoAppStart && _appStartEnd == null) {
-          await _appStartEndCompleter.future
-              .timeout(const Duration(seconds: 10));
+        DateTime? appStartEnd;
+        if (options.autoAppStart) {
+          // ignore: invalid_use_of_internal_member
+          appStartEnd = options.clock();
+        } else if (_appStartEnd == null) {
+          await _appStartEndCompleter.future.timeout(
+            const Duration(seconds: 10),
+          );
+          appStartEnd = _appStartEnd;
+        } else {
+          appStartEnd = null;
+        }
+        if (appStartEnd != null) {
+          await _nativeAppStartHandler.call(
+            hub,
+            options,
+            appStartEnd: appStartEnd,
+          );
         }
-        await _nativeAppStartHandler.call(
-          hub,
-          options,
-          appStartEnd: _appStartEnd,
-        );
       } catch (exception, stackTrace) {
         options.logger(
           SentryLevel.error,

diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart
@@ -78,7 +78,6 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
     bool setRouteNameAsTransaction = false,
     RouteNameExtractor? routeNameExtractor,
     AdditionalInfoExtractor? additionalInfoProvider,
-    @visibleForTesting TimeToDisplayTracker? timeToDisplayTracker,
     List<String>? ignoreRoutes,
   })  : _hub = hub ?? HubAdapter(),
         _enableAutoTransactions = enableAutoTransactions,
@@ -92,19 +91,17 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
     if (enableAutoTransactions) {
       _hub.options.sdk.addIntegration('UINavigationTracing');
     }
-    _timeToDisplayTracker =
-        timeToDisplayTracker ?? _initializeTimeToDisplayTracker();
+    _timeToDisplayTracker = _initializeTimeToDisplayTracker();
   }
 
   /// Initializes the TimeToDisplayTracker with the option to enable time to full display tracing.
-  TimeToDisplayTracker _initializeTimeToDisplayTracker() {
-    bool enableTimeToFullDisplayTracing = false;
+  TimeToDisplayTracker? _initializeTimeToDisplayTracker() {
     final options = _hub.options;
     if (options is SentryFlutterOptions) {
-      enableTimeToFullDisplayTracing = options.enableTimeToFullDisplayTracing;
+      return options.timeToDisplayTracker;
+    } else {
+      return null;
     }
-    return TimeToDisplayTracker(
-        enableTimeToFullDisplayTracing: enableTimeToFullDisplayTracing);
   }
 
   final Hub _hub;
@@ -115,11 +112,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
   final AdditionalInfoExtractor? _additionalInfoProvider;
   final SentryNativeBinding? _native;
   final List<String> _ignoreRoutes;
-  static TimeToDisplayTracker? _timeToDisplayTracker;
-
-  @internal
-  static TimeToDisplayTracker? get timeToDisplayTracker =>
-      _timeToDisplayTracker;
+  TimeToDisplayTracker? _timeToDisplayTracker;
 
   ISentrySpan? _transaction;
 
@@ -362,7 +355,7 @@ class SentryNavigatorObserver extends RouteObserver<PageRoute<dynamic>> {
       }
 
       if (!isAppStart) {
-        await _timeToDisplayTracker?.trackRegularRouteTTD(
+        await _timeToDisplayTracker?.track(
           transaction,
           startTimestamp: startTimestamp,
         );

diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart
@@ -9,38 +9,50 @@ import 'time_to_initial_display_tracker.dart';
 @internal
 class TimeToDisplayTracker {
   final TimeToInitialDisplayTracker _ttidTracker;
-  final TimeToFullDisplayTracker? _ttfdTracker;
-  final bool enableTimeToFullDisplayTracing;
+  final TimeToFullDisplayTracker _ttfdTracker;
+  final SentryFlutterOptions options;
 
   TimeToDisplayTracker({
     TimeToInitialDisplayTracker? ttidTracker,
     TimeToFullDisplayTracker? ttfdTracker,
-    required this.enableTimeToFullDisplayTracing,
+    required this.options,
   })  : _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(),
-        _ttfdTracker = enableTimeToFullDisplayTracing
-            ? ttfdTracker ?? TimeToFullDisplayTracker()
-            : null;
-
-  Future<void> trackRegularRouteTTD(ISentrySpan transaction,
-      {required DateTime startTimestamp}) async {
-    await _ttidTracker.trackRegularRoute(transaction, startTimestamp);
-    await _trackTTFDIfEnabled(transaction, startTimestamp);
-  }
-
-  Future<void> _trackTTFDIfEnabled(
-      ISentrySpan transaction, DateTime startTimestamp) async {
-    if (enableTimeToFullDisplayTracing) {
-      await _ttfdTracker?.track(transaction, startTimestamp);
+        _ttfdTracker = ttfdTracker ?? TimeToFullDisplayTracker();
+
+  Future<void> track(
+    ISentrySpan transaction, {
+    required DateTime startTimestamp,
+    DateTime? endTimestamp,
+    String? origin,
+  }) async {
+    // TTID
+    await _ttidTracker.track(
+      transaction: transaction,
+      startTimestamp: startTimestamp,
+      endTimestamp: endTimestamp,
+      origin: origin,
+    );
+
+    // TTFD
+    if (options.enableTimeToFullDisplayTracing) {
+      await _ttfdTracker.track(
+        transaction: transaction,
+        startTimestamp: startTimestamp,
+      );
     }
   }
 
   @internal
   Future<void> reportFullyDisplayed() async {
-    return _ttfdTracker?.reportFullyDisplayed();
+    if (options.enableTimeToFullDisplayTracing) {
+      return _ttfdTracker.reportFullyDisplayed();
+    }
   }
 
   void clear() {
     _ttidTracker.clear();
-    _ttfdTracker?.clear();
+    if (options.enableTimeToFullDisplayTracing) {
+      _ttfdTracker.clear();
+    }
   }
 }