Skip to content

Commit

Permalink
fix(frame-tracking): app lag when long running span finishes (#2311)
Browse files Browse the repository at this point in the history
* add frame retention option

* add safeguard for frames tracking that stops frame tracking after a limit

* add test

* update test

* update option name

* update comment and fix tests

* remove debugging code

* update changelog

* change debug to warning log

* Update sentry_flutter_options.dart

* dont make option configurable

* fix test
  • Loading branch information
buenaflor authored Sep 26, 2024
1 parent a4c4f8c commit 84c28cd
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 1 deletion.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

### Fixes

-- Incoming Change
- App lag with frame tracking enabled when span finishes after a long time ([#2311](https://github.com/getsentry/sentry-dart/pull/2311))
- Only start frame tracking if we receive valid display refresh data ([#2307](https://github.com/getsentry/sentry-dart/pull/2307))
- Rounding error used on frames.total and reject frame measurements if frames.total is less than frames.slow or frames.frozen ([#2308](https://github.com/getsentry/sentry-dart/pull/2308))
- iOS replay integration when only `onErrorSampleRate` is specified ([#2306](https://github.com/getsentry/sentry-dart/pull/2306))
Expand Down
11 changes: 11 additions & 0 deletions flutter/lib/src/span_frame_metrics_collector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector {
@visibleForTesting
int? displayRefreshRate;

@visibleForTesting
int maxFramesToTrack = 10800;

final _stopwatch = Stopwatch();

SpanFrameMetricsCollector(this.options,
Expand Down Expand Up @@ -118,6 +121,13 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector {
///
/// This method is called for each frame when frame tracking is active.
Future<void> measureFrameDuration(Duration duration) async {
if (frames.length >= maxFramesToTrack) {
options.logger(SentryLevel.warning,
'Frame tracking limit reached. Clearing frames and cancelling frame tracking for all active spans');
clear();
return;
}

// Using the stopwatch to measure the frame duration is flaky in ci
if (_isTestMode) {
// ignore: invalid_use_of_internal_member
Expand Down Expand Up @@ -267,6 +277,7 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector {
@override
void clear() {
_isTrackingPaused = true;
_stopwatch.reset();
frames.clear();
activeSpans.clear();
displayRefreshRate = null;
Expand Down
27 changes: 27 additions & 0 deletions flutter/test/span_frame_metrics_collector_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,33 @@ void main() {

expect(sut.isTrackingPaused, isTrue);
});

test(
'measureFrameDuration stops and removes all frames when reaching frame limit',
() async {
final sut = fixture.sut;
final span = MockSentrySpan();

when(span.startTimestamp).thenReturn(DateTime.now());
sut.activeSpans.add(span);
sut.frames[DateTime.now()] = 1;
const maxFramesToTrack = 1000;
sut.maxFramesToTrack = maxFramesToTrack;

for (var i = 1; i <= maxFramesToTrack; i++) {
await Future<void>.delayed(
Duration(milliseconds: 1)); // Add a small delay
if (i == maxFramesToTrack - 1) {
expect(sut.frames.length, maxFramesToTrack - 1);
}
await sut.measureFrameDuration(Duration.zero);
}

expect(sut.frames, isEmpty);
expect(sut.activeSpans, isEmpty);
expect(sut.displayRefreshRate, isNull);
expect(sut.isTrackingPaused, isTrue);
});
}

class Fixture {
Expand Down

0 comments on commit 84c28cd

Please sign in to comment.