diff --git a/CHANGELOG.md b/CHANGELOG.md index 65339560d7..14ce713182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ ### Fixes +-- Incoming Change +- 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)) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index 87cee4bac5..94891f136b 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -26,10 +26,12 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { /// Stores frame timestamps and their durations in milliseconds. /// Keys are frame timestamps, values are frame durations. /// The timestamps mark the end of the frame. + @visibleForTesting final frames = SplayTreeMap(); /// Stores the spans that are actively being tracked. /// After the frames are calculated and stored in the span the span is removed from this list. + @visibleForTesting final activeSpans = SplayTreeSet( (a, b) => a.startTimestamp.compareTo(b.startTimestamp)); @@ -39,7 +41,8 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { bool get isTrackingRegistered => _isTrackingRegistered; bool _isTrackingRegistered = false; - int displayRefreshRate = 60; + @visibleForTesting + int? displayRefreshRate; final _stopwatch = Stopwatch(); @@ -59,25 +62,33 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { } final fetchedDisplayRefreshRate = await _native?.displayRefreshRate(); - if (fetchedDisplayRefreshRate != null) { + if (fetchedDisplayRefreshRate != null && fetchedDisplayRefreshRate > 0) { options.logger(SentryLevel.debug, 'Retrieved display refresh rate at $fetchedDisplayRefreshRate'); displayRefreshRate = fetchedDisplayRefreshRate; + + // Start tracking frames only when refresh rate is valid + activeSpans.add(span); + startFrameTracking(); } else { options.logger(SentryLevel.debug, - 'Could not fetch display refresh rate, keeping at 60hz by default'); + 'Retrieved invalid display refresh rate: $fetchedDisplayRefreshRate. Not starting frame tracking.'); } - - activeSpans.add(span); - startFrameTracking(); } @override Future onSpanFinished(ISentrySpan span, DateTime endTimestamp) async { if (span is NoOpSentrySpan || !activeSpans.contains(span)) return; + if (displayRefreshRate == null || displayRefreshRate! <= 0) { + options.logger(SentryLevel.warning, + 'Invalid display refresh rate. Skipping frame tracking for all active spans.'); + clear(); + return; + } + final frameMetrics = - calculateFrameMetrics(span, endTimestamp, displayRefreshRate); + calculateFrameMetrics(span, endTimestamp, displayRefreshRate!); _applyFrameMetricsToSpan(span, frameMetrics); activeSpans.remove(span); @@ -258,6 +269,6 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { _isTrackingPaused = true; frames.clear(); activeSpans.clear(); - displayRefreshRate = 60; + displayRefreshRate = null; } } diff --git a/flutter/test/span_frame_metrics_collector_test.dart b/flutter/test/span_frame_metrics_collector_test.dart index f0ba7fc043..c9f088a8f1 100644 --- a/flutter/test/span_frame_metrics_collector_test.dart +++ b/flutter/test/span_frame_metrics_collector_test.dart @@ -47,31 +47,32 @@ void main() { expect(sut.isTrackingRegistered, isFalse); }); - test( - 'captures metrics with display refresh rate of 60 if native refresh rate is null', - () async { + test('does not start frame tracking if native refresh rate is null', () { final sut = fixture.sut; fixture.options.tracesSampleRate = 1.0; fixture.options.addPerformanceCollector(sut); - final startTimestamp = DateTime.now(); - final endTimestamp = - startTimestamp.add(Duration(milliseconds: 1000)).toUtc(); when(fixture.mockSentryNative.displayRefreshRate()) .thenAnswer((_) async => null); - final tracer = SentryTracer( - SentryTransactionContext('name', 'op', description: 'tracerDesc'), - fixture.hub, - startTimestamp: startTimestamp); + final span = MockSentrySpan(); + sut.onSpanStarted(span); - await Future.delayed(Duration(milliseconds: 500)); - await tracer.finish(endTimestamp: endTimestamp); + expect(sut.isTrackingRegistered, isFalse); + }); - expect(tracer.data['frames.slow'], expectedSlowFrames); - expect(tracer.data['frames.frozen'], expectedFrozenFrames); - expect(tracer.data['frames.delay'], expectedFramesDelay); - expect(tracer.data['frames.total'], expectedTotalFrames); + test('does not start frame tracking if native refresh rate is 0', () { + final sut = fixture.sut; + fixture.options.tracesSampleRate = 1.0; + fixture.options.addPerformanceCollector(sut); + + when(fixture.mockSentryNative.displayRefreshRate()) + .thenAnswer((_) async => 0); + + final span = MockSentrySpan(); + sut.onSpanStarted(span); + + expect(sut.isTrackingRegistered, isFalse); }); test('onSpanFinished removes frames older than span start timestamp', @@ -83,6 +84,7 @@ void main() { final span2 = MockSentrySpan(); final spanStartTimestamp = DateTime.now(); final spanEndTimestamp = spanStartTimestamp.add(Duration(seconds: 1)); + sut.displayRefreshRate = 60; when(span1.isRootSpan).thenReturn(false); when(span1.startTimestamp).thenReturn(spanStartTimestamp);