From 8800f80f8f55ce2dc0eeb34e13e946129b04f7cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Fri, 20 Aug 2021 17:12:05 +0200 Subject: [PATCH 1/5] Set name of current route as transaction --- .../web_enricher_event_processor.dart | 1 + flutter/example/lib/main.dart | 6 ++++++ .../navigation/sentry_navigator_observer.dart | 21 ++++++++++++++----- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/dart/lib/src/enricher/web_enricher_event_processor.dart b/dart/lib/src/enricher/web_enricher_event_processor.dart index 2dd78fd166..7786ed8276 100644 --- a/dart/lib/src/enricher/web_enricher_event_processor.dart +++ b/dart/lib/src/enricher/web_enricher_event_processor.dart @@ -29,6 +29,7 @@ class WebEnricherEventProcessor extends EventProcessor { return event.copyWith( contexts: contexts, request: _getRequest(event.request), + transaction: event.transaction ?? _window.location.pathname, ); } diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index a66a4efc67..738f80eaa7 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -418,6 +418,12 @@ class SecondaryScaffold extends StatelessWidget { Navigator.pop(context); }, ), + MaterialButton( + child: const Text('throw uncaught exception'), + onPressed: () { + throw Exception('Exception from SecondaryScaffold'); + }, + ), ], ), ), diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 917bc67c08..620efcc351 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -35,13 +35,16 @@ const _navigationKey = 'navigation'; /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) /// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments) class SentryNavigatorObserver extends RouteObserver> { - SentryNavigatorObserver({Hub? hub}) : hub = hub ?? HubAdapter(); + SentryNavigatorObserver({Hub? hub, this.setTransaction = true}) + : _hub = hub ?? HubAdapter(); - final Hub hub; + final Hub _hub; + final bool setTransaction; @override void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); + setCurrentRoute(route.settings.name); _addBreadcrumb( type: 'didPush', from: previousRoute?.settings, @@ -52,7 +55,7 @@ class SentryNavigatorObserver extends RouteObserver> { @override void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); - + setCurrentRoute(newRoute?.settings.name); _addBreadcrumb( type: 'didReplace', from: oldRoute?.settings, @@ -63,7 +66,7 @@ class SentryNavigatorObserver extends RouteObserver> { @override void didPop(Route route, Route? previousRoute) { super.didPop(route, previousRoute); - + setCurrentRoute(previousRoute?.settings.name); _addBreadcrumb( type: 'didPop', from: route.settings, @@ -76,12 +79,20 @@ class SentryNavigatorObserver extends RouteObserver> { RouteSettings? from, RouteSettings? to, }) { - hub.addBreadcrumb(RouteObserverBreadcrumb( + _hub.addBreadcrumb(RouteObserverBreadcrumb( navigationType: type, from: from, to: to, )); } + + void setCurrentRoute(String? name) { + if (setTransaction) { + _hub.configureScope((scope) { + scope.transaction = name; + }); + } + } } /// This class makes it easier to record breadcrumbs for events of Flutters From 929e61b743d0b5cc8e84a2c54582ed59282ec807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Fri, 20 Aug 2021 18:05:35 +0200 Subject: [PATCH 2/5] Add tests --- CHANGELOG.md | 2 + dart/test/enricher/web_enricher_test.dart | 14 ++++++ .../test/sentry_navigator_observer_test.dart | 43 +++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6e7e72366..bdf802839f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* Feat: Add current route as transaction (#560) + # 6.0.0-beta.4 ## Breaking Changes: diff --git a/dart/test/enricher/web_enricher_test.dart b/dart/test/enricher/web_enricher_test.dart index da9e0f24ae..28337b1bda 100644 --- a/dart/test/enricher/web_enricher_test.dart +++ b/dart/test/enricher/web_enricher_test.dart @@ -16,6 +16,20 @@ void main() { fixture = Fixture(); }); + test('add path as transaction if transaction is null', () async { + var enricher = fixture.getSut(); + final event = await enricher.apply(SentryEvent()); + + expect(event.transaction, isNotNull); + }); + + test("don't overwrite transaction", () async { + var enricher = fixture.getSut(); + final event = await enricher.apply(SentryEvent(transaction: 'foobar')); + + expect(event.transaction, 'foobar'); + }); + test('add request with user-agent header', () async { var enricher = fixture.getSut(); final event = await enricher.apply(SentryEvent()); diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index fd08c2a61d..9ccf1f62c2 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -3,6 +3,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:mockito/mockito.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; +import 'mocks.dart'; import 'mocks.mocks.dart'; void main() { @@ -226,5 +227,47 @@ void main() { ).data, ); }); + + test('route name as transaction', () { + final hub = _MockHub(); + final observer = SentryNavigatorObserver(hub: hub, setTransaction: true); + + final to = routeSettings('to'); + final previous = routeSettings('previous'); + + observer.didPush(route(to), route(previous)); + expect(hub.scope.transaction, 'to'); + + observer.didPop(route(to), route(previous)); + expect(hub.scope.transaction, 'previous'); + + observer.didReplace(newRoute: route(to), oldRoute: route(previous)); + expect(hub.scope.transaction, 'to'); + }); + + test('disabled route as transaction', () { + final hub = _MockHub(); + final observer = SentryNavigatorObserver(hub: hub, setTransaction: false); + + final to = routeSettings('to'); + final previous = routeSettings('previous'); + + observer.didPush(route(to), route(previous)); + expect(hub.scope.transaction, null); + + observer.didPop(route(to), route(previous)); + expect(hub.scope.transaction, null); + + observer.didReplace(newRoute: route(to), oldRoute: route(previous)); + expect(hub.scope.transaction, null); + }); }); } + +class _MockHub extends MockHub { + final Scope scope = Scope(SentryOptions(dsn: fakeDsn)); + @override + void configureScope(ScopeCallback? callback) { + callback?.call(scope); + } +} From f2277f551dfe7aadf9d709283e6eda3da9a6f3c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Thu, 7 Oct 2021 18:52:51 +0200 Subject: [PATCH 3/5] fixture and changelog --- CHANGELOG.md | 3 +- .../navigation/sentry_navigator_observer.dart | 20 ++++--- .../test/sentry_navigator_observer_test.dart | 52 ++++++++++++++++--- 3 files changed, 60 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdcf935866..efeef18b76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Unreleased -* Feat: Add current route as transaction (#560) +* Feat: Add current route as transaction (#615) + # 6.1.0-alpha.2 * Bump Sentry Android SDK to [5.2.0](https://github.com/getsentry/sentry-dart/pull/594) (#594) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 620efcc351..55516f1171 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -35,16 +35,19 @@ const _navigationKey = 'navigation'; /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) /// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments) class SentryNavigatorObserver extends RouteObserver> { - SentryNavigatorObserver({Hub? hub, this.setTransaction = true}) + SentryNavigatorObserver({Hub? hub, this.setRouteNameAsTransaction = true}) : _hub = hub ?? HubAdapter(); final Hub _hub; - final bool setTransaction; + + /// Enabling this option overrides the current [Scope.transaction]. + /// So be careful when this is used together with performance tracing. + final bool setRouteNameAsTransaction; @override void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); - setCurrentRoute(route.settings.name); + _setCurrentRoute(route.settings.name); _addBreadcrumb( type: 'didPush', from: previousRoute?.settings, @@ -55,7 +58,7 @@ class SentryNavigatorObserver extends RouteObserver> { @override void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); - setCurrentRoute(newRoute?.settings.name); + _setCurrentRoute(newRoute?.settings.name); _addBreadcrumb( type: 'didReplace', from: oldRoute?.settings, @@ -66,7 +69,7 @@ class SentryNavigatorObserver extends RouteObserver> { @override void didPop(Route route, Route? previousRoute) { super.didPop(route, previousRoute); - setCurrentRoute(previousRoute?.settings.name); + _setCurrentRoute(previousRoute?.settings.name); _addBreadcrumb( type: 'didPop', from: route.settings, @@ -86,8 +89,11 @@ class SentryNavigatorObserver extends RouteObserver> { )); } - void setCurrentRoute(String? name) { - if (setTransaction) { + void _setCurrentRoute(String? name) { + if (name == null) { + return; + } + if (setRouteNameAsTransaction) { _hub.configureScope((scope) { scope.transaction = name; }); diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 9ccf1f62c2..0ca5496d09 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -7,6 +7,12 @@ import 'mocks.dart'; import 'mocks.mocks.dart'; void main() { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + group('RouteObserverBreadcrumb', () { test('happy path with string route agrument', () { const fromRouteSettings = RouteSettings( @@ -141,12 +147,12 @@ void main() { settings: settings, ); - RouteSettings routeSettings(String name, [Object? arguments]) => + RouteSettings routeSettings(String? name, [Object? arguments]) => RouteSettings(name: name, arguments: arguments); test('Test recording of Breadcrumbs', () { final hub = MockHub(); - final observer = SentryNavigatorObserver(hub: hub); + final observer = fixture.getSut(hub: hub); final to = routeSettings('to', 'foobar'); final previous = routeSettings('previous', 'foobar'); @@ -167,7 +173,7 @@ void main() { test('No arguments', () { final hub = MockHub(); - final observer = SentryNavigatorObserver(hub: hub); + final observer = fixture.getSut(hub: hub); final to = routeSettings('to'); final previous = routeSettings('previous'); @@ -188,7 +194,7 @@ void main() { test('No arguments & no name', () { final hub = MockHub(); - final observer = SentryNavigatorObserver(hub: hub); + final observer = fixture.getSut(hub: hub); final to = route(null); final previous = route(null); @@ -211,7 +217,7 @@ void main() { ); final hub = MockHub(); - final observer = SentryNavigatorObserver(hub: hub); + final observer = fixture.getSut(hub: hub); final to = route(); final previous = route(); @@ -230,7 +236,10 @@ void main() { test('route name as transaction', () { final hub = _MockHub(); - final observer = SentryNavigatorObserver(hub: hub, setTransaction: true); + final observer = fixture.getSut( + hub: hub, + setRouteNameAsTransaction: true, + ); final to = routeSettings('to'); final previous = routeSettings('previous'); @@ -245,9 +254,26 @@ void main() { expect(hub.scope.transaction, 'to'); }); + test('route name does nothing if null', () { + final hub = _MockHub(); + final observer = fixture.getSut( + hub: hub, + setRouteNameAsTransaction: true, + ); + + hub.scope.transaction = 'foo bar'; + + final to = routeSettings(null); + final previous = routeSettings(null); + + observer.didPush(route(to), route(previous)); + expect(hub.scope.transaction, 'foo bar'); + }); + test('disabled route as transaction', () { final hub = _MockHub(); - final observer = SentryNavigatorObserver(hub: hub, setTransaction: false); + final observer = + fixture.getSut(hub: hub, setRouteNameAsTransaction: false); final to = routeSettings('to'); final previous = routeSettings('previous'); @@ -264,6 +290,18 @@ void main() { }); } +class Fixture { + SentryNavigatorObserver getSut({ + required Hub hub, + bool setRouteNameAsTransaction = false, + }) { + return SentryNavigatorObserver( + hub: hub, + setRouteNameAsTransaction: setRouteNameAsTransaction, + ); + } +} + class _MockHub extends MockHub { final Scope scope = Scope(SentryOptions(dsn: fakeDsn)); @override From 1d55711e88eb3ea6ba286f4b6bf54911a37de80c Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Fri, 8 Oct 2021 07:45:18 +0200 Subject: [PATCH 4/5] Update flutter/lib/src/navigation/sentry_navigator_observer.dart --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 55516f1171..43b49b53e1 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -41,7 +41,7 @@ class SentryNavigatorObserver extends RouteObserver> { final Hub _hub; /// Enabling this option overrides the current [Scope.transaction]. - /// So be careful when this is used together with performance tracing. + /// So be careful when this is used together with performance monitoring. final bool setRouteNameAsTransaction; @override From 058a7f9b2eff78fbed25103945c5eeba5fc1c853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Fri, 8 Oct 2021 11:21:04 +0200 Subject: [PATCH 5/5] make option private --- .../src/navigation/sentry_navigator_observer.dart | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 43b49b53e1..2193ab568d 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -30,19 +30,20 @@ const _navigationKey = 'navigation'; /// // other parameter ... /// ) /// ``` +/// Enabling the [setRouteNameAsTransaction] option overrides the current +/// [Scope.transaction]. So be careful when this is used together with +/// performance monitoring. /// /// See also: /// - [RouteObserver](https://api.flutter.dev/flutter/widgets/RouteObserver-class.html) /// - [Navigating with arguments](https://flutter.dev/docs/cookbook/navigation/navigate-with-arguments) class SentryNavigatorObserver extends RouteObserver> { - SentryNavigatorObserver({Hub? hub, this.setRouteNameAsTransaction = true}) - : _hub = hub ?? HubAdapter(); + SentryNavigatorObserver({Hub? hub, bool setRouteNameAsTransaction = false}) + : _hub = hub ?? HubAdapter(), + _setRouteNameAsTransaction = setRouteNameAsTransaction; final Hub _hub; - - /// Enabling this option overrides the current [Scope.transaction]. - /// So be careful when this is used together with performance monitoring. - final bool setRouteNameAsTransaction; + final bool _setRouteNameAsTransaction; @override void didPush(Route route, Route? previousRoute) { @@ -93,7 +94,7 @@ class SentryNavigatorObserver extends RouteObserver> { if (name == null) { return; } - if (setRouteNameAsTransaction) { + if (_setRouteNameAsTransaction) { _hub.configureScope((scope) { scope.transaction = name; });