From ee54f5ae29cb95507ef95f5b6be0db0d055bebeb Mon Sep 17 00:00:00 2001 From: Sergei Lavinov Date: Thu, 16 Jun 2022 22:09:17 +0300 Subject: [PATCH 1/4] Fix duplicative metrics breadcrumbs --- flutter/example/lib/main.dart | 1 + flutter/lib/src/widgets_binding_observer.dart | 43 ++++++++---- .../test/widgets_binding_observer_test.dart | 65 +++++++++++++++++-- 3 files changed, 94 insertions(+), 15 deletions(-) diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index e194e8d08c..d17b9247b0 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -27,6 +27,7 @@ Future main() async { options.addInAppInclude('sentry_flutter_example'); options.considerInAppFramesByDefault = false; options.attachThreads = true; + options.enableWindowMetricBreadcrumbs = true; }, // Init your App. appRunner: () => runApp( diff --git a/flutter/lib/src/widgets_binding_observer.dart b/flutter/lib/src/widgets_binding_observer.dart index f3ba815045..b2650a2c4b 100644 --- a/flutter/lib/src/widgets_binding_observer.dart +++ b/flutter/lib/src/widgets_binding_observer.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import '../sentry_flutter.dart'; import 'binding_utils.dart'; @@ -20,11 +21,23 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver { Hub? hub, required SentryFlutterOptions options, }) : _hub = hub ?? HubAdapter(), - _options = options; + _options = options { + if (_options.enableWindowMetricBreadcrumbs) { + final window = BindingUtils.getWidgetsBindingInstance()?.window; + + _lastSentMetrics = { + 'new_pixel_ratio': window?.devicePixelRatio, + 'new_height': window?.physicalSize.height, + 'new_width': window?.physicalSize.width, + }; + } + } final Hub _hub; final SentryFlutterOptions _options; + Map? _lastSentMetrics; + /// This method records lifecycle events. /// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry /// Android for lifecycle events. @@ -62,16 +75,24 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver { return; } final window = BindingUtils.getWidgetsBindingInstance()?.window; - _hub.addBreadcrumb(Breadcrumb( - message: 'Screen size changed', - category: 'device.screen', - type: 'navigation', - data: { - 'new_pixel_ratio': window?.devicePixelRatio, - 'new_height': window?.physicalSize.height, - 'new_width': window?.physicalSize.width, - }, - )); + final data = { + 'new_pixel_ratio': window?.devicePixelRatio, + 'new_height': window?.physicalSize.height, + 'new_width': window?.physicalSize.width, + }; + + final dataChanged = !mapEquals(data, _lastSentMetrics); + + if (dataChanged) { + _lastSentMetrics = data; + + _hub.addBreadcrumb(Breadcrumb( + message: 'Screen size changed', + category: 'device.screen', + type: 'navigation', + data: data, + )); + } } /// See also: diff --git a/flutter/test/widgets_binding_observer_test.dart b/flutter/test/widgets_binding_observer_test.dart index 656008045e..9e738fcd41 100644 --- a/flutter/test/widgets_binding_observer_test.dart +++ b/flutter/test/widgets_binding_observer_test.dart @@ -1,3 +1,5 @@ +import 'dart:ui'; + import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; @@ -168,19 +170,22 @@ void main() { instance.removeObserver(observer); }); - testWidgets('metrics changed breadcrumb', (WidgetTester tester) async { + testWidgets('metrics changed breadcrumb (screen size)', + (WidgetTester tester) async { final hub = MockHub(); final observer = SentryWidgetsBindingObserver( hub: hub, options: flutterTrackingEnabledOptions, ); - final instance = BindingUtils.getWidgetsBindingInstance(); - instance!.addObserver(observer); + final instance = tester.binding; + instance.addObserver(observer); final window = instance.window; - window.onMetricsChanged!(); + const newWidth = 123.0; + const newHeight = 456.0; + window.physicalSizeTestValue = Size(newWidth, newHeight); final breadcrumb = verify(hub.addBreadcrumb(captureAny)).captured.single as Breadcrumb; @@ -191,6 +196,38 @@ void main() { expect(breadcrumb.level, SentryLevel.info); expect(breadcrumb.data, { 'new_pixel_ratio': window.devicePixelRatio, + 'new_height': newHeight, + 'new_width': newWidth, + }); + + instance.removeObserver(observer); + }); + + testWidgets('metrics changed breadcrumb (pixel ratio)', + (WidgetTester tester) async { + final hub = MockHub(); + + final observer = SentryWidgetsBindingObserver( + hub: hub, + options: flutterTrackingEnabledOptions, + ); + final instance = tester.binding; + instance.addObserver(observer); + + final window = instance.window; + + const newPixelRatio = 1.618; + window.devicePixelRatioTestValue = newPixelRatio; + + final breadcrumb = + verify(hub.addBreadcrumb(captureAny)).captured.single as Breadcrumb; + + expect(breadcrumb.message, 'Screen size changed'); + expect(breadcrumb.category, 'device.screen'); + expect(breadcrumb.type, 'navigation'); + expect(breadcrumb.level, SentryLevel.info); + expect(breadcrumb.data, { + 'new_pixel_ratio': newPixelRatio, 'new_height': window.physicalSize.height, 'new_width': window.physicalSize.width, }); @@ -198,6 +235,26 @@ void main() { instance.removeObserver(observer); }); + testWidgets('no breadcrumb on unrelated metrics changes', + (WidgetTester tester) async { + final hub = MockHub(); + + final observer = SentryWidgetsBindingObserver( + hub: hub, + options: flutterTrackingEnabledOptions, + ); + final instance = tester.binding; + instance.addObserver(observer); + + final window = instance.window; + + window.viewInsetsTestValue = WindowPadding.zero; + + verifyNever(hub.addBreadcrumb(captureAny)); + + instance.removeObserver(observer); + }); + testWidgets('disable metrics changed breadcrumb', (WidgetTester tester) async { final hub = MockHub(); From 4ad806aba3d057821f0b5377785dc4391bb49a5d Mon Sep 17 00:00:00 2001 From: Sergei Lavinov Date: Thu, 16 Jun 2022 22:46:49 +0300 Subject: [PATCH 2/4] Update docs link --- flutter/lib/src/widgets_binding_observer.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/lib/src/widgets_binding_observer.dart b/flutter/lib/src/widgets_binding_observer.dart index b2650a2c4b..2bf24eb60d 100644 --- a/flutter/lib/src/widgets_binding_observer.dart +++ b/flutter/lib/src/widgets_binding_observer.dart @@ -68,7 +68,7 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver { /// when a phone is rotated or an application window is resized. /// /// See also: - /// - [Window.onMetricsChanged](https://api.flutter.dev/flutter/dart-ui/Window/onMetricsChanged.html) + /// - [SingletonFlutterWindow.onMetricsChanged](https://api.flutter.dev/flutter/dart-ui/SingletonFlutterWindow/onMetricsChanged.html) @override void didChangeMetrics() { if (!_options.enableWindowMetricBreadcrumbs) { From 3150be7d8c85abe4da444c4714302194c4b9e65c Mon Sep 17 00:00:00 2001 From: Sergei Lavinov Date: Mon, 27 Jun 2022 18:49:25 +0300 Subject: [PATCH 3/4] Refactor to stream + distinct --- flutter/lib/src/widgets_binding_observer.dart | 55 ++++++++++--------- .../test/widgets_binding_observer_test.dart | 8 +-- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/flutter/lib/src/widgets_binding_observer.dart b/flutter/lib/src/widgets_binding_observer.dart index 2bf24eb60d..c55a074558 100644 --- a/flutter/lib/src/widgets_binding_observer.dart +++ b/flutter/lib/src/widgets_binding_observer.dart @@ -1,3 +1,6 @@ +import 'dart:async'; +import 'dart:ui'; + import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import '../sentry_flutter.dart'; @@ -21,22 +24,30 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver { Hub? hub, required SentryFlutterOptions options, }) : _hub = hub ?? HubAdapter(), - _options = options { + _options = options, + _screenSizeStreamController = StreamController(sync: true) { if (_options.enableWindowMetricBreadcrumbs) { - final window = BindingUtils.getWidgetsBindingInstance()?.window; + _screenSizeStreamController.stream + .map( + (window) => { + 'new_pixel_ratio': window?.devicePixelRatio, + 'new_height': window?.physicalSize.height, + 'new_width': window?.physicalSize.width, + }, + ) + .distinct(mapEquals) + .skip(1) // Skip initial event added below in constructor + .listen(_onScreenSizeChanged); - _lastSentMetrics = { - 'new_pixel_ratio': window?.devicePixelRatio, - 'new_height': window?.physicalSize.height, - 'new_width': window?.physicalSize.width, - }; + final window = BindingUtils.getWidgetsBindingInstance()?.window; + _screenSizeStreamController.add(window); } } final Hub _hub; final SentryFlutterOptions _options; - Map? _lastSentMetrics; + final StreamController _screenSizeStreamController; /// This method records lifecycle events. /// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry @@ -75,24 +86,16 @@ class SentryWidgetsBindingObserver with WidgetsBindingObserver { return; } final window = BindingUtils.getWidgetsBindingInstance()?.window; - final data = { - 'new_pixel_ratio': window?.devicePixelRatio, - 'new_height': window?.physicalSize.height, - 'new_width': window?.physicalSize.width, - }; - - final dataChanged = !mapEquals(data, _lastSentMetrics); - - if (dataChanged) { - _lastSentMetrics = data; - - _hub.addBreadcrumb(Breadcrumb( - message: 'Screen size changed', - category: 'device.screen', - type: 'navigation', - data: data, - )); - } + _screenSizeStreamController.add(window); + } + + void _onScreenSizeChanged(Map data) { + _hub.addBreadcrumb(Breadcrumb( + message: 'Screen size changed', + category: 'device.screen', + type: 'navigation', + data: data, + )); } /// See also: diff --git a/flutter/test/widgets_binding_observer_test.dart b/flutter/test/widgets_binding_observer_test.dart index 9e738fcd41..ba37d37fc8 100644 --- a/flutter/test/widgets_binding_observer_test.dart +++ b/flutter/test/widgets_binding_observer_test.dart @@ -170,8 +170,7 @@ void main() { instance.removeObserver(observer); }); - testWidgets('metrics changed breadcrumb (screen size)', - (WidgetTester tester) async { + testWidgets('metrics changed breadcrumb', (WidgetTester tester) async { final hub = MockHub(); final observer = SentryWidgetsBindingObserver( @@ -203,8 +202,7 @@ void main() { instance.removeObserver(observer); }); - testWidgets('metrics changed breadcrumb (pixel ratio)', - (WidgetTester tester) async { + testWidgets('only unique metrics emit events', (WidgetTester tester) async { final hub = MockHub(); final observer = SentryWidgetsBindingObserver( @@ -216,6 +214,8 @@ void main() { final window = instance.window; + window.physicalSizeTestValue = window.physicalSize; + const newPixelRatio = 1.618; window.devicePixelRatioTestValue = newPixelRatio; From 3469eb47c4571bde99955d249027ec9fba6c999e Mon Sep 17 00:00:00 2001 From: Sergei Lavinov Date: Mon, 11 Jul 2022 10:19:27 +0300 Subject: [PATCH 4/4] Add changelog note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a2788c54e..77b67101e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * Breadcrumbs should not be duplicated if there is no mechanism on Android ([#936](https://github.com/getsentry/sentry-dart/pull/936)) * Maps with Key Object, Object would fail during serialization if not String, Object ([#935](https://github.com/getsentry/sentry-dart/pull/935)) +* Fix duplicative Screen size changed breadcrumbs ([#888](https://github.com/getsentry/sentry-dart/pull/888)) ## 6.6.3