From 0b21267e355c83f7c09611cea6ed1df107d3d34e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 1 Mar 2024 13:19:43 +0100 Subject: [PATCH 01/57] Change app start integration in a way that works with ttid as well --- .../native_app_start_event_processor.dart | 52 ++------- flutter/lib/src/frame_callback_handler.dart | 12 ++ .../native_app_start_integration.dart | 104 ++++++++++++++++-- flutter/lib/src/sentry_flutter.dart | 4 +- flutter/test/fake_frame_callback_handler.dart | 18 +++ .../native_app_start_integration_test.dart | 10 +- 6 files changed, 145 insertions(+), 55 deletions(-) create mode 100644 flutter/lib/src/frame_callback_handler.dart create mode 100644 flutter/test/fake_frame_callback_handler.dart diff --git a/flutter/lib/src/event_processor/native_app_start_event_processor.dart b/flutter/lib/src/event_processor/native_app_start_event_processor.dart index a7abe62e05..9289dc0388 100644 --- a/flutter/lib/src/event_processor/native_app_start_event_processor.dart +++ b/flutter/lib/src/event_processor/native_app_start_event_processor.dart @@ -2,57 +2,29 @@ import 'dart:async'; import 'package:sentry/sentry.dart'; -import '../native/sentry_native.dart'; +import '../integrations/integrations.dart'; /// EventProcessor that enriches [SentryTransaction] objects with app start /// measurement. class NativeAppStartEventProcessor implements EventProcessor { - /// We filter out App starts more than 60s - static const _maxAppStartMillis = 60000; + NativeAppStartEventProcessor(); - NativeAppStartEventProcessor( - this._native, - ); - - final SentryNative _native; + // We want the app start measurement to only be added once to the first transaction + bool _didAddAppStartMeasurement = false; @override Future apply(SentryEvent event, {Hint? hint}) async { - final appStartEnd = _native.appStartEnd; + if (_didAddAppStartMeasurement || event is! SentryTransaction) { + return event; + } - if (appStartEnd != null && - event is SentryTransaction && - !_native.didFetchAppStart) { - final nativeAppStart = await _native.fetchNativeAppStart(); - if (nativeAppStart == null) { - return event; - } - final measurement = nativeAppStart.toMeasurement(appStartEnd); - // 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 (measurement.value >= _maxAppStartMillis) { - return event; - } + final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); + final measurement = appStartInfo?.toMeasurement(); + + if (measurement != null) { event.measurements[measurement.name] = measurement; + _didAddAppStartMeasurement = true; } return event; } } - -extension NativeAppStartMeasurement on NativeAppStart { - SentryMeasurement toMeasurement(DateTime appStartEnd) { - final appStartDateTime = - DateTime.fromMillisecondsSinceEpoch(appStartTime.toInt()); - final duration = appStartEnd.difference(appStartDateTime); - - return isColdStart - ? SentryMeasurement.coldAppStart(duration) - : SentryMeasurement.warmAppStart(duration); - } -} diff --git a/flutter/lib/src/frame_callback_handler.dart b/flutter/lib/src/frame_callback_handler.dart new file mode 100644 index 0000000000..aa920504f3 --- /dev/null +++ b/flutter/lib/src/frame_callback_handler.dart @@ -0,0 +1,12 @@ +import 'package:flutter/scheduler.dart'; + +abstract class FrameCallbackHandler { + void addPostFrameCallback(FrameCallback callback); +} + +class DefaultFrameCallbackHandler implements FrameCallbackHandler { + @override + void addPostFrameCallback(FrameCallback callback) { + SchedulerBinding.instance.addPostFrameCallback(callback); + } +} diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 47bf79dff4..db2e22524c 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -1,10 +1,29 @@ -import 'package:flutter/scheduler.dart'; -import 'package:sentry/sentry.dart'; +import 'dart:async'; -import '../sentry_flutter_options.dart'; +import 'package:meta/meta.dart'; + +import '../../sentry_flutter.dart'; +import '../frame_callback_handler.dart'; import '../native/sentry_native.dart'; import '../event_processor/native_app_start_event_processor.dart'; +enum AppStartType { cold, warm } + +class AppStartInfo { + AppStartInfo(this.type, {required this.start, required this.end}); + + final AppStartType type; + final DateTime start; + final DateTime end; + + SentryMeasurement toMeasurement() { + final duration = end.difference(start); + return type == AppStartType.cold + ? SentryMeasurement.coldAppStart(duration) + : SentryMeasurement.warmAppStart(duration); + } +} + /// Integration which handles communication with native frameworks in order to /// enrich [SentryTransaction] objects with app start data for mobile vitals. class NativeAppStartIntegration extends Integration { @@ -13,6 +32,35 @@ class NativeAppStartIntegration extends Integration { final SentryNative _native; final SchedulerBindingProvider _schedulerBindingProvider; + /// We filter out App starts more than 60s + static const _maxAppStartMillis = 60000; + + static Completer _appStartCompleter = + Completer(); + static AppStartInfo? _appStartInfo; + + @internal + static void setAppStartInfo(AppStartInfo? appStartInfo) { + _appStartInfo = appStartInfo; + _appStartCompleter.complete(appStartInfo); + } + + @internal + static Future getAppStartInfo() { + if (_appStartInfo != null) { + return Future.value(_appStartInfo); + } + return _appStartCompleter.future; + } + + @internal + static void clearAppStartInfo() { + _appStartInfo = null; + if (_appStartCompleter.isCompleted) { + _appStartCompleter = Completer(); + } + } + @override void call(Hub hub, SentryFlutterOptions options) { if (options.autoAppStart) { @@ -21,18 +69,60 @@ class NativeAppStartIntegration extends Integration { options.logger(SentryLevel.debug, 'Scheduler binding is null. Can\'t auto detect app start time.'); } else { - schedulerBinding.addPostFrameCallback((timeStamp) { + schedulerBinding.addPostFrameCallback((timeStamp) async { // ignore: invalid_use_of_internal_member - _native.appStartEnd = options.clock(); + // We only assign the current time if it's not already set + // this is useful in tests + _native.appStartEnd ??= options.clock(); + final appStartEnd = _native.appStartEnd; + + if (_native.didFetchAppStart || appStartEnd == null) { + setAppStartInfo(null); + return; + } + + final nativeAppStart = await _native.fetchNativeAppStart(); + + if (nativeAppStart == null) { + setAppStartInfo(null); + return; + } + + final appStartDateTime = DateTime.fromMillisecondsSinceEpoch( + nativeAppStart.appStartTime.toInt()); + 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) { + setAppStartInfo(null); + return; + } + + final appStartInfo = AppStartInfo( + nativeAppStart.isColdStart + ? AppStartType.cold + : AppStartType.warm, + start: DateTime.fromMillisecondsSinceEpoch( + nativeAppStart.appStartTime.toInt()), + end: appStartEnd); + setAppStartInfo(appStartInfo); }); } } - options.addEventProcessor(NativeAppStartEventProcessor(_native)); + options.addEventProcessor(NativeAppStartEventProcessor()); options.sdk.addIntegration('nativeAppStartIntegration'); } } /// Used to provide scheduler binding at call time. -typedef SchedulerBindingProvider = SchedulerBinding? Function(); +typedef SchedulerBindingProvider = FrameCallbackHandler? Function(); + diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index 62a9043bc9..2b2c2f928e 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -1,7 +1,6 @@ import 'dart:async'; import 'dart:ui'; -import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; @@ -9,6 +8,7 @@ import '../sentry_flutter.dart'; import 'event_processor/android_platform_exception_event_processor.dart'; import 'event_processor/flutter_exception_event_processor.dart'; import 'event_processor/platform_exception_event_processor.dart'; +import 'frame_callback_handler.dart'; import 'integrations/connectivity/connectivity_integration.dart'; import 'integrations/screenshot_integration.dart'; import 'native/factory.dart'; @@ -192,7 +192,7 @@ mixin SentryFlutter { () { try { /// Flutter >= 2.12 throws if SchedulerBinding.instance isn't initialized. - return SchedulerBinding.instance; + return DefaultFrameCallbackHandler(); } catch (_) {} return null; }, diff --git a/flutter/test/fake_frame_callback_handler.dart b/flutter/test/fake_frame_callback_handler.dart new file mode 100644 index 0000000000..55434371d4 --- /dev/null +++ b/flutter/test/fake_frame_callback_handler.dart @@ -0,0 +1,18 @@ +import 'package:flutter/scheduler.dart'; +import 'package:sentry_flutter/src/frame_callback_handler.dart'; + +class FakeFrameCallbackHandler implements FrameCallbackHandler { + FrameCallback? storedCallback; + + final Duration _finishAfterDuration; + + FakeFrameCallbackHandler( + {Duration finishAfterDuration = const Duration(milliseconds: 500)}) + : _finishAfterDuration = finishAfterDuration; + + @override + void addPostFrameCallback(FrameCallback callback) async { + await Future.delayed(_finishAfterDuration); + callback(Duration.zero); + } +} diff --git a/flutter/test/integrations/native_app_start_integration_test.dart b/flutter/test/integrations/native_app_start_integration_test.dart index d4b8deaaf5..1f26dca311 100644 --- a/flutter/test/integrations/native_app_start_integration_test.dart +++ b/flutter/test/integrations/native_app_start_integration_test.dart @@ -1,5 +1,4 @@ @TestOn('vm') - import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; @@ -7,6 +6,7 @@ import 'package:sentry_flutter/src/integrations/native_app_start_integration.dar import 'package:sentry_flutter/src/native/sentry_native.dart'; import 'package:sentry/src/sentry_tracer.dart'; +import '../fake_frame_callback_handler.dart'; import '../mocks.dart'; import '../mocks.mocks.dart'; @@ -18,10 +18,10 @@ void main() { TestWidgetsFlutterBinding.ensureInitialized(); fixture = Fixture(); + NativeAppStartIntegration.clearAppStartInfo(); }); test('native app start measurement added to first transaction', () async { - fixture.options.autoAppStart = false; fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10); fixture.binding.nativeAppStart = NativeAppStart(0, true); @@ -40,7 +40,6 @@ void main() { test('native app start measurement not added to following transactions', () async { - fixture.options.autoAppStart = false; fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10); fixture.binding.nativeAppStart = NativeAppStart(0, true); @@ -58,7 +57,6 @@ void main() { }); test('measurements appended', () async { - fixture.options.autoAppStart = false; fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10); fixture.binding.nativeAppStart = NativeAppStart(0, true); final measurement = SentryMeasurement.warmAppStart(Duration(seconds: 1)); @@ -79,7 +77,6 @@ void main() { }); test('native app start measurement not added if more than 60s', () async { - fixture.options.autoAppStart = false; fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(60001); fixture.binding.nativeAppStart = NativeAppStart(0, true); @@ -111,7 +108,8 @@ class Fixture { return NativeAppStartIntegration( native, () { - return TestWidgetsFlutterBinding.ensureInitialized(); + TestWidgetsFlutterBinding.ensureInitialized(); + return FakeFrameCallbackHandler(); }, ); } From 6f6e71c716cc1ffa632a7ca40bc68fadeba4562f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 1 Mar 2024 13:21:09 +0100 Subject: [PATCH 02/57] Formatting --- .../native_app_start_integration.dart | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index db2e22524c..49a46ec5ee 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -7,23 +7,6 @@ import '../frame_callback_handler.dart'; import '../native/sentry_native.dart'; import '../event_processor/native_app_start_event_processor.dart'; -enum AppStartType { cold, warm } - -class AppStartInfo { - AppStartInfo(this.type, {required this.start, required this.end}); - - final AppStartType type; - final DateTime start; - final DateTime end; - - SentryMeasurement toMeasurement() { - final duration = end.difference(start); - return type == AppStartType.cold - ? SentryMeasurement.coldAppStart(duration) - : SentryMeasurement.warmAppStart(duration); - } -} - /// Integration which handles communication with native frameworks in order to /// enrich [SentryTransaction] objects with app start data for mobile vitals. class NativeAppStartIntegration extends Integration { @@ -126,3 +109,19 @@ class NativeAppStartIntegration extends Integration { /// Used to provide scheduler binding at call time. typedef SchedulerBindingProvider = FrameCallbackHandler? Function(); +enum AppStartType { cold, warm } + +class AppStartInfo { + AppStartInfo(this.type, {required this.start, required this.end}); + + final AppStartType type; + final DateTime start; + final DateTime end; + + SentryMeasurement toMeasurement() { + final duration = end.difference(start); + return type == AppStartType.cold + ? SentryMeasurement.coldAppStart(duration) + : SentryMeasurement.warmAppStart(duration); + } +} From 6ed77001fd8de6bfa1aa4541836f87782d9ef1ec Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 1 Mar 2024 13:26:20 +0100 Subject: [PATCH 03/57] Update --- .../integrations/native_app_start_integration.dart | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 49a46ec5ee..01e0ea8348 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -53,20 +53,18 @@ class NativeAppStartIntegration extends Integration { 'Scheduler binding is null. Can\'t auto detect app start time.'); } else { schedulerBinding.addPostFrameCallback((timeStamp) async { - // ignore: invalid_use_of_internal_member - // We only assign the current time if it's not already set - // this is useful in tests - _native.appStartEnd ??= options.clock(); - final appStartEnd = _native.appStartEnd; - - if (_native.didFetchAppStart || appStartEnd == null) { + if (_native.didFetchAppStart) { setAppStartInfo(null); return; } + // ignore: invalid_use_of_internal_member + // We only assign the current time if it's not already set - this is useful in tests + _native.appStartEnd ??= options.clock(); + final appStartEnd = _native.appStartEnd; final nativeAppStart = await _native.fetchNativeAppStart(); - if (nativeAppStart == null) { + if (nativeAppStart == null || appStartEnd == null) { setAppStartInfo(null); return; } From 2f8a47d7ff3901791de377e109073dc51b00e0c7 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 1 Mar 2024 13:48:34 +0100 Subject: [PATCH 04/57] add visibleForTesting --- .../lib/src/integrations/native_app_start_integration.dart | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 01e0ea8348..251a23f907 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -36,12 +36,10 @@ class NativeAppStartIntegration extends Integration { return _appStartCompleter.future; } - @internal + @visibleForTesting static void clearAppStartInfo() { _appStartInfo = null; - if (_appStartCompleter.isCompleted) { - _appStartCompleter = Completer(); - } + _appStartCompleter = Completer(); } @override From 50f298bd31d92b6a72bc6e43bbc6fb1e7770353c Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Fri, 1 Mar 2024 14:14:31 +0100 Subject: [PATCH 05/57] Update --- flutter/lib/src/integrations/native_app_start_integration.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 251a23f907..8ede500c5e 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -25,6 +25,9 @@ class NativeAppStartIntegration extends Integration { @internal static void setAppStartInfo(AppStartInfo? appStartInfo) { _appStartInfo = appStartInfo; + if (_appStartCompleter.isCompleted) { + _appStartCompleter = Completer(); + } _appStartCompleter.complete(appStartInfo); } From 3477e6f25541f45d2cbe98be59c016ab8b0444ee Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 10:00:59 +0100 Subject: [PATCH 06/57] update --- CHANGELOG.md | 6 ++++++ .../native_app_start_event_processor.dart | 10 +++++----- .../src/integrations/native_app_start_integration.dart | 4 ++-- flutter/lib/src/native/sentry_native.dart | 9 +++++++++ flutter/test/fake_frame_callback_handler.dart | 1 + flutter/test/mocks.dart | 10 ++++++++++ 6 files changed, 33 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52f902b373..e0d6578d29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Improvements + +- App start is now fetched within integration instead of event processor ([#1905](https://github.com/getsentry/sentry-dart/pull/1905)) + ## 7.16.1 ### Fixes diff --git a/flutter/lib/src/event_processor/native_app_start_event_processor.dart b/flutter/lib/src/event_processor/native_app_start_event_processor.dart index 9289dc0388..f66c8ffbc8 100644 --- a/flutter/lib/src/event_processor/native_app_start_event_processor.dart +++ b/flutter/lib/src/event_processor/native_app_start_event_processor.dart @@ -3,18 +3,18 @@ import 'dart:async'; import 'package:sentry/sentry.dart'; import '../integrations/integrations.dart'; +import '../native/sentry_native.dart'; /// EventProcessor that enriches [SentryTransaction] objects with app start /// measurement. class NativeAppStartEventProcessor implements EventProcessor { - NativeAppStartEventProcessor(); + final SentryNative _native; - // We want the app start measurement to only be added once to the first transaction - bool _didAddAppStartMeasurement = false; + NativeAppStartEventProcessor(this._native); @override Future apply(SentryEvent event, {Hint? hint}) async { - if (_didAddAppStartMeasurement || event is! SentryTransaction) { + if (_native.didAddAppStartMeasurement || event is! SentryTransaction) { return event; } @@ -23,7 +23,7 @@ class NativeAppStartEventProcessor implements EventProcessor { if (measurement != null) { event.measurements[measurement.name] = measurement; - _didAddAppStartMeasurement = true; + _native.setDidAddAppStartMeasurement(true); } return event; } diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 8ede500c5e..adc2f46722 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -59,8 +59,8 @@ class NativeAppStartIntegration extends Integration { return; } - // ignore: invalid_use_of_internal_member // We only assign the current time if it's not already set - this is useful in tests + // ignore: invalid_use_of_internal_member _native.appStartEnd ??= options.clock(); final appStartEnd = _native.appStartEnd; final nativeAppStart = await _native.fetchNativeAppStart(); @@ -99,7 +99,7 @@ class NativeAppStartIntegration extends Integration { } } - options.addEventProcessor(NativeAppStartEventProcessor()); + options.addEventProcessor(NativeAppStartEventProcessor(_native)); options.sdk.addIntegration('nativeAppStartIntegration'); } diff --git a/flutter/lib/src/native/sentry_native.dart b/flutter/lib/src/native/sentry_native.dart index b8d2206a8d..445a7d061c 100644 --- a/flutter/lib/src/native/sentry_native.dart +++ b/flutter/lib/src/native/sentry_native.dart @@ -27,6 +27,15 @@ class SentryNative { /// Flag indicating if app start was already fetched. bool get didFetchAppStart => _didFetchAppStart; + bool _didAddAppStartMeasurement = false; + + /// Flag indicating if app start measurement was added to the first transaction. + bool get didAddAppStartMeasurement => _didAddAppStartMeasurement; + + void setDidAddAppStartMeasurement(bool value) { + _didAddAppStartMeasurement = value; + } + /// Fetch [NativeAppStart] from native channels. Can only be called once. Future fetchNativeAppStart() async { _didFetchAppStart = true; diff --git a/flutter/test/fake_frame_callback_handler.dart b/flutter/test/fake_frame_callback_handler.dart index 55434371d4..103045d0e2 100644 --- a/flutter/test/fake_frame_callback_handler.dart +++ b/flutter/test/fake_frame_callback_handler.dart @@ -12,6 +12,7 @@ class FakeFrameCallbackHandler implements FrameCallbackHandler { @override void addPostFrameCallback(FrameCallback callback) async { + // ignore: inference_failure_on_instance_creation await Future.delayed(_finishAfterDuration); callback(Duration.zero); } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index b2e01788c1..e9fa743a0e 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -171,6 +171,11 @@ class TestMockSentryNative implements SentryNative { @override bool get didFetchAppStart => _didFetchAppStart; + bool _didAddAppStartMeasurement = false; + + @override + bool get didAddAppStartMeasurement => _didAddAppStartMeasurement; + Breadcrumb? breadcrumb; var numberOfAddBreadcrumbCalls = 0; var numberOfBeginNativeFramesCollectionCalls = 0; @@ -290,6 +295,11 @@ class TestMockSentryNative implements SentryNative { numberOfDiscardProfilerCalls++; return Future.value(null); } + + @override + void setDidAddAppStartMeasurement(bool value) { + _didAddAppStartMeasurement = value; + } } // TODO can this be replaced with https://pub.dev/packages/mockito#verifying-exact-number-of-invocations--at-least-x--never From ed3d4ae1ada33c34a798166fb091ad3b0525e717 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 10:16:50 +0100 Subject: [PATCH 07/57] Add app start info test --- .../native_app_start_integration_test.dart | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/flutter/test/integrations/native_app_start_integration_test.dart b/flutter/test/integrations/native_app_start_integration_test.dart index 1f26dca311..f79ca2e7b3 100644 --- a/flutter/test/integrations/native_app_start_integration_test.dart +++ b/flutter/test/integrations/native_app_start_integration_test.dart @@ -90,6 +90,18 @@ void main() { expect(enriched.measurements.isEmpty, true); }); + + + test('native app start integration is called and sets app start info', () async { + fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10); + fixture.binding.nativeAppStart = NativeAppStart(0, true); + + fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options); + + final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); + expect(appStartInfo?.start, DateTime.fromMillisecondsSinceEpoch(0)); + expect(appStartInfo?.end, DateTime.fromMillisecondsSinceEpoch(10)); + }); }); } From 81346de4356f1bb169e3dc16ede302c7106cd0e9 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 10:18:43 +0100 Subject: [PATCH 08/57] Remove set app start info null --- flutter/lib/src/integrations/native_app_start_integration.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index adc2f46722..bd006a5596 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -66,7 +66,6 @@ class NativeAppStartIntegration extends Integration { final nativeAppStart = await _native.fetchNativeAppStart(); if (nativeAppStart == null || appStartEnd == null) { - setAppStartInfo(null); return; } From cf5af40e626e04602ce5ce9952f8812b203484f7 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 11:14:06 +0100 Subject: [PATCH 09/57] Review improvements --- .../native_app_start_event_processor.dart | 2 +- flutter/lib/src/frame_callback_handler.dart | 5 +- .../native_app_start_integration.dart | 95 ++++++++----------- flutter/lib/src/native/sentry_native.dart | 8 +- flutter/lib/src/sentry_flutter.dart | 9 +- .../native_app_start_integration_test.dart | 9 +- flutter/test/mocks.dart | 9 +- 7 files changed, 54 insertions(+), 83 deletions(-) diff --git a/flutter/lib/src/event_processor/native_app_start_event_processor.dart b/flutter/lib/src/event_processor/native_app_start_event_processor.dart index f66c8ffbc8..fd1a5ec169 100644 --- a/flutter/lib/src/event_processor/native_app_start_event_processor.dart +++ b/flutter/lib/src/event_processor/native_app_start_event_processor.dart @@ -23,7 +23,7 @@ class NativeAppStartEventProcessor implements EventProcessor { if (measurement != null) { event.measurements[measurement.name] = measurement; - _native.setDidAddAppStartMeasurement(true); + _native.didAddAppStartMeasurement = true; } return event; } diff --git a/flutter/lib/src/frame_callback_handler.dart b/flutter/lib/src/frame_callback_handler.dart index aa920504f3..71a8f928b1 100644 --- a/flutter/lib/src/frame_callback_handler.dart +++ b/flutter/lib/src/frame_callback_handler.dart @@ -7,6 +7,9 @@ abstract class FrameCallbackHandler { class DefaultFrameCallbackHandler implements FrameCallbackHandler { @override void addPostFrameCallback(FrameCallback callback) { - SchedulerBinding.instance.addPostFrameCallback(callback); + try { + /// Flutter >= 2.12 throws if SchedulerBinding.instance isn't initialized. + SchedulerBinding.instance.addPostFrameCallback(callback); + } catch (_) {} } } diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index bd006a5596..4ea3049ec4 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -10,10 +10,10 @@ import '../event_processor/native_app_start_event_processor.dart'; /// Integration which handles communication with native frameworks in order to /// enrich [SentryTransaction] objects with app start data for mobile vitals. class NativeAppStartIntegration extends Integration { - NativeAppStartIntegration(this._native, this._schedulerBindingProvider); + NativeAppStartIntegration(this._native, this._frameCallbackHandler); final SentryNative _native; - final SchedulerBindingProvider _schedulerBindingProvider; + final FrameCallbackHandler _frameCallbackHandler; /// We filter out App starts more than 60s static const _maxAppStartMillis = 60000; @@ -48,54 +48,46 @@ class NativeAppStartIntegration extends Integration { @override void call(Hub hub, SentryFlutterOptions options) { if (options.autoAppStart) { - final schedulerBinding = _schedulerBindingProvider(); - if (schedulerBinding == null) { - options.logger(SentryLevel.debug, - 'Scheduler binding is null. Can\'t auto detect app start time.'); - } else { - schedulerBinding.addPostFrameCallback((timeStamp) async { - if (_native.didFetchAppStart) { - setAppStartInfo(null); - return; - } - - // We only assign the current time if it's not already set - this is useful in tests - // ignore: invalid_use_of_internal_member - _native.appStartEnd ??= options.clock(); - final appStartEnd = _native.appStartEnd; - final nativeAppStart = await _native.fetchNativeAppStart(); - - if (nativeAppStart == null || appStartEnd == null) { - return; - } - - final appStartDateTime = DateTime.fromMillisecondsSinceEpoch( - nativeAppStart.appStartTime.toInt()); - 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) { - setAppStartInfo(null); - return; - } - - final appStartInfo = AppStartInfo( - nativeAppStart.isColdStart - ? AppStartType.cold - : AppStartType.warm, - start: DateTime.fromMillisecondsSinceEpoch( - nativeAppStart.appStartTime.toInt()), - end: appStartEnd); - setAppStartInfo(appStartInfo); - }); - } + _frameCallbackHandler.addPostFrameCallback((timeStamp) async { + if (_native.didFetchAppStart) { + setAppStartInfo(null); + return; + } + + // We only assign the current time if it's not already set - this is useful in tests + // ignore: invalid_use_of_internal_member + _native.appStartEnd ??= options.clock(); + final appStartEnd = _native.appStartEnd; + final nativeAppStart = await _native.fetchNativeAppStart(); + + if (nativeAppStart == null || appStartEnd == null) { + return; + } + + final appStartDateTime = DateTime.fromMillisecondsSinceEpoch( + nativeAppStart.appStartTime.toInt()); + 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) { + setAppStartInfo(null); + return; + } + + final appStartInfo = AppStartInfo( + nativeAppStart.isColdStart ? AppStartType.cold : AppStartType.warm, + start: DateTime.fromMillisecondsSinceEpoch( + nativeAppStart.appStartTime.toInt()), + end: appStartEnd); + setAppStartInfo(appStartInfo); + }); } options.addEventProcessor(NativeAppStartEventProcessor(_native)); @@ -104,9 +96,6 @@ class NativeAppStartIntegration extends Integration { } } -/// Used to provide scheduler binding at call time. -typedef SchedulerBindingProvider = FrameCallbackHandler? Function(); - enum AppStartType { cold, warm } class AppStartInfo { diff --git a/flutter/lib/src/native/sentry_native.dart b/flutter/lib/src/native/sentry_native.dart index 445a7d061c..a7973f8a12 100644 --- a/flutter/lib/src/native/sentry_native.dart +++ b/flutter/lib/src/native/sentry_native.dart @@ -27,14 +27,8 @@ class SentryNative { /// Flag indicating if app start was already fetched. bool get didFetchAppStart => _didFetchAppStart; - bool _didAddAppStartMeasurement = false; - /// Flag indicating if app start measurement was added to the first transaction. - bool get didAddAppStartMeasurement => _didAddAppStartMeasurement; - - void setDidAddAppStartMeasurement(bool value) { - _didAddAppStartMeasurement = value; - } + bool didAddAppStartMeasurement = false; /// Fetch [NativeAppStart] from native channels. Can only be called once. Future fetchNativeAppStart() async { diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index 2b2c2f928e..9822b49665 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -189,13 +189,7 @@ mixin SentryFlutter { if (_native != null) { integrations.add(NativeAppStartIntegration( _native!, - () { - try { - /// Flutter >= 2.12 throws if SchedulerBinding.instance isn't initialized. - return DefaultFrameCallbackHandler(); - } catch (_) {} - return null; - }, + DefaultFrameCallbackHandler(), )); } return integrations; @@ -231,6 +225,7 @@ mixin SentryFlutter { @internal static SentryNative? get native => _native; + @internal static set native(SentryNative? value) => _native = value; static SentryNative? _native; diff --git a/flutter/test/integrations/native_app_start_integration_test.dart b/flutter/test/integrations/native_app_start_integration_test.dart index f79ca2e7b3..135d90f34b 100644 --- a/flutter/test/integrations/native_app_start_integration_test.dart +++ b/flutter/test/integrations/native_app_start_integration_test.dart @@ -91,8 +91,8 @@ void main() { expect(enriched.measurements.isEmpty, true); }); - - test('native app start integration is called and sets app start info', () async { + test('native app start integration is called and sets app start info', + () async { fixture.native.appStartEnd = DateTime.fromMillisecondsSinceEpoch(10); fixture.binding.nativeAppStart = NativeAppStart(0, true); @@ -119,10 +119,7 @@ class Fixture { NativeAppStartIntegration getNativeAppStartIntegration() { return NativeAppStartIntegration( native, - () { - TestWidgetsFlutterBinding.ensureInitialized(); - return FakeFrameCallbackHandler(); - }, + FakeFrameCallbackHandler(), ); } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index e9fa743a0e..41c3269393 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -171,10 +171,8 @@ class TestMockSentryNative implements SentryNative { @override bool get didFetchAppStart => _didFetchAppStart; - bool _didAddAppStartMeasurement = false; - @override - bool get didAddAppStartMeasurement => _didAddAppStartMeasurement; + bool didAddAppStartMeasurement = false; Breadcrumb? breadcrumb; var numberOfAddBreadcrumbCalls = 0; @@ -295,11 +293,6 @@ class TestMockSentryNative implements SentryNative { numberOfDiscardProfilerCalls++; return Future.value(null); } - - @override - void setDidAddAppStartMeasurement(bool value) { - _didAddAppStartMeasurement = value; - } } // TODO can this be replaced with https://pub.dev/packages/mockito#verifying-exact-number-of-invocations--at-least-x--never From 487e55f64c96d70c1eff0cb4eb635eb8ac0040ae Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 12:55:47 +0100 Subject: [PATCH 10/57] Add TTID --- dart/lib/src/sentry_span_operations.dart | 0 .../native_app_start_integration.dart | 2 +- .../src/navigation/sentry_display_widget.dart | 60 ++++++ .../navigation/sentry_navigator_observer.dart | 153 ++++++--------- .../navigation/time_to_display_tracker.dart | 87 +++++++++ .../time_to_display_transaction_handler.dart | 80 ++++++++ .../time_to_initial_display_tracker.dart | 123 ++++++++++++ .../lib/src/sentry_flutter_measurement.dart | 23 +++ .../sentry_display_widget_test.dart | 121 ++++++++++++ .../time_to_display_tracker_test.dart | 136 ++++++++++++++ .../time_to_initial_display_tracker_test.dart | 177 ++++++++++++++++++ .../test/sentry_navigator_observer_test.dart | 140 ++++++++++---- 12 files changed, 971 insertions(+), 131 deletions(-) create mode 100644 dart/lib/src/sentry_span_operations.dart create mode 100644 flutter/lib/src/navigation/sentry_display_widget.dart create mode 100644 flutter/lib/src/navigation/time_to_display_tracker.dart create mode 100644 flutter/lib/src/navigation/time_to_display_transaction_handler.dart create mode 100644 flutter/lib/src/navigation/time_to_initial_display_tracker.dart create mode 100644 flutter/lib/src/sentry_flutter_measurement.dart create mode 100644 flutter/test/navigation/sentry_display_widget_test.dart create mode 100644 flutter/test/navigation/time_to_display_tracker_test.dart create mode 100644 flutter/test/navigation/time_to_initial_display_tracker_test.dart diff --git a/dart/lib/src/sentry_span_operations.dart b/dart/lib/src/sentry_span_operations.dart new file mode 100644 index 0000000000..e69de29bb2 diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 4ea3049ec4..219721af5a 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -104,9 +104,9 @@ class AppStartInfo { final AppStartType type; final DateTime start; final DateTime end; + Duration get duration => end.difference(start); SentryMeasurement toMeasurement() { - final duration = end.difference(start); return type == AppStartType.cold ? SentryMeasurement.coldAppStart(duration) : SentryMeasurement.warmAppStart(duration); diff --git a/flutter/lib/src/navigation/sentry_display_widget.dart b/flutter/lib/src/navigation/sentry_display_widget.dart new file mode 100644 index 0000000000..ffed11cce7 --- /dev/null +++ b/flutter/lib/src/navigation/sentry_display_widget.dart @@ -0,0 +1,60 @@ +import 'package:flutter/cupertino.dart'; +import 'time_to_initial_display_tracker.dart'; + +import '../frame_callback_handler.dart'; + +/// A widget that reports the Time To Initially Displayed (TTID) of its child widget. +/// +/// This widget wraps around another widget to measure and report the time it takes +/// for the child widget to be initially displayed on the screen. This method +/// allows a more accurate measurement than what the default TTID implementation +/// provides. The TTID measurement begins when the route to the widget is pushed and ends +/// when [WidgetsBinding.instance.addPostFrameCallback] is triggered. +/// +/// Wrap the widget you want to measure with [SentryDisplayWidget], and ensure that you +/// have set up Sentry's routing instrumentation according to the Sentry documentation. +/// +/// ```dart +/// SentryDisplayWidget( +/// child: MyWidget(), +/// ) +/// ``` +/// +/// Make sure to configure Sentry's routing instrumentation in your app by following +/// the guidelines provided in Sentry's documentation for Flutter integrations: +/// https://docs.sentry.io/platforms/flutter/integrations/routing-instrumentation/ +/// +/// See also: +/// - [Sentry's documentation on Flutter integrations](https://docs.sentry.io/platforms/flutter/) +/// for more information on how to integrate Sentry into your Flutter application. +class SentryDisplayWidget extends StatefulWidget { + final Widget child; + final FrameCallbackHandler _frameCallbackHandler; + + SentryDisplayWidget({ + super.key, + required this.child, + FrameCallbackHandler? frameCallbackHandler, + }) : _frameCallbackHandler = + frameCallbackHandler ?? DefaultFrameCallbackHandler(); + + @override + _SentryDisplayWidgetState createState() => _SentryDisplayWidgetState(); +} + +class _SentryDisplayWidgetState extends State { + @override + void initState() { + super.initState(); + TimeToInitialDisplayTracker().markAsManual(); + + widget._frameCallbackHandler.addPostFrameCallback((_) { + TimeToInitialDisplayTracker().completeTracking(); + }); + } + + @override + Widget build(BuildContext context) { + return widget.child; + } +} diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 30beaa75bc..5402603c25 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -1,9 +1,16 @@ +// ignore_for_file: invalid_use_of_internal_member + +import 'dart:async'; + +import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import 'time_to_display_transaction_handler.dart'; import '../../sentry_flutter.dart'; import '../event_processor/flutter_enricher_event_processor.dart'; import '../native/sentry_native.dart'; +import 'time_to_display_tracker.dart'; /// This key must be used so that the web interface displays the events nicely /// See https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/ @@ -19,6 +26,8 @@ typedef AdditionalInfoExtractor = Map? Function( /// This is a navigation observer to record navigational breadcrumbs. /// For now it only records navigation events and no gestures. /// +/// It also records Time to Initial Display (TTID) and Time to Full Display (TTFD). +/// /// [Route]s can always be null and their [Route.settings] can also always be null. /// For example, if the application starts, there is no previous route. /// The [RouteSettings] is null if a developer has not specified any @@ -44,22 +53,21 @@ typedef AdditionalInfoExtractor = Map? Function( /// ) /// ``` /// -/// See the constructor docs for the argument documentation. +/// The option [enableAutoTransactions] is enabled by default. For every new +/// route a transaction is started. It's automatically finished after +/// [autoFinishAfter] duration or when all child spans are finished, +/// if those happen to take longer. The transaction will be set to [Scope.span] +/// if the latter is empty. +/// +/// Enabling the [setRouteNameAsTransaction] option overrides the current +/// [Scope.transaction] which will also override the name of the current +/// [Scope.span]. So be careful when this is used together with performance +/// monitoring. /// /// See also: /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) /// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments) class SentryNavigatorObserver extends RouteObserver> { - /// The option [enableAutoTransactions] is enabled by default. - /// For every new route a transaction is started. It's automatically finished - /// after [autoFinishAfter] duration or when all child spans are - /// finished, if those happen to take longer. - /// The transaction will be set to [Scope.span] if the latter is empty. - /// - /// Enabling the [setRouteNameAsTransaction] option overrides the - /// current [Scope.transaction] which will also override the name of the current - /// [Scope.span]. So be careful when this is used together with performance - /// monitoring. SentryNavigatorObserver({ Hub? hub, bool enableAutoTransactions = true, @@ -67,34 +75,51 @@ class SentryNavigatorObserver extends RouteObserver> { bool setRouteNameAsTransaction = false, RouteNameExtractor? routeNameExtractor, AdditionalInfoExtractor? additionalInfoProvider, + @internal TimeToDisplayTracker? timeToDisplayTracker, }) : _hub = hub ?? HubAdapter(), - _enableAutoTransactions = enableAutoTransactions, - _autoFinishAfter = autoFinishAfter, _setRouteNameAsTransaction = setRouteNameAsTransaction, _routeNameExtractor = routeNameExtractor, - _additionalInfoProvider = additionalInfoProvider, - _native = SentryFlutter.native { + _additionalInfoProvider = additionalInfoProvider { if (enableAutoTransactions) { - // ignore: invalid_use_of_internal_member _hub.options.sdk.addIntegration('UINavigationTracing'); } + final options = _hub.options; + if (options is SentryFlutterOptions) { + const enableTimeToFullDisplayTracing = + false; // TODO: replace with options.enableTimeToFullDisplayTracing in TTFD + _timeToDisplayTracker = timeToDisplayTracker ?? + TimeToDisplayTracker( + enableTimeToFullDisplayTracing: enableTimeToFullDisplayTracing, + ttdTransactionHandler: TimeToDisplayTransactionHandler( + hub: hub, + enableAutoTransactions: enableAutoTransactions, + autoFinishAfter: autoFinishAfter, + ), + ); + } } final Hub _hub; - final bool _enableAutoTransactions; - final Duration _autoFinishAfter; final bool _setRouteNameAsTransaction; final RouteNameExtractor? _routeNameExtractor; final AdditionalInfoExtractor? _additionalInfoProvider; - final SentryNative? _native; - ISentrySpan? _transaction; + static TimeToDisplayTracker? _timeToDisplayTracker; + + @internal + static TimeToDisplayTracker? get timeToDisplayTracker => + _timeToDisplayTracker; static String? _currentRouteName; @internal static String? get currentRouteName => _currentRouteName; + Completer? _completedDisplayTracking; + + @visibleForTesting + Completer? get completedDisplayTracking => _completedDisplayTracking; + @override void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); @@ -109,7 +134,16 @@ class SentryNavigatorObserver extends RouteObserver> { ); _finishTransaction(); - _startTransaction(route); + _startTimeToDisplayTracking(route); + } + + Future _finishTransaction() async { + final transaction = _hub.getSpan(); + if (transaction == null || transaction.finished) { + return; + } + transaction.status ??= SpanStatus.ok(); + await transaction.finish(); } @override @@ -140,7 +174,6 @@ class SentryNavigatorObserver extends RouteObserver> { ); _finishTransaction(); - _startTransaction(previousRoute); } void _addBreadcrumb({ @@ -152,7 +185,6 @@ class SentryNavigatorObserver extends RouteObserver> { navigationType: type, from: _routeNameExtractor?.call(from) ?? from, to: _routeNameExtractor?.call(to) ?? to, - // ignore: invalid_use_of_internal_member timestamp: _hub.options.clock(), data: _additionalInfoProvider?.call(from, to), )); @@ -179,77 +211,14 @@ class SentryNavigatorObserver extends RouteObserver> { } } - Future _startTransaction(Route? route) async { - if (!_enableAutoTransactions) { - return; - } + Future _startTimeToDisplayTracking(Route? route) async { + _completedDisplayTracking = Completer(); + final routeName = _getRouteName(route); + _currentRouteName = routeName; - String? name = _getRouteName(route); final arguments = route?.settings.arguments; - - if (name == null) { - return; - } - - if (name == '/') { - name = 'root ("/")'; - } - final transactionContext = SentryTransactionContext( - name, - 'navigation', - transactionNameSource: SentryTransactionNameSource.component, - // ignore: invalid_use_of_internal_member - origin: SentryTraceOrigins.autoNavigationRouteObserver, - ); - - _transaction = _hub.startTransactionWithContext( - transactionContext, - waitForChildren: true, - autoFinishAfter: _autoFinishAfter, - trimEnd: true, - onFinish: (transaction) async { - _transaction = null; - final nativeFrames = await _native - ?.endNativeFramesCollection(transaction.context.traceId); - if (nativeFrames != null) { - final measurements = nativeFrames.toMeasurements(); - for (final item in measurements.entries) { - final measurement = item.value; - transaction.setMeasurement( - item.key, - measurement.value, - unit: measurement.unit, - ); - } - } - }, - ); - - // if _enableAutoTransactions is enabled but there's no traces sample rate - if (_transaction is NoOpSentrySpan) { - _transaction = null; - return; - } - - if (arguments != null) { - _transaction?.setData('route_settings_arguments', arguments); - } - - await _hub.configureScope((scope) { - scope.span ??= _transaction; - }); - - await _native?.beginNativeFramesCollection(); - } - - Future _finishTransaction() async { - final transaction = _transaction; - _transaction = null; - if (transaction == null || transaction.finished) { - return; - } - transaction.status ??= SpanStatus.ok(); - await transaction.finish(); + await _timeToDisplayTracker?.startTracking(routeName, arguments); + completedDisplayTracking?.complete(); } } diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart new file mode 100644 index 0000000000..a4312d4a2e --- /dev/null +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -0,0 +1,87 @@ +import 'dart:async'; + +import 'package:meta/meta.dart'; +import '../integrations/integrations.dart'; + +import '../../sentry_flutter.dart'; +import '../native/sentry_native.dart'; +import 'time_to_display_transaction_handler.dart'; +import 'time_to_initial_display_tracker.dart'; + +@internal +class TimeToDisplayTracker { + final SentryNative? _native; + final TimeToDisplayTransactionHandler _ttdTransactionHandler; + final TimeToInitialDisplayTracker _ttidTracker; + final bool _enableTimeToFullDisplayTracing; + + TimeToDisplayTracker({ + required bool enableTimeToFullDisplayTracing, + required TimeToDisplayTransactionHandler ttdTransactionHandler, + TimeToInitialDisplayTracker? ttidTracker, + }) : _native = SentryFlutter.native, + _enableTimeToFullDisplayTracing = enableTimeToFullDisplayTracing, + _ttdTransactionHandler = ttdTransactionHandler, + _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); + + Future startTracking(String? routeName, Object? arguments) async { + final startTimestamp = DateTime.now(); + if (routeName == '/') { + routeName = 'root ("/")'; + } + final isRootScreen = routeName == 'root ("/")'; + final didFetchAppStart = _native?.didFetchAppStart; + if (isRootScreen && didFetchAppStart == false) { + // Dart cannot infer here that routeName is not nullable + if (routeName == null) return; + return _trackAppStartTTD(routeName, arguments); + } else { + return _trackRegularRouteTTD(routeName, arguments, startTimestamp); + } + } + + /// This method listens for the completion of the app's start process via + /// [AppStartTracker], then: + /// - Starts a transaction with the app start start timestamp + /// - Starts TTID and optionally TTFD spans based on the app start start timestamp + /// - Finishes the TTID span immediately with the app start end timestamp + /// + /// We start and immediately finish the TTID span since we cannot mutate the history of spans. + Future _trackAppStartTTD(String routeName, Object? arguments) async { + final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); + final name = routeName; + + if (appStartInfo == null) return; + + final transaction = await _ttdTransactionHandler + .startTransaction(name, arguments, startTimestamp: appStartInfo.start); + if (transaction == null) return; + + if (_enableTimeToFullDisplayTracing) { + // TODO: implement TTFD + } + + await _ttidTracker.trackAppStart(transaction, appStartInfo, name); + } + + /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. + Future _trackRegularRouteTTD( + String? routeName, Object? arguments, DateTime startTimestamp) async { + final transaction = await _ttdTransactionHandler + .startTransaction(routeName, arguments, startTimestamp: startTimestamp); + + if (transaction == null || routeName == null) return; + + if (_enableTimeToFullDisplayTracing) { + // TODO: implement TTFD + } + + await _ttidTracker.trackRegularRoute( + transaction, startTimestamp, routeName); + } + + @internal + Future reportFullyDisplayed() async { + // TODO: implement TTFD + } +} diff --git a/flutter/lib/src/navigation/time_to_display_transaction_handler.dart b/flutter/lib/src/navigation/time_to_display_transaction_handler.dart new file mode 100644 index 0000000000..94f411496a --- /dev/null +++ b/flutter/lib/src/navigation/time_to_display_transaction_handler.dart @@ -0,0 +1,80 @@ +// ignore_for_file: invalid_use_of_internal_member + +import 'package:meta/meta.dart'; +import '../../sentry_flutter.dart'; +import '../native/sentry_native.dart'; + +@internal +class TimeToDisplayTransactionHandler { + final Hub? _hub; + final bool? _enableAutoTransactions; + final Duration? _autoFinishAfter; + final SentryNative? _native; + + TimeToDisplayTransactionHandler({ + required Hub? hub, + required bool? enableAutoTransactions, + required Duration? autoFinishAfter, + }) : _hub = hub ?? HubAdapter(), + _enableAutoTransactions = enableAutoTransactions, + _autoFinishAfter = autoFinishAfter, + _native = SentryFlutter.native; + + Future startTransaction(String? routeName, Object? arguments, + {DateTime? startTimestamp}) async { + if (_enableAutoTransactions == false) { + return null; + } + + if (routeName == null) { + return null; + } + + final transactionContext = SentryTransactionContext( + routeName, + SentrySpanOperations.uiLoad, + transactionNameSource: SentryTransactionNameSource.component, + origin: SentryTraceOrigins.autoNavigationRouteObserver, + ); + + final transaction = _hub?.startTransactionWithContext( + transactionContext, + waitForChildren: true, + autoFinishAfter: _autoFinishAfter, + trimEnd: true, + startTimestamp: startTimestamp, + onFinish: (transaction) async { + final nativeFrames = await _native + ?.endNativeFramesCollection(transaction.context.traceId); + if (nativeFrames != null) { + final measurements = nativeFrames.toMeasurements(); + for (final item in measurements.entries) { + final measurement = item.value; + transaction.setMeasurement( + item.key, + measurement.value, + unit: measurement.unit, + ); + } + } + }, + ); + + // if _enableAutoTransactions is enabled but there's no traces sample rate + if (transaction is NoOpSentrySpan) { + return null; + } + + if (arguments != null) { + transaction?.setData('route_settings_arguments', arguments); + } + + _hub?.configureScope((scope) { + scope.span ??= transaction; + }); + + await _native?.beginNativeFramesCollection(); + + return transaction; + } +} diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart new file mode 100644 index 0000000000..a86d52bf17 --- /dev/null +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -0,0 +1,123 @@ +// ignore_for_file: invalid_use_of_internal_member + +import 'dart:async'; + +import 'package:meta/meta.dart'; +import '../integrations/integrations.dart'; + +import '../../sentry_flutter.dart'; +import '../frame_callback_handler.dart'; +import '../sentry_flutter_measurement.dart'; + +@internal +class TimeToInitialDisplayTracker { + static final TimeToInitialDisplayTracker _instance = + TimeToInitialDisplayTracker._(); + + factory TimeToInitialDisplayTracker( + {FrameCallbackHandler? frameCallbackHandler}) { + if (frameCallbackHandler != null) { + _instance._frameCallbackHandler = frameCallbackHandler; + } + return _instance; + } + + TimeToInitialDisplayTracker._(); + + FrameCallbackHandler _frameCallbackHandler = DefaultFrameCallbackHandler(); + bool _isManual = false; + Completer? _trackingCompleter; + DateTime? _endTimestamp; + + /// This endTimestamp is needed in the [TimeToFullDisplayTracker] class + @internal + DateTime? get endTimestamp => _endTimestamp; + + Future trackRegularRoute(ISentrySpan transaction, + DateTime startTimestamp, String routeName) async { + final endTimestamp = await determineEndTime(); + if (endTimestamp == null) return; + + final ttidSpan = transaction.startChild( + SentrySpanOperations.uiTimeToInitialDisplay, + description: '$routeName initial display', + startTimestamp: startTimestamp); + + if (_isManual) { + ttidSpan.origin = SentryTraceOrigins.manualUiTimeToDisplay; + } else { + ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; + } + + final ttidMeasurement = SentryFlutterMeasurement.timeToInitialDisplay( + Duration( + milliseconds: + endTimestamp.difference(startTimestamp).inMilliseconds)); + transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, + unit: ttidMeasurement.unit); + await ttidSpan.finish(endTimestamp: endTimestamp); + + // We can clear the state after creating and finishing the ttid span has finished + clear(); + } + + Future trackAppStart(ISentrySpan transaction, AppStartInfo appStartInfo, + String routeName) async { + final ttidSpan = transaction.startChild( + SentrySpanOperations.uiTimeToInitialDisplay, + description: '$routeName initial display', + startTimestamp: appStartInfo.start, + ); + ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; + + final ttidMeasurement = + SentryFlutterMeasurement.timeToInitialDisplay(appStartInfo.duration); + transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, + unit: ttidMeasurement.unit); + + // Since app start measurement is immediate, finish the TTID span with the app start's end timestamp + await ttidSpan.finish(endTimestamp: appStartInfo.end); + + // Store the end timestamp for potential use by TTFD tracking + _endTimestamp = appStartInfo.end; + } + + Future? determineEndTime() { + _trackingCompleter = Completer(); + + // If we already know it's manual we can return the future immediately + if (_isManual) { + return _trackingCompleter?.future; + } + + // Schedules a check at the end of the frame to determine if the tracking + // should be completed immediately (approximation mode) or deferred (manual mode). + _frameCallbackHandler.addPostFrameCallback((_) { + if (!_isManual) { + completeTracking(); + } + }); + + return _trackingCompleter?.future; + } + + void markAsManual() { + _isManual = true; + } + + void completeTracking() { + if (_trackingCompleter != null && !_trackingCompleter!.isCompleted) { + final endTimestamp = DateTime.now(); + _endTimestamp = endTimestamp; + // Reset after completion + _trackingCompleter?.complete(endTimestamp); + } + } + + void clear() { + _isManual = false; + _trackingCompleter = null; + // We can't clear the ttid end time stamp here, because it might be needed + // in the [TimeToFullDisplayTracker] class + } +} diff --git a/flutter/lib/src/sentry_flutter_measurement.dart b/flutter/lib/src/sentry_flutter_measurement.dart new file mode 100644 index 0000000000..8b47a3fc56 --- /dev/null +++ b/flutter/lib/src/sentry_flutter_measurement.dart @@ -0,0 +1,23 @@ +import '../sentry_flutter.dart'; + +extension SentryFlutterMeasurement on SentryMeasurement { + /// Duration of the time to initial display in milliseconds + static SentryMeasurement timeToInitialDisplay(Duration duration) { + assert(!duration.isNegative); + return SentryMeasurement( + 'time_to_initial_display', + duration.inMilliseconds.toDouble(), + unit: DurationSentryMeasurementUnit.milliSecond, + ); + } + + /// Duration of the time to full display in milliseconds + static SentryMeasurement timeToFullDisplay(Duration duration) { + assert(!duration.isNegative); + return SentryMeasurement( + 'time_to_full_display', + duration.inMilliseconds.toDouble(), + unit: DurationSentryMeasurementUnit.milliSecond, + ); + } +} diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart new file mode 100644 index 0000000000..6eac88653b --- /dev/null +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -0,0 +1,121 @@ +// ignore_for_file: invalid_use_of_internal_member + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry/src/sentry_tracer.dart'; +import 'package:sentry_flutter/src/integrations/integrations.dart'; +import 'package:sentry_flutter/src/navigation/sentry_display_widget.dart'; +import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; + +import '../fake_frame_callback_handler.dart'; +import '../mocks.dart'; + +void main() { + PageRoute route(RouteSettings? settings) => PageRouteBuilder( + pageBuilder: (_, __, ___) => Container(), + settings: settings, + ); + + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + testWidgets('SentryDisplayWidget reports manual ttid span after didPush', + (WidgetTester tester) async { + final currentRoute = route(RouteSettings(name: 'Current Route')); + + await tester.runAsync(() async { + fixture.navigatorObserver.didPush(currentRoute, null); + await tester.pumpWidget(fixture.getSut()); + await fixture.navigatorObserver.completedDisplayTracking?.future; + }); + + final tracer = fixture.hub.getSpan() as SentryTracer; + final spans = tracer.children.where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay); + + expect(spans, hasLength(1)); + final ttidSpan = spans.first; + expect(ttidSpan.context.operation, + SentrySpanOperations.uiTimeToInitialDisplay); + expect(ttidSpan.finished, isTrue); + expect(ttidSpan.context.description, 'Current Route initial display'); + expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); + expect(tracer.measurements, hasLength(1)); + final measurement = tracer.measurements['time_to_initial_display']; + expect(measurement, isNotNull); + expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); + }); + + testWidgets('SentryDisplayWidget is ignored for app starts', + (WidgetTester tester) async { + final currentRoute = route(RouteSettings(name: '/')); + + await tester.runAsync(() async { + fixture.navigatorObserver.didPush(currentRoute, null); + await tester.pumpWidget(fixture.getSut()); + final appStartInfo = AppStartInfo( + AppStartType.cold, + start: DateTime.fromMillisecondsSinceEpoch(0), + end: DateTime.fromMillisecondsSinceEpoch(10), + ); + NativeAppStartIntegration.setAppStartInfo(appStartInfo); + await fixture.navigatorObserver.completedDisplayTracking?.future; + }); + + final tracer = fixture.hub.getSpan() as SentryTracer; + final spans = tracer.children.where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay); + + expect(spans, hasLength(1)); + + final ttidSpan = spans.first; + expect(ttidSpan.context.operation, + SentrySpanOperations.uiTimeToInitialDisplay); + expect(ttidSpan.finished, isTrue); + expect(ttidSpan.context.description, 'root ("/") initial display'); + expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); + + expect(ttidSpan.startTimestamp, + DateTime.fromMillisecondsSinceEpoch(0).toUtc()); + expect( + ttidSpan.endTimestamp, DateTime.fromMillisecondsSinceEpoch(10).toUtc()); + + expect(tracer.measurements, hasLength(1)); + final measurement = tracer.measurements['time_to_initial_display']; + expect(measurement, isNotNull); + expect(measurement?.value, 10); + expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); + }); +} + +class Fixture { + final Hub hub = + Hub(SentryFlutterOptions(dsn: fakeDsn)..tracesSampleRate = 1.0); + late final SentryNavigatorObserver navigatorObserver; + late final TimeToInitialDisplayTracker timeToInitialDisplayTracker; + final fakeFrameCallbackHandler = FakeFrameCallbackHandler(); + + Fixture() { + SentryFlutter.native = TestMockSentryNative(); + navigatorObserver = SentryNavigatorObserver(hub: hub); + timeToInitialDisplayTracker = TimeToInitialDisplayTracker( + frameCallbackHandler: fakeFrameCallbackHandler); + } + + MaterialApp getSut() { + return MaterialApp( + home: SentryDisplayWidget( + frameCallbackHandler: FakeFrameCallbackHandler( + finishAfterDuration: Duration(milliseconds: 50), + ), + child: Text('my text'), + ), + ); + } +} diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart new file mode 100644 index 0000000000..5754a7f7f9 --- /dev/null +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -0,0 +1,136 @@ +// ignore_for_file: invalid_use_of_internal_member +// ignore_for_file: inference_failure_on_instance_creation + +import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/integrations/integrations.dart'; +import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; +import 'package:sentry/src/sentry_tracer.dart'; +import 'package:sentry_flutter/src/navigation/time_to_display_transaction_handler.dart'; +import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; + +import '../fake_frame_callback_handler.dart'; +import '../mocks.dart'; + +void main() { + late Fixture fixture; + + setUp(() { + TestWidgetsFlutterBinding.ensureInitialized(); + fixture = Fixture(); + }); + + group('time to initial display', () { + group('in root screen app start route', () { + test('startMeasurement finishes ttid span', () async { + SentryFlutter.native = TestMockSentryNative(); + final sut = fixture.getSut(); + + Future.delayed(const Duration(milliseconds: 500), () async { + final appStartInfo = AppStartInfo(AppStartType.cold, + start: DateTime.fromMillisecondsSinceEpoch(0), + end: DateTime.fromMillisecondsSinceEpoch(10)); + + NativeAppStartIntegration.setAppStartInfo(appStartInfo); + }); + + await sut.startTracking('/', null); + + await Future.delayed(const Duration(milliseconds: 100)); + + final transaction = fixture.hub.getSpan() as SentryTracer; + + final spans = transaction.children; + expect(transaction.children, hasLength(1)); + expect(spans[0].context.operation, + SentrySpanOperations.uiTimeToInitialDisplay); + expect(spans[0].finished, isTrue); + }); + }); + + group('in regular routes', () { + group('with approximation strategy', () { + test('startMeasurement finishes ttid span', () async { + final sut = fixture.getSut(); + + await sut.startTracking('Current Route', null); + + final transaction = fixture.hub.getSpan() as SentryTracer; + await Future.delayed(const Duration(milliseconds: 2000)); + + final spans = transaction.children; + expect(transaction.children, hasLength(1)); + expect(spans[0].context.operation, + SentrySpanOperations.uiTimeToInitialDisplay); + expect(spans[0].finished, isTrue); + }); + }); + + group('with manual strategy', () { + test('finishes ttid span after reporting with manual api', () async { + final sut = fixture.getSut(); + + Future.delayed(const Duration(milliseconds: 100), () { + fixture.ttidTracker.markAsManual(); + fixture.ttidTracker.completeTracking(); + }); + await sut.startTracking('Current Route', null); + + final transaction = fixture.hub.getSpan() as SentryTracer; + + await Future.delayed(const Duration(milliseconds: 100)); + + final ttidSpan = transaction.children + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + expect(ttidSpan, isNotNull); + + await Future.delayed(const Duration(milliseconds: 100)); + + expect(ttidSpan.finished, isTrue); + }); + }); + }); + }); + + test('screen load tracking creates ui.load transaction', () async { + final sut = fixture.getSut(); + + await sut.startTracking('Current Route', null); + + final transaction = fixture.hub.getSpan(); + expect(transaction, isNotNull); + expect(transaction?.context.operation, SentrySpanOperations.uiLoad); + }); +} + +class Fixture { + final options = SentryFlutterOptions() + ..dsn = fakeDsn + ..tracesSampleRate = 1.0; + + late final hub = Hub(options); + + final frameCallbackHandler = FakeFrameCallbackHandler(); + late final ttidTracker = + TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); + + final ttfdAutoFinishAfter = Duration(milliseconds: 500); + + TimeToDisplayTracker getSut() { + // TODO: replace with options.enableTimeToFullDisplayTracing in TTFD + const enableTimeToFullDisplayTracing = false; + + return TimeToDisplayTracker( + enableTimeToFullDisplayTracing: enableTimeToFullDisplayTracing, + ttdTransactionHandler: TimeToDisplayTransactionHandler( + hub: hub, + enableAutoTransactions: true, + autoFinishAfter: const Duration(seconds: 30), + ), + ttidTracker: ttidTracker, + ); + } +} diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart new file mode 100644 index 0000000000..58b8fe69ce --- /dev/null +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -0,0 +1,177 @@ +// ignore_for_file: invalid_use_of_internal_member + +import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/integrations/native_app_start_integration.dart'; +import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; + +import '../fake_frame_callback_handler.dart'; +import '../mocks.dart'; +import 'package:sentry/src/sentry_tracer.dart'; + +void main() { + late Fixture fixture; + late TimeToInitialDisplayTracker sut; + + setUp(() { + fixture = Fixture(); + sut = fixture.getSut(); + }); + + tearDown(() { + sut.clear(); + }); + + group('app start', () { + test('tracking creates and finishes ttid span with correct measurements', + () async { + final transaction = + fixture.hub.startTransaction('fake', 'fake') as SentryTracer; + final startTimestamp = DateTime.now(); + final appStartInfo = AppStartInfo(AppStartType.cold, + start: startTimestamp, + end: startTimestamp.add(Duration(milliseconds: 10))); + + await sut.trackAppStart(transaction, appStartInfo, 'route ("/")'); + + 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, 'route ("/") initial display'); + expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); + + final ttidMeasurement = + transaction.measurements['time_to_initial_display']; + expect(ttidMeasurement, isNotNull); + expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); + expect(ttidMeasurement?.value, 10); + }); + }); + + group('regular route', () { + test( + 'approximation tracking creates and finishes ttid span with correct measurements', + () async { + final transaction = + fixture.hub.startTransaction('fake', 'fake') as SentryTracer; + final startTimestamp = DateTime.now(); + + await sut.trackRegularRoute(transaction, startTimestamp, 'regular route'); + + 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.autoUiTimeToDisplay); + + final ttidMeasurement = + transaction.measurements['time_to_initial_display']; + expect(ttidMeasurement, isNotNull); + expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); + expect( + ttidMeasurement?.value, + greaterThanOrEqualTo( + fixture.finishFrameAfterDuration.inMilliseconds)); + }); + + test( + 'manual tracking creates and finishes ttid span with correct measurements', + () async { + final transaction = + fixture.hub.startTransaction('fake', 'fake') as SentryTracer; + final startTimestamp = DateTime.now(); + + sut.markAsManual(); + Future.delayed(fixture.finishFrameAfterDuration, () { + sut.completeTracking(); + }); + await sut.trackRegularRoute(transaction, startTimestamp, 'regular route'); + + 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.finishFrameAfterDuration.inMilliseconds)); + }); + }); + + group('determineEndtime', () { + test('can complete automatically in approximation mode', () async { + final futureEndTime = sut.determineEndTime(); + + expect(futureEndTime, completes); + }); + + test('prevents automatic completion in manual mode', () async { + sut.markAsManual(); + final futureEndTime = sut.determineEndTime(); + + expect(futureEndTime, doesNotComplete); + }); + + test('can complete manually in manual mode', () async { + sut.markAsManual(); + final futureEndTime = sut.determineEndTime(); + + sut.completeTracking(); + expect(futureEndTime, completes); + }); + + test('returns the correct approximation end time', () async { + final startTime = DateTime.now(); + + final futureEndTime = sut.determineEndTime(); + + final endTime = await futureEndTime; + expect(endTime?.difference(startTime).inSeconds, + fixture.finishFrameAfterDuration.inSeconds); + }); + + test('returns the correct manual end time', () async { + final startTime = DateTime.now(); + + sut.markAsManual(); + final futureEndTime = sut.determineEndTime(); + + Future.delayed(fixture.finishFrameAfterDuration, () { + sut.completeTracking(); + }); + + final endTime = await futureEndTime; + expect(endTime?.difference(startTime).inSeconds, + fixture.finishFrameAfterDuration.inSeconds); + }); + }); +} + +class Fixture { + final hub = Hub(SentryFlutterOptions(dsn: fakeDsn)..tracesSampleRate = 1.0); + final finishFrameAfterDuration = Duration(milliseconds: 100); + late final fakeFrameCallbackHandler = + FakeFrameCallbackHandler(finishAfterDuration: finishFrameAfterDuration); + + TimeToInitialDisplayTracker getSut() { + return TimeToInitialDisplayTracker( + frameCallbackHandler: fakeFrameCallbackHandler); + } +} diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index aca9646c0b..6b35342e1c 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -7,7 +7,11 @@ import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/native/sentry_native.dart'; import 'package:sentry/src/sentry_tracer.dart'; +import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; +import 'package:sentry_flutter/src/navigation/time_to_display_transaction_handler.dart'; +import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; +import 'fake_frame_callback_handler.dart'; import 'mocks.dart'; import 'mocks.mocks.dart'; @@ -30,10 +34,12 @@ void main() { customSamplingContext: anyNamed('customSamplingContext'), startTimestamp: anyNamed('startTimestamp'), )).thenReturn(thenReturnSpan); + when(mockHub.getSpan()).thenReturn(thenReturnSpan); } setUp(() { fixture = Fixture(); + WidgetsFlutterBinding.ensureInitialized(); }); group('NativeFrames', () { @@ -55,6 +61,12 @@ void main() { final tracer = getMockSentryTracer(); _whenAnyStart(mockHub, tracer); + when(tracer.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); + when(tracer.finished).thenReturn(false); + when(tracer.status).thenReturn(SpanStatus.ok()); final sut = fixture.getSut(hub: mockHub); @@ -73,13 +85,14 @@ void main() { options.tracesSampleRate = 1; final hub = Hub(options); + mockNativeChannel = MockNativeChannel(); + SentryFlutter.native = + SentryNative(SentryFlutterOptions(dsn: fakeDsn), mockNativeChannel); + final nativeFrames = NativeFrames(3, 2, 1); mockNativeChannel.nativeFrames = nativeFrames; - final sut = fixture.getSut( - hub: hub, - autoFinishAfter: Duration(milliseconds: 50), - ); + final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); @@ -91,13 +104,15 @@ void main() { actualTransaction = scope.span as SentryTracer; }); - await Future.delayed(Duration(milliseconds: 500)); + await sut.completedDisplayTracking?.future; + + await Future.delayed(Duration(milliseconds: 1500)); expect(mockNativeChannel.numberOfEndNativeFramesCalls, 1); final measurements = actualTransaction?.measurements ?? {}; - expect(measurements.length, 3); + expect(measurements.length, 4); final expectedTotal = SentryMeasurement.totalFrames(3); final expectedSlow = SentryMeasurement.slowFrames(2); @@ -117,15 +132,21 @@ void main() { }); group('$SentryNavigatorObserver', () { - test('didPush starts transaction', () { + test('didPush starts transaction', () async { const name = 'Current Route'; final currentRoute = route(RouteSettings(name: name)); - const op = 'navigation'; + const op = 'ui.load'; final hub = _MockHub(); final span = getMockSentryTracer(name: name); when(span.context).thenReturn(SentrySpanContext(operation: op)); _whenAnyStart(hub, span); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); final sut = fixture.getSut( hub: hub, @@ -136,6 +157,7 @@ void main() { final context = verify(hub.startTransactionWithContext( captureAny, + startTimestamp: anyNamed('startTimestamp'), waitForChildren: true, autoFinishAfter: anyNamed('autoFinishAfter'), trimEnd: true, @@ -157,6 +179,7 @@ void main() { final span = NoOpSentrySpan(); _whenAnyStart(hub, span); + when(hub.getSpan()).thenReturn(null); final sut = fixture.getSut( hub: hub, @@ -167,6 +190,7 @@ void main() { verify(hub.startTransactionWithContext( any, + startTimestamp: anyNamed('startTimestamp'), waitForChildren: true, autoFinishAfter: Duration(seconds: 5), trimEnd: true, @@ -185,6 +209,8 @@ void main() { final hub = _MockHub(); final span = getMockSentryTracer(); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -193,6 +219,7 @@ void main() { verifyNever(hub.startTransactionWithContext( any, + startTimestamp: anyNamed('startTimestamp'), waitForChildren: true, autoFinishAfter: anyNamed('autoFinishAfter'), trimEnd: true, @@ -211,6 +238,8 @@ void main() { final span = getMockSentryTracer(); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); _whenAnyStart(hub, span); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); final sut = fixture.getSut(hub: hub, enableAutoTransactions: false); @@ -218,6 +247,7 @@ void main() { verifyNever(hub.startTransactionWithContext( any, + startTimestamp: anyNamed('startTimestamp'), waitForChildren: true, autoFinishAfter: anyNamed('autoFinishAfter'), trimEnd: true, @@ -237,6 +267,12 @@ void main() { final span = getMockSentryTracer(); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -245,6 +281,7 @@ void main() { verify(hub.startTransactionWithContext( any, + startTimestamp: anyNamed('startTimestamp'), waitForChildren: true, autoFinishAfter: anyNamed('autoFinishAfter'), trimEnd: true, @@ -256,7 +293,7 @@ void main() { }); }); - test('didPush finishes previous transaction', () { + test('didPush finishes previous transaction', () async { final firstRoute = route(RouteSettings(name: 'First Route')); final secondRoute = route(RouteSettings(name: 'Second Route')); @@ -264,6 +301,11 @@ void main() { final span = getMockSentryTracer(finished: false); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); when(span.status).thenReturn(null); + when(span.finished).thenReturn(false); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -282,6 +324,11 @@ void main() { final span = getMockSentryTracer(finished: false); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); when(span.status).thenReturn(null); + when(span.finished).thenReturn(false); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -305,40 +352,14 @@ void main() { final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); + when(span.finished).thenReturn(true); + sut.didPop(currentRoute, null); sut.didPop(currentRoute, null); verify(span.finish()).called(1); }); - test('didPop re-starts previous', () { - final previousRoute = route(RouteSettings(name: 'Previous Route')); - final currentRoute = route(RouteSettings(name: 'Current Route')); - - final hub = _MockHub(); - final previousSpan = getMockSentryTracer(); - when(previousSpan.context).thenReturn(SentrySpanContext(operation: 'op')); - when(previousSpan.status).thenReturn(null); - - _whenAnyStart(hub, previousSpan); - - final sut = fixture.getSut(hub: hub); - - sut.didPop(currentRoute, previousRoute); - - verify(hub.startTransactionWithContext( - any, - waitForChildren: true, - autoFinishAfter: anyNamed('autoFinishAfter'), - trimEnd: true, - onFinish: anyNamed('onFinish'), - )); - - hub.configureScope((scope) { - expect(scope.span, previousSpan); - }); - }); - test('route arguments are set on transaction', () { final arguments = {'foo': 'bar'}; final currentRoute = route(RouteSettings( @@ -350,6 +371,11 @@ void main() { final span = getMockSentryTracer(); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); when(span.status).thenReturn(null); + when(span.finished).thenReturn(false); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -365,6 +391,12 @@ void main() { final hub = _MockHub(); final span = getMockSentryTracer(name: '/'); when(span.context).thenReturn(SentrySpanContext(operation: 'op')); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut(hub: hub); @@ -374,6 +406,7 @@ void main() { final context = verify(hub.startTransactionWithContext( captureAny, waitForChildren: true, + startTimestamp: anyNamed('startTimestamp'), autoFinishAfter: anyNamed('autoFinishAfter'), trimEnd: true, onFinish: anyNamed('onFinish'), @@ -394,6 +427,12 @@ void main() { final hub = _MockHub(); final span = getMockSentryTracer(name: name); when(span.context).thenReturn(SentrySpanContext(operation: op)); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut( @@ -416,6 +455,12 @@ void main() { final hub = _MockHub(); final span = getMockSentryTracer(name: oldRouteName); when(span.context).thenReturn(SentrySpanContext(operation: op)); + when(span.finished).thenReturn(false); + when(span.status).thenReturn(SpanStatus.ok()); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut( @@ -440,6 +485,11 @@ void main() { final span = getMockSentryTracer(name: oldRouteName); when(span.context).thenReturn(SentrySpanContext(operation: op)); when(span.status).thenReturn(null); + when(span.finished).thenReturn(false); + when(span.startChild('ui.load.initial_display', + description: anyNamed('description'), + startTimestamp: anyNamed('startTimestamp'))) + .thenReturn(NoOpSentrySpan()); _whenAnyStart(hub, span); final sut = fixture.getSut( @@ -659,6 +709,7 @@ void main() { final hub = _MockHub(); final observer = fixture.getSut(hub: hub); + when(hub.getSpan()).thenReturn(NoOpSentrySpan()); final to = route(); final previous = route(); @@ -818,11 +869,23 @@ class Fixture { SentryNavigatorObserver getSut({ required Hub hub, bool enableAutoTransactions = true, - Duration autoFinishAfter = const Duration(seconds: 3), + Duration autoFinishAfter = const Duration(seconds: 1), bool setRouteNameAsTransaction = false, RouteNameExtractor? routeNameExtractor, AdditionalInfoExtractor? additionalInfoProvider, }) { + final frameCallbackHandler = FakeFrameCallbackHandler(); + final timeToInitialDisplayTracker = + TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); + final timeToDisplayTracker = TimeToDisplayTracker( + enableTimeToFullDisplayTracing: false, + ttdTransactionHandler: TimeToDisplayTransactionHandler( + hub: hub, + enableAutoTransactions: enableAutoTransactions, + autoFinishAfter: autoFinishAfter, + ), + ttidTracker: timeToInitialDisplayTracker, + ); return SentryNavigatorObserver( hub: hub, enableAutoTransactions: enableAutoTransactions, @@ -830,6 +893,7 @@ class Fixture { setRouteNameAsTransaction: setRouteNameAsTransaction, routeNameExtractor: routeNameExtractor, additionalInfoProvider: additionalInfoProvider, + timeToDisplayTracker: timeToDisplayTracker, ); } From 845e6a8d2d5b77a5a40119016c146de3c900e3d5 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 13:05:29 +0100 Subject: [PATCH 11/57] Improvements --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 5402603c25..be8d488bb3 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -117,6 +117,7 @@ class SentryNavigatorObserver extends RouteObserver> { Completer? _completedDisplayTracking; + // Since didPush does not have a future, we can keep track of when the display tracking has finished @visibleForTesting Completer? get completedDisplayTracking => _completedDisplayTracking; From 96c876679f332a5a0dd2af4e2773e875fc676a90 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 13:10:03 +0100 Subject: [PATCH 12/57] Improvements --- CHANGELOG.md | 10 ++++++++++ dart/lib/sentry.dart | 2 ++ dart/lib/src/sentry_span_operations.dart | 8 ++++++++ dart/lib/src/sentry_trace_origins.dart | 2 ++ 4 files changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f259f8b0..4dd77c62b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ ### Features +- Add TTID, allows you to measure the time it takes to render the first frame of your screen ([#1882](https://github.com/getsentry/sentry-dart/pull/1882)) + - Requires using the [routing instrumentation](https://docs.sentry.io/platforms/flutter/integrations/routing-instrumentation/). + - Introduces two modes: `automatic` and `manual`. + - `automatic` mode is enabled by default for all screens and will yield only an approximation result. + - `manual` mode requires manual instrumentation and will yield a more accurate result. + - To use `manual` mode, you need to wrap your desired widget: `SentryDisplayWidget(child: MyScreen())`. + - You can mix and match both modes in your app. + - Other significant fixes + - `didPop` doesn't trigger a new transaction + - Change transaction operation name to `ui.load` instead of `navigation` - Use `recordHttpBreadcrumbs` to set iOS `enableNetworkBreadcrumbs` ([#1884](https://github.com/getsentry/sentry-dart/pull/1884)) ### Improvements diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart index 5419aa45b8..f416d0b797 100644 --- a/dart/lib/sentry.dart +++ b/dart/lib/sentry.dart @@ -49,6 +49,8 @@ export 'src/utils/http_header_utils.dart'; // ignore: invalid_export_of_internal_element export 'src/sentry_trace_origins.dart'; // ignore: invalid_export_of_internal_element +export 'src/sentry_span_operations.dart'; +// ignore: invalid_export_of_internal_element export 'src/utils.dart'; // spotlight debugging export 'src/spotlight.dart'; diff --git a/dart/lib/src/sentry_span_operations.dart b/dart/lib/src/sentry_span_operations.dart index e69de29bb2..eba53aae5b 100644 --- a/dart/lib/src/sentry_span_operations.dart +++ b/dart/lib/src/sentry_span_operations.dart @@ -0,0 +1,8 @@ +import 'package:meta/meta.dart'; + +@internal +class SentrySpanOperations { + static const String uiLoad = 'ui.load'; + static const String uiTimeToInitialDisplay = 'ui.load.initial_display'; + static const String uiTimeToFullDisplay = 'ui.load.full_display'; +} \ No newline at end of file diff --git a/dart/lib/src/sentry_trace_origins.dart b/dart/lib/src/sentry_trace_origins.dart index 5a7c49b339..4377fa2c03 100644 --- a/dart/lib/src/sentry_trace_origins.dart +++ b/dart/lib/src/sentry_trace_origins.dart @@ -27,4 +27,6 @@ class SentryTraceOrigins { static const autoDbDriftQueryExecutor = 'auto.db.drift.query.executor'; static const autoDbDriftTransactionExecutor = 'auto.db.drift.transaction.executor'; + static const autoUiTimeToDisplay = 'auto.ui.time_to_display'; + static const manualUiTimeToDisplay = 'manual.ui.time_to_display'; } From 1d9e71fff42c22730e4a43aec3a580335dc5ac5e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 14:24:57 +0100 Subject: [PATCH 13/57] Fix integration test --- .../example/integration_test/integration_test.dart | 10 ++++++++++ flutter/example/lib/main.dart | 1 + .../integrations/native_app_start_integration.dart | 12 +++++++++++- .../src/navigation/sentry_navigator_observer.dart | 1 + .../lib/src/navigation/time_to_display_tracker.dart | 4 ++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/flutter/example/integration_test/integration_test.dart b/flutter/example/integration_test/integration_test.dart index c4c71edb41..92dfa3400d 100644 --- a/flutter/example/integration_test/integration_test.dart +++ b/flutter/example/integration_test/integration_test.dart @@ -1,4 +1,5 @@ // ignore_for_file: avoid_print +// ignore_for_file: invalid_use_of_internal_member import 'dart:async'; import 'dart:convert'; @@ -8,6 +9,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter_example/main.dart'; import 'package:http/http.dart'; +import 'package:sentry_flutter/src/integrations/native_app_start_integration.dart'; void main() { // const org = 'sentry-sdks'; @@ -24,6 +26,8 @@ void main() { // Using fake DSN for testing purposes. Future setupSentryAndApp(WidgetTester tester, {String? dsn, BeforeSendCallback? beforeSendCallback}) async { + NativeAppStartIntegration.isIntegrationTest = true; + await setupSentry( () async { await tester.pumpWidget(SentryScreenshotWidget( @@ -128,8 +132,14 @@ void main() { testWidgets('setup sentry and start transaction', (tester) async { await setupSentryAndApp(tester); + print('here'); final transaction = Sentry.startTransaction('transaction', 'test'); + + print('here2'); + await transaction.finish(); + + print('here3'); }); testWidgets('setup sentry and start transaction with context', diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 6a0c0a5b81..a46a9ca641 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -80,6 +80,7 @@ Future setupSentry( // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. options.debug = true; + options.spotlight = Spotlight(enabled: true); options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; diff --git a/flutter/lib/src/integrations/native_app_start_integration.dart b/flutter/lib/src/integrations/native_app_start_integration.dart index 219721af5a..0ba5c77c4f 100644 --- a/flutter/lib/src/integrations/native_app_start_integration.dart +++ b/flutter/lib/src/integrations/native_app_start_integration.dart @@ -22,6 +22,9 @@ class NativeAppStartIntegration extends Integration { Completer(); static AppStartInfo? _appStartInfo; + @internal + static bool isIntegrationTest = false; + @internal static void setAppStartInfo(AppStartInfo? appStartInfo) { _appStartInfo = appStartInfo; @@ -47,10 +50,17 @@ class NativeAppStartIntegration extends Integration { @override void call(Hub hub, SentryFlutterOptions options) { + if (isIntegrationTest) { + final appStartInfo = AppStartInfo(AppStartType.cold, + start: DateTime.now(), + end: DateTime.now().add(const Duration(milliseconds: 100))); + setAppStartInfo(appStartInfo); + return; + } + if (options.autoAppStart) { _frameCallbackHandler.addPostFrameCallback((timeStamp) async { if (_native.didFetchAppStart) { - setAppStartInfo(null); return; } diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index be8d488bb3..9862ae5817 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -145,6 +145,7 @@ class SentryNavigatorObserver extends RouteObserver> { } transaction.status ??= SpanStatus.ok(); await transaction.finish(); + timeToDisplayTracker?.clear(); } @override diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index a4312d4a2e..b02c1b46bd 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -84,4 +84,8 @@ class TimeToDisplayTracker { Future reportFullyDisplayed() async { // TODO: implement TTFD } + + void clear() { + _ttidTracker.clear(); + } } From cea12c5f55782873654c97c7a3ea7d2ac4e669da Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 14:58:48 +0100 Subject: [PATCH 14/57] Update --- flutter/example/integration_test/integration_test.dart | 6 ------ flutter/example/lib/main.dart | 1 - 2 files changed, 7 deletions(-) diff --git a/flutter/example/integration_test/integration_test.dart b/flutter/example/integration_test/integration_test.dart index 92dfa3400d..f292d04cfc 100644 --- a/flutter/example/integration_test/integration_test.dart +++ b/flutter/example/integration_test/integration_test.dart @@ -132,14 +132,8 @@ void main() { testWidgets('setup sentry and start transaction', (tester) async { await setupSentryAndApp(tester); - print('here'); final transaction = Sentry.startTransaction('transaction', 'test'); - - print('here2'); - await transaction.finish(); - - print('here3'); }); testWidgets('setup sentry and start transaction with context', diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index a46a9ca641..6a0c0a5b81 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -80,7 +80,6 @@ Future setupSentry( // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. options.debug = true; - options.spotlight = Spotlight(enabled: true); options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; From 9af34553000adf8f50da52f35607af8179b40b61 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:03:56 +0100 Subject: [PATCH 15/57] Clear after tracking --- flutter/lib/src/navigation/time_to_display_tracker.dart | 5 +++-- .../lib/src/navigation/time_to_initial_display_tracker.dart | 3 --- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index b02c1b46bd..b224f6f272 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -34,10 +34,11 @@ class TimeToDisplayTracker { if (isRootScreen && didFetchAppStart == false) { // Dart cannot infer here that routeName is not nullable if (routeName == null) return; - return _trackAppStartTTD(routeName, arguments); + await _trackAppStartTTD(routeName, arguments); } else { - return _trackRegularRouteTTD(routeName, arguments, startTimestamp); + await _trackRegularRouteTTD(routeName, arguments, startTimestamp); } + _ttidTracker.clear(); } /// This method listens for the completion of the app's start process via diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index a86d52bf17..3acb56b838 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -56,9 +56,6 @@ class TimeToInitialDisplayTracker { transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, unit: ttidMeasurement.unit); await ttidSpan.finish(endTimestamp: endTimestamp); - - // We can clear the state after creating and finishing the ttid span has finished - clear(); } Future trackAppStart(ISentrySpan transaction, AppStartInfo appStartInfo, From 71fd7ec80427c088b6e37d5f2ad148d46922c116 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:06:36 +0100 Subject: [PATCH 16/57] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dd77c62b2..9881683ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Add TTID, allows you to measure the time it takes to render the first frame of your screen ([#1882](https://github.com/getsentry/sentry-dart/pull/1882)) +- Add TTID, allows you to measure the time it takes to render the first frame of your screen ([#1910](https://github.com/getsentry/sentry-dart/pull/1910)) - Requires using the [routing instrumentation](https://docs.sentry.io/platforms/flutter/integrations/routing-instrumentation/). - Introduces two modes: `automatic` and `manual`. - `automatic` mode is enabled by default for all screens and will yield only an approximation result. From 7a977fb00effdd8ea016113ad8c15df74820a429 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:18:23 +0100 Subject: [PATCH 17/57] Format --- dart/lib/src/sentry_span_operations.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dart/lib/src/sentry_span_operations.dart b/dart/lib/src/sentry_span_operations.dart index eba53aae5b..27b1d22496 100644 --- a/dart/lib/src/sentry_span_operations.dart +++ b/dart/lib/src/sentry_span_operations.dart @@ -5,4 +5,4 @@ class SentrySpanOperations { static const String uiLoad = 'ui.load'; static const String uiTimeToInitialDisplay = 'ui.load.initial_display'; static const String uiTimeToFullDisplay = 'ui.load.full_display'; -} \ No newline at end of file +} From d7a6a83006e9b193fbc0a5ce5a27d1f086fab48d Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:22:45 +0100 Subject: [PATCH 18/57] Update --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 3 ++- .../lib/src/navigation/time_to_initial_display_tracker.dart | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 9862ae5817..b0733e8154 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -139,13 +139,14 @@ class SentryNavigatorObserver extends RouteObserver> { } Future _finishTransaction() async { + timeToDisplayTracker?.clear(); + final transaction = _hub.getSpan(); if (transaction == null || transaction.finished) { return; } transaction.status ??= SpanStatus.ok(); await transaction.finish(); - timeToDisplayTracker?.clear(); } @override diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index 3acb56b838..477c3d4328 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -106,7 +106,6 @@ class TimeToInitialDisplayTracker { if (_trackingCompleter != null && !_trackingCompleter!.isCompleted) { final endTimestamp = DateTime.now(); _endTimestamp = endTimestamp; - // Reset after completion _trackingCompleter?.complete(endTimestamp); } } From cde560f8fe3c4399b19deca68a9b63c7cfb37898 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:30:55 +0100 Subject: [PATCH 19/57] Update --- flutter/lib/sentry_flutter.dart | 1 + flutter/lib/src/navigation/sentry_navigator_observer.dart | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index 0f43bf741b..d15c8b7a70 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -16,3 +16,4 @@ export 'src/screenshot/sentry_screenshot_quality.dart'; export 'src/user_interaction/sentry_user_interaction_widget.dart'; export 'src/binding_wrapper.dart'; export 'src/sentry_widget.dart'; +export 'src/navigation/sentry_display_widget.dart'; diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index b0733e8154..d493735bac 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -5,12 +5,12 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import 'time_to_display_tracker.dart'; import 'time_to_display_transaction_handler.dart'; import '../../sentry_flutter.dart'; import '../event_processor/flutter_enricher_event_processor.dart'; import '../native/sentry_native.dart'; -import 'time_to_display_tracker.dart'; /// This key must be used so that the web interface displays the events nicely /// See https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/ @@ -26,7 +26,7 @@ typedef AdditionalInfoExtractor = Map? Function( /// This is a navigation observer to record navigational breadcrumbs. /// For now it only records navigation events and no gestures. /// -/// It also records Time to Initial Display (TTID) and Time to Full Display (TTFD). +/// It also records Time to Initial Display (TTID). /// /// [Route]s can always be null and their [Route.settings] can also always be null. /// For example, if the application starts, there is no previous route. From f66206da2d5d86aca753843c41330f43461c8153 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 15:36:22 +0100 Subject: [PATCH 20/57] remove import --- flutter/lib/sentry_flutter.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index d15c8b7a70..0f43bf741b 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -16,4 +16,3 @@ export 'src/screenshot/sentry_screenshot_quality.dart'; export 'src/user_interaction/sentry_user_interaction_widget.dart'; export 'src/binding_wrapper.dart'; export 'src/sentry_widget.dart'; -export 'src/navigation/sentry_display_widget.dart'; From 4dedf37f828d3ea4f5bf3cf7c4a95683379c2492 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Mon, 4 Mar 2024 16:56:08 +0100 Subject: [PATCH 21/57] Update sentry tracer --- dart/lib/src/sentry_tracer.dart | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index c57912fe46..ed3fb35799 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -109,18 +109,24 @@ class SentryTracer extends ISentrySpan { } var _rootEndTimestamp = commonEndTimestamp; + + // Trim the end timestamp of the transaction to the very last timestamp of child spans if (_trimEnd && children.isNotEmpty) { - final childEndTimestamps = children - .where((child) => child.endTimestamp != null) - .map((child) => child.endTimestamp!); - - if (childEndTimestamps.isNotEmpty) { - final oldestChildEndTimestamp = - childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b); - if (_rootEndTimestamp.isAfter(oldestChildEndTimestamp)) { - _rootEndTimestamp = oldestChildEndTimestamp; + DateTime? latestEndTime; + + for (var child in children) { + final childEndTimestamp = child.endTimestamp; + if (childEndTimestamp != null) { + if (latestEndTime == null || + childEndTimestamp.isAfter(latestEndTime)) { + latestEndTime = child.endTimestamp; + } } } + + if (latestEndTime != null) { + _rootEndTimestamp = latestEndTime; + } } // the callback should run before because if the span is finished, From 5dd824eff3e5df080a4b2f8ff53e185eeafcdb73 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 12:47:09 +0100 Subject: [PATCH 22/57] Add (not all) improvements for pr review --- dart/lib/src/sentry_measurement.dart | 14 +++++++++++ .../navigation/sentry_navigator_observer.dart | 2 +- .../time_to_initial_display_tracker.dart | 9 +++----- .../lib/src/sentry_flutter_measurement.dart | 23 ------------------- 4 files changed, 18 insertions(+), 30 deletions(-) delete mode 100644 flutter/lib/src/sentry_flutter_measurement.dart diff --git a/dart/lib/src/sentry_measurement.dart b/dart/lib/src/sentry_measurement.dart index 481c513e9b..8f803990ec 100644 --- a/dart/lib/src/sentry_measurement.dart +++ b/dart/lib/src/sentry_measurement.dart @@ -39,6 +39,20 @@ class SentryMeasurement { value = duration.inMilliseconds, unit = DurationSentryMeasurementUnit.milliSecond; + /// Duration of the time to initial display in milliseconds + SentryMeasurement.timeToInitialDisplay(Duration duration) : + assert(!duration.isNegative), + name = 'time_to_initial_display', + value = duration.inMilliseconds, + unit = DurationSentryMeasurementUnit.milliSecond; + + /// Duration of the time to full display in milliseconds + SentryMeasurement.timeToFullDisplay(Duration duration) : + assert(!duration.isNegative), + name = 'time_to_full_display', + value = duration.inMilliseconds, + unit = DurationSentryMeasurementUnit.milliSecond; + final String name; final num value; final SentryMeasurementUnit? unit; diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index d493735bac..a7aba9d5f5 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -221,7 +221,7 @@ class SentryNavigatorObserver extends RouteObserver> { final arguments = route?.settings.arguments; await _timeToDisplayTracker?.startTracking(routeName, arguments); - completedDisplayTracking?.complete(); + _completedDisplayTracking?.complete(); } } diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index 477c3d4328..29c3f126a2 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -7,7 +7,6 @@ import '../integrations/integrations.dart'; import '../../sentry_flutter.dart'; import '../frame_callback_handler.dart'; -import '../sentry_flutter_measurement.dart'; @internal class TimeToInitialDisplayTracker { @@ -49,10 +48,8 @@ class TimeToInitialDisplayTracker { ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; } - final ttidMeasurement = SentryFlutterMeasurement.timeToInitialDisplay( - Duration( - milliseconds: - endTimestamp.difference(startTimestamp).inMilliseconds)); + final ttidMeasurement = SentryMeasurement.timeToInitialDisplay(Duration( + milliseconds: endTimestamp.difference(startTimestamp).inMilliseconds)); transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, unit: ttidMeasurement.unit); await ttidSpan.finish(endTimestamp: endTimestamp); @@ -68,7 +65,7 @@ class TimeToInitialDisplayTracker { ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; final ttidMeasurement = - SentryFlutterMeasurement.timeToInitialDisplay(appStartInfo.duration); + SentryMeasurement.timeToInitialDisplay(appStartInfo.duration); transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, unit: ttidMeasurement.unit); diff --git a/flutter/lib/src/sentry_flutter_measurement.dart b/flutter/lib/src/sentry_flutter_measurement.dart deleted file mode 100644 index 8b47a3fc56..0000000000 --- a/flutter/lib/src/sentry_flutter_measurement.dart +++ /dev/null @@ -1,23 +0,0 @@ -import '../sentry_flutter.dart'; - -extension SentryFlutterMeasurement on SentryMeasurement { - /// Duration of the time to initial display in milliseconds - static SentryMeasurement timeToInitialDisplay(Duration duration) { - assert(!duration.isNegative); - return SentryMeasurement( - 'time_to_initial_display', - duration.inMilliseconds.toDouble(), - unit: DurationSentryMeasurementUnit.milliSecond, - ); - } - - /// Duration of the time to full display in milliseconds - static SentryMeasurement timeToFullDisplay(Duration duration) { - assert(!duration.isNegative); - return SentryMeasurement( - 'time_to_full_display', - duration.inMilliseconds.toDouble(), - unit: DurationSentryMeasurementUnit.milliSecond, - ); - } -} From bf2ad2f23d68875f0deeccaeb9d18bc7e4f0e15e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 14:34:25 +0100 Subject: [PATCH 23/57] combine transaction handler --- dart/lib/src/sentry_measurement.dart | 20 ++--- .../navigation/sentry_navigator_observer.dart | 21 ++--- .../navigation/time_to_display_tracker.dart | 83 ++++++++++++++++--- .../time_to_display_transaction_handler.dart | 80 ------------------ .../test/sentry_navigator_observer_test.dart | 10 +-- 5 files changed, 91 insertions(+), 123 deletions(-) delete mode 100644 flutter/lib/src/navigation/time_to_display_transaction_handler.dart diff --git a/dart/lib/src/sentry_measurement.dart b/dart/lib/src/sentry_measurement.dart index 8f803990ec..663197654d 100644 --- a/dart/lib/src/sentry_measurement.dart +++ b/dart/lib/src/sentry_measurement.dart @@ -40,18 +40,18 @@ class SentryMeasurement { unit = DurationSentryMeasurementUnit.milliSecond; /// Duration of the time to initial display in milliseconds - SentryMeasurement.timeToInitialDisplay(Duration duration) : - assert(!duration.isNegative), - name = 'time_to_initial_display', - value = duration.inMilliseconds, - unit = DurationSentryMeasurementUnit.milliSecond; + SentryMeasurement.timeToInitialDisplay(Duration duration) + : assert(!duration.isNegative), + name = 'time_to_initial_display', + value = duration.inMilliseconds, + unit = DurationSentryMeasurementUnit.milliSecond; /// Duration of the time to full display in milliseconds - SentryMeasurement.timeToFullDisplay(Duration duration) : - assert(!duration.isNegative), - name = 'time_to_full_display', - value = duration.inMilliseconds, - unit = DurationSentryMeasurementUnit.milliSecond; + SentryMeasurement.timeToFullDisplay(Duration duration) + : assert(!duration.isNegative), + name = 'time_to_full_display', + value = duration.inMilliseconds, + unit = DurationSentryMeasurementUnit.milliSecond; final String name; final num value; diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index a7aba9d5f5..092f61837d 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -6,7 +6,6 @@ import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; import 'time_to_display_tracker.dart'; -import 'time_to_display_transaction_handler.dart'; import '../../sentry_flutter.dart'; import '../event_processor/flutter_enricher_event_processor.dart'; @@ -83,20 +82,12 @@ class SentryNavigatorObserver extends RouteObserver> { if (enableAutoTransactions) { _hub.options.sdk.addIntegration('UINavigationTracing'); } - final options = _hub.options; - if (options is SentryFlutterOptions) { - const enableTimeToFullDisplayTracing = - false; // TODO: replace with options.enableTimeToFullDisplayTracing in TTFD - _timeToDisplayTracker = timeToDisplayTracker ?? - TimeToDisplayTracker( - enableTimeToFullDisplayTracing: enableTimeToFullDisplayTracing, - ttdTransactionHandler: TimeToDisplayTransactionHandler( - hub: hub, - enableAutoTransactions: enableAutoTransactions, - autoFinishAfter: autoFinishAfter, - ), - ); - } + _timeToDisplayTracker = timeToDisplayTracker ?? + TimeToDisplayTracker( + hub: hub, + enableAutoTransactions: enableAutoTransactions, + autoFinishAfter: autoFinishAfter, + ); } final Hub _hub; diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index b224f6f272..cef6dc312a 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -5,25 +5,88 @@ import '../integrations/integrations.dart'; import '../../sentry_flutter.dart'; import '../native/sentry_native.dart'; -import 'time_to_display_transaction_handler.dart'; import 'time_to_initial_display_tracker.dart'; @internal class TimeToDisplayTracker { + final Hub? _hub; + final bool? _enableAutoTransactions; + final Duration? _autoFinishAfter; final SentryNative? _native; - final TimeToDisplayTransactionHandler _ttdTransactionHandler; final TimeToInitialDisplayTracker _ttidTracker; - final bool _enableTimeToFullDisplayTracing; + + // TODO We can use _hub.options to fetch the ttfd flag + bool get _enableTimeToFullDisplayTracing => false; TimeToDisplayTracker({ - required bool enableTimeToFullDisplayTracing, - required TimeToDisplayTransactionHandler ttdTransactionHandler, + required Hub? hub, + required bool? enableAutoTransactions, + required Duration? autoFinishAfter, TimeToInitialDisplayTracker? ttidTracker, }) : _native = SentryFlutter.native, - _enableTimeToFullDisplayTracing = enableTimeToFullDisplayTracing, - _ttdTransactionHandler = ttdTransactionHandler, + _hub = hub ?? HubAdapter(), + _enableAutoTransactions = enableAutoTransactions, + _autoFinishAfter = autoFinishAfter, _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); + Future _startTransaction(String? routeName, Object? arguments, + {DateTime? startTimestamp}) async { + if (_enableAutoTransactions == false) { + return null; + } + + if (routeName == null) { + return null; + } + + final transactionContext = SentryTransactionContext( + routeName, + SentrySpanOperations.uiLoad, + transactionNameSource: SentryTransactionNameSource.component, + origin: SentryTraceOrigins.autoNavigationRouteObserver, + ); + + final transaction = _hub?.startTransactionWithContext( + transactionContext, + waitForChildren: true, + autoFinishAfter: _autoFinishAfter, + trimEnd: true, + startTimestamp: startTimestamp, + onFinish: (transaction) async { + final nativeFrames = await _native + ?.endNativeFramesCollection(transaction.context.traceId); + if (nativeFrames != null) { + final measurements = nativeFrames.toMeasurements(); + for (final item in measurements.entries) { + final measurement = item.value; + transaction.setMeasurement( + item.key, + measurement.value, + unit: measurement.unit, + ); + } + } + }, + ); + + // if _enableAutoTransactions is enabled but there's no traces sample rate + if (transaction is NoOpSentrySpan) { + return null; + } + + if (arguments != null) { + transaction?.setData('route_settings_arguments', arguments); + } + + _hub?.configureScope((scope) { + scope.span ??= transaction; + }); + + await _native?.beginNativeFramesCollection(); + + return transaction; + } + Future startTracking(String? routeName, Object? arguments) async { final startTimestamp = DateTime.now(); if (routeName == '/') { @@ -54,8 +117,7 @@ class TimeToDisplayTracker { if (appStartInfo == null) return; - final transaction = await _ttdTransactionHandler - .startTransaction(name, arguments, startTimestamp: appStartInfo.start); + final transaction = await _startTransaction(name, arguments, startTimestamp: appStartInfo.start); if (transaction == null) return; if (_enableTimeToFullDisplayTracing) { @@ -68,8 +130,7 @@ class TimeToDisplayTracker { /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. Future _trackRegularRouteTTD( String? routeName, Object? arguments, DateTime startTimestamp) async { - final transaction = await _ttdTransactionHandler - .startTransaction(routeName, arguments, startTimestamp: startTimestamp); + final transaction = await _startTransaction(routeName, arguments, startTimestamp: startTimestamp); if (transaction == null || routeName == null) return; diff --git a/flutter/lib/src/navigation/time_to_display_transaction_handler.dart b/flutter/lib/src/navigation/time_to_display_transaction_handler.dart deleted file mode 100644 index 94f411496a..0000000000 --- a/flutter/lib/src/navigation/time_to_display_transaction_handler.dart +++ /dev/null @@ -1,80 +0,0 @@ -// ignore_for_file: invalid_use_of_internal_member - -import 'package:meta/meta.dart'; -import '../../sentry_flutter.dart'; -import '../native/sentry_native.dart'; - -@internal -class TimeToDisplayTransactionHandler { - final Hub? _hub; - final bool? _enableAutoTransactions; - final Duration? _autoFinishAfter; - final SentryNative? _native; - - TimeToDisplayTransactionHandler({ - required Hub? hub, - required bool? enableAutoTransactions, - required Duration? autoFinishAfter, - }) : _hub = hub ?? HubAdapter(), - _enableAutoTransactions = enableAutoTransactions, - _autoFinishAfter = autoFinishAfter, - _native = SentryFlutter.native; - - Future startTransaction(String? routeName, Object? arguments, - {DateTime? startTimestamp}) async { - if (_enableAutoTransactions == false) { - return null; - } - - if (routeName == null) { - return null; - } - - final transactionContext = SentryTransactionContext( - routeName, - SentrySpanOperations.uiLoad, - transactionNameSource: SentryTransactionNameSource.component, - origin: SentryTraceOrigins.autoNavigationRouteObserver, - ); - - final transaction = _hub?.startTransactionWithContext( - transactionContext, - waitForChildren: true, - autoFinishAfter: _autoFinishAfter, - trimEnd: true, - startTimestamp: startTimestamp, - onFinish: (transaction) async { - final nativeFrames = await _native - ?.endNativeFramesCollection(transaction.context.traceId); - if (nativeFrames != null) { - final measurements = nativeFrames.toMeasurements(); - for (final item in measurements.entries) { - final measurement = item.value; - transaction.setMeasurement( - item.key, - measurement.value, - unit: measurement.unit, - ); - } - } - }, - ); - - // if _enableAutoTransactions is enabled but there's no traces sample rate - if (transaction is NoOpSentrySpan) { - return null; - } - - if (arguments != null) { - transaction?.setData('route_settings_arguments', arguments); - } - - _hub?.configureScope((scope) { - scope.span ??= transaction; - }); - - await _native?.beginNativeFramesCollection(); - - return transaction; - } -} diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 6b35342e1c..01325cdba0 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -8,7 +8,6 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/native/sentry_native.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; -import 'package:sentry_flutter/src/navigation/time_to_display_transaction_handler.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; import 'fake_frame_callback_handler.dart'; @@ -878,12 +877,9 @@ class Fixture { final timeToInitialDisplayTracker = TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); final timeToDisplayTracker = TimeToDisplayTracker( - enableTimeToFullDisplayTracing: false, - ttdTransactionHandler: TimeToDisplayTransactionHandler( - hub: hub, - enableAutoTransactions: enableAutoTransactions, - autoFinishAfter: autoFinishAfter, - ), + hub: hub, + enableAutoTransactions: enableAutoTransactions, + autoFinishAfter: autoFinishAfter, ttidTracker: timeToInitialDisplayTracker, ); return SentryNavigatorObserver( From 4d33aabaf8521d861d268a6b773036a19a202537 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 14:56:19 +0100 Subject: [PATCH 24/57] Refactor trackAppStart and trackRegularRoute to use private method --- .../navigation/time_to_display_tracker.dart | 15 ++-- .../time_to_initial_display_tracker.dart | 81 ++++++++++++------- .../time_to_display_tracker_test.dart | 13 +-- 3 files changed, 60 insertions(+), 49 deletions(-) diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index cef6dc312a..44a8929a5e 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -94,9 +94,11 @@ class TimeToDisplayTracker { } final isRootScreen = routeName == 'root ("/")'; final didFetchAppStart = _native?.didFetchAppStart; + + if (routeName == null) return; + if (isRootScreen && didFetchAppStart == false) { // Dart cannot infer here that routeName is not nullable - if (routeName == null) return; await _trackAppStartTTD(routeName, arguments); } else { await _trackRegularRouteTTD(routeName, arguments, startTimestamp); @@ -113,26 +115,23 @@ class TimeToDisplayTracker { /// We start and immediately finish the TTID span since we cannot mutate the history of spans. Future _trackAppStartTTD(String routeName, Object? arguments) async { final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); - final name = routeName; - if (appStartInfo == null) return; - final transaction = await _startTransaction(name, arguments, startTimestamp: appStartInfo.start); + final transaction = await _startTransaction(routeName, arguments, startTimestamp: appStartInfo.start); if (transaction == null) return; if (_enableTimeToFullDisplayTracing) { // TODO: implement TTFD } - await _ttidTracker.trackAppStart(transaction, appStartInfo, name); + await _ttidTracker.trackAppStart(transaction, appStartInfo, routeName); } /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. Future _trackRegularRouteTTD( - String? routeName, Object? arguments, DateTime startTimestamp) async { + String routeName, Object? arguments, DateTime startTimestamp) async { final transaction = await _startTransaction(routeName, arguments, startTimestamp: startTimestamp); - - if (transaction == null || routeName == null) return; + if (transaction == null) return; if (_enableTimeToFullDisplayTracing) { // TODO: implement TTFD diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index 29c3f126a2..e0895a51fc 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -32,48 +32,67 @@ class TimeToInitialDisplayTracker { @internal DateTime? get endTimestamp => _endTimestamp; - Future trackRegularRoute(ISentrySpan transaction, - DateTime startTimestamp, String routeName) async { - final endTimestamp = await determineEndTime(); - if (endTimestamp == null) return; - - final ttidSpan = transaction.startChild( - SentrySpanOperations.uiTimeToInitialDisplay, - description: '$routeName initial display', - startTimestamp: startTimestamp); + Future trackRegularRoute( + ISentrySpan transaction, + DateTime startTimestamp, + String routeName, + ) async { + await _trackTimeToInitialDisplay( + transaction: transaction, + startTimestamp: startTimestamp, + routeName: routeName, + // endTimestamp is null by default, determined inside the private method + // origin could be set here if needed, or determined inside the private method + ); + } - if (_isManual) { - ttidSpan.origin = SentryTraceOrigins.manualUiTimeToDisplay; - } else { - ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; - } + Future trackAppStart( + ISentrySpan transaction, + AppStartInfo appStartInfo, + String routeName, + ) async { + await _trackTimeToInitialDisplay( + transaction: transaction, + startTimestamp: appStartInfo.start, + routeName: routeName, + endTimestamp: appStartInfo.end, + origin: SentryTraceOrigins.autoUiTimeToDisplay, + ); - final ttidMeasurement = SentryMeasurement.timeToInitialDisplay(Duration( - milliseconds: endTimestamp.difference(startTimestamp).inMilliseconds)); - transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, - unit: ttidMeasurement.unit); - await ttidSpan.finish(endTimestamp: endTimestamp); + // Store the end timestamp for potential use by TTFD tracking + _endTimestamp = appStartInfo.end; } - Future trackAppStart(ISentrySpan transaction, AppStartInfo appStartInfo, - String routeName) async { + Future _trackTimeToInitialDisplay({ + required ISentrySpan transaction, + required DateTime startTimestamp, + required String routeName, + DateTime? endTimestamp, + String? origin, + }) async { + // Determine endTimestamp if not provided + final _endTimestamp = endTimestamp ?? await determineEndTime(); + if (_endTimestamp == null) return; + final ttidSpan = transaction.startChild( SentrySpanOperations.uiTimeToInitialDisplay, description: '$routeName initial display', - startTimestamp: appStartInfo.start, + startTimestamp: startTimestamp, ); - ttidSpan.origin = SentryTraceOrigins.autoUiTimeToDisplay; - final ttidMeasurement = - SentryMeasurement.timeToInitialDisplay(appStartInfo.duration); - transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, - unit: ttidMeasurement.unit); + // Set the origin based on provided value or determine based on _isManual + ttidSpan.origin = origin ?? + (_isManual + ? SentryTraceOrigins.manualUiTimeToDisplay + : SentryTraceOrigins.autoUiTimeToDisplay); - // Since app start measurement is immediate, finish the TTID span with the app start's end timestamp - await ttidSpan.finish(endTimestamp: appStartInfo.end); + final duration = Duration( + milliseconds: _endTimestamp.difference(startTimestamp).inMilliseconds); + final ttidMeasurement = SentryMeasurement.timeToInitialDisplay(duration); - // Store the end timestamp for potential use by TTFD tracking - _endTimestamp = appStartInfo.end; + transaction.setMeasurement(ttidMeasurement.name, ttidMeasurement.value, + unit: ttidMeasurement.unit); + await ttidSpan.finish(endTimestamp: _endTimestamp); } Future? determineEndTime() { diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 5754a7f7f9..13b8f77b1a 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -6,7 +6,6 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry_flutter/src/navigation/time_to_display_transaction_handler.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; import '../fake_frame_callback_handler.dart'; @@ -120,16 +119,10 @@ class Fixture { final ttfdAutoFinishAfter = Duration(milliseconds: 500); TimeToDisplayTracker getSut() { - // TODO: replace with options.enableTimeToFullDisplayTracing in TTFD - const enableTimeToFullDisplayTracing = false; - return TimeToDisplayTracker( - enableTimeToFullDisplayTracing: enableTimeToFullDisplayTracing, - ttdTransactionHandler: TimeToDisplayTransactionHandler( - hub: hub, - enableAutoTransactions: true, - autoFinishAfter: const Duration(seconds: 30), - ), + hub: hub, + enableAutoTransactions: true, + autoFinishAfter: const Duration(seconds: 30), ttidTracker: ttidTracker, ); } From 248cb8fea9fcad807ce09ed017468952c4487f29 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 14:57:43 +0100 Subject: [PATCH 25/57] Fix dart analyzer --- flutter/lib/src/navigation/time_to_display_tracker.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index 44a8929a5e..b4bd0fb087 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -1,3 +1,5 @@ +// ignore_for_file: invalid_use_of_internal_member + import 'dart:async'; import 'package:meta/meta.dart'; From 35ec31c17ce72968e1ccb674eb57baf5c0c638e3 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 14:58:49 +0100 Subject: [PATCH 26/57] Remove clear --- flutter/lib/src/navigation/time_to_display_tracker.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index b4bd0fb087..2483a33ef7 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -105,7 +105,6 @@ class TimeToDisplayTracker { } else { await _trackRegularRouteTTD(routeName, arguments, startTimestamp); } - _ttidTracker.clear(); } /// This method listens for the completion of the app's start process via From e1fde584fa4498a2f90e16bdc03bc57ec557d23e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 15:21:09 +0100 Subject: [PATCH 27/57] Clear in tearDown --- flutter/test/navigation/time_to_display_tracker_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 13b8f77b1a..a13e957f04 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -19,6 +19,10 @@ void main() { fixture = Fixture(); }); + tearDown(() { + fixture.ttidTracker.clear(); + }); + group('time to initial display', () { group('in root screen app start route', () { test('startMeasurement finishes ttid span', () async { From 4efd9f984ffcc080482db7fb56f167df333957b4 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Tue, 5 Mar 2024 15:58:04 +0100 Subject: [PATCH 28/57] Apply suggestions from code review Co-authored-by: Philipp Hofmann --- CHANGELOG.md | 4 ++-- dart/lib/src/sentry_measurement.dart | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9881683ad2..0d62743098 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,9 @@ ### Features -- Add TTID, allows you to measure the time it takes to render the first frame of your screen ([#1910](https://github.com/getsentry/sentry-dart/pull/1910)) +- Add TTID (time to initial display), which allows you to measure the time it takes to render the first frame of your screen ([#1910](https://github.com/getsentry/sentry-dart/pull/1910)) - Requires using the [routing instrumentation](https://docs.sentry.io/platforms/flutter/integrations/routing-instrumentation/). - - Introduces two modes: `automatic` and `manual`. + - Introduces two modes: - `automatic` mode is enabled by default for all screens and will yield only an approximation result. - `manual` mode requires manual instrumentation and will yield a more accurate result. - To use `manual` mode, you need to wrap your desired widget: `SentryDisplayWidget(child: MyScreen())`. diff --git a/dart/lib/src/sentry_measurement.dart b/dart/lib/src/sentry_measurement.dart index 663197654d..bc8b40069c 100644 --- a/dart/lib/src/sentry_measurement.dart +++ b/dart/lib/src/sentry_measurement.dart @@ -46,12 +46,6 @@ class SentryMeasurement { value = duration.inMilliseconds, unit = DurationSentryMeasurementUnit.milliSecond; - /// Duration of the time to full display in milliseconds - SentryMeasurement.timeToFullDisplay(Duration duration) - : assert(!duration.isNegative), - name = 'time_to_full_display', - value = duration.inMilliseconds, - unit = DurationSentryMeasurementUnit.milliSecond; final String name; final num value; From 579a3c5cebeb75caab5552a52e06d4d56d2825c6 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 17:10:53 +0100 Subject: [PATCH 29/57] Apply PR suggestions --- dart/lib/src/sentry_measurement.dart | 1 - dart/lib/src/sentry_tracer.dart | 23 ++--- .../navigation/sentry_navigator_observer.dart | 87 ++++++++++++++++-- .../navigation/time_to_display_tracker.dart | 92 +++---------------- .../time_to_initial_display_tracker.dart | 16 ++-- .../time_to_display_tracker_test.dart | 24 +++-- .../test/sentry_navigator_observer_test.dart | 5 +- 7 files changed, 127 insertions(+), 121 deletions(-) diff --git a/dart/lib/src/sentry_measurement.dart b/dart/lib/src/sentry_measurement.dart index bc8b40069c..b95e506aab 100644 --- a/dart/lib/src/sentry_measurement.dart +++ b/dart/lib/src/sentry_measurement.dart @@ -46,7 +46,6 @@ class SentryMeasurement { value = duration.inMilliseconds, unit = DurationSentryMeasurementUnit.milliSecond; - final String name; final num value; final SentryMeasurementUnit? unit; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index ed3fb35799..86b4bd35e8 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -110,23 +110,18 @@ class SentryTracer extends ISentrySpan { var _rootEndTimestamp = commonEndTimestamp; - // Trim the end timestamp of the transaction to the very last timestamp of child spans if (_trimEnd && children.isNotEmpty) { - DateTime? latestEndTime; - - for (var child in children) { - final childEndTimestamp = child.endTimestamp; - if (childEndTimestamp != null) { - if (latestEndTime == null || - childEndTimestamp.isAfter(latestEndTime)) { - latestEndTime = child.endTimestamp; - } + final childEndTimestamps = children + .where((child) => child.endTimestamp != null) + .map((child) => child.endTimestamp!); + + if (childEndTimestamps.isNotEmpty) { + final oldestChildEndTimestamp = + childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b); + if (_rootEndTimestamp.isAfter(oldestChildEndTimestamp)) { + _rootEndTimestamp = oldestChildEndTimestamp; } } - - if (latestEndTime != null) { - _rootEndTimestamp = latestEndTime; - } } // the callback should run before because if the span is finished, diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 092f61837d..b059b0e902 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -74,26 +74,31 @@ class SentryNavigatorObserver extends RouteObserver> { bool setRouteNameAsTransaction = false, RouteNameExtractor? routeNameExtractor, AdditionalInfoExtractor? additionalInfoProvider, - @internal TimeToDisplayTracker? timeToDisplayTracker, + @visibleForTesting TimeToDisplayTracker? timeToDisplayTracker, }) : _hub = hub ?? HubAdapter(), + _enableAutoTransactions = enableAutoTransactions, + _autoFinishAfter = autoFinishAfter, _setRouteNameAsTransaction = setRouteNameAsTransaction, _routeNameExtractor = routeNameExtractor, - _additionalInfoProvider = additionalInfoProvider { + _additionalInfoProvider = additionalInfoProvider, + _native = SentryFlutter.native { if (enableAutoTransactions) { _hub.options.sdk.addIntegration('UINavigationTracing'); } _timeToDisplayTracker = timeToDisplayTracker ?? TimeToDisplayTracker( - hub: hub, - enableAutoTransactions: enableAutoTransactions, - autoFinishAfter: autoFinishAfter, + // TODO: ttfd flag + enableTimeToFullDisplayTracing: false, ); } final Hub _hub; + final bool _enableAutoTransactions; + final Duration _autoFinishAfter; final bool _setRouteNameAsTransaction; final RouteNameExtractor? _routeNameExtractor; final AdditionalInfoExtractor? _additionalInfoProvider; + final SentryNative? _native; static TimeToDisplayTracker? _timeToDisplayTracker; @@ -101,6 +106,8 @@ class SentryNavigatorObserver extends RouteObserver> { static TimeToDisplayTracker? get timeToDisplayTracker => _timeToDisplayTracker; + ISentrySpan? _transaction; + static String? _currentRouteName; @internal @@ -205,13 +212,81 @@ class SentryNavigatorObserver extends RouteObserver> { } } + Future _startTransaction(Route? route) async { + if (!_enableAutoTransactions) { + return null; + } + + String? name = _getRouteName(route); + final arguments = route?.settings.arguments; + + if (name == null) { + return null; + } + + if (name == '/') { + name = 'root ("/")'; + } + final transactionContext = SentryTransactionContext( + name, + 'navigation', + transactionNameSource: SentryTransactionNameSource.component, + // ignore: invalid_use_of_internal_member + origin: SentryTraceOrigins.autoNavigationRouteObserver, + ); + + _transaction = _hub.startTransactionWithContext( + transactionContext, + waitForChildren: true, + autoFinishAfter: _autoFinishAfter, + trimEnd: true, + onFinish: (transaction) async { + _transaction = null; + final nativeFrames = await _native + ?.endNativeFramesCollection(transaction.context.traceId); + if (nativeFrames != null) { + final measurements = nativeFrames.toMeasurements(); + for (final item in measurements.entries) { + final measurement = item.value; + transaction.setMeasurement( + item.key, + measurement.value, + unit: measurement.unit, + ); + } + } + }, + ); + + // if _enableAutoTransactions is enabled but there's no traces sample rate + if (_transaction is NoOpSentrySpan) { + _transaction = null; + return null; + } + + if (arguments != null) { + _transaction?.setData('route_settings_arguments', arguments); + } + + await _hub.configureScope((scope) { + scope.span ??= _transaction; + }); + + await _native?.beginNativeFramesCollection(); + + return _transaction; + } + Future _startTimeToDisplayTracking(Route? route) async { _completedDisplayTracking = Completer(); final routeName = _getRouteName(route); _currentRouteName = routeName; final arguments = route?.settings.arguments; - await _timeToDisplayTracker?.startTracking(routeName, arguments); + final transaction = await _startTransaction(route); + if (transaction == null) return; + await _timeToDisplayTracker?.startTracking( + transaction, routeName, arguments); _completedDisplayTracking?.complete(); } } diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index 2483a33ef7..fac68595d4 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -11,85 +11,21 @@ import 'time_to_initial_display_tracker.dart'; @internal class TimeToDisplayTracker { - final Hub? _hub; - final bool? _enableAutoTransactions; - final Duration? _autoFinishAfter; final SentryNative? _native; final TimeToInitialDisplayTracker _ttidTracker; // TODO We can use _hub.options to fetch the ttfd flag - bool get _enableTimeToFullDisplayTracing => false; + final bool _enableTimeToFullDisplayTracing; TimeToDisplayTracker({ - required Hub? hub, - required bool? enableAutoTransactions, - required Duration? autoFinishAfter, + required bool enableTimeToFullDisplayTracing, TimeToInitialDisplayTracker? ttidTracker, }) : _native = SentryFlutter.native, - _hub = hub ?? HubAdapter(), - _enableAutoTransactions = enableAutoTransactions, - _autoFinishAfter = autoFinishAfter, + _enableTimeToFullDisplayTracing = enableTimeToFullDisplayTracing, _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); - Future _startTransaction(String? routeName, Object? arguments, - {DateTime? startTimestamp}) async { - if (_enableAutoTransactions == false) { - return null; - } - - if (routeName == null) { - return null; - } - - final transactionContext = SentryTransactionContext( - routeName, - SentrySpanOperations.uiLoad, - transactionNameSource: SentryTransactionNameSource.component, - origin: SentryTraceOrigins.autoNavigationRouteObserver, - ); - - final transaction = _hub?.startTransactionWithContext( - transactionContext, - waitForChildren: true, - autoFinishAfter: _autoFinishAfter, - trimEnd: true, - startTimestamp: startTimestamp, - onFinish: (transaction) async { - final nativeFrames = await _native - ?.endNativeFramesCollection(transaction.context.traceId); - if (nativeFrames != null) { - final measurements = nativeFrames.toMeasurements(); - for (final item in measurements.entries) { - final measurement = item.value; - transaction.setMeasurement( - item.key, - measurement.value, - unit: measurement.unit, - ); - } - } - }, - ); - - // if _enableAutoTransactions is enabled but there's no traces sample rate - if (transaction is NoOpSentrySpan) { - return null; - } - - if (arguments != null) { - transaction?.setData('route_settings_arguments', arguments); - } - - _hub?.configureScope((scope) { - scope.span ??= transaction; - }); - - await _native?.beginNativeFramesCollection(); - - return transaction; - } - - Future startTracking(String? routeName, Object? arguments) async { + Future startTracking( + ISentrySpan transaction, String? routeName, Object? arguments) async { final startTimestamp = DateTime.now(); if (routeName == '/') { routeName = 'root ("/")'; @@ -101,9 +37,10 @@ class TimeToDisplayTracker { if (isRootScreen && didFetchAppStart == false) { // Dart cannot infer here that routeName is not nullable - await _trackAppStartTTD(routeName, arguments); + await _trackAppStartTTD(transaction, routeName, arguments); } else { - await _trackRegularRouteTTD(routeName, arguments, startTimestamp); + await _trackRegularRouteTTD( + transaction, routeName, arguments, startTimestamp); } } @@ -114,13 +51,11 @@ class TimeToDisplayTracker { /// - Finishes the TTID span immediately with the app start end timestamp /// /// We start and immediately finish the TTID span since we cannot mutate the history of spans. - Future _trackAppStartTTD(String routeName, Object? arguments) async { + Future _trackAppStartTTD( + ISentrySpan transaction, String routeName, Object? arguments) async { final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); if (appStartInfo == null) return; - final transaction = await _startTransaction(routeName, arguments, startTimestamp: appStartInfo.start); - if (transaction == null) return; - if (_enableTimeToFullDisplayTracing) { // TODO: implement TTFD } @@ -129,11 +64,8 @@ class TimeToDisplayTracker { } /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. - Future _trackRegularRouteTTD( - String routeName, Object? arguments, DateTime startTimestamp) async { - final transaction = await _startTransaction(routeName, arguments, startTimestamp: startTimestamp); - if (transaction == null) return; - + Future _trackRegularRouteTTD(ISentrySpan transaction, String routeName, + Object? arguments, DateTime startTimestamp) async { if (_enableTimeToFullDisplayTracing) { // TODO: implement TTFD } diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index e0895a51fc..acc263c830 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -33,10 +33,10 @@ class TimeToInitialDisplayTracker { DateTime? get endTimestamp => _endTimestamp; Future trackRegularRoute( - ISentrySpan transaction, - DateTime startTimestamp, - String routeName, - ) async { + ISentrySpan transaction, + DateTime startTimestamp, + String routeName, + ) async { await _trackTimeToInitialDisplay( transaction: transaction, startTimestamp: startTimestamp, @@ -47,10 +47,10 @@ class TimeToInitialDisplayTracker { } Future trackAppStart( - ISentrySpan transaction, - AppStartInfo appStartInfo, - String routeName, - ) async { + ISentrySpan transaction, + AppStartInfo appStartInfo, + String routeName, + ) async { await _trackTimeToInitialDisplay( transaction: transaction, startTimestamp: appStartInfo.start, diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index a13e957f04..0ab081b489 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -29,15 +29,16 @@ void main() { SentryFlutter.native = TestMockSentryNative(); final sut = fixture.getSut(); + // Fake app start Future.delayed(const Duration(milliseconds: 500), () async { final appStartInfo = AppStartInfo(AppStartType.cold, - start: DateTime.fromMillisecondsSinceEpoch(0), - end: DateTime.fromMillisecondsSinceEpoch(10)); + start: DateTime.now(), + end: DateTime.now().add(const Duration(milliseconds: 10))); NativeAppStartIntegration.setAppStartInfo(appStartInfo); }); - await sut.startTracking('/', null); + await sut.startTracking(fixture.getTransaction(), '/', null); await Future.delayed(const Duration(milliseconds: 100)); @@ -56,7 +57,8 @@ void main() { test('startMeasurement finishes ttid span', () async { final sut = fixture.getSut(); - await sut.startTracking('Current Route', null); + await sut.startTracking( + fixture.getTransaction(), 'Current Route', null); final transaction = fixture.hub.getSpan() as SentryTracer; await Future.delayed(const Duration(milliseconds: 2000)); @@ -77,7 +79,8 @@ void main() { fixture.ttidTracker.markAsManual(); fixture.ttidTracker.completeTracking(); }); - await sut.startTracking('Current Route', null); + await sut.startTracking( + fixture.getTransaction(), 'Current Route', null); final transaction = fixture.hub.getSpan() as SentryTracer; @@ -101,7 +104,7 @@ void main() { test('screen load tracking creates ui.load transaction', () async { final sut = fixture.getSut(); - await sut.startTracking('Current Route', null); + await sut.startTracking(fixture.getTransaction(), 'Current Route', null); final transaction = fixture.hub.getSpan(); expect(transaction, isNotNull); @@ -122,11 +125,14 @@ class Fixture { final ttfdAutoFinishAfter = Duration(milliseconds: 500); + ISentrySpan getTransaction({String? name = "Current route"}) { + return hub.startTransaction(name!, 'ui.load', bindToScope: true); + } + TimeToDisplayTracker getSut() { return TimeToDisplayTracker( - hub: hub, - enableAutoTransactions: true, - autoFinishAfter: const Duration(seconds: 30), + // TODO: ttfd flag + enableTimeToFullDisplayTracing: false, ttidTracker: ttidTracker, ); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 01325cdba0..95fadf33ae 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -877,9 +877,8 @@ class Fixture { final timeToInitialDisplayTracker = TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); final timeToDisplayTracker = TimeToDisplayTracker( - hub: hub, - enableAutoTransactions: enableAutoTransactions, - autoFinishAfter: autoFinishAfter, + // TODO: ttfd flag + enableTimeToFullDisplayTracing: false, ttidTracker: timeToInitialDisplayTracker, ); return SentryNavigatorObserver( From f2ba992f4f92ecdc2c3092dea41d0499cefd425e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 17:11:55 +0100 Subject: [PATCH 30/57] fix analyze --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index b059b0e902..067bff5944 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -231,7 +231,6 @@ class SentryNavigatorObserver extends RouteObserver> { name, 'navigation', transactionNameSource: SentryTransactionNameSource.component, - // ignore: invalid_use_of_internal_member origin: SentryTraceOrigins.autoNavigationRouteObserver, ); From 7c64a3f121a4de27e57035bf42ce025f0d06e4a9 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 17:15:20 +0100 Subject: [PATCH 31/57] update --- dart/lib/src/sentry_tracer.dart | 6 +++--- .../navigation/sentry_navigator_observer.dart | 17 +++++++---------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 86b4bd35e8..ee53287246 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -109,7 +109,6 @@ class SentryTracer extends ISentrySpan { } var _rootEndTimestamp = commonEndTimestamp; - if (_trimEnd && children.isNotEmpty) { final childEndTimestamps = children .where((child) => child.endTimestamp != null) @@ -117,7 +116,7 @@ class SentryTracer extends ISentrySpan { if (childEndTimestamps.isNotEmpty) { final oldestChildEndTimestamp = - childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b); + childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b); if (_rootEndTimestamp.isAfter(oldestChildEndTimestamp)) { _rootEndTimestamp = oldestChildEndTimestamp; } @@ -363,7 +362,8 @@ class SentryTracer extends ISentrySpan { Dsn.parse(_hub.options.dsn!).publicKey, release: _hub.options.release, environment: _hub.options.environment, - userId: null, // because of PII not sending it for now + userId: null, + // because of PII not sending it for now userSegment: user?.segment, transaction: _isHighQualityTransactionName(transactionNameSource) ? name : null, diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 067bff5944..40fedf5e87 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -212,16 +212,16 @@ class SentryNavigatorObserver extends RouteObserver> { } } - Future _startTransaction(Route? route) async { + Future _startTransaction(Route? route) async { if (!_enableAutoTransactions) { - return null; + return; } String? name = _getRouteName(route); final arguments = route?.settings.arguments; if (name == null) { - return null; + return; } if (name == '/') { @@ -260,7 +260,7 @@ class SentryNavigatorObserver extends RouteObserver> { // if _enableAutoTransactions is enabled but there's no traces sample rate if (_transaction is NoOpSentrySpan) { _transaction = null; - return null; + return; } if (arguments != null) { @@ -272,18 +272,15 @@ class SentryNavigatorObserver extends RouteObserver> { }); await _native?.beginNativeFramesCollection(); - - return _transaction; } Future _startTimeToDisplayTracking(Route? route) async { _completedDisplayTracking = Completer(); - final routeName = _getRouteName(route); - _currentRouteName = routeName; - final arguments = route?.settings.arguments; - final transaction = await _startTransaction(route); + await _startTransaction(route); + final transaction = _transaction; if (transaction == null) return; + final routeName = _getRouteName(route); await _timeToDisplayTracker?.startTracking( transaction, routeName, arguments); _completedDisplayTracking?.complete(); From d90a7ed8ce408c7245877ec2d172daad1a45b9a2 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 17:32:57 +0100 Subject: [PATCH 32/57] update --- .../src/navigation/sentry_display_widget.dart | 2 +- .../navigation/sentry_navigator_observer.dart | 20 +++++++++---------- .../navigation/time_to_display_tracker.dart | 4 ++++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_display_widget.dart b/flutter/lib/src/navigation/sentry_display_widget.dart index ffed11cce7..d5995999c5 100644 --- a/flutter/lib/src/navigation/sentry_display_widget.dart +++ b/flutter/lib/src/navigation/sentry_display_widget.dart @@ -34,7 +34,7 @@ class SentryDisplayWidget extends StatefulWidget { SentryDisplayWidget({ super.key, required this.child, - FrameCallbackHandler? frameCallbackHandler, + @visibleForTesting FrameCallbackHandler? frameCallbackHandler, }) : _frameCallbackHandler = frameCallbackHandler ?? DefaultFrameCallbackHandler(); diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 40fedf5e87..39da8066b2 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -136,17 +136,6 @@ class SentryNavigatorObserver extends RouteObserver> { _startTimeToDisplayTracking(route); } - Future _finishTransaction() async { - timeToDisplayTracker?.clear(); - - final transaction = _hub.getSpan(); - if (transaction == null || transaction.finished) { - return; - } - transaction.status ??= SpanStatus.ok(); - await transaction.finish(); - } - @override void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); @@ -212,6 +201,15 @@ class SentryNavigatorObserver extends RouteObserver> { } } + Future _finishTransaction() async { + final transaction = _hub.getSpan(); + if (transaction == null || transaction.finished) { + return; + } + transaction.status ??= SpanStatus.ok(); + await transaction.finish(); + } + Future _startTransaction(Route? route) async { if (!_enableAutoTransactions) { return; diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index fac68595d4..c8af669fe8 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -26,6 +26,8 @@ class TimeToDisplayTracker { Future startTracking( ISentrySpan transaction, String? routeName, Object? arguments) async { + clear(); + final startTimestamp = DateTime.now(); if (routeName == '/') { routeName = 'root ("/")'; @@ -42,6 +44,8 @@ class TimeToDisplayTracker { await _trackRegularRouteTTD( transaction, routeName, arguments, startTimestamp); } + + clear(); } /// This method listens for the completion of the app's start process via From b2767aa660e07188174d8ea330944bc389a9b934 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 18:28:21 +0100 Subject: [PATCH 33/57] Fix tests --- .../navigation/sentry_navigator_observer.dart | 4 +++- .../navigation/time_to_display_tracker.dart | 2 -- .../sentry_display_widget_test.dart | 22 ++++++++----------- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 39da8066b2..3a89e7d86f 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -202,6 +202,8 @@ class SentryNavigatorObserver extends RouteObserver> { } Future _finishTransaction() async { + _timeToDisplayTracker?.clear(); + final transaction = _hub.getSpan(); if (transaction == null || transaction.finished) { return; @@ -227,7 +229,7 @@ class SentryNavigatorObserver extends RouteObserver> { } final transactionContext = SentryTransactionContext( name, - 'navigation', + SentrySpanOperations.uiLoad, transactionNameSource: SentryTransactionNameSource.component, origin: SentryTraceOrigins.autoNavigationRouteObserver, ); diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index c8af669fe8..6af6a92e28 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -26,8 +26,6 @@ class TimeToDisplayTracker { Future startTracking( ISentrySpan transaction, String? routeName, Object? arguments) async { - clear(); - final startTimestamp = DateTime.now(); if (routeName == '/') { routeName = 'root ("/")'; diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart index 6eac88653b..4156de13c9 100644 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -54,16 +54,16 @@ void main() { testWidgets('SentryDisplayWidget is ignored for app starts', (WidgetTester tester) async { final currentRoute = route(RouteSettings(name: '/')); + final appStartInfo = AppStartInfo( + AppStartType.cold, + start: DateTime.now().add(Duration(seconds: 1)), + end: DateTime.now().add(Duration(seconds: 2)), + ); + NativeAppStartIntegration.setAppStartInfo(appStartInfo); await tester.runAsync(() async { fixture.navigatorObserver.didPush(currentRoute, null); await tester.pumpWidget(fixture.getSut()); - final appStartInfo = AppStartInfo( - AppStartType.cold, - start: DateTime.fromMillisecondsSinceEpoch(0), - end: DateTime.fromMillisecondsSinceEpoch(10), - ); - NativeAppStartIntegration.setAppStartInfo(appStartInfo); await fixture.navigatorObserver.completedDisplayTracking?.future; }); @@ -81,15 +81,14 @@ void main() { expect(ttidSpan.context.description, 'root ("/") initial display'); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); - expect(ttidSpan.startTimestamp, - DateTime.fromMillisecondsSinceEpoch(0).toUtc()); + expect(ttidSpan.startTimestamp.toUtc(), appStartInfo.start.toUtc()); expect( - ttidSpan.endTimestamp, DateTime.fromMillisecondsSinceEpoch(10).toUtc()); + ttidSpan.endTimestamp?.toUtc(), appStartInfo.end.toUtc()); expect(tracer.measurements, hasLength(1)); final measurement = tracer.measurements['time_to_initial_display']; expect(measurement, isNotNull); - expect(measurement?.value, 10); + expect(measurement?.value, 1000); expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); }); } @@ -98,14 +97,11 @@ class Fixture { final Hub hub = Hub(SentryFlutterOptions(dsn: fakeDsn)..tracesSampleRate = 1.0); late final SentryNavigatorObserver navigatorObserver; - late final TimeToInitialDisplayTracker timeToInitialDisplayTracker; final fakeFrameCallbackHandler = FakeFrameCallbackHandler(); Fixture() { SentryFlutter.native = TestMockSentryNative(); navigatorObserver = SentryNavigatorObserver(hub: hub); - timeToInitialDisplayTracker = TimeToInitialDisplayTracker( - frameCallbackHandler: fakeFrameCallbackHandler); } MaterialApp getSut() { From 4c6301a1e173f99c531f70c21ffca91b2f0d5ef4 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 18:29:01 +0100 Subject: [PATCH 34/57] Fix analyze --- flutter/example/android/app/build.gradle | 2 +- flutter/example/lib/main.dart | 1 + flutter/test/navigation/sentry_display_widget_test.dart | 4 +--- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/flutter/example/android/app/build.gradle b/flutter/example/android/app/build.gradle index b54cd7fdf9..ac4557e2cd 100644 --- a/flutter/example/android/app/build.gradle +++ b/flutter/example/android/app/build.gradle @@ -48,7 +48,7 @@ android { defaultConfig { applicationId "io.sentry.samples.flutter" - minSdkVersion 19 + minSdkVersion flutter.minSdkVersion targetSdkVersion 33 versionCode flutterVersionCode.toInteger() versionName flutterVersionName diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 6a0c0a5b81..a46a9ca641 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -80,6 +80,7 @@ Future setupSentry( // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. options.debug = true; + options.spotlight = Spotlight(enabled: true); options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart index 4156de13c9..317ed30b3a 100644 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -6,7 +6,6 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/navigation/sentry_display_widget.dart'; -import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; import '../fake_frame_callback_handler.dart'; import '../mocks.dart'; @@ -82,8 +81,7 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); expect(ttidSpan.startTimestamp.toUtc(), appStartInfo.start.toUtc()); - expect( - ttidSpan.endTimestamp?.toUtc(), appStartInfo.end.toUtc()); + expect(ttidSpan.endTimestamp?.toUtc(), appStartInfo.end.toUtc()); expect(tracer.measurements, hasLength(1)); final measurement = tracer.measurements['time_to_initial_display']; From eaa6d8df98b2802ba965ecd91bfb5d89ea0ba770 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 18:34:42 +0100 Subject: [PATCH 35/57] revert sample --- flutter/example/android/app/build.gradle | 2 +- flutter/example/lib/main.dart | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/flutter/example/android/app/build.gradle b/flutter/example/android/app/build.gradle index ac4557e2cd..b54cd7fdf9 100644 --- a/flutter/example/android/app/build.gradle +++ b/flutter/example/android/app/build.gradle @@ -48,7 +48,7 @@ android { defaultConfig { applicationId "io.sentry.samples.flutter" - minSdkVersion flutter.minSdkVersion + minSdkVersion 19 targetSdkVersion 33 versionCode flutterVersionCode.toInteger() versionName flutterVersionName diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index a46a9ca641..6a0c0a5b81 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -80,7 +80,6 @@ Future setupSentry( // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. options.debug = true; - options.spotlight = Spotlight(enabled: true); options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; From 6ca06ddfa39ced6d60e88852d342b89212cd78cb Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:09:43 +0100 Subject: [PATCH 36/57] Update --- dart/lib/src/sentry_span_operations.dart | 1 - dart/lib/src/sentry_tracer.dart | 3 +-- .../navigation/sentry_navigator_observer.dart | 12 +++++----- .../navigation/time_to_display_tracker.dart | 22 +------------------ .../sentry_display_widget_test.dart | 2 +- .../time_to_display_tracker_test.dart | 4 ---- .../test/sentry_navigator_observer_test.dart | 2 +- 7 files changed, 9 insertions(+), 37 deletions(-) diff --git a/dart/lib/src/sentry_span_operations.dart b/dart/lib/src/sentry_span_operations.dart index 27b1d22496..fca22fc1e9 100644 --- a/dart/lib/src/sentry_span_operations.dart +++ b/dart/lib/src/sentry_span_operations.dart @@ -4,5 +4,4 @@ import 'package:meta/meta.dart'; class SentrySpanOperations { static const String uiLoad = 'ui.load'; static const String uiTimeToInitialDisplay = 'ui.load.initial_display'; - static const String uiTimeToFullDisplay = 'ui.load.full_display'; } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index ee53287246..c57912fe46 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -362,8 +362,7 @@ class SentryTracer extends ISentrySpan { Dsn.parse(_hub.options.dsn!).publicKey, release: _hub.options.release, environment: _hub.options.environment, - userId: null, - // because of PII not sending it for now + userId: null, // because of PII not sending it for now userSegment: user?.segment, transaction: _isHighQualityTransactionName(transactionNameSource) ? name : null, diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 3a89e7d86f..219dd627d3 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -86,10 +86,7 @@ class SentryNavigatorObserver extends RouteObserver> { _hub.options.sdk.addIntegration('UINavigationTracing'); } _timeToDisplayTracker = timeToDisplayTracker ?? - TimeToDisplayTracker( - // TODO: ttfd flag - enableTimeToFullDisplayTracing: false, - ); + TimeToDisplayTracker(); } final Hub _hub; @@ -202,14 +199,15 @@ class SentryNavigatorObserver extends RouteObserver> { } Future _finishTransaction() async { - _timeToDisplayTracker?.clear(); - - final transaction = _hub.getSpan(); + final transaction = _transaction; + _transaction = null; if (transaction == null || transaction.finished) { return; } transaction.status ??= SpanStatus.ok(); await transaction.finish(); + + _timeToDisplayTracker?.clear(); } Future _startTransaction(Route? route) async { diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index 6af6a92e28..d678ca86d4 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -14,14 +14,9 @@ class TimeToDisplayTracker { final SentryNative? _native; final TimeToInitialDisplayTracker _ttidTracker; - // TODO We can use _hub.options to fetch the ttfd flag - final bool _enableTimeToFullDisplayTracing; - TimeToDisplayTracker({ - required bool enableTimeToFullDisplayTracing, TimeToInitialDisplayTracker? ttidTracker, }) : _native = SentryFlutter.native, - _enableTimeToFullDisplayTracing = enableTimeToFullDisplayTracing, _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); Future startTracking( @@ -36,7 +31,6 @@ class TimeToDisplayTracker { if (routeName == null) return; if (isRootScreen && didFetchAppStart == false) { - // Dart cannot infer here that routeName is not nullable await _trackAppStartTTD(transaction, routeName, arguments); } else { await _trackRegularRouteTTD( @@ -49,7 +43,7 @@ class TimeToDisplayTracker { /// This method listens for the completion of the app's start process via /// [AppStartTracker], then: /// - Starts a transaction with the app start start timestamp - /// - Starts TTID and optionally TTFD spans based on the app start start timestamp + /// - Starts a TTID span based on the app start start timestamp /// - Finishes the TTID span immediately with the app start end timestamp /// /// We start and immediately finish the TTID span since we cannot mutate the history of spans. @@ -57,30 +51,16 @@ class TimeToDisplayTracker { ISentrySpan transaction, String routeName, Object? arguments) async { final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); if (appStartInfo == null) return; - - if (_enableTimeToFullDisplayTracing) { - // TODO: implement TTFD - } - await _ttidTracker.trackAppStart(transaction, appStartInfo, routeName); } /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. Future _trackRegularRouteTTD(ISentrySpan transaction, String routeName, Object? arguments, DateTime startTimestamp) async { - if (_enableTimeToFullDisplayTracing) { - // TODO: implement TTFD - } - await _ttidTracker.trackRegularRoute( transaction, startTimestamp, routeName); } - @internal - Future reportFullyDisplayed() async { - // TODO: implement TTFD - } - void clear() { _ttidTracker.clear(); } diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart index 317ed30b3a..106912f6bc 100644 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -86,7 +86,7 @@ void main() { expect(tracer.measurements, hasLength(1)); final measurement = tracer.measurements['time_to_initial_display']; expect(measurement, isNotNull); - expect(measurement?.value, 1000); + expect(measurement?.value, appStartInfo.duration.inMilliseconds); expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); }); } diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 0ab081b489..98d6856639 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -123,16 +123,12 @@ class Fixture { late final ttidTracker = TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); - final ttfdAutoFinishAfter = Duration(milliseconds: 500); - ISentrySpan getTransaction({String? name = "Current route"}) { return hub.startTransaction(name!, 'ui.load', bindToScope: true); } TimeToDisplayTracker getSut() { return TimeToDisplayTracker( - // TODO: ttfd flag - enableTimeToFullDisplayTracing: false, ttidTracker: ttidTracker, ); } diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 95fadf33ae..e22aec02cf 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -877,7 +877,7 @@ class Fixture { final timeToInitialDisplayTracker = TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); final timeToDisplayTracker = TimeToDisplayTracker( - // TODO: ttfd flag + // TODO: ttfd flag via options enableTimeToFullDisplayTracing: false, ttidTracker: timeToInitialDisplayTracker, ); From c73de7bbeda6c206bca27fdbac1d9db98ebc436e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:10:37 +0100 Subject: [PATCH 37/57] Update --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 3 +-- flutter/test/sentry_navigator_observer_test.dart | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 219dd627d3..aa6c0c2ab5 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -85,8 +85,7 @@ class SentryNavigatorObserver extends RouteObserver> { if (enableAutoTransactions) { _hub.options.sdk.addIntegration('UINavigationTracing'); } - _timeToDisplayTracker = timeToDisplayTracker ?? - TimeToDisplayTracker(); + _timeToDisplayTracker = timeToDisplayTracker ?? TimeToDisplayTracker(); } final Hub _hub; diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index e22aec02cf..bb995a466c 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -877,8 +877,6 @@ class Fixture { final timeToInitialDisplayTracker = TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); final timeToDisplayTracker = TimeToDisplayTracker( - // TODO: ttfd flag via options - enableTimeToFullDisplayTracing: false, ttidTracker: timeToInitialDisplayTracker, ); return SentryNavigatorObserver( From 238995e30639e104a604411e11fd78d8695a52e0 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 10:27:23 +0100 Subject: [PATCH 38/57] Fix test --- flutter/test/sentry_navigator_observer_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index bb995a466c..2e54fed326 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -351,7 +351,6 @@ void main() { final sut = fixture.getSut(hub: hub); sut.didPush(currentRoute, null); - when(span.finished).thenReturn(true); sut.didPop(currentRoute, null); sut.didPop(currentRoute, null); From 133e16636559953f19287359fc9b15236378634f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 10:35:25 +0100 Subject: [PATCH 39/57] Move clear to the beginning of function --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index aa6c0c2ab5..adbeccc9c6 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -198,6 +198,8 @@ class SentryNavigatorObserver extends RouteObserver> { } Future _finishTransaction() async { + _timeToDisplayTracker?.clear(); + final transaction = _transaction; _transaction = null; if (transaction == null || transaction.finished) { @@ -205,8 +207,6 @@ class SentryNavigatorObserver extends RouteObserver> { } transaction.status ??= SpanStatus.ok(); await transaction.finish(); - - _timeToDisplayTracker?.clear(); } Future _startTransaction(Route? route) async { From cc4398d2a6d6439a7fac4077c37531e7394580ea Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 12:48:54 +0100 Subject: [PATCH 40/57] Fix start time --- .../navigation/sentry_navigator_observer.dart | 46 ++++++++++-- .../navigation/time_to_display_tracker.dart | 53 +++----------- .../time_to_initial_display_tracker.dart | 24 +++---- .../time_to_display_tracker_test.dart | 72 ++++++++++++++----- .../time_to_initial_display_tracker_test.dart | 45 ++++++------ 5 files changed, 135 insertions(+), 105 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index adbeccc9c6..053dbb2b08 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; +import '../integrations/integrations.dart'; import 'time_to_display_tracker.dart'; import '../../sentry_flutter.dart'; @@ -209,7 +210,8 @@ class SentryNavigatorObserver extends RouteObserver> { await transaction.finish(); } - Future _startTransaction(Route? route) async { + Future _startTransaction( + Route? route, DateTime startTimestamp) async { if (!_enableAutoTransactions) { return; } @@ -233,6 +235,7 @@ class SentryNavigatorObserver extends RouteObserver> { _transaction = _hub.startTransactionWithContext( transactionContext, + startTimestamp: startTimestamp, waitForChildren: true, autoFinishAfter: _autoFinishAfter, trimEnd: true, @@ -273,14 +276,43 @@ class SentryNavigatorObserver extends RouteObserver> { Future _startTimeToDisplayTracking(Route? route) async { _completedDisplayTracking = Completer(); - final arguments = route?.settings.arguments; - await _startTransaction(route); + String? routeName = _currentRouteName; + + if (routeName == null) return; + + DateTime startTimestamp = _hub.options.clock(); + DateTime? endTimestamp; + + if (routeName == '/') { + final appStartInfo = await NativeAppStartIntegration + .getAppStartInfo(); + if (appStartInfo == null) { + return; + } + + startTimestamp = + appStartInfo.start; // Adjust start timestamp based on app start info. + endTimestamp = + appStartInfo.end; // Set end timestamp for app start tracking. + } + + await _startTransaction(route, startTimestamp); final transaction = _transaction; - if (transaction == null) return; - final routeName = _getRouteName(route); - await _timeToDisplayTracker?.startTracking( - transaction, routeName, arguments); + if (transaction == null) { + return; + } + + if (routeName == '/' && endTimestamp != null) { + await _timeToDisplayTracker?.trackAppStartTTD(transaction, + startTimestamp: startTimestamp, endTimestamp: endTimestamp); + } else { + await _timeToDisplayTracker?.trackRegularRouteTTD(transaction, + startTimestamp: startTimestamp); + } + + // Mark the tracking as completed and clear any temporary state. _completedDisplayTracking?.complete(); + _timeToDisplayTracker?.clear(); } } diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index d678ca86d4..38ec1e55c6 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -11,54 +11,23 @@ import 'time_to_initial_display_tracker.dart'; @internal class TimeToDisplayTracker { - final SentryNative? _native; final TimeToInitialDisplayTracker _ttidTracker; TimeToDisplayTracker({ TimeToInitialDisplayTracker? ttidTracker, - }) : _native = SentryFlutter.native, - _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); - - Future startTracking( - ISentrySpan transaction, String? routeName, Object? arguments) async { - final startTimestamp = DateTime.now(); - if (routeName == '/') { - routeName = 'root ("/")'; - } - final isRootScreen = routeName == 'root ("/")'; - final didFetchAppStart = _native?.didFetchAppStart; - - if (routeName == null) return; - - if (isRootScreen && didFetchAppStart == false) { - await _trackAppStartTTD(transaction, routeName, arguments); - } else { - await _trackRegularRouteTTD( - transaction, routeName, arguments, startTimestamp); - } - - clear(); - } - - /// This method listens for the completion of the app's start process via - /// [AppStartTracker], then: - /// - Starts a transaction with the app start start timestamp - /// - Starts a TTID span based on the app start start timestamp - /// - Finishes the TTID span immediately with the app start end timestamp - /// - /// We start and immediately finish the TTID span since we cannot mutate the history of spans. - Future _trackAppStartTTD( - ISentrySpan transaction, String routeName, Object? arguments) async { - final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); - if (appStartInfo == null) return; - await _ttidTracker.trackAppStart(transaction, appStartInfo, routeName); + }) : _ttidTracker = ttidTracker ?? TimeToInitialDisplayTracker(); + + Future trackAppStartTTD(ISentrySpan transaction, + {required DateTime startTimestamp, + required DateTime endTimestamp}) async { + // We start and immediately finish the spans since we cannot mutate the history of spans. + await _ttidTracker.trackAppStart(transaction, + startTimestamp: startTimestamp, endTimestamp: endTimestamp); } - /// Starts and finishes Time To Display spans for regular routes meaning routes that are not root. - Future _trackRegularRouteTTD(ISentrySpan transaction, String routeName, - Object? arguments, DateTime startTimestamp) async { - await _ttidTracker.trackRegularRoute( - transaction, startTimestamp, routeName); + Future trackRegularRouteTTD(ISentrySpan transaction, + {required DateTime startTimestamp}) async { + await _ttidTracker.trackRegularRoute(transaction, startTimestamp); } void clear() { diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index acc263c830..b32e9e9a6e 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -3,7 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; -import '../integrations/integrations.dart'; +import 'package:sentry/src/sentry_tracer.dart'; import '../../sentry_flutter.dart'; import '../frame_callback_handler.dart'; @@ -35,38 +35,32 @@ class TimeToInitialDisplayTracker { Future trackRegularRoute( ISentrySpan transaction, DateTime startTimestamp, - String routeName, ) async { await _trackTimeToInitialDisplay( transaction: transaction, startTimestamp: startTimestamp, - routeName: routeName, // endTimestamp is null by default, determined inside the private method // origin could be set here if needed, or determined inside the private method ); } - Future trackAppStart( - ISentrySpan transaction, - AppStartInfo appStartInfo, - String routeName, - ) async { + Future trackAppStart(ISentrySpan transaction, + {required DateTime startTimestamp, + required DateTime endTimestamp}) async { await _trackTimeToInitialDisplay( transaction: transaction, - startTimestamp: appStartInfo.start, - routeName: routeName, - endTimestamp: appStartInfo.end, + startTimestamp: startTimestamp, + endTimestamp: endTimestamp, origin: SentryTraceOrigins.autoUiTimeToDisplay, ); // Store the end timestamp for potential use by TTFD tracking - _endTimestamp = appStartInfo.end; + _endTimestamp = endTimestamp; } Future _trackTimeToInitialDisplay({ required ISentrySpan transaction, required DateTime startTimestamp, - required String routeName, DateTime? endTimestamp, String? origin, }) async { @@ -74,9 +68,11 @@ class TimeToInitialDisplayTracker { final _endTimestamp = endTimestamp ?? await determineEndTime(); if (_endTimestamp == null) return; + final tracer = transaction as SentryTracer; + final ttidSpan = transaction.startChild( SentrySpanOperations.uiTimeToInitialDisplay, - description: '$routeName initial display', + description: '${tracer.name} initial display', startTimestamp: startTimestamp, ); diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 98d6856639..acdd2ba653 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -3,7 +3,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; @@ -25,20 +24,34 @@ void main() { group('time to initial display', () { group('in root screen app start route', () { - test('startMeasurement finishes ttid span', () async { - SentryFlutter.native = TestMockSentryNative(); + test('matches startTimestamp of transaction', () async { final sut = fixture.getSut(); - // Fake app start - Future.delayed(const Duration(milliseconds: 500), () async { - final appStartInfo = AppStartInfo(AppStartType.cold, - start: DateTime.now(), - end: DateTime.now().add(const Duration(milliseconds: 10))); + await sut.trackRegularRouteTTD(fixture.getTransaction(name: '/'), + startTimestamp: fixture.startTimestamp); - NativeAppStartIntegration.setAppStartInfo(appStartInfo); - }); + final transaction = fixture.hub.getSpan() as SentryTracer; + + final spans = transaction.children; + final ttidSpan = spans + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + + expect(transaction, isNotNull); + expect(transaction.context.operation, SentrySpanOperations.uiLoad); + expect(transaction.startTimestamp, ttidSpan.startTimestamp); + }); - await sut.startTracking(fixture.getTransaction(), '/', null); + test('startMeasurement finishes ttid span', () async { + SentryFlutter.native = TestMockSentryNative(); + final sut = fixture.getSut(); + final endTimestamp = + fixture.startTimestamp.add(const Duration(milliseconds: 10)); + + await sut.trackAppStartTTD(fixture.getTransaction(name: '/'), + startTimestamp: fixture.startTimestamp, endTimestamp: endTimestamp); await Future.delayed(const Duration(milliseconds: 100)); @@ -53,12 +66,32 @@ void main() { }); group('in regular routes', () { + test('matches startTimestamp of transaction', () async { + final sut = fixture.getSut(); + + await sut.trackRegularRouteTTD(fixture.getTransaction(), + startTimestamp: fixture.startTimestamp); + + final transaction = fixture.hub.getSpan() as SentryTracer; + + final spans = transaction.children; + final ttidSpan = spans + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + + expect(transaction, isNotNull); + expect(transaction.context.operation, SentrySpanOperations.uiLoad); + expect(transaction.startTimestamp, ttidSpan.startTimestamp); + }); + group('with approximation strategy', () { test('startMeasurement finishes ttid span', () async { final sut = fixture.getSut(); - await sut.startTracking( - fixture.getTransaction(), 'Current Route', null); + await sut.trackRegularRouteTTD(fixture.getTransaction(), + startTimestamp: fixture.startTimestamp); final transaction = fixture.hub.getSpan() as SentryTracer; await Future.delayed(const Duration(milliseconds: 2000)); @@ -79,8 +112,8 @@ void main() { fixture.ttidTracker.markAsManual(); fixture.ttidTracker.completeTracking(); }); - await sut.startTracking( - fixture.getTransaction(), 'Current Route', null); + await sut.trackRegularRouteTTD(fixture.getTransaction(), + startTimestamp: fixture.startTimestamp); final transaction = fixture.hub.getSpan() as SentryTracer; @@ -103,8 +136,11 @@ void main() { test('screen load tracking creates ui.load transaction', () async { final sut = fixture.getSut(); + final startTimestamp = + getUtcDateTime().add(const Duration(milliseconds: 100)); - await sut.startTracking(fixture.getTransaction(), 'Current Route', null); + await sut.trackRegularRouteTTD(fixture.getTransaction(), + startTimestamp: startTimestamp); final transaction = fixture.hub.getSpan(); expect(transaction, isNotNull); @@ -113,6 +149,7 @@ void main() { } class Fixture { + final startTimestamp = getUtcDateTime(); final options = SentryFlutterOptions() ..dsn = fakeDsn ..tracesSampleRate = 1.0; @@ -124,7 +161,8 @@ class Fixture { TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); ISentrySpan getTransaction({String? name = "Current route"}) { - return hub.startTransaction(name!, 'ui.load', bindToScope: true); + return hub.startTransaction(name!, 'ui.load', + bindToScope: true, startTimestamp: startTimestamp); } TimeToDisplayTracker getSut() { diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index 58b8fe69ce..f8443c44ae 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -2,7 +2,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -import 'package:sentry_flutter/src/integrations/native_app_start_integration.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; import '../fake_frame_callback_handler.dart'; @@ -25,14 +24,12 @@ void main() { group('app start', () { test('tracking creates and finishes ttid span with correct measurements', () async { - final transaction = - fixture.hub.startTransaction('fake', 'fake') as SentryTracer; - final startTimestamp = DateTime.now(); - final appStartInfo = AppStartInfo(AppStartType.cold, - start: startTimestamp, - end: startTimestamp.add(Duration(milliseconds: 10))); + final transaction = fixture.getTransaction(name: 'root ("/")') as SentryTracer; + final endTimestamp = + fixture.startTimestamp.add(const Duration(milliseconds: 10)); - await sut.trackAppStart(transaction, appStartInfo, 'route ("/")'); + await sut.trackAppStart(transaction, + startTimestamp: fixture.startTimestamp, endTimestamp: endTimestamp); final children = transaction.children; expect(children, hasLength(1)); @@ -41,7 +38,7 @@ void main() { expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); expect(ttidSpan.finished, isTrue); - expect(ttidSpan.context.description, 'route ("/") initial display'); + expect(ttidSpan.context.description, 'root ("/") initial display'); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); final ttidMeasurement = @@ -56,11 +53,9 @@ void main() { test( 'approximation tracking creates and finishes ttid span with correct measurements', () async { - final transaction = - fixture.hub.startTransaction('fake', 'fake') as SentryTracer; - final startTimestamp = DateTime.now(); + final transaction = fixture.getTransaction() as SentryTracer; - await sut.trackRegularRoute(transaction, startTimestamp, 'regular route'); + await sut.trackRegularRoute(transaction, fixture.startTimestamp); final children = transaction.children; expect(children, hasLength(1)); @@ -69,7 +64,7 @@ void main() { expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); expect(ttidSpan.finished, isTrue); - expect(ttidSpan.context.description, 'regular route initial display'); + expect(ttidSpan.context.description, 'Regular route initial display'); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); final ttidMeasurement = @@ -85,15 +80,13 @@ void main() { test( 'manual tracking creates and finishes ttid span with correct measurements', () async { - final transaction = - fixture.hub.startTransaction('fake', 'fake') as SentryTracer; - final startTimestamp = DateTime.now(); + final transaction = fixture.getTransaction() as SentryTracer; sut.markAsManual(); Future.delayed(fixture.finishFrameAfterDuration, () { sut.completeTracking(); }); - await sut.trackRegularRoute(transaction, startTimestamp, 'regular route'); + await sut.trackRegularRoute(transaction, fixture.startTimestamp); final children = transaction.children; expect(children, hasLength(1)); @@ -102,7 +95,7 @@ void main() { expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); expect(ttidSpan.finished, isTrue); - expect(ttidSpan.context.description, 'regular route initial display'); + expect(ttidSpan.context.description, 'Regular route initial display'); expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); final ttidMeasurement = transaction.measurements['time_to_initial_display']; @@ -138,18 +131,14 @@ void main() { }); test('returns the correct approximation end time', () async { - final startTime = DateTime.now(); - final futureEndTime = sut.determineEndTime(); final endTime = await futureEndTime; - expect(endTime?.difference(startTime).inSeconds, + expect(endTime?.difference(fixture.startTimestamp).inSeconds, fixture.finishFrameAfterDuration.inSeconds); }); test('returns the correct manual end time', () async { - final startTime = DateTime.now(); - sut.markAsManual(); final futureEndTime = sut.determineEndTime(); @@ -158,18 +147,24 @@ void main() { }); final endTime = await futureEndTime; - expect(endTime?.difference(startTime).inSeconds, + expect(endTime?.difference(fixture.startTimestamp).inSeconds, fixture.finishFrameAfterDuration.inSeconds); }); }); } class Fixture { + final startTimestamp = getUtcDateTime(); final hub = Hub(SentryFlutterOptions(dsn: fakeDsn)..tracesSampleRate = 1.0); final finishFrameAfterDuration = Duration(milliseconds: 100); late final fakeFrameCallbackHandler = FakeFrameCallbackHandler(finishAfterDuration: finishFrameAfterDuration); + ISentrySpan getTransaction({String? name = "Regular route"}) { + return hub.startTransaction(name!, 'ui.load', + bindToScope: true, startTimestamp: startTimestamp); + } + TimeToInitialDisplayTracker getSut() { return TimeToInitialDisplayTracker( frameCallbackHandler: fakeFrameCallbackHandler); From c0f41d8bf2ead5d63c4a580a45940e0c9e3c26c1 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 12:52:20 +0100 Subject: [PATCH 41/57] Fix analyze --- flutter/lib/src/navigation/time_to_display_tracker.dart | 2 -- flutter/lib/src/navigation/time_to_initial_display_tracker.dart | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/time_to_display_tracker.dart b/flutter/lib/src/navigation/time_to_display_tracker.dart index 38ec1e55c6..713bc16ff4 100644 --- a/flutter/lib/src/navigation/time_to_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_display_tracker.dart @@ -3,10 +3,8 @@ import 'dart:async'; import 'package:meta/meta.dart'; -import '../integrations/integrations.dart'; import '../../sentry_flutter.dart'; -import '../native/sentry_native.dart'; import 'time_to_initial_display_tracker.dart'; @internal diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index b32e9e9a6e..12ab26149c 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; +// ignore: implementation_imports import 'package:sentry/src/sentry_tracer.dart'; import '../../sentry_flutter.dart'; From 7434ca165d201606cec1eba3433b733e9edc7662 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 12:54:13 +0100 Subject: [PATCH 42/57] remove comment --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 053dbb2b08..92564ca25b 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -291,9 +291,9 @@ class SentryNavigatorObserver extends RouteObserver> { } startTimestamp = - appStartInfo.start; // Adjust start timestamp based on app start info. + appStartInfo.start; endTimestamp = - appStartInfo.end; // Set end timestamp for app start tracking. + appStartInfo.end; } await _startTransaction(route, startTimestamp); From e0298fe55383e5a46fdeb9f860e15c60b420e94f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 12:56:14 +0100 Subject: [PATCH 43/57] Formatting --- flutter/example/android/app/build.gradle | 2 +- flutter/example/lib/main.dart | 1 + .../lib/src/navigation/sentry_navigator_observer.dart | 9 +++------ .../test/navigation/time_to_display_tracker_test.dart | 4 ++-- .../navigation/time_to_initial_display_tracker_test.dart | 3 ++- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/flutter/example/android/app/build.gradle b/flutter/example/android/app/build.gradle index b54cd7fdf9..ac4557e2cd 100644 --- a/flutter/example/android/app/build.gradle +++ b/flutter/example/android/app/build.gradle @@ -48,7 +48,7 @@ android { defaultConfig { applicationId "io.sentry.samples.flutter" - minSdkVersion 19 + minSdkVersion flutter.minSdkVersion targetSdkVersion 33 versionCode flutterVersionCode.toInteger() versionName flutterVersionName diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 6a0c0a5b81..a46a9ca641 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -80,6 +80,7 @@ Future setupSentry( // going to log too much for your app, but can be useful when figuring out // configuration issues, e.g. finding out why your events are not uploaded. options.debug = true; + options.spotlight = Spotlight(enabled: true); options.maxRequestBodySize = MaxRequestBodySize.always; options.maxResponseBodySize = MaxResponseBodySize.always; diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 92564ca25b..f71f8def9f 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -284,16 +284,13 @@ class SentryNavigatorObserver extends RouteObserver> { DateTime? endTimestamp; if (routeName == '/') { - final appStartInfo = await NativeAppStartIntegration - .getAppStartInfo(); + final appStartInfo = await NativeAppStartIntegration.getAppStartInfo(); if (appStartInfo == null) { return; } - startTimestamp = - appStartInfo.start; - endTimestamp = - appStartInfo.end; + startTimestamp = appStartInfo.start; + endTimestamp = appStartInfo.end; } await _startTransaction(route, startTimestamp); diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index acdd2ba653..81c4598309 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -77,8 +77,8 @@ void main() { final spans = transaction.children; final ttidSpan = spans .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(transaction, isNotNull); diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index f8443c44ae..b8dc651c3d 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -24,7 +24,8 @@ void main() { group('app start', () { test('tracking creates and finishes ttid span with correct measurements', () async { - final transaction = fixture.getTransaction(name: 'root ("/")') as SentryTracer; + final transaction = + fixture.getTransaction(name: 'root ("/")') as SentryTracer; final endTimestamp = fixture.startTimestamp.add(const Duration(milliseconds: 10)); From 7e8478cc7dca5df42bd9c9761f3090a1e7bd122e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 13:30:42 +0100 Subject: [PATCH 44/57] fix test --- .../src/navigation/sentry_navigator_observer.dart | 1 - flutter/test/sentry_navigator_observer_test.dart | 12 +++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index f71f8def9f..a9b91f0ce1 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -277,7 +277,6 @@ class SentryNavigatorObserver extends RouteObserver> { Future _startTimeToDisplayTracking(Route? route) async { _completedDisplayTracking = Completer(); String? routeName = _currentRouteName; - if (routeName == null) return; DateTime startTimestamp = _hub.options.clock(); diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 2e54fed326..5a7528d904 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -5,6 +5,7 @@ import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/integrations/integrations.dart'; import 'package:sentry_flutter/src/native/sentry_native.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; @@ -383,8 +384,15 @@ void main() { verify(span.setData('route_settings_arguments', arguments)); }); - test('flutter root name is replaced', () { + test('flutter root name is replaced', () async { final rootRoute = route(RouteSettings(name: '/')); + NativeAppStartIntegration.setAppStartInfo( + AppStartInfo( + AppStartType.cold, + start: DateTime.now().add(const Duration(seconds: 1)), + end: DateTime.now().add(const Duration(seconds: 2)), + ), + ); final hub = _MockHub(); final span = getMockSentryTracer(name: '/'); @@ -401,6 +409,8 @@ void main() { sut.didPush(rootRoute, null); + await Future.delayed(const Duration(milliseconds: 100)); + final context = verify(hub.startTransactionWithContext( captureAny, waitForChildren: true, From 67c6e8f4d48d95e6e620e8711202cb0f130e2b66 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 16:45:07 +0100 Subject: [PATCH 45/57] add ttid duration assertion and determineEndTime timeout --- .../time_to_initial_display_tracker.dart | 19 +++++++++++-------- .../sentry_display_widget_test.dart | 16 ++++++++++++---- .../time_to_initial_display_tracker_test.dart | 18 ++++++++++++++++-- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index 12ab26149c..c251559595 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; + // ignore: implementation_imports import 'package:sentry/src/sentry_tracer.dart'; @@ -26,8 +27,9 @@ class TimeToInitialDisplayTracker { FrameCallbackHandler _frameCallbackHandler = DefaultFrameCallbackHandler(); bool _isManual = false; - Completer? _trackingCompleter; + Completer? _trackingCompleter; DateTime? _endTimestamp; + final Duration _determineEndtimeTimeout = Duration(seconds: 5); /// This endTimestamp is needed in the [TimeToFullDisplayTracker] class @internal @@ -40,8 +42,6 @@ class TimeToInitialDisplayTracker { await _trackTimeToInitialDisplay( transaction: transaction, startTimestamp: startTimestamp, - // endTimestamp is null by default, determined inside the private method - // origin could be set here if needed, or determined inside the private method ); } @@ -65,7 +65,6 @@ class TimeToInitialDisplayTracker { DateTime? endTimestamp, String? origin, }) async { - // Determine endTimestamp if not provided final _endTimestamp = endTimestamp ?? await determineEndTime(); if (_endTimestamp == null) return; @@ -77,7 +76,6 @@ class TimeToInitialDisplayTracker { startTimestamp: startTimestamp, ); - // Set the origin based on provided value or determine based on _isManual ttidSpan.origin = origin ?? (_isManual ? SentryTraceOrigins.manualUiTimeToDisplay @@ -92,8 +90,8 @@ class TimeToInitialDisplayTracker { await ttidSpan.finish(endTimestamp: _endTimestamp); } - Future? determineEndTime() { - _trackingCompleter = Completer(); + Future? determineEndTime() { + _trackingCompleter = Completer(); // If we already know it's manual we can return the future immediately if (_isManual) { @@ -108,7 +106,12 @@ class TimeToInitialDisplayTracker { } }); - return _trackingCompleter?.future; + return _trackingCompleter?.future.timeout( + _determineEndtimeTimeout, + onTimeout: () { + return Future.value(null); + }, + ); } void markAsManual() { diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart index 106912f6bc..70547be33b 100644 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -38,16 +38,21 @@ void main() { SentrySpanOperations.uiTimeToInitialDisplay); expect(spans, hasLength(1)); + final ttidSpan = spans.first; expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); expect(ttidSpan.finished, isTrue); expect(ttidSpan.context.description, 'Current Route initial display'); expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); + final ttidSpanDuration = + ttidSpan.endTimestamp!.difference(ttidSpan.startTimestamp); + expect(tracer.measurements, hasLength(1)); final measurement = tracer.measurements['time_to_initial_display']; expect(measurement, isNotNull); expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); + expect(measurement?.value, ttidSpanDuration.inMilliseconds); }); testWidgets('SentryDisplayWidget is ignored for app starts', @@ -55,8 +60,8 @@ void main() { final currentRoute = route(RouteSettings(name: '/')); final appStartInfo = AppStartInfo( AppStartType.cold, - start: DateTime.now().add(Duration(seconds: 1)), - end: DateTime.now().add(Duration(seconds: 2)), + start: getUtcDateTime().add(Duration(seconds: 1)), + end: getUtcDateTime().add(Duration(seconds: 2)), ); NativeAppStartIntegration.setAppStartInfo(appStartInfo); @@ -80,13 +85,16 @@ void main() { expect(ttidSpan.context.description, 'root ("/") initial display'); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); - expect(ttidSpan.startTimestamp.toUtc(), appStartInfo.start.toUtc()); - expect(ttidSpan.endTimestamp?.toUtc(), appStartInfo.end.toUtc()); + expect(ttidSpan.startTimestamp, appStartInfo.start); + expect(ttidSpan.endTimestamp, appStartInfo.end); + final ttidSpanDuration = + ttidSpan.endTimestamp!.difference(ttidSpan.startTimestamp); expect(tracer.measurements, hasLength(1)); final measurement = tracer.measurements['time_to_initial_display']; expect(measurement, isNotNull); expect(measurement?.value, appStartInfo.duration.inMilliseconds); + expect(measurement?.value, ttidSpanDuration.inMilliseconds); expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond); }); } diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index b8dc651c3d..0503696fca 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -2,6 +2,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/frame_callback_handler.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; import '../fake_frame_callback_handler.dart'; @@ -41,6 +42,8 @@ void main() { expect(ttidSpan.finished, isTrue); expect(ttidSpan.context.description, 'root ("/") initial display'); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); + expect(ttidSpan.startTimestamp, fixture.startTimestamp); + expect(ttidSpan.endTimestamp, endTimestamp); final ttidMeasurement = transaction.measurements['time_to_initial_display']; @@ -110,6 +113,15 @@ void main() { }); group('determineEndtime', () { + test('can complete with null in approximation mode with timeout', () async { + // this test will trigger the timeout + final futureEndTime = await fixture + .getSut(frameCallbackHandler: DefaultFrameCallbackHandler()) + .determineEndTime(); + + expect(futureEndTime, null); + }); + test('can complete automatically in approximation mode', () async { final futureEndTime = sut.determineEndTime(); @@ -166,8 +178,10 @@ class Fixture { bindToScope: true, startTimestamp: startTimestamp); } - TimeToInitialDisplayTracker getSut() { + TimeToInitialDisplayTracker getSut( + {FrameCallbackHandler? frameCallbackHandler}) { + frameCallbackHandler ??= fakeFrameCallbackHandler; return TimeToInitialDisplayTracker( - frameCallbackHandler: fakeFrameCallbackHandler); + frameCallbackHandler: frameCallbackHandler); } } From 320f92b36a8cf6f9c6364fbce798cb2a01f92a0f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 16:52:48 +0100 Subject: [PATCH 46/57] Rename finish transaction and do an early exit with enableAutoTransactions --- .../navigation/sentry_navigator_observer.dart | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index a9b91f0ce1..4a9d010419 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -129,7 +129,7 @@ class SentryNavigatorObserver extends RouteObserver> { to: route.settings, ); - _finishTransaction(); + _finishDisplayTracking(); _startTimeToDisplayTracking(route); } @@ -160,7 +160,7 @@ class SentryNavigatorObserver extends RouteObserver> { to: previousRoute?.settings, ); - _finishTransaction(); + _finishDisplayTracking(); } void _addBreadcrumb({ @@ -198,24 +198,8 @@ class SentryNavigatorObserver extends RouteObserver> { } } - Future _finishTransaction() async { - _timeToDisplayTracker?.clear(); - - final transaction = _transaction; - _transaction = null; - if (transaction == null || transaction.finished) { - return; - } - transaction.status ??= SpanStatus.ok(); - await transaction.finish(); - } - Future _startTransaction( Route? route, DateTime startTimestamp) async { - if (!_enableAutoTransactions) { - return; - } - String? name = _getRouteName(route); final arguments = route?.settings.arguments; @@ -274,7 +258,23 @@ class SentryNavigatorObserver extends RouteObserver> { await _native?.beginNativeFramesCollection(); } + Future _finishDisplayTracking() async { + _timeToDisplayTracker?.clear(); + + final transaction = _transaction; + _transaction = null; + if (transaction == null || transaction.finished) { + return; + } + transaction.status ??= SpanStatus.ok(); + await transaction.finish(); + } + Future _startTimeToDisplayTracking(Route? route) async { + if (!_enableAutoTransactions) { + return; + } + _completedDisplayTracking = Completer(); String? routeName = _currentRouteName; if (routeName == null) return; From a823366dea3a164b8aae62238c2dab646512f682 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 17:01:03 +0100 Subject: [PATCH 47/57] Rename function --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 4a9d010419..fb411e6a6a 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -129,7 +129,7 @@ class SentryNavigatorObserver extends RouteObserver> { to: route.settings, ); - _finishDisplayTracking(); + _finishTimeToDisplayTracking(); _startTimeToDisplayTracking(route); } @@ -160,7 +160,7 @@ class SentryNavigatorObserver extends RouteObserver> { to: previousRoute?.settings, ); - _finishDisplayTracking(); + _finishTimeToDisplayTracking(); } void _addBreadcrumb({ @@ -258,7 +258,7 @@ class SentryNavigatorObserver extends RouteObserver> { await _native?.beginNativeFramesCollection(); } - Future _finishDisplayTracking() async { + Future _finishTimeToDisplayTracking() async { _timeToDisplayTracker?.clear(); final transaction = _transaction; From dbc4320e3a560baf41b59d88aa5eb2bd54a9f975 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 10:10:41 +0100 Subject: [PATCH 48/57] Remove static and getter for in navigator observer --- .../lib/src/navigation/sentry_navigator_observer.dart | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index fb411e6a6a..5eb7884006 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -82,11 +82,11 @@ class SentryNavigatorObserver extends RouteObserver> { _setRouteNameAsTransaction = setRouteNameAsTransaction, _routeNameExtractor = routeNameExtractor, _additionalInfoProvider = additionalInfoProvider, - _native = SentryFlutter.native { + _native = SentryFlutter.native, + _timeToDisplayTracker = timeToDisplayTracker ?? TimeToDisplayTracker() { if (enableAutoTransactions) { _hub.options.sdk.addIntegration('UINavigationTracing'); } - _timeToDisplayTracker = timeToDisplayTracker ?? TimeToDisplayTracker(); } final Hub _hub; @@ -96,12 +96,7 @@ class SentryNavigatorObserver extends RouteObserver> { final RouteNameExtractor? _routeNameExtractor; final AdditionalInfoExtractor? _additionalInfoProvider; final SentryNative? _native; - - static TimeToDisplayTracker? _timeToDisplayTracker; - - @internal - static TimeToDisplayTracker? get timeToDisplayTracker => - _timeToDisplayTracker; + final TimeToDisplayTracker? _timeToDisplayTracker; ISentrySpan? _transaction; From 9c6e2b15827c6275df695e60835efc867610f7eb Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 10:50:50 +0100 Subject: [PATCH 49/57] Expose SentryDisplayWidget as public api and add it to example app --- flutter/example/lib/main.dart | 3 ++- flutter/lib/sentry_flutter.dart | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index a46a9ca641..19050ec79e 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -13,6 +13,7 @@ import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_isar/sentry_isar.dart'; import 'package:sentry_sqflite/sentry_sqflite.dart'; import 'package:sqflite/sqflite.dart'; + // import 'package:sqflite_common_ffi/sqflite_ffi.dart'; // import 'package:sqflite_common_ffi_web/sqflite_ffi_web.dart'; import 'package:universal_platform/universal_platform.dart'; @@ -733,7 +734,7 @@ void navigateToAutoCloseScreen(BuildContext context) { context, MaterialPageRoute( settings: const RouteSettings(name: 'AutoCloseScreen'), - builder: (context) => const AutoCloseScreen(), + builder: (context) => SentryDisplayWidget(child: const AutoCloseScreen()), ), ); } diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index 0f43bf741b..d15c8b7a70 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -16,3 +16,4 @@ export 'src/screenshot/sentry_screenshot_quality.dart'; export 'src/user_interaction/sentry_user_interaction_widget.dart'; export 'src/binding_wrapper.dart'; export 'src/sentry_widget.dart'; +export 'src/navigation/sentry_display_widget.dart'; From 1aa234350c5d00005733dc3e0b3524c91de4db88 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 10:54:43 +0100 Subject: [PATCH 50/57] Fix dart analyze --- flutter/test/navigation/sentry_display_widget_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart index 70547be33b..6fb61c5b8f 100644 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ b/flutter/test/navigation/sentry_display_widget_test.dart @@ -5,7 +5,6 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/integrations/integrations.dart'; -import 'package:sentry_flutter/src/navigation/sentry_display_widget.dart'; import '../fake_frame_callback_handler.dart'; import '../mocks.dart'; From b15eccfca5a57c5ddbf82fb88a66ef088a19cdae Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 12:38:54 +0100 Subject: [PATCH 51/57] Fix dart doc --- flutter/lib/src/navigation/sentry_display_widget.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_display_widget.dart b/flutter/lib/src/navigation/sentry_display_widget.dart index d5995999c5..e42f5c4f72 100644 --- a/flutter/lib/src/navigation/sentry_display_widget.dart +++ b/flutter/lib/src/navigation/sentry_display_widget.dart @@ -9,7 +9,7 @@ import '../frame_callback_handler.dart'; /// for the child widget to be initially displayed on the screen. This method /// allows a more accurate measurement than what the default TTID implementation /// provides. The TTID measurement begins when the route to the widget is pushed and ends -/// when [WidgetsBinding.instance.addPostFrameCallback] is triggered. +/// when `addPostFramecallback` is triggered. /// /// Wrap the widget you want to measure with [SentryDisplayWidget], and ensure that you /// have set up Sentry's routing instrumentation according to the Sentry documentation. From aeaebfae53ff0ba13e9e25d4724b43cacf6cb785 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 13:57:24 +0100 Subject: [PATCH 52/57] Improve tests --- .../time_to_initial_display_tracker.dart | 15 +- .../time_to_display_tracker_test.dart | 129 +++++++++++------- .../time_to_initial_display_tracker_test.dart | 56 ++++---- 3 files changed, 117 insertions(+), 83 deletions(-) diff --git a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index c251559595..ce2f7b9e9c 100644 --- a/flutter/lib/src/navigation/time_to_initial_display_tracker.dart +++ b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart @@ -92,10 +92,16 @@ class TimeToInitialDisplayTracker { Future? determineEndTime() { _trackingCompleter = Completer(); + final future = _trackingCompleter?.future.timeout( + _determineEndtimeTimeout, + onTimeout: () { + return Future.value(null); + }, + ); // If we already know it's manual we can return the future immediately if (_isManual) { - return _trackingCompleter?.future; + return future; } // Schedules a check at the end of the frame to determine if the tracking @@ -106,12 +112,7 @@ class TimeToInitialDisplayTracker { } }); - return _trackingCompleter?.future.timeout( - _determineEndtimeTimeout, - onTimeout: () { - return Future.value(null); - }, - ); + return future; } void markAsManual() { diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 81c4598309..c3d20ae895 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -3,6 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import 'package:sentry_flutter/src/frame_callback_handler.dart'; import 'package:sentry_flutter/src/navigation/time_to_display_tracker.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry_flutter/src/navigation/time_to_initial_display_tracker.dart'; @@ -19,7 +20,7 @@ void main() { }); tearDown(() { - fixture.ttidTracker.clear(); + fixture.ttidTracker?.clear(); }); group('time to initial display', () { @@ -27,35 +28,48 @@ void main() { test('matches startTimestamp of transaction', () async { final sut = fixture.getSut(); - await sut.trackRegularRouteTTD(fixture.getTransaction(name: '/'), + final transaction = fixture.getTransaction(name: '/') as SentryTracer; + await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - final transaction = fixture.hub.getSpan() as SentryTracer; - - final spans = transaction.children; - final ttidSpan = spans + final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; - expect(transaction, isNotNull); expect(transaction.context.operation, SentrySpanOperations.uiLoad); expect(transaction.startTimestamp, ttidSpan.startTimestamp); }); - test('startMeasurement finishes ttid span', () async { + test('trackAppStartTTD finishes ttid span', () async { SentryFlutter.native = TestMockSentryNative(); final sut = fixture.getSut(); final endTimestamp = fixture.startTimestamp.add(const Duration(milliseconds: 10)); - await sut.trackAppStartTTD(fixture.getTransaction(name: '/'), + final transaction = fixture.getTransaction(name: '/') as SentryTracer; + await sut.trackAppStartTTD(transaction, startTimestamp: fixture.startTimestamp, endTimestamp: endTimestamp); - await Future.delayed(const Duration(milliseconds: 100)); + final ttidSpan = transaction.children + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + expect(ttidSpan.context.operation, + SentrySpanOperations.uiTimeToInitialDisplay); + expect(ttidSpan.finished, isTrue); + expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); + }); - final transaction = fixture.hub.getSpan() as SentryTracer; + test('trackRegularRoute finishes ttid span', () async { + SentryFlutter.native = TestMockSentryNative(); + final sut = fixture.getSut(); + + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, + startTimestamp: fixture.startTimestamp); final spans = transaction.children; expect(transaction.children, hasLength(1)); @@ -69,38 +83,47 @@ void main() { test('matches startTimestamp of transaction', () async { final sut = fixture.getSut(); - await sut.trackRegularRouteTTD(fixture.getTransaction(), + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - final transaction = fixture.hub.getSpan() as SentryTracer; - - final spans = transaction.children; - final ttidSpan = spans + final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; - expect(transaction, isNotNull); expect(transaction.context.operation, SentrySpanOperations.uiLoad); expect(transaction.startTimestamp, ttidSpan.startTimestamp); }); group('with approximation strategy', () { - test('startMeasurement finishes ttid span', () async { + test('trackRegularRouteTTD finishes ttid span', () async { final sut = fixture.getSut(); - await sut.trackRegularRouteTTD(fixture.getTransaction(), + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - final transaction = fixture.hub.getSpan() as SentryTracer; - await Future.delayed(const Duration(milliseconds: 2000)); - - final spans = transaction.children; - expect(transaction.children, hasLength(1)); - expect(spans[0].context.operation, + final ttidSpan = transaction.children + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); - expect(spans[0].finished, isTrue); + expect(ttidSpan.finished, isTrue); + expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); + }); + + test('timeout triggered when not completing the tracking', () async { + final sut = fixture.getSut(triggerApproximationTimeout: true); + + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, + startTimestamp: fixture.startTimestamp); + + expect(transaction.children, hasLength(0)); }); }); @@ -108,27 +131,35 @@ void main() { test('finishes ttid span after reporting with manual api', () async { final sut = fixture.getSut(); - Future.delayed(const Duration(milliseconds: 100), () { - fixture.ttidTracker.markAsManual(); - fixture.ttidTracker.completeTracking(); + Future.delayed(const Duration(milliseconds: 1), () { + fixture.ttidTracker?.markAsManual(); + fixture.ttidTracker?.completeTracking(); }); - await sut.trackRegularRouteTTD(fixture.getTransaction(), + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - final transaction = fixture.hub.getSpan() as SentryTracer; - - await Future.delayed(const Duration(milliseconds: 100)); - final ttidSpan = transaction.children .where((element) => element.context.operation == SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(ttidSpan, isNotNull); + expect(ttidSpan.finished, isTrue); + expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); + }); - await Future.delayed(const Duration(milliseconds: 100)); + test('timeout triggered when not completing the tracking', () async { + final sut = fixture.getSut(); - expect(ttidSpan.finished, isTrue); + fixture.ttidTracker?.markAsManual(); + // Not calling completeTracking() triggers the manual timeout + + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, + startTimestamp: fixture.startTimestamp); + + expect(transaction.children, hasLength(0)); }); }); }); @@ -139,12 +170,11 @@ void main() { final startTimestamp = getUtcDateTime().add(const Duration(milliseconds: 100)); - await sut.trackRegularRouteTTD(fixture.getTransaction(), - startTimestamp: startTimestamp); + final transaction = fixture.getTransaction() as SentryTracer; + await sut.trackRegularRouteTTD(transaction, startTimestamp: startTimestamp); - final transaction = fixture.hub.getSpan(); expect(transaction, isNotNull); - expect(transaction?.context.operation, SentrySpanOperations.uiLoad); + expect(transaction.context.operation, SentrySpanOperations.uiLoad); }); } @@ -155,17 +185,18 @@ class Fixture { ..tracesSampleRate = 1.0; late final hub = Hub(options); - - final frameCallbackHandler = FakeFrameCallbackHandler(); - late final ttidTracker = - TimeToInitialDisplayTracker(frameCallbackHandler: frameCallbackHandler); + TimeToInitialDisplayTracker? ttidTracker; ISentrySpan getTransaction({String? name = "Current route"}) { return hub.startTransaction(name!, 'ui.load', bindToScope: true, startTimestamp: startTimestamp); } - TimeToDisplayTracker getSut() { + TimeToDisplayTracker getSut({bool triggerApproximationTimeout = false}) { + ttidTracker = TimeToInitialDisplayTracker( + frameCallbackHandler: triggerApproximationTimeout + ? DefaultFrameCallbackHandler() + : FakeFrameCallbackHandler()); return TimeToDisplayTracker( ttidTracker: ttidTracker, ); diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index 0503696fca..9870f9feca 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -25,11 +25,11 @@ void main() { group('app start', () { test('tracking creates and finishes ttid span with correct measurements', () async { - final transaction = - fixture.getTransaction(name: 'root ("/")') as SentryTracer; final endTimestamp = fixture.startTimestamp.add(const Duration(milliseconds: 10)); + final transaction = + fixture.getTransaction(name: 'root ("/")') as SentryTracer; await sut.trackAppStart(transaction, startTimestamp: fixture.startTimestamp, endTimestamp: endTimestamp); @@ -58,7 +58,6 @@ void main() { 'approximation tracking creates and finishes ttid span with correct measurements', () async { final transaction = fixture.getTransaction() as SentryTracer; - await sut.trackRegularRoute(transaction, fixture.startTimestamp); final children = transaction.children; @@ -84,12 +83,12 @@ void main() { test( 'manual tracking creates and finishes ttid span with correct measurements', () async { - final transaction = fixture.getTransaction() as SentryTracer; - sut.markAsManual(); Future.delayed(fixture.finishFrameAfterDuration, () { sut.completeTracking(); }); + + final transaction = fixture.getTransaction() as SentryTracer; await sut.trackRegularRoute(transaction, fixture.startTimestamp); final children = transaction.children; @@ -113,53 +112,55 @@ void main() { }); group('determineEndtime', () { - test('can complete with null in approximation mode with timeout', () async { - // this test will trigger the timeout + test('can complete as null in approximation mode with timeout', () async { final futureEndTime = await fixture - .getSut(frameCallbackHandler: DefaultFrameCallbackHandler()) + .getSut(triggerApproximationTimeout: true) .determineEndTime(); expect(futureEndTime, null); }); - test('can complete automatically in approximation mode', () async { - final futureEndTime = sut.determineEndTime(); + test('can complete as null in manual mode with timeout', () async { + final sut = fixture.getSut(); + sut.markAsManual(); + // Not calling completeTracking() triggers the manual timeout + + final futureEndTime = await sut.determineEndTime(); - expect(futureEndTime, completes); + expect(futureEndTime, null); }); - test('prevents automatic completion in manual mode', () async { - sut.markAsManual(); - final futureEndTime = sut.determineEndTime(); + test('can complete automatically in approximation mode', () async { + final futureEndTime = await sut.determineEndTime(); - expect(futureEndTime, doesNotComplete); + expect(futureEndTime, isNotNull); }); test('can complete manually in manual mode', () async { sut.markAsManual(); - final futureEndTime = sut.determineEndTime(); + Future.delayed(Duration(milliseconds: 1), () { + sut.completeTracking(); + }); + final futureEndTime = await sut.determineEndTime(); - sut.completeTracking(); - expect(futureEndTime, completes); + expect(futureEndTime, isNotNull); }); test('returns the correct approximation end time', () async { - final futureEndTime = sut.determineEndTime(); + final endTme = await sut.determineEndTime(); - final endTime = await futureEndTime; - expect(endTime?.difference(fixture.startTimestamp).inSeconds, + expect(endTme?.difference(fixture.startTimestamp).inSeconds, fixture.finishFrameAfterDuration.inSeconds); }); test('returns the correct manual end time', () async { sut.markAsManual(); - final futureEndTime = sut.determineEndTime(); - Future.delayed(fixture.finishFrameAfterDuration, () { sut.completeTracking(); }); - final endTime = await futureEndTime; + final endTime = await sut.determineEndTime(); + expect(endTime?.difference(fixture.startTimestamp).inSeconds, fixture.finishFrameAfterDuration.inSeconds); }); @@ -179,9 +180,10 @@ class Fixture { } TimeToInitialDisplayTracker getSut( - {FrameCallbackHandler? frameCallbackHandler}) { - frameCallbackHandler ??= fakeFrameCallbackHandler; + {bool triggerApproximationTimeout = false}) { return TimeToInitialDisplayTracker( - frameCallbackHandler: frameCallbackHandler); + frameCallbackHandler: triggerApproximationTimeout + ? DefaultFrameCallbackHandler() + : FakeFrameCallbackHandler()); } } From 5552732d6ae48be1be97d95247a2d7f468ac4515 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 14:08:08 +0100 Subject: [PATCH 53/57] Reduce fake frame finishing time and improve tests --- flutter/test/fake_frame_callback_handler.dart | 7 ++-- .../time_to_display_tracker_test.dart | 32 ++++++++++--------- .../time_to_initial_display_tracker_test.dart | 28 ++++++++-------- 3 files changed, 33 insertions(+), 34 deletions(-) diff --git a/flutter/test/fake_frame_callback_handler.dart b/flutter/test/fake_frame_callback_handler.dart index 103045d0e2..0dc968f22c 100644 --- a/flutter/test/fake_frame_callback_handler.dart +++ b/flutter/test/fake_frame_callback_handler.dart @@ -4,16 +4,15 @@ import 'package:sentry_flutter/src/frame_callback_handler.dart'; class FakeFrameCallbackHandler implements FrameCallbackHandler { FrameCallback? storedCallback; - final Duration _finishAfterDuration; + final Duration finishAfterDuration; FakeFrameCallbackHandler( - {Duration finishAfterDuration = const Duration(milliseconds: 500)}) - : _finishAfterDuration = finishAfterDuration; + {this.finishAfterDuration = const Duration(milliseconds: 50)}); @override void addPostFrameCallback(FrameCallback callback) async { // ignore: inference_failure_on_instance_creation - await Future.delayed(_finishAfterDuration); + await Future.delayed(finishAfterDuration); callback(Duration.zero); } } diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index c3d20ae895..32471e7887 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -34,8 +34,8 @@ void main() { final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(transaction, isNotNull); expect(transaction.context.operation, SentrySpanOperations.uiLoad); @@ -54,8 +54,8 @@ void main() { final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); @@ -71,11 +71,14 @@ void main() { await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - final spans = transaction.children; - expect(transaction.children, hasLength(1)); - expect(spans[0].context.operation, + final ttidSpan = transaction.children + .where((element) => + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) + .first; + expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); - expect(spans[0].finished, isTrue); + expect(ttidSpan.finished, isTrue); }); }); @@ -89,8 +92,8 @@ void main() { final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(transaction, isNotNull); expect(transaction.context.operation, SentrySpanOperations.uiLoad); @@ -107,8 +110,8 @@ void main() { final ttidSpan = transaction.children .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) + element.context.operation == + SentrySpanOperations.uiTimeToInitialDisplay) .first; expect(ttidSpan.context.operation, SentrySpanOperations.uiTimeToInitialDisplay); @@ -167,11 +170,10 @@ void main() { test('screen load tracking creates ui.load transaction', () async { final sut = fixture.getSut(); - final startTimestamp = - getUtcDateTime().add(const Duration(milliseconds: 100)); final transaction = fixture.getTransaction() as SentryTracer; - await sut.trackRegularRouteTTD(transaction, startTimestamp: startTimestamp); + await sut.trackRegularRouteTTD(transaction, + startTimestamp: fixture.startTimestamp); expect(transaction, isNotNull); expect(transaction.context.operation, SentrySpanOperations.uiLoad); diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index 9870f9feca..d7cc91ea9c 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -74,17 +74,15 @@ void main() { transaction.measurements['time_to_initial_display']; expect(ttidMeasurement, isNotNull); expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); - expect( - ttidMeasurement?.value, - greaterThanOrEqualTo( - fixture.finishFrameAfterDuration.inMilliseconds)); + expect(ttidMeasurement?.value, + greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds)); }); test( 'manual tracking creates and finishes ttid span with correct measurements', () async { sut.markAsManual(); - Future.delayed(fixture.finishFrameAfterDuration, () { + Future.delayed(fixture.finishFrameDuration, () { sut.completeTracking(); }); @@ -104,10 +102,8 @@ void main() { transaction.measurements['time_to_initial_display']; expect(ttidMeasurement, isNotNull); expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); - expect( - ttidMeasurement?.value, - greaterThanOrEqualTo( - fixture.finishFrameAfterDuration.inMilliseconds)); + expect(ttidMeasurement?.value, + greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds)); }); }); @@ -150,19 +146,19 @@ void main() { final endTme = await sut.determineEndTime(); expect(endTme?.difference(fixture.startTimestamp).inSeconds, - fixture.finishFrameAfterDuration.inSeconds); + fixture.finishFrameDuration.inSeconds); }); test('returns the correct manual end time', () async { sut.markAsManual(); - Future.delayed(fixture.finishFrameAfterDuration, () { + Future.delayed(fixture.finishFrameDuration, () { sut.completeTracking(); }); final endTime = await sut.determineEndTime(); expect(endTime?.difference(fixture.startTimestamp).inSeconds, - fixture.finishFrameAfterDuration.inSeconds); + fixture.finishFrameDuration.inSeconds); }); }); } @@ -170,15 +166,17 @@ void main() { class Fixture { final startTimestamp = getUtcDateTime(); final hub = Hub(SentryFlutterOptions(dsn: fakeDsn)..tracesSampleRate = 1.0); - final finishFrameAfterDuration = Duration(milliseconds: 100); - late final fakeFrameCallbackHandler = - FakeFrameCallbackHandler(finishAfterDuration: finishFrameAfterDuration); + late final fakeFrameCallbackHandler = FakeFrameCallbackHandler(); ISentrySpan getTransaction({String? name = "Regular route"}) { return hub.startTransaction(name!, 'ui.load', bindToScope: true, startTimestamp: startTimestamp); } + /// The time it takes until a fake frame has been triggered + Duration get finishFrameDuration => + fakeFrameCallbackHandler.finishAfterDuration; + TimeToInitialDisplayTracker getSut( {bool triggerApproximationTimeout = false}) { return TimeToInitialDisplayTracker( From 524935cd92f76ea8da85e7b9955056dd40104bf2 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 14:11:41 +0100 Subject: [PATCH 54/57] Improve test names --- .../time_to_display_tracker_test.dart | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 32471e7887..900344ab40 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -42,7 +42,7 @@ void main() { expect(transaction.startTimestamp, ttidSpan.startTimestamp); }); - test('trackAppStartTTD finishes ttid span', () async { + test('finishes ttid span', () async { SentryFlutter.native = TestMockSentryNative(); final sut = fixture.getSut(); final endTimestamp = @@ -62,24 +62,6 @@ void main() { expect(ttidSpan.finished, isTrue); expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); }); - - test('trackRegularRoute finishes ttid span', () async { - SentryFlutter.native = TestMockSentryNative(); - final sut = fixture.getSut(); - - final transaction = fixture.getTransaction() as SentryTracer; - await sut.trackRegularRouteTTD(transaction, - startTimestamp: fixture.startTimestamp); - - final ttidSpan = transaction.children - .where((element) => - element.context.operation == - SentrySpanOperations.uiTimeToInitialDisplay) - .first; - expect(ttidSpan.context.operation, - SentrySpanOperations.uiTimeToInitialDisplay); - expect(ttidSpan.finished, isTrue); - }); }); group('in regular routes', () { @@ -101,7 +83,7 @@ void main() { }); group('with approximation strategy', () { - test('trackRegularRouteTTD finishes ttid span', () async { + test('finishes ttid span', () async { final sut = fixture.getSut(); final transaction = fixture.getTransaction() as SentryTracer; @@ -131,7 +113,7 @@ void main() { }); group('with manual strategy', () { - test('finishes ttid span after reporting with manual api', () async { + test('finishes ttid span', () async { final sut = fixture.getSut(); Future.delayed(const Duration(milliseconds: 1), () { @@ -152,7 +134,7 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); }); - test('timeout triggered when not completing the tracking', () async { + test('triggers timeout when not completing the tracking', () async { final sut = fixture.getSut(); fixture.ttidTracker?.markAsManual(); From 6e865232b4c74e9d0c78a1eaa6c190106556103b Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 14:23:25 +0100 Subject: [PATCH 55/57] Fix tests --- .../test/navigation/time_to_display_tracker_test.dart | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 900344ab40..93abad8e89 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -101,14 +101,12 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); }); - test('timeout triggered when not completing the tracking', () async { + test('completes with timeout when not completing the tracking', () async { final sut = fixture.getSut(triggerApproximationTimeout: true); final transaction = fixture.getTransaction() as SentryTracer; await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - - expect(transaction.children, hasLength(0)); }); }); @@ -134,7 +132,7 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); }); - test('triggers timeout when not completing the tracking', () async { + test('completes with timeout when not completing the tracking', () async { final sut = fixture.getSut(); fixture.ttidTracker?.markAsManual(); @@ -143,8 +141,6 @@ void main() { final transaction = fixture.getTransaction() as SentryTracer; await sut.trackRegularRouteTTD(transaction, startTimestamp: fixture.startTimestamp); - - expect(transaction.children, hasLength(0)); }); }); }); @@ -172,8 +168,7 @@ class Fixture { TimeToInitialDisplayTracker? ttidTracker; ISentrySpan getTransaction({String? name = "Current route"}) { - return hub.startTransaction(name!, 'ui.load', - bindToScope: true, startTimestamp: startTimestamp); + return hub.startTransaction(name!, 'ui.load', startTimestamp: startTimestamp); } TimeToDisplayTracker getSut({bool triggerApproximationTimeout = false}) { From 5fa126e85d2a224692bac902b3bea92e6048d11d Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 14:23:52 +0100 Subject: [PATCH 56/57] Apply formatting --- .../test/navigation/time_to_display_tracker_test.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/flutter/test/navigation/time_to_display_tracker_test.dart b/flutter/test/navigation/time_to_display_tracker_test.dart index 93abad8e89..942f9ccb35 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -101,7 +101,8 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.autoUiTimeToDisplay); }); - test('completes with timeout when not completing the tracking', () async { + test('completes with timeout when not completing the tracking', + () async { final sut = fixture.getSut(triggerApproximationTimeout: true); final transaction = fixture.getTransaction() as SentryTracer; @@ -132,7 +133,8 @@ void main() { expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); }); - test('completes with timeout when not completing the tracking', () async { + test('completes with timeout when not completing the tracking', + () async { final sut = fixture.getSut(); fixture.ttidTracker?.markAsManual(); @@ -168,7 +170,8 @@ class Fixture { TimeToInitialDisplayTracker? ttidTracker; ISentrySpan getTransaction({String? name = "Current route"}) { - return hub.startTransaction(name!, 'ui.load', startTimestamp: startTimestamp); + return hub.startTransaction(name!, 'ui.load', + startTimestamp: startTimestamp); } TimeToDisplayTracker getSut({bool triggerApproximationTimeout = false}) { From d209276c2b722ff80f84fbd6017c4cc1b65a91f8 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 7 Mar 2024 14:27:52 +0100 Subject: [PATCH 57/57] Add extra assertion in tests --- .../time_to_initial_display_tracker_test.dart | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/flutter/test/navigation/time_to_initial_display_tracker_test.dart b/flutter/test/navigation/time_to_initial_display_tracker_test.dart index d7cc91ea9c..6e55029572 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -49,7 +49,11 @@ void main() { transaction.measurements['time_to_initial_display']; expect(ttidMeasurement, isNotNull); expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); - expect(ttidMeasurement?.value, 10); + expect( + ttidMeasurement?.value, + ttidSpan.endTimestamp! + .difference(ttidSpan.startTimestamp) + .inMilliseconds); }); }); @@ -76,6 +80,11 @@ void main() { expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); expect(ttidMeasurement?.value, greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds)); + expect( + ttidMeasurement?.value, + ttidSpan.endTimestamp! + .difference(ttidSpan.startTimestamp) + .inMilliseconds); }); test( @@ -104,6 +113,11 @@ void main() { expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); expect(ttidMeasurement?.value, greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds)); + expect( + ttidMeasurement?.value, + ttidSpan.endTimestamp! + .difference(ttidSpan.startTimestamp) + .inMilliseconds); }); });