From 19284d6c6b8387bcc12e6b76e7f87004ade7df6e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 10:48:49 +0200 Subject: [PATCH 1/9] update refresh rate condition --- flutter/lib/src/span_frame_metrics_collector.dart | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index ecf0ca961c..bae355ee13 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -59,17 +59,18 @@ 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'); + 'Fetched invalid refresh rate: $fetchedDisplayRefreshRate. Not starting frame tracking.'); } - - activeSpans.add(span); - startFrameTracking(); } @override From 55015d3fa879e81c807d094304512ed3aa56e029 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 10:58:25 +0200 Subject: [PATCH 2/9] update test --- .../span_frame_metrics_collector_test.dart | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/flutter/test/span_frame_metrics_collector_test.dart b/flutter/test/span_frame_metrics_collector_test.dart index 41cb56b6d5..4f4ec4fbc8 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', From f6a586a46bda21e76de76c6e88d781203dbe7802 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 11:00:16 +0200 Subject: [PATCH 3/9] update log --- flutter/lib/src/span_frame_metrics_collector.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index bae355ee13..edf32d94e6 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -69,7 +69,7 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { startFrameTracking(); } else { options.logger(SentryLevel.debug, - 'Fetched invalid refresh rate: $fetchedDisplayRefreshRate. Not starting frame tracking.'); + 'Retrieved invalid display refresh rate: $fetchedDisplayRefreshRate. Not starting frame tracking.'); } } From 5a2451090572cc92d55cf0d2822aa18237292aca Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 11:05:12 +0200 Subject: [PATCH 4/9] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c3e09669f..96d10c1630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ ### Fixes +- Only start frame tracking if we receive valid dispaly refresh data ([#2307](https://github.com/getsentry/sentry-dart/pull/2307)) - iOS replay integration when only `onErrorSampleRate` is specified ([#2306](https://github.com/getsentry/sentry-dart/pull/2306)) ## 8.9.0 From a60355d8232f1961c8c90146df2ce66f60eb998a Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 11:06:46 +0200 Subject: [PATCH 5/9] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96d10c1630..d668a71265 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Fixes -- Only start frame tracking if we receive valid dispaly refresh data ([#2307](https://github.com/getsentry/sentry-dart/pull/2307)) +- Only start frame tracking if we receive valid display refresh data ([#2307](https://github.com/getsentry/sentry-dart/pull/2307)) - iOS replay integration when only `onErrorSampleRate` is specified ([#2306](https://github.com/getsentry/sentry-dart/pull/2306)) ## 8.9.0 From 4267f4d0ea5e9131f2490d50cf86f8a20ce275d3 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 13:12:38 +0200 Subject: [PATCH 6/9] update --- flutter/lib/src/span_frame_metrics_collector.dart | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index edf32d94e6..4152850e5f 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -39,7 +39,7 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { bool get isTrackingRegistered => _isTrackingRegistered; bool _isTrackingRegistered = false; - int displayRefreshRate = 60; + int? displayRefreshRate; final _stopwatch = Stopwatch(); @@ -75,10 +75,12 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { @override Future onSpanFinished(ISentrySpan span, DateTime endTimestamp) async { - if (span is NoOpSentrySpan || !activeSpans.contains(span)) return; + if (span is NoOpSentrySpan || + !activeSpans.contains(span) || + displayRefreshRate == null) return; final frameMetrics = - calculateFrameMetrics(span, endTimestamp, displayRefreshRate); + calculateFrameMetrics(span, endTimestamp, displayRefreshRate!); _applyFrameMetricsToSpan(span, frameMetrics); activeSpans.remove(span); @@ -252,6 +254,6 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { _isTrackingPaused = true; frames.clear(); activeSpans.clear(); - displayRefreshRate = 60; + displayRefreshRate = null; } } From c76bdc53f5f49641bd5c987be4811abce3a0839f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 14:29:25 +0200 Subject: [PATCH 7/9] fix tests --- flutter/lib/src/span_frame_metrics_collector.dart | 4 ++++ flutter/test/span_frame_metrics_collector_test.dart | 1 + 2 files changed, 5 insertions(+) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index 4152850e5f..ff846295f6 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,6 +41,7 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { bool get isTrackingRegistered => _isTrackingRegistered; bool _isTrackingRegistered = false; + @visibleForTesting int? displayRefreshRate; final _stopwatch = Stopwatch(); @@ -75,6 +78,7 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { @override Future onSpanFinished(ISentrySpan span, DateTime endTimestamp) async { + print(displayRefreshRate); if (span is NoOpSentrySpan || !activeSpans.contains(span) || displayRefreshRate == null) return; diff --git a/flutter/test/span_frame_metrics_collector_test.dart b/flutter/test/span_frame_metrics_collector_test.dart index 4f4ec4fbc8..33ab6346e6 100644 --- a/flutter/test/span_frame_metrics_collector_test.dart +++ b/flutter/test/span_frame_metrics_collector_test.dart @@ -84,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); From fd3cc7344487771bd9e0613fcba9d77f3811f1ac Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 25 Sep 2024 15:57:32 +0200 Subject: [PATCH 8/9] avoid print --- flutter/lib/src/span_frame_metrics_collector.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index ff846295f6..c13986d71b 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -78,7 +78,6 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { @override Future onSpanFinished(ISentrySpan span, DateTime endTimestamp) async { - print(displayRefreshRate); if (span is NoOpSentrySpan || !activeSpans.contains(span) || displayRefreshRate == null) return; From c67205bfe9c749f4c8ab6e5c1148becb2c716514 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Thu, 26 Sep 2024 00:27:56 +0200 Subject: [PATCH 9/9] update --- flutter/lib/src/span_frame_metrics_collector.dart | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/flutter/lib/src/span_frame_metrics_collector.dart b/flutter/lib/src/span_frame_metrics_collector.dart index 2797fd435f..94891f136b 100644 --- a/flutter/lib/src/span_frame_metrics_collector.dart +++ b/flutter/lib/src/span_frame_metrics_collector.dart @@ -78,9 +78,14 @@ class SpanFrameMetricsCollector implements PerformanceContinuousCollector { @override Future onSpanFinished(ISentrySpan span, DateTime endTimestamp) async { - if (span is NoOpSentrySpan || - !activeSpans.contains(span) || - displayRefreshRate == null) return; + 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!);