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

feat: ttid/ttfd #1882

Closed
wants to merge 49 commits into from
Closed
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
83626bd
test
buenaflor Jan 31, 2024
1dbc007
draft impl for ttid
buenaflor Feb 2, 2024
f2668bb
poc
buenaflor Feb 6, 2024
6a707da
use epoch
buenaflor Feb 6, 2024
51d6c6d
remove duration
buenaflor Feb 6, 2024
4c976b3
update
buenaflor Feb 8, 2024
75a76fc
update poc
buenaflor Feb 9, 2024
7f2ab85
update
buenaflor Feb 9, 2024
16c5ca0
update
buenaflor Feb 12, 2024
69e4d3e
update
buenaflor Feb 13, 2024
ab8bddc
working ttid and ttfd for non root app start navigations
buenaflor Feb 13, 2024
7490a83
update
buenaflor Feb 13, 2024
c8f553f
refactor to navigator observer
buenaflor Feb 13, 2024
f0420ad
update
buenaflor Feb 14, 2024
4da60a1
update
buenaflor Feb 14, 2024
5db69e7
fix timestamps
buenaflor Feb 14, 2024
ab6abbf
update
buenaflor Feb 14, 2024
2bfa150
refactor
buenaflor Feb 14, 2024
0f1b10b
add tests
buenaflor Feb 14, 2024
bee35cf
update
buenaflor Feb 15, 2024
18a3fdf
improve
buenaflor Feb 16, 2024
07e67a3
update
buenaflor Feb 19, 2024
a3903f0
update
buenaflor Feb 19, 2024
65626df
update
buenaflor Feb 19, 2024
2928099
Fix tests
buenaflor Feb 20, 2024
0c33885
Update origin and operation
buenaflor Feb 20, 2024
00e7684
Improve code
buenaflor Feb 21, 2024
b3da58a
Improve code
buenaflor Feb 21, 2024
f888f83
Update
buenaflor Feb 21, 2024
1478af3
Refactor ttfd
buenaflor Feb 21, 2024
b01c5d1
Refactor and fix tests
buenaflor Feb 21, 2024
96b9f83
Add tests
buenaflor Feb 22, 2024
f535386
add ttid tracker tests
buenaflor Feb 22, 2024
7a748b3
Update tests
buenaflor Feb 23, 2024
c17025b
Merge branch 'main' into feat/ttid-ttfd
buenaflor Feb 23, 2024
852684d
fix
buenaflor Feb 23, 2024
ae62328
remove comments app start tracker
buenaflor Feb 23, 2024
d912922
fix tests
buenaflor Feb 28, 2024
8f9c5da
add comments
buenaflor Feb 28, 2024
67f622a
adjust test
buenaflor Feb 28, 2024
011533d
try other test
buenaflor Feb 28, 2024
9e7c1f0
format and fix test
buenaflor Feb 28, 2024
1b25138
Merge branch 'main' into feat/ttid-ttfd
buenaflor Feb 28, 2024
82b86fb
add comment
buenaflor Feb 28, 2024
aa42e95
exit early in app start event processor
buenaflor Feb 29, 2024
25291cb
Apply review
buenaflor Feb 29, 2024
32d3bc0
Update review
buenaflor Feb 29, 2024
34033f9
remove debug label from framecallbackhandler
buenaflor Mar 1, 2024
32faabf
pr improvements
buenaflor Mar 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export 'src/utils/http_header_utils.dart';
// ignore: invalid_export_of_internal_element
export 'src/sentry_trace_origins.dart';
// ignore: invalid_export_of_internal_element
export 'src/sentry_span_operations.dart';
// ignore: invalid_export_of_internal_element
export 'src/utils.dart';
// spotlight debugging
export 'src/spotlight.dart';
8 changes: 8 additions & 0 deletions dart/lib/src/sentry_span_operations.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import 'package:meta/meta.dart';

