Skip to content

Commit

Permalink
fix: event processor blocking transactions from being sent if `autoAp…
Browse files Browse the repository at this point in the history
…pStart` is false (#2028)

* Add fix

* Update CHANGELOG

* Fix tests

* Update native_app_start_integration.dart

* Implement code review

* Apply auto ttid only when autoAppStart is true

* Revert

* Revert
  • Loading branch information
buenaflor authored May 7, 2024
1 parent 9fe67d5 commit e0f6628
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 44 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixes

- Event processor blocking transactions from being sent if `autoAppStart` is false ([#2028](https://github.com/getsentry/sentry-dart/pull/2028))

### Features

- Adds app start spans to first transaction ([#2009](https://github.com/getsentry/sentry-dart/pull/2009))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,25 @@ class NativeAppStartEventProcessor implements EventProcessor {

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

final appStartInfo = await NativeAppStartIntegration.getAppStartInfo();

final appStartEnd = _native.appStartEnd;
if (!options.autoAppStart) {
if (appStartEnd != null) {
appStartInfo?.end = appStartEnd;
} else {
// If autoAppStart is disabled and appStartEnd is not set, we can't add app starts
return event;
}
}

final measurement = appStartInfo?.toMeasurement();

if (measurement != null) {
Expand All @@ -44,6 +58,10 @@ class NativeAppStartEventProcessor implements EventProcessor {
Future<void> _attachAppStartSpans(
AppStartInfo appStartInfo, SentryTracer transaction) async {
final transactionTraceId = transaction.context.traceId;
final appStartEnd = appStartInfo.end;
if (appStartEnd == null) {
return;
}

final appStartSpan = await _createAndFinishSpan(
tracer: transaction,
Expand All @@ -52,7 +70,7 @@ class NativeAppStartEventProcessor implements EventProcessor {
parentSpanId: transaction.context.spanId,
traceId: transactionTraceId,
startTimestamp: appStartInfo.start,
endTimestamp: appStartInfo.end);
endTimestamp: appStartEnd);

final pluginRegistrationSpan = await _createAndFinishSpan(
tracer: transaction,
Expand All @@ -79,7 +97,7 @@ class NativeAppStartEventProcessor implements EventProcessor {
parentSpanId: appStartSpan.context.spanId,
traceId: transactionTraceId,
startTimestamp: appStartInfo.mainIsolateStart,
endTimestamp: appStartInfo.end);
endTimestamp: appStartEnd);

transaction.children.addAll([
appStartSpan,
Expand Down
94 changes: 54 additions & 40 deletions flutter/lib/src/integrations/native_app_start_integration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
final SentryNative _native;
final FrameCallbackHandler _frameCallbackHandler;

/// Timeout duration to wait for the app start info to be fetched.
static const _timeoutDuration = Duration(seconds: 30);

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

Expand All @@ -39,7 +42,8 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
if (_appStartInfo != null) {
return Future.value(_appStartInfo);
}
return _appStartCompleter.future;
return _appStartCompleter.future
.timeout(_timeoutDuration, onTimeout: () => null);
}

@visibleForTesting
Expand All @@ -62,31 +66,31 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
return;
}

if (options.autoAppStart) {
_frameCallbackHandler.addPostFrameCallback((timeStamp) async {
if (_native.didFetchAppStart) {
return;
}
if (_native.didFetchAppStart) {
return;
}

_frameCallbackHandler.addPostFrameCallback((timeStamp) async {
final nativeAppStart = await _native.fetchNativeAppStart();
if (nativeAppStart == null) {
setAppStartInfo(null);
return;
}

final mainIsolateStartDateTime = SentryFlutter.mainIsolateStartTime;
final appStartDateTime = DateTime.fromMillisecondsSinceEpoch(
nativeAppStart.appStartTime.toInt());
final pluginRegistrationDateTime = DateTime.fromMillisecondsSinceEpoch(
nativeAppStart.pluginRegistrationTime);
DateTime? appStartEndDateTime;

if (options.autoAppStart) {
// 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 appStartEndDateTime = _native.appStartEnd;
final nativeAppStart = await _native.fetchNativeAppStart();
final pluginRegistrationTime = nativeAppStart?.pluginRegistrationTime;
final mainIsolateStartDateTime = SentryFlutter.mainIsolateStartTime;

if (nativeAppStart == null ||
appStartEndDateTime == null ||
pluginRegistrationTime == null) {
return;
}
appStartEndDateTime = _native.appStartEnd;

final appStartDateTime = DateTime.fromMillisecondsSinceEpoch(
nativeAppStart.appStartTime.toInt());
final duration = appStartEndDateTime.difference(appStartDateTime);
final pluginRegistrationDateTime =
DateTime.fromMillisecondsSinceEpoch(pluginRegistrationTime);
final duration = appStartEndDateTime?.difference(appStartDateTime);

// We filter out app start more than 60s.
// This could be due to many different reasons.
Expand All @@ -96,23 +100,23 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
// 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) {
if (duration != null && duration.inMilliseconds > _maxAppStartMillis) {
setAppStartInfo(null);
return;
}
}

final appStartInfo = AppStartInfo(
nativeAppStart.isColdStart ? AppStartType.cold : AppStartType.warm,
start: appStartDateTime,
end: appStartEndDateTime,
pluginRegistration: pluginRegistrationDateTime,
mainIsolateStart: mainIsolateStartDateTime);
final appStartInfo = AppStartInfo(
nativeAppStart.isColdStart ? AppStartType.cold : AppStartType.warm,
start: appStartDateTime,
end: appStartEndDateTime,
pluginRegistration: pluginRegistrationDateTime,
mainIsolateStart: mainIsolateStartDateTime);

setAppStartInfo(appStartInfo);
});
}
setAppStartInfo(appStartInfo);
});

options.addEventProcessor(NativeAppStartEventProcessor(_native));
options.addEventProcessor(NativeAppStartEventProcessor(_native, hub: hub));

options.sdk.addIntegration('nativeAppStartIntegration');
}
Expand All @@ -121,21 +125,31 @@ class NativeAppStartIntegration extends Integration<SentryFlutterOptions> {
enum AppStartType { cold, warm }

class AppStartInfo {
AppStartInfo(this.type,
{required this.start,
required this.end,
required this.pluginRegistration,
required this.mainIsolateStart});
AppStartInfo(
this.type, {
required this.start,
required this.pluginRegistration,
required this.mainIsolateStart,
this.end,
});

final AppStartType type;
final DateTime start;
final DateTime end;

// We allow the end to be null, since it might be set at a later time
// with setAppStartEnd when autoAppStart is disabled
DateTime? end;

final DateTime pluginRegistration;
final DateTime mainIsolateStart;

Duration get duration => end.difference(start);
Duration? get duration => end?.difference(start);

SentryMeasurement toMeasurement() {
SentryMeasurement? toMeasurement() {
final duration = this.duration;
if (duration == null) {
return null;
}
return type == AppStartType.cold
? SentryMeasurement.coldAppStart(duration)
: SentryMeasurement.warmAppStart(duration);
Expand Down
64 changes: 64 additions & 0 deletions flutter/test/integrations/native_app_start_integration_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,70 @@ void main() {
expect(appStartInfo?.start, DateTime.fromMillisecondsSinceEpoch(0));
expect(appStartInfo?.end, DateTime.fromMillisecondsSinceEpoch(10));
});

test(
'autoAppStart is false and appStartEnd is not set does not add app start measurement',
() async {
fixture.options.autoAppStart = false;
fixture.binding.nativeAppStart = NativeAppStart(
appStartTime: 0, pluginRegistrationTime: 10, isColdStart: true);

fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer);

final processor = fixture.options.eventProcessors.first;
final enriched =
await processor.apply(transaction, Hint()) as SentryTransaction;

expect(enriched.measurements.isEmpty, true);
expect(enriched.spans.isEmpty, true);
});

test(
'autoAppStart is false and appStartEnd is set adds app start measurement',
() async {
fixture.options.autoAppStart = false;
fixture.binding.nativeAppStart = NativeAppStart(
appStartTime: 0, pluginRegistrationTime: 10, isColdStart: true);
SentryFlutter.native = fixture.native;

fixture.getNativeAppStartIntegration().call(fixture.hub, fixture.options);

SentryFlutter.setAppStartEnd(DateTime.fromMillisecondsSinceEpoch(10));

final tracer = fixture.createTracer();
final transaction = SentryTransaction(tracer);

final processor = fixture.options.eventProcessors.first;
final enriched =
await processor.apply(transaction, Hint()) as SentryTransaction;

final measurement = enriched.measurements['app_start_cold']!;
expect(measurement.value, 10);
expect(measurement.unit, DurationSentryMeasurementUnit.milliSecond);

final appStartInfo = await NativeAppStartIntegration.getAppStartInfo();

final appStartSpan = enriched.spans.firstWhereOrNull((element) =>
element.context.description == appStartInfo!.appStartTypeDescription);
final pluginRegistrationSpan = enriched.spans.firstWhereOrNull(
(element) =>
element.context.description ==
appStartInfo!.pluginRegistrationDescription);
final mainIsolateSetupSpan = enriched.spans.firstWhereOrNull((element) =>
element.context.description ==
appStartInfo!.mainIsolateSetupDescription);
final firstFrameRenderSpan = enriched.spans.firstWhereOrNull((element) =>
element.context.description ==
appStartInfo!.firstFrameRenderDescription);

expect(appStartSpan, isNotNull);
expect(pluginRegistrationSpan, isNotNull);
expect(mainIsolateSetupSpan, isNotNull);
expect(firstFrameRenderSpan, isNotNull);
});
});

group('App start spans', () {
Expand Down
2 changes: 1 addition & 1 deletion flutter/test/navigation/sentry_display_widget_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void main() {
expect(tracer.measurements, hasLength(1));
final measurement = tracer.measurements['time_to_initial_display'];
expect(measurement, isNotNull);
expect(measurement?.value, appStartInfo.duration.inMilliseconds);
expect(measurement?.value, appStartInfo.duration?.inMilliseconds);
expect(measurement?.value, ttidSpanDuration.inMilliseconds);
expect(measurement?.unit, DurationSentryMeasurementUnit.milliSecond);
});
Expand Down

0 comments on commit e0f6628

Please sign in to comment.