Skip to content

Commit

Permalink
feat: use addTimingsCallback instead of addPostFrameCallback to d…
Browse files Browse the repository at this point in the history
…etermine app start end (#2405)

* use addTimingsCallback

* update test

* update

* update naming

* update naming

* update changelog

* fix test

* check if using custom timingscallback works
  • Loading branch information
buenaflor authored Nov 20, 2024
1 parent 404f5a9 commit 07cd9e8
Show file tree
Hide file tree
Showing 11 changed files with 199 additions and 137 deletions.
22 changes: 12 additions & 10 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
# Changelog

## Unreleased

### Enhancements

- Only send debug images referenced in the stacktrace for events ([#2329](https://github.com/getsentry/sentry-dart/pull/2329))
- Remove `sentry` frames if SDK falls back to current stack trace ([#2351](https://github.com/getsentry/sentry-dart/pull/2351))
- Flutter doesn't always provide stack traces for unhandled errors - this is normal Flutter behavior
- When no stack trace is provided (in Flutter errors, `captureException`, or `captureMessage`):
- SDK creates a synthetic trace using `StackTrace.current`
- Internal SDK frames are removed to reduce noise
- Original stack traces (when provided) are left unchanged

### Features

- Improve app start measurements by using `addTimingsCallback` instead of `addPostFrameCallback` to determine app start end ([#2405](https://github.com/getsentry/sentry-dart/pull/2405))
- ⚠️ This change may result in reporting of shorter app start durations
- 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`
Expand All @@ -29,6 +21,16 @@
```
- ⚠️ Frame tracking will be disabled if a different binding is used

### Enhancements

- Only send debug images referenced in the stacktrace for events ([#2329](https://github.com/getsentry/sentry-dart/pull/2329))
- Remove `sentry` frames if SDK falls back to current stack trace ([#2351](https://github.com/getsentry/sentry-dart/pull/2351))
- Flutter doesn't always provide stack traces for unhandled errors - this is normal Flutter behavior
- When no stack trace is provided (in Flutter errors, `captureException`, or `captureMessage`):
- SDK creates a synthetic trace using `StackTrace.current`
- Internal SDK frames are removed to reduce noise
- Original stack traces (when provided) are left unchanged

### Fixes

- Apply default IP address (`{{auto}}`) to transactions ([#2395](https://github.com/getsentry/sentry-dart/pull/2395))
Expand Down
19 changes: 9 additions & 10 deletions flutter/lib/src/frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import 'package:flutter/cupertino.dart';
import 'package:flutter/scheduler.dart';

/// Use instead of TimingsCallback as it is not available in the Flutter min version
typedef SentryTimingsCallback = void Function(List<FrameTiming> timings);

abstract class FrameCallbackHandler {
void addPostFrameCallback(FrameCallback callback);
void addPersistentFrameCallback(FrameCallback callback);
Future<void> get endOfFrame;
bool get hasScheduledFrame;
void removeTimingsCallback(SentryTimingsCallback callback);
void addTimingsCallback(SentryTimingsCallback callback);
}

class DefaultFrameCallbackHandler implements FrameCallbackHandler {
Expand All @@ -18,19 +20,16 @@ class DefaultFrameCallbackHandler implements FrameCallbackHandler {
}

@override
void addPersistentFrameCallback(FrameCallback callback) {
void addTimingsCallback(SentryTimingsCallback callback) {
try {
WidgetsBinding.instance.addPersistentFrameCallback(callback);
WidgetsBinding.instance.addTimingsCallback(callback);
} catch (_) {}
}

@override
Future<void> get endOfFrame async {
void removeTimingsCallback(SentryTimingsCallback callback) {
try {
await WidgetsBinding.instance.endOfFrame;
WidgetsBinding.instance.removeTimingsCallback(callback);
} catch (_) {}
}

@override
bool get hasScheduledFrame => WidgetsBinding.instance.hasScheduledFrame;
}
20 changes: 17 additions & 3 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:async';
import 'dart:ui';

import 'package:meta/meta.dart';

Expand Down Expand Up @@ -28,15 +29,24 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
}

final Completer<void> _appStartEndCompleter = Completer<void>();
bool _allowProcessing = true;

@override
void call(Hub hub, SentryFlutterOptions options) async {
_frameCallbackHandler.addPostFrameCallback((timeStamp) async {
void timingsCallback(List<FrameTiming> timings) async {
if (!_allowProcessing) {
return;
}
// Set immediately to prevent multiple executions
// we only care about the first frame
_allowProcessing = false;

try {
DateTime? appStartEnd;
if (options.autoAppStart) {
// ignore: invalid_use_of_internal_member
appStartEnd = options.clock();
appStartEnd = DateTime.fromMicrosecondsSinceEpoch(timings.first
.timestampInMicroseconds(FramePhase.rasterFinishWallTime));
} else if (_appStartEnd == null) {
await _appStartEndCompleter.future.timeout(
const Duration(seconds: 10),
Expand All @@ -62,8 +72,12 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
if (options.automatedTestMode) {
rethrow;
}
} finally {
_frameCallbackHandler.removeTimingsCallback(timingsCallback);
}
});
}

_frameCallbackHandler.addTimingsCallback(timingsCallback);
options.sdk.addIntegration('nativeAppStartIntegration');
}
}
14 changes: 6 additions & 8 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ mixin SentryFlutter {
}
integrations.add(LoadImageListIntegration(native));
integrations.add(FramesTrackingIntegration(native));
integrations.add(
NativeAppStartIntegration(
DefaultFrameCallbackHandler(),
NativeAppStartHandler(native),
),
);
options.enableDartSymbolication = false;
}

Expand All @@ -193,14 +199,6 @@ mixin SentryFlutter {
// in errors.
integrations.add(LoadReleaseIntegration());

if (native != null) {
integrations.add(
NativeAppStartIntegration(
DefaultFrameCallbackHandler(),
NativeAppStartHandler(native),
),
);
}
return integrations;
}

Expand Down
34 changes: 17 additions & 17 deletions flutter/test/fake_frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
import 'package:flutter/scheduler.dart';
import 'package:sentry_flutter/src/frame_callback_handler.dart';

import 'mocks.dart';

class FakeFrameCallbackHandler implements FrameCallbackHandler {
final Duration finishAfterDuration;
FakeFrameCallbackHandler({this.postFrameCallbackDelay});

FakeFrameCallbackHandler(
{this.finishAfterDuration = const Duration(milliseconds: 50)});
/// If set, it automatically executes the callback after the delay
Duration? postFrameCallbackDelay;
FrameCallback? postFrameCallback;
TimingsCallback? timingsCallback;

@override
void addPostFrameCallback(FrameCallback callback) async {
// ignore: inference_failure_on_instance_creation
await Future.delayed(finishAfterDuration);
callback(Duration.zero);
}
postFrameCallback = callback;

@override
Future<void> addPersistentFrameCallback(FrameCallback callback) async {
for (final duration in fakeFrameDurations) {
// Let's wait a bit so the timestamp intervals are large enough
await Future<void>.delayed(Duration(milliseconds: 20));
callback(duration);
if (postFrameCallbackDelay != null) {
await Future<void>.delayed(postFrameCallbackDelay!);
callback(Duration.zero);
}
}

@override
bool hasScheduledFrame = true;
void addTimingsCallback(TimingsCallback callback) {
timingsCallback = callback;
}

@override
Future<void> get endOfFrame => Future<void>.value();
void removeTimingsCallback(TimingsCallback callback) {
if (timingsCallback == callback) {
timingsCallback = null;
}
}
}
Loading

0 comments on commit 07cd9e8

Please sign in to comment.