@internal
class SentrySpanOperations {
static const String uiLoad = 'ui.load';
static const String uiTimeToInitialDisplay = 'ui.load.initial_display';
static const String uiTimeToFullDisplay = 'ui.load.full_display';
}
2 changes: 2 additions & 0 deletions dart/lib/src/sentry_trace_origins.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ class SentryTraceOrigins {
static const autoDbDriftQueryExecutor = 'auto.db.drift.query.executor';
static const autoDbDriftTransactionExecutor =
'auto.db.drift.transaction.executor';
static const autoUiTimeToDisplay = 'auto.ui.time_to_display';
static const manualUiTimeToDisplay = 'manual.ui.time_to_display';
}
27 changes: 17 additions & 10 deletions dart/lib/src/sentry_tracer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,24 @@ class SentryTracer extends ISentrySpan {
}

var _rootEndTimestamp = commonEndTimestamp;

// Trim the end timestamp of the transaction to the very last timestamp of child spans
if (_trimEnd && children.isNotEmpty) {
final childEndTimestamps = children
.where((child) => child.endTimestamp != null)
.map((child) => child.endTimestamp!);

if (childEndTimestamps.isNotEmpty) {
final oldestChildEndTimestamp =
childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b);
if (_rootEndTimestamp.isAfter(oldestChildEndTimestamp)) {
_rootEndTimestamp = oldestChildEndTimestamp;
DateTime? latestEndTime;

for (var child in children) {
final childEndTimestamp = child.endTimestamp;
if (childEndTimestamp != null) {
if (latestEndTime == null ||
childEndTimestamp.isAfter(latestEndTime)) {
latestEndTime = child.endTimestamp;
}
}
}

if (latestEndTime != null) {
_rootEndTimestamp = latestEndTime;
}
}

// the callback should run before because if the span is finished,
Expand Down Expand Up @@ -362,7 +368,8 @@ class SentryTracer extends ISentrySpan {
Dsn.parse(_hub.options.dsn!).publicKey,
release: _hub.options.release,
environment: _hub.options.environment,
userId: null, // because of PII not sending it for now
userId: null,
// because of PII not sending it for now
userSegment: user?.segment,
transaction:
_isHighQualityTransactionName(transactionNameSource) ? name : null,
Expand Down
6 changes: 4 additions & 2 deletions flutter/example/lib/auto_close_screen.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:flutter/material.dart';
import 'package:sentry/sentry.dart';
import 'package:sentry_flutter/sentry_flutter.dart';

/// This screen is only used to demonstrate how route navigation works.
/// Init will create a child span and pop the screen after 3 seconds.
Expand All @@ -25,9 +26,10 @@ class AutoCloseScreenState extends State<AutoCloseScreen> {
final childSpan = activeSpan?.startChild('complex operation',
description: 'running a $delayInSeconds seconds operation');
await Future.delayed(const Duration(seconds: delayInSeconds));
childSpan?.finish();
await childSpan?.finish();
SentryFlutter.reportFullyDisplayed();
// ignore: use_build_context_synchronously
Navigator.of(context).pop();
// Navigator.of(context).pop();
}

@override
Expand Down
29 changes: 24 additions & 5 deletions flutter/example/lib/main.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:sentry_flutter/sentry_flutter.dart';
import 'package:sentry_isar/sentry_isar.dart';
import 'package:sentry_sqflite/sentry_sqflite.dart';
import 'package:sqflite/sqflite.dart';

// import 'package:sqflite_common_ffi/sqflite_ffi.dart';
// import 'package:sqflite_common_ffi_web/sqflite_ffi_web.dart';
import 'package:universal_platform/universal_platform.dart';
Expand Down Expand Up @@ -71,10 +72,13 @@ Future<void> setupSentry(AppRunner appRunner, String dsn,
options.attachScreenshot = true;
options.screenshotQuality = SentryScreenshotQuality.low;
options.attachViewHierarchy = true;
options.enableTimeToFullDisplayTracing = false;
options.spotlight = Spotlight(enabled: true);
// We can enable Sentry debug logging during development. This is likely
// going to log too much for your app, but can be useful when figuring out
// configuration issues, e.g. finding out why your events are not uploaded.
options.debug = true;
// options.enableTimeToFullDisplayTracing = true;

options.maxRequestBodySize = MaxRequestBodySize.always;
options.maxResponseBodySize = MaxResponseBodySize.always;
Expand All @@ -98,6 +102,16 @@ class MyApp extends StatefulWidget {
}

class _MyAppState extends State<MyApp> {
@override
void initState() {
super.initState();
// Example of reporting TTFD
Future.delayed(
const Duration(seconds: 1),
() => SentryFlutter.reportFullyDisplayed(),
);
}

@override
Widget build(BuildContext context) {
return feedback.BetterFeedback(
Expand Down Expand Up @@ -723,9 +737,10 @@ void navigateToAutoCloseScreen(BuildContext context) {
Navigator.push(
context,
MaterialPageRoute(
settings: const RouteSettings(name: 'AutoCloseScreen'),
builder: (context) => const AutoCloseScreen(),
),
settings: const RouteSettings(name: 'AutoCloseScreen'),
builder: (context) => SentryDisplayWidget(
child: const AutoCloseScreen(),
)),
);
}

Expand Down Expand Up @@ -842,7 +857,11 @@ int loop(int val) {
}

class SecondaryScaffold extends StatelessWidget {
const SecondaryScaffold({Key? key}) : super(key: key);
SecondaryScaffold({Key? key}) : super(key: key) {
Timer(const Duration(milliseconds: 500), () {
SentryFlutter.reportFullyDisplayed();
});
}

static Future<void> openSecondaryScaffold(BuildContext context) {
return Navigator.push(
Expand All @@ -851,7 +870,7 @@ class SecondaryScaffold extends StatelessWidget {
settings:
const RouteSettings(name: 'SecondaryScaffold', arguments: 'foobar'),
builder: (context) {
return const SecondaryScaffold();
return SecondaryScaffold();
},
),
);
Expand Down
1 change: 1 addition & 0 deletions flutter/lib/sentry_flutter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ export 'src/screenshot/sentry_screenshot_quality.dart';
export 'src/user_interaction/sentry_user_interaction_widget.dart';
export 'src/binding_wrapper.dart';
export 'src/sentry_widget.dart';
export 'src/navigation/sentry_display_widget.dart';
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import 'dart:async';

import 'package:sentry/sentry.dart';

import '../integrations/app_start/app_start_tracker.dart';
import '../integrations/integrations.dart';
import '../native/sentry_native.dart';

/// EventProcessor that enriches [SentryTransaction] objects with app start
Expand All @@ -10,36 +12,24 @@ class NativeAppStartEventProcessor implements EventProcessor {
/// We filter out App starts more than 60s
static const _maxAppStartMillis = 60000;

NativeAppStartEventProcessor(
this._native,
);
final AppStartTracker? _appStartTracker;

final SentryNative _native;
NativeAppStartEventProcessor({
AppStartTracker? appStartTracker,
}) : _appStartTracker = appStartTracker ?? AppStartTracker();

bool didAddAppStartMeasurement = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m: This seems to be a duplicate state of SentryNative.didFetchAppStart. It would be better to only store the state once if we already got the app start or added it as a measurement. Why did we add this flag?

Copy link
Contributor Author

@buenaflor buenaflor Feb 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because SentryNative.didFetchAppStart doesn't tell us 'we've already added this measurement to the transaction`

since the event processor is always run for every event checking if (didFetchAppStart == true) and then adding the measurement would mean we are always adding measurement because after the app start it's always true.

buenaflor marked this conversation as resolved.
Show resolved Hide resolved

@override
Future<SentryEvent?> apply(SentryEvent event, {Hint? hint}) async {
final appStartEnd = _native.appStartEnd;

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 _appStartTracker?.getAppStartInfo();
final measurement = appStartInfo?.measurement;
if (!didAddAppStartMeasurement &&
measurement != null &&
measurement.value.toInt() <= _maxAppStartMillis &&
event is SentryTransaction) {
buenaflor marked this conversation as resolved.
Show resolved Hide resolved
event.measurements[measurement.name] = measurement;
didAddAppStartMeasurement = true;
buenaflor marked this conversation as resolved.
Show resolved Hide resolved
}
return event;
}
Expand Down
14 changes: 14 additions & 0 deletions flutter/lib/src/frame_callback_handler.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';

abstract class FrameCallbackHandler {
void addPostFrameCallback(FrameCallback callback, {String debugLabel});
buenaflor marked this conversation as resolved.
Show resolved Hide resolved
}

class DefaultFrameCallbackHandler implements FrameCallbackHandler {
@override
void addPostFrameCallback(FrameCallback callback,
{String debugLabel = 'callback'}) {
WidgetsBinding.instance.addPostFrameCallback(callback);
}
}
41 changes: 41 additions & 0 deletions flutter/lib/src/integrations/app_start/app_start_tracker.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import 'dart:async';

import 'package:meta/meta.dart';

import '../../../sentry_flutter.dart';

@internal
class AppStartInfo {
final DateTime start;
final DateTime end;
final SentryMeasurement measurement;
buenaflor marked this conversation as resolved.
Show resolved Hide resolved

AppStartInfo(this.start, this.end, this.measurement);
}

@internal
class AppStartTracker {
static final AppStartTracker _instance = AppStartTracker._internal();
factory AppStartTracker() => _instance;
AppStartTracker._internal();
buenaflor marked this conversation as resolved.
Show resolved Hide resolved

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

void setAppStartInfo(AppStartInfo? appStartInfo) {
_appStartInfo = appStartInfo;
if (!_appStartCompleter.isCompleted) {
_appStartCompleter.complete(appStartInfo);
} else {
_appStartCompleter = Completer<AppStartInfo?>();
_appStartCompleter.complete(appStartInfo);
}
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
}

Future<AppStartInfo?> getAppStartInfo() {
if (_appStartInfo != null) {
return Future.value(_appStartInfo);
}
return _appStartCompleter.future;
}
}
48 changes: 44 additions & 4 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
import 'package:flutter/scheduler.dart';
import 'package:meta/meta.dart';
import 'package:sentry/sentry.dart';

import '../../sentry_flutter.dart';
import '../sentry_flutter_options.dart';
import '../native/sentry_native.dart';
import '../event_processor/native_app_start_event_processor.dart';
import 'app_start/app_start_tracker.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._schedulerBindingProvider,
{AppStartTracker? appStartTracker})
: _appStartTracker = appStartTracker ?? AppStartTracker();

final SentryNative _native;
final SchedulerBindingProvider _schedulerBindingProvider;
final AppStartTracker? _appStartTracker;

@override
void call(Hub hub, SentryFlutterOptions options) {
Expand All @@ -21,14 +27,48 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
options.logger(SentryLevel.debug,
'Scheduler binding is null. Can\'t auto detect app start time.');
} else {
schedulerBinding.addPostFrameCallback((timeStamp) {
schedulerBinding.addPostFrameCallback((timeStamp) async {
// ignore: invalid_use_of_internal_member
_native.appStartEnd = options.clock();
final appStartEnd = options.clock();
_native.appStartEnd = appStartEnd;

if (_native.didFetchAppStart) {
_appStartTracker?.setAppStartInfo(null);
return;
}

final nativeAppStart = await _native.fetchNativeAppStart();
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 (nativeAppStart == null ||
measurement == null ||
measurement.value >= 60000) {
buenaflor marked this conversation as resolved.
Show resolved Hide resolved
_appStartTracker?.setAppStartInfo(null);
return;
}

final appStartInfo = AppStartInfo(
DateTime.fromMillisecondsSinceEpoch(
nativeAppStart.appStartTime.toInt()),
appStartEnd,
measurement,
);

_appStartTracker?.setAppStartInfo(appStartInfo);
});
}
}

options.addEventProcessor(NativeAppStartEventProcessor(_native));
options.addEventProcessor(
NativeAppStartEventProcessor(appStartTracker: _appStartTracker));

options.sdk.addIntegration('nativeAppStartIntegration');
}
Expand Down
Loading
Loading