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
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 '../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<SentryEvent?> 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);
}
}
12 changes: 12 additions & 0 deletions flutter/lib/src/frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -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);
buenaflor marked this conversation as resolved.
Show resolved Hide resolved
}
}
102 changes: 95 additions & 7 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
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';

Expand All @@ -13,6 +15,36 @@
final SentryNative _native;
final SchedulerBindingProvider _schedulerBindingProvider;
buenaflor marked this conversation as resolved.
Show resolved Hide resolved

/// We filter out App starts more than 60s
static const _maxAppStartMillis = 60000;

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

@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 (options.autoAppStart) {
Expand All @@ -21,18 +53,74 @@
options.logger(SentryLevel.debug,
'Scheduler binding is null. Can\'t auto detect app start time.');
} else {
schedulerBinding.addPostFrameCallback((timeStamp) {
schedulerBinding.addPostFrameCallback((timeStamp) async {
if (_native.didFetchAppStart) {
setAppStartInfo(null);
return;
}

// 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();

Check warning on line 64 in flutter/lib/src/integrations/native_app_start_integration.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The member 'clock' can only be used within its package.

See https://dart.dev/diagnostics/invalid_use_of_internal_member to learn more about this problem.
final appStartEnd = _native.appStartEnd;
final nativeAppStart = await _native.fetchNativeAppStart();

if (nativeAppStart == null || appStartEnd == null) {
setAppStartInfo(null);
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
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();

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);
}
}
4 changes: 2 additions & 2 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 @@ -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;
},
Expand Down
18 changes: 18 additions & 0 deletions flutter/test/fake_frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -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);

Check warning on line 15 in flutter/test/fake_frame_callback_handler.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The type argument(s) of the constructor 'Future.delayed' can't be inferred.

Use explicit type argument(s) for 'Future.delayed'.
callback(Duration.zero);
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
@TestOn('vm')

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/native_app_start_integration.dart';
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';

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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));
Expand All @@ -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);

Expand Down Expand Up @@ -111,7 +108,8 @@ class Fixture {
return NativeAppStartIntegration(
native,
() {
return TestWidgetsFlutterBinding.ensureInitialized();
TestWidgetsFlutterBinding.ensureInitialized();
return FakeFrameCallbackHandler();
},
);
}
Expand Down
Loading