From 9406132b8b374986cfe1bd26767b6f6d6ae4970a Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 6 Feb 2025 14:19:05 +0100 Subject: [PATCH 1/4] remove manual TTID --- flutter/example/lib/main.dart | 2 +- flutter/lib/sentry_flutter.dart | 27 +++---- .../src/navigation/sentry_display_widget.dart | 64 --------------- .../time_to_initial_display_tracker.dart | 33 +------- .../sentry_display_widget_test.dart | 79 ------------------- .../time_to_display_tracker_test.dart | 29 ------- .../time_to_initial_display_tracker_test.dart | 69 ---------------- 7 files changed, 16 insertions(+), 287 deletions(-) delete mode 100644 flutter/lib/src/navigation/sentry_display_widget.dart delete mode 100644 flutter/test/navigation/sentry_display_widget_test.dart diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index b2c5c8e626..87977f7113 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -730,7 +730,7 @@ void navigateToAutoCloseScreen(BuildContext context) { MaterialPageRoute( settings: const RouteSettings(name: 'AutoCloseScreen'), // ignore: deprecated_member_use - builder: (context) => SentryDisplayWidget(child: const AutoCloseScreen()), + builder: (context) => const AutoCloseScreen(), ), ); } diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index 8e3e80afe9..b1571ec875 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -4,24 +4,23 @@ library; // ignore: invalid_export_of_internal_element export 'package:sentry/sentry.dart'; +export 'src/binding_wrapper.dart' + show BindingWrapper, SentryWidgetsFlutterBinding; +export 'src/feedback/sentry_feedback_widget.dart'; +export 'src/flutter_sentry_attachment.dart'; export 'src/integrations/load_release_integration.dart'; +export 'src/integrations/on_error_integration.dart'; export 'src/navigation/sentry_navigator_observer.dart'; -export 'src/sentry_flutter.dart'; -export 'src/sentry_flutter_options.dart'; -export 'src/sentry_replay_options.dart'; export 'src/replay/replay_quality.dart'; -export 'src/sentry_privacy_options.dart'; -export 'src/flutter_sentry_attachment.dart'; -export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; -export 'src/integrations/on_error_integration.dart'; export 'src/screenshot/masking_config.dart' show SentryMaskingDecision; export 'src/screenshot/sentry_mask_widget.dart'; -export 'src/screenshot/sentry_unmask_widget.dart'; -export 'src/screenshot/sentry_screenshot_widget.dart'; export 'src/screenshot/sentry_screenshot_quality.dart'; -export 'src/user_interaction/sentry_user_interaction_widget.dart'; -export 'src/binding_wrapper.dart' - show BindingWrapper, SentryWidgetsFlutterBinding; +export 'src/screenshot/sentry_screenshot_widget.dart'; +export 'src/screenshot/sentry_unmask_widget.dart'; +export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; +export 'src/sentry_flutter.dart'; +export 'src/sentry_flutter_options.dart'; +export 'src/sentry_privacy_options.dart'; +export 'src/sentry_replay_options.dart'; export 'src/sentry_widget.dart'; -export 'src/navigation/sentry_display_widget.dart'; -export 'src/feedback/sentry_feedback_widget.dart'; +export 'src/user_interaction/sentry_user_interaction_widget.dart'; diff --git a/flutter/lib/src/navigation/sentry_display_widget.dart b/flutter/lib/src/navigation/sentry_display_widget.dart deleted file mode 100644 index 77a5ac2882..0000000000 --- a/flutter/lib/src/navigation/sentry_display_widget.dart +++ /dev/null @@ -1,64 +0,0 @@ -import 'package:flutter/cupertino.dart'; - -import '../frame_callback_handler.dart'; -import 'time_to_initial_display_tracker.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 `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. -@Deprecated( - 'Manual TTID tracking is no longer needed - Sentry automatically tracks TTID accurately.' - ' You can safely remove this widget.') -class SentryDisplayWidget extends StatefulWidget { - final Widget child; - final FrameCallbackHandler _frameCallbackHandler; - - SentryDisplayWidget({ - super.key, - required this.child, - @visibleForTesting FrameCallbackHandler? frameCallbackHandler, - }) : _frameCallbackHandler = - frameCallbackHandler ?? DefaultFrameCallbackHandler(); - - @override - _SentryDisplayWidgetState createState() => _SentryDisplayWidgetState(); -} - -// ignore: deprecated_member_use_from_same_package -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/time_to_initial_display_tracker.dart b/flutter/lib/src/navigation/time_to_initial_display_tracker.dart index 3509dc6c4d..690dc433a0 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,6 @@ import 'dart:async'; import 'package:meta/meta.dart'; - // ignore: implementation_imports import 'package:sentry/src/sentry_tracer.dart'; @@ -26,10 +25,8 @@ class TimeToInitialDisplayTracker { TimeToInitialDisplayTracker._(); FrameCallbackHandler _frameCallbackHandler = DefaultFrameCallbackHandler(); - bool _isManual = false; Completer? _trackingCompleter; DateTime? _endTimestamp; - DateTime? _completeTrackingTimeStamp; final Duration _determineEndtimeTimeout = Duration(seconds: 5); @@ -61,10 +58,7 @@ class TimeToInitialDisplayTracker { startTimestamp: startTimestamp, ); - ttidSpan.origin = origin ?? - (_isManual - ? SentryTraceOrigins.manualUiTimeToDisplay - : SentryTraceOrigins.autoUiTimeToDisplay); + ttidSpan.origin = origin ?? SentryTraceOrigins.autoUiTimeToDisplay; final duration = Duration( milliseconds: _endTimestamp.difference(startTimestamp).inMilliseconds); @@ -87,46 +81,23 @@ class TimeToInitialDisplayTracker { }, ); - // If we already know it's manual we can return the future immediately - if (_isManual) { - final completeTrackingTimeStamp = _completeTrackingTimeStamp; - if (completeTrackingTimeStamp != null) { - // If complete was called before we could call start, complete it here. - _endTimestamp = completeTrackingTimeStamp; - _trackingCompleter?.complete(completeTrackingTimeStamp); - _completeTrackingTimeStamp = null; - } - return future; - } - - // 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(); - } + completeTracking(); }); return future; } - void markAsManual() { - _isManual = true; - } - void completeTracking() { final timestamp = DateTime.now(); if (_trackingCompleter != null && !_trackingCompleter!.isCompleted) { _endTimestamp = timestamp; _trackingCompleter?.complete(timestamp); - } else { - _completeTrackingTimeStamp = timestamp; } } 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/test/navigation/sentry_display_widget_test.dart b/flutter/test/navigation/sentry_display_widget_test.dart deleted file mode 100644 index 334c75b9b0..0000000000 --- a/flutter/test/navigation/sentry_display_widget_test.dart +++ /dev/null @@ -1,79 +0,0 @@ -// ignore_for_file: invalid_use_of_internal_member - -import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry_flutter/sentry_flutter.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); - 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); - }); -} - -class Fixture { - final Hub hub = Hub(defaultTestOptions()..tracesSampleRate = 1.0); - late final SentryNavigatorObserver navigatorObserver; - final fakeFrameCallbackHandler = FakeFrameCallbackHandler(); - final frameCallbackHandler = FakeFrameCallbackHandler( - postFrameCallbackDelay: Duration(milliseconds: 10)); - - Fixture() { - navigatorObserver = SentryNavigatorObserver(hub: hub); - } - - MaterialApp getSut() { - return MaterialApp( - // ignore: deprecated_member_use_from_same_package - home: SentryDisplayWidget( - frameCallbackHandler: frameCallbackHandler, - 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 index 9a8d820d29..a5fecdcafd 100644 --- a/flutter/test/navigation/time_to_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_display_tracker_test.dart @@ -83,35 +83,6 @@ void main() { await sut.track(transaction, startTimestamp: fixture.startTimestamp); }); }); - - group('with manual strategy', () { - test('finishes ttid span', () async { - final sut = fixture.getSut(); - - Future.delayed(const Duration(milliseconds: 1), () { - fixture.ttidTracker?.markAsManual(); - fixture.ttidTracker?.completeTracking(); - }); - final transaction = fixture.getTransaction() as SentryTracer; - await sut.track(transaction, startTimestamp: fixture.startTimestamp); - - final ttidSpan = _getTTIDSpan(transaction); - expect(ttidSpan, isNotNull); - expect(ttidSpan?.finished, isTrue); - expect(ttidSpan?.origin, SentryTraceOrigins.manualUiTimeToDisplay); - }); - - // skipping test, flaky - test('completes with timeout when not completing the tracking', () async { - final sut = fixture.getSut(); - - fixture.ttidTracker?.markAsManual(); - // Not calling completeTracking() triggers the manual timeout - - final transaction = fixture.getTransaction() as SentryTracer; - await sut.track(transaction, startTimestamp: fixture.startTimestamp); - }, skip: true); - }); }); group('time to full display', () { 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 ebf691da68..e5331f984b 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -55,45 +55,8 @@ void main() { .inMilliseconds); }); - test( - 'manual tracking creates and finishes ttid span with correct measurements', - () async { - sut.markAsManual(); - Future.delayed(fixture.finishFrameDuration, () { - sut.completeTracking(); - }); - - final transaction = fixture.getTransaction() as SentryTracer; - await sut.track( - transaction: transaction, - startTimestamp: fixture.startTimestamp, - ); - - final children = transaction.children; - expect(children, hasLength(1)); - - final ttidSpan = children.first; - expect(ttidSpan.context.operation, - SentrySpanOperations.uiTimeToInitialDisplay); - expect(ttidSpan.finished, isTrue); - expect(ttidSpan.context.description, 'Regular route initial display'); - expect(ttidSpan.origin, SentryTraceOrigins.manualUiTimeToDisplay); - final ttidMeasurement = - transaction.measurements['time_to_initial_display']; - expect(ttidMeasurement, isNotNull); - expect(ttidMeasurement?.unit, DurationSentryMeasurementUnit.milliSecond); - expect(ttidMeasurement?.value, - greaterThanOrEqualTo(fixture.finishFrameDuration.inMilliseconds)); - expect( - ttidMeasurement?.value, - ttidSpan.endTimestamp! - .difference(ttidSpan.startTimestamp) - .inMilliseconds); - }); - test('starting after completing still finished correctly', () async { await Future.delayed(fixture.finishFrameDuration, () { - sut.markAsManual(); sut.completeTracking(); }); @@ -173,50 +136,18 @@ void main() { expect(futureEndTime, null); }); - 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, null); - }); - test('can complete automatically in approximation mode', () async { final futureEndTime = await sut.determineEndTime(); expect(futureEndTime, isNotNull); }); - test('can complete manually in manual mode', () async { - sut.markAsManual(); - Future.delayed(Duration(milliseconds: 1), () { - sut.completeTracking(); - }); - final futureEndTime = await sut.determineEndTime(); - - expect(futureEndTime, isNotNull); - }); - test('returns the correct approximation end time', () async { final endTme = await sut.determineEndTime(); expect(endTme?.difference(fixture.startTimestamp).inSeconds, fixture.finishFrameDuration.inSeconds); }); - - test('returns the correct manual end time', () async { - sut.markAsManual(); - Future.delayed(fixture.finishFrameDuration, () { - sut.completeTracking(); - }); - - final endTime = await sut.determineEndTime(); - - expect(endTime?.difference(fixture.startTimestamp).inSeconds, - fixture.finishFrameDuration.inSeconds); - }); }); } From c61972a49f48ceaabbd05c26c7d5eaeb4d604b0e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 6 Feb 2025 14:26:12 +0100 Subject: [PATCH 2/4] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43b148d0d0..45334f5688 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased 9.0.0 +- Remove `SentryDisplayWidget` and manual TTID implementation ([#2668](https://github.com/getsentry/sentry-dart/pull/2668)) - Increase minimum SDK version requirements to Dart v3.5.0 and Flutter v3.24.0 ([#2643](https://github.com/getsentry/sentry-dart/pull/2643)) ### Dependencies From c8bf923bd46986157145ae98b7cca814105fcb2c Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 6 Feb 2025 14:34:31 +0100 Subject: [PATCH 3/4] remove assertion --- .../test/navigation/time_to_initial_display_tracker_test.dart | 1 - 1 file changed, 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 e5331f984b..4e36fd3e15 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -74,7 +74,6 @@ void main() { 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); From e124b8b3da373c21f859ed3afd77e886b44e16e4 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 6 Feb 2025 14:35:08 +0100 Subject: [PATCH 4/4] use SentryTraceOrigins.autoUiTimeToDisplay as origin --- .../test/navigation/time_to_initial_display_tracker_test.dart | 1 + 1 file changed, 1 insertion(+) 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 4e36fd3e15..1be86954cf 100644 --- a/flutter/test/navigation/time_to_initial_display_tracker_test.dart +++ b/flutter/test/navigation/time_to_initial_display_tracker_test.dart @@ -74,6 +74,7 @@ void main() { 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);