Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: fetch app start in integration instead of event processor #1905

Merged
merged 14 commits into from
Mar 4, 2024
2 changes: 1 addition & 1 deletion .github/workflows/flutter_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ jobs:

cocoa:
name: "${{ matrix.target }} | ${{ matrix.sdk }}"
runs-on: macos-13
runs-on: macos-latest-xlarge
timeout-minutes: 30
defaults:
run:
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Use `recordHttpBreadcrumbs` to set iOS `enableNetworkBreadcrumbs` ([#1884](https://github.com/getsentry/sentry-dart/pull/1884))

### 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
Expand Down
4 changes: 4 additions & 0 deletions flutter/example/integration_test/integration_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// ignore_for_file: avoid_print
// ignore_for_file: invalid_use_of_internal_member

import 'dart:async';
import 'dart:convert';
Expand All @@ -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';
Expand All @@ -24,6 +26,8 @@ void main() {
// Using fake DSN for testing purposes.
Future<void> setupSentryAndApp(WidgetTester tester,
{String? dsn, BeforeSendCallback? beforeSendCallback}) async {
NativeAppStartIntegration.isIntegrationTest = true;

await setupSentry(
() async {
await tester.pumpWidget(SentryScreenshotWidget(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,57 +2,29 @@ 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 {
/// We filter out App starts more than 60s
static const _maxAppStartMillis = 60000;

NativeAppStartEventProcessor(
this._native,
);

final SentryNative _native;

NativeAppStartEventProcessor(this._native);

@override
Future<SentryEvent?> apply(SentryEvent event, {Hint? hint}) async {
final appStartEnd = _native.appStartEnd;
if (_native.didAddAppStartMeasurement || event is! SentryTransaction) {
return event;
}

final appStartInfo = await NativeAppStartIntegration.getAppStartInfo();
final measurement = appStartInfo?.toMeasurement();

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;
}
if (measurement != null) {
event.measurements[measurement.name] = measurement;
_native.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);
}
}
15 changes: 15 additions & 0 deletions flutter/lib/src/frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import 'package:flutter/scheduler.dart';

abstract class FrameCallbackHandler {
void addPostFrameCallback(FrameCallback callback);
}

class DefaultFrameCallbackHandler implements FrameCallbackHandler {
@override
void addPostFrameCallback(FrameCallback callback) {
try {
/// Flutter >= 2.12 throws if SchedulerBinding.instance isn't initialized.
SchedulerBinding.instance.addPostFrameCallback(callback);
} catch (_) {}
}
}
120 changes: 103 additions & 17 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
@@ -1,31 +1,103 @@
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';

/// Integration which handles communication with native frameworks in order to
/// enrich [SentryTransaction] objects with app start data for mobile vitals.
class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
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;

static Completer<AppStartInfo?> _appStartCompleter =
Completer<AppStartInfo?>();
static AppStartInfo? _appStartInfo;

@internal
static bool isIntegrationTest = false;

@internal
static void setAppStartInfo(AppStartInfo? appStartInfo) {
_appStartInfo = appStartInfo;
if (_appStartCompleter.isCompleted) {
_appStartCompleter = Completer<AppStartInfo?>();
}
_appStartCompleter.complete(appStartInfo);
}

@internal
static Future<AppStartInfo?> getAppStartInfo() {
if (_appStartInfo != null) {
return Future.value(_appStartInfo);
}
return _appStartCompleter.future;
}

@visibleForTesting
static void clearAppStartInfo() {
_appStartInfo = null;
_appStartCompleter = Completer<AppStartInfo?>();
}

@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) {
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) {
// ignore: invalid_use_of_internal_member
_native.appStartEnd = options.clock();
});
}
_frameCallbackHandler.addPostFrameCallback((timeStamp) async {
if (_native.didFetchAppStart) {
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));
Expand All @@ -34,5 +106,19 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
}
}

/// Used to provide scheduler binding at call time.
typedef SchedulerBindingProvider = SchedulerBinding? 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);
}
}
3 changes: 3 additions & 0 deletions flutter/lib/src/native/sentry_native.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class SentryNative {
/// Flag indicating if app start was already fetched.
bool get didFetchAppStart => _didFetchAppStart;

/// Flag indicating if app start measurement was added to the first transaction.
bool didAddAppStartMeasurement = false;

/// Fetch [NativeAppStart] from native channels. Can only be called once.
Future<NativeAppStart?> fetchNativeAppStart() async {
_didFetchAppStart = true;
Expand Down
11 changes: 3 additions & 8 deletions flutter/lib/src/sentry_flutter.dart
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
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';
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';
Expand Down Expand Up @@ -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 SchedulerBinding.instance;
} catch (_) {}
return null;
},
DefaultFrameCallbackHandler(),
));
}
return integrations;
Expand Down Expand Up @@ -231,6 +225,7 @@ mixin SentryFlutter {

@internal
static SentryNative? get native => _native;

@internal
static set native(SentryNative? value) => _native = value;
static SentryNative? _native;
Expand Down
19 changes: 19 additions & 0 deletions flutter/test/fake_frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
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 {
// ignore: inference_failure_on_instance_creation
await Future.delayed(_finishAfterDuration);
callback(Duration.zero);
}
}
Loading
Loading