Skip to content

Commit

Permalink
fix(ttfd): measurements should only be added for successful ttfd (#2348)
Browse files Browse the repository at this point in the history
* fix

* update

* update

* update
  • Loading branch information
buenaflor authored Oct 14, 2024
1 parent 8da6ae0 commit 4d763a5
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
- Start missing TTFD for root screen transaction ([#2332](https://github.com/getsentry/sentry-dart/pull/2332))
- Accessing invalid json fields from `fetchNativeAppStart` should return null ([#2340](https://github.com/getsentry/sentry-dart/pull/2340))
- Error when calling `SentryFlutter.reportFullyDisplayed()` twice ([#2339](https://github.com/getsentry/sentry-dart/pull/2339))
- TTFD measurements should only be added for successful TTFD spans ([#2348](https://github.com/getsentry/sentry-dart/pull/2348))

### Deprecate

Expand Down
33 changes: 24 additions & 9 deletions flutter/lib/src/navigation/time_to_full_display_tracker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class TimeToFullDisplayTracker {
final endTimestamp = timestamp ?? _endTimestampProvider();

if (ttfdSpan == null ||
ttfdSpan.finished == true ||
ttfdSpan.finished ||
startTimestamp == null ||
endTimestamp == null) {
options.logger(
Expand All @@ -83,14 +83,29 @@ class TimeToFullDisplayTracker {
return;
}

_setTTFDMeasurement(startTimestamp, endTimestamp);
await ttfdSpan.finish(
status:
timestamp != null ? SpanStatus.ok() : SpanStatus.deadlineExceeded(),
endTimestamp: endTimestamp,
);
_completedTTFDTracking.complete();
clear();
// If a timestamp is provided, the operation was successful; otherwise, it timed out
final status =
timestamp != null ? SpanStatus.ok() : SpanStatus.deadlineExceeded();
try {
// Should only add measurements if the span is successful
if (status == SpanStatus.ok()) {
_setTTFDMeasurement(startTimestamp, endTimestamp);
}
await ttfdSpan.finish(
status: status,
endTimestamp: endTimestamp,
);
} catch (e, stackTrace) {
options.logger(
SentryLevel.error,
'Failed to finish TTFD span',
exception: e,
stackTrace: stackTrace,
);
} finally {
_completedTTFDTracking.complete();
clear();
}
}

void _setTTFDMeasurement(DateTime startTimestamp, DateTime endTimestamp) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ void main() {
final differenceInSeconds =
actualEndTimestamp.difference(expectedEndTimestamp).inSeconds.abs();
expect(differenceInSeconds, lessThanOrEqualTo(1));
expect(transaction.measurements, isNotEmpty);
});

test(
Expand All @@ -66,6 +67,7 @@ void main() {
expect(ttfdSpan.status, equals(SpanStatus.deadlineExceeded()));
expect(ttfdSpan.context.description, equals('Current route full display'));
expect(ttfdSpan.origin, equals(SentryTraceOrigins.manualUiTimeToDisplay));
expect(transaction.measurements, isEmpty);
});

test('finishing ttfd twice does not throw', () async {
Expand Down

0 comments on commit 4d763a5

Please sign in to comment.