feat: improve frame tracking accuracy (#2372)

* update test' * revert example * update test * fix fluttera analyze and update frame tracker doc * update CHANGELOG * update * update test * working integration test * update * fix test * update * improve tests * fix test * Update CHANGELOG.md * Update span_frame_metrics_collector.dart * iterate over relevant frames and not all delayed frames * improve frame valid check * change actualDurationMs to frameDurationMs * improve logger * remove unnecessary log * decouple frame tracker from widgetsbinding * use clock callback
getsentry · Nov 11, 2024 · 6d5e62f · 6d5e62f
1 parent 15868f3
commit 6d5e62f
Show file tree

Hide file tree

Showing 20 changed files with 3,489 additions and 789 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,24 @@
 # Changelog
 
+## Unreleased
+
+### Features
+
+- Improve frame tracking accuracy ([#2372](https://github.com/getsentry/sentry-dart/pull/2372))
+  - Introduces `SentryWidgetsFlutterBinding` that tracks a frame starting from `handleBeginFrame` and ending in `handleDrawFrame`, this is approximately the [buildDuration](https://api.flutter.dev/flutter/dart-ui/FrameTiming/buildDuration.html) time
+  - By default, `SentryFlutter.init()` automatically initializes `SentryWidgetsFlutterBinding` through the `WidgetsFlutterBindingIntegration`
+  - If you need to initialize the binding before `SentryFlutter.init`, use `SentryWidgetsFlutterBinding.ensureInitialized` instead of `WidgetsFlutterBinding.ensureInitialized`:
+  ```dart
+  void main() async {
+    // Replace WidgetsFlutterBinding.ensureInitialized()
+    SentryWidgetsFlutterBinding.ensureInitialized();
+    
+    await SentryFlutter.init(...);
+    runApp(MyApp());
+  }
+  ```
+  - ⚠️ Frame tracking will be disabled if a different binding is used
+
 ## 8.10.1
 
 ### Fixes

diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart
@@ -60,3 +60,5 @@ export 'src/spotlight.dart';
 export 'src/protocol/sentry_proxy.dart';
 // feedback
 export 'src/protocol/sentry_feedback.dart';
+// constants
+export 'src/span_data_convention.dart';
diff --git a/dart/lib/src/sentry_measurement.dart b/dart/lib/src/sentry_measurement.dart
@@ -7,22 +7,32 @@ class SentryMeasurement {
     this.unit,
   });
 
+  static const totalFramesName = 'frames_total';
+  static const slowFramesName = 'frames_slow';
+  static const frozenFramesName = 'frames_frozen';
+  static const framesDelayName = 'frames_delay';
+
   /// Amount of frames drawn during a transaction
   SentryMeasurement.totalFrames(this.value)
-      : name = 'frames_total',
+      : name = totalFramesName,
         unit = SentryMeasurementUnit.none;
 
   /// Amount of slow frames drawn during a transaction.
   /// A slow frame is any frame longer than 1s / refreshrate.
   /// So for example any frame slower than 16ms for a refresh rate of 60hz.
   SentryMeasurement.slowFrames(this.value)
-      : name = 'frames_slow',
+      : name = slowFramesName,
         unit = SentryMeasurementUnit.none;
 
   /// Amount of frozen frames drawn during a transaction.
   /// Typically defined as frames slower than 500ms.
   SentryMeasurement.frozenFrames(this.value)
-      : name = 'frames_frozen',
+      : name = frozenFramesName,
+        unit = SentryMeasurementUnit.none;
+
+  /// Total duration of frames delayed.
+  SentryMeasurement.framesDelay(this.value)
+      : name = framesDelayName,
         unit = SentryMeasurementUnit.none;
 
   /// Duration of the Cold App start in milliseconds

diff --git a/dart/lib/src/span_data_convention.dart b/dart/lib/src/span_data_convention.dart
@@ -0,0 +1,10 @@
+class SpanDataConvention {
+  SpanDataConvention._();
+
+  static const totalFrames = 'frames.total';
+  static const slowFrames = 'frames.slow';
+  static const frozenFrames = 'frames.frozen';
+  static const framesDelay = 'frames.delay';
+
+  // TODO: eventually add other data keys here as well
+}
diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart
@@ -18,7 +18,8 @@ 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';
+export 'src/binding_wrapper.dart'
+    show BindingWrapper, SentryWidgetsFlutterBinding;
 export 'src/sentry_widget.dart';
 export 'src/navigation/sentry_display_widget.dart';
 export 'src/feedback/sentry_feedback_widget.dart';
diff --git a/flutter/lib/src/binding_wrapper.dart b/flutter/lib/src/binding_wrapper.dart
@@ -1,5 +1,7 @@
 // ignore_for_file: invalid_use_of_internal_member
 
+import 'package:flutter/foundation.dart';
+
 import '../sentry_flutter.dart';
 import 'package:flutter/widgets.dart';
 import 'package:meta/meta.dart';
@@ -39,7 +41,87 @@ class BindingWrapper {
   /// You only need to call this method if you need the binding to be
   /// initialized before calling [runApp].
   WidgetsBinding ensureInitialized() =>
-      WidgetsFlutterBinding.ensureInitialized();
+      SentryWidgetsFlutterBinding.ensureInitialized();
 }
 
 WidgetsBinding? _ambiguate(WidgetsBinding? binding) => binding;
+
+class SentryWidgetsFlutterBinding extends WidgetsFlutterBinding
+    with SentryWidgetsBindingMixin {
+  @override
+  void initInstances() {
+    super.initInstances();
+    _instance = this;
+  }
+
+  static SentryWidgetsFlutterBinding get instance =>
+      BindingBase.checkInstance(_instance);
+  static SentryWidgetsFlutterBinding? _instance;
+
+  /// Returns an instance of [SentryWidgetsFlutterBinding].
+  /// If no binding has yet been initialized, creates and initializes one.
+  ///
+  /// If the binding was already initialized with a different implementation,
+  /// returns the existing [WidgetsBinding] instance instead.
+  static WidgetsBinding ensureInitialized() {
+    try {
+      if (SentryWidgetsFlutterBinding._instance == null) {
+        SentryWidgetsFlutterBinding();
+      }
+      return SentryWidgetsFlutterBinding.instance;
+    } catch (e) {
+      Sentry.currentHub.options.logger(
+          SentryLevel.info,
+          'WidgetsFlutterBinding already initialized. '
+          'Falling back to default WidgetsBinding instance.');
+      return WidgetsBinding.instance;
+    }
+  }
+}
+
+@internal
+typedef FrameTimingCallback = void Function(
+    DateTime startTimestamp, DateTime endTimestamp);
+
+mixin SentryWidgetsBindingMixin on WidgetsBinding {
+  DateTime? _startTimestamp;
+  FrameTimingCallback? _frameTimingCallback;
+  ClockProvider? _clock;
+
+  @internal
+  void registerFramesTracking(
+      FrameTimingCallback callback, ClockProvider clock) {
+    _frameTimingCallback ??= callback;
+    _clock ??= clock;
+  }
+
+  @visibleForTesting
+  bool isFramesTrackingInitialized() {
+    return _frameTimingCallback != null && _clock != null;
+  }
+
+  @internal
+  void removeFramesTracking() {
+    _frameTimingCallback = null;
+    _clock = null;
+  }
+
+  @override
+  void handleBeginFrame(Duration? rawTimeStamp) {
+    _startTimestamp = _clock?.call();
+
+    super.handleBeginFrame(rawTimeStamp);
+  }
+
+  @override
+  void handleDrawFrame() {
+    super.handleDrawFrame();
+
+    final endTimestamp = _clock?.call();
+    if (_startTimestamp != null &&
+        endTimestamp != null &&
+        _startTimestamp!.isBefore(endTimestamp)) {
+      _frameTimingCallback?.call(_startTimestamp!, endTimestamp);
+    }
+  }
+}