From 3de8b9b92406e56cedb15086ae779e72d5cdfb08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Denis=20Andra=C5=A1ec?= Date: Tue, 31 Oct 2023 14:35:48 +0000 Subject: [PATCH] Add `SentryNavigatorObserver` current route to `event.app.contexts.viewNames` (#1545) --- CHANGELOG.md | 2 + dart/lib/src/protocol/sentry_app.dart | 40 +++++++---- dart/test/protocol/sentry_app_test.dart | 30 ++++++--- .../flutter_enricher_event_processor.dart | 17 +++++ .../navigation/sentry_navigator_observer.dart | 27 ++++++-- ...flutter_enricher_event_processor_test.dart | 23 +++++++ .../test/sentry_navigator_observer_test.dart | 67 +++++++++++++++++++ 7 files changed, 180 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c640f46f6b..41975567fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ ### Features - Initial (alpha) support for profiling on iOS and macOS ([#1611](https://github.com/getsentry/sentry-dart/pull/1611)) +- Add `SentryNavigatorObserver` current route to `event.app.contexts.viewNames` ([#1545](https://github.com/getsentry/sentry-dart/pull/1545)) + - Requires relay version [23.9.0](https://github.com/getsentry/relay/blob/master/CHANGELOG.md#2390) for self-hosted instances ## 7.11.0 diff --git a/dart/lib/src/protocol/sentry_app.dart b/dart/lib/src/protocol/sentry_app.dart index 67accebe1a..b5163e5856 100644 --- a/dart/lib/src/protocol/sentry_app.dart +++ b/dart/lib/src/protocol/sentry_app.dart @@ -18,6 +18,7 @@ class SentryApp { this.deviceAppHash, this.appMemory, this.inForeground, + this.viewNames, }); /// Human readable application name, as it appears on the platform. @@ -48,20 +49,27 @@ class SentryApp { /// An app is in foreground when it's visible to the user. final bool? inForeground; + /// The names of the currently visible views. + final List? viewNames; + /// Deserializes a [SentryApp] from JSON [Map]. - factory SentryApp.fromJson(Map data) => SentryApp( - name: data['app_name'], - version: data['app_version'], - identifier: data['app_identifier'], - build: data['app_build'], - buildType: data['build_type'], - startTime: data['app_start_time'] != null - ? DateTime.tryParse(data['app_start_time']) - : null, - deviceAppHash: data['device_app_hash'], - appMemory: data['app_memory'], - inForeground: data['in_foreground'], - ); + factory SentryApp.fromJson(Map data) { + final viewNamesJson = data['view_names'] as List?; + return SentryApp( + name: data['app_name'], + version: data['app_version'], + identifier: data['app_identifier'], + build: data['app_build'], + buildType: data['build_type'], + startTime: data['app_start_time'] != null + ? DateTime.tryParse(data['app_start_time']) + : null, + deviceAppHash: data['device_app_hash'], + appMemory: data['app_memory'], + inForeground: data['in_foreground'], + viewNames: viewNamesJson?.map((e) => e as String).toList(), + ); + } /// Produces a [Map] that can be serialized to JSON. Map toJson() { @@ -71,10 +79,11 @@ class SentryApp { if (identifier != null) 'app_identifier': identifier!, if (build != null) 'app_build': build!, if (buildType != null) 'build_type': buildType!, + if (startTime != null) 'app_start_time': startTime!.toIso8601String(), if (deviceAppHash != null) 'device_app_hash': deviceAppHash!, if (appMemory != null) 'app_memory': appMemory!, - if (startTime != null) 'app_start_time': startTime!.toIso8601String(), if (inForeground != null) 'in_foreground': inForeground!, + if (viewNames != null && viewNames!.isNotEmpty) 'view_names': viewNames!, }; } @@ -88,6 +97,7 @@ class SentryApp { deviceAppHash: deviceAppHash, appMemory: appMemory, inForeground: inForeground, + viewNames: viewNames, ); SentryApp copyWith({ @@ -100,6 +110,7 @@ class SentryApp { String? deviceAppHash, int? appMemory, bool? inForeground, + List? viewNames, }) => SentryApp( name: name ?? this.name, @@ -111,5 +122,6 @@ class SentryApp { deviceAppHash: deviceAppHash ?? this.deviceAppHash, appMemory: appMemory ?? this.appMemory, inForeground: inForeground ?? this.inForeground, + viewNames: viewNames ?? this.viewNames, ); } diff --git a/dart/test/protocol/sentry_app_test.dart b/dart/test/protocol/sentry_app_test.dart index 6c72e956d3..6fd4236efc 100644 --- a/dart/test/protocol/sentry_app_test.dart +++ b/dart/test/protocol/sentry_app_test.dart @@ -14,6 +14,7 @@ void main() { startTime: testStartTime, deviceAppHash: 'fixture-deviceAppHash', inForeground: true, + viewNames: ['fixture-viewName', 'fixture-viewName2'], ); final sentryAppJson = { @@ -25,25 +26,36 @@ void main() { 'app_start_time': testStartTime.toIso8601String(), 'device_app_hash': 'fixture-deviceAppHash', 'in_foreground': true, + 'view_names': ['fixture-viewName', 'fixture-viewName2'], }; group('json', () { test('toJson', () { final json = sentryApp.toJson(); - expect( - MapEquality().equals(sentryAppJson, json), - true, - ); + expect(json['app_name'], 'fixture-name'); + expect(json['app_version'], 'fixture-version'); + expect(json['app_identifier'], 'fixture-identifier'); + expect(json['app_build'], 'fixture-build'); + expect(json['build_type'], 'fixture-buildType'); + expect(json['app_start_time'], testStartTime.toIso8601String()); + expect(json['device_app_hash'], 'fixture-deviceAppHash'); + expect(json['in_foreground'], true); + expect(json['view_names'], ['fixture-viewName', 'fixture-viewName2']); }); test('fromJson', () { final sentryApp = SentryApp.fromJson(sentryAppJson); final json = sentryApp.toJson(); - expect( - MapEquality().equals(sentryAppJson, json), - true, - ); + expect(json['app_name'], 'fixture-name'); + expect(json['app_version'], 'fixture-version'); + expect(json['app_identifier'], 'fixture-identifier'); + expect(json['app_build'], 'fixture-build'); + expect(json['build_type'], 'fixture-buildType'); + expect(json['app_start_time'], testStartTime.toIso8601String()); + expect(json['device_app_hash'], 'fixture-deviceAppHash'); + expect(json['in_foreground'], true); + expect(json['view_names'], ['fixture-viewName', 'fixture-viewName2']); }); }); @@ -73,6 +85,7 @@ void main() { startTime: startTime, deviceAppHash: 'hash1', inForeground: true, + viewNames: ['screen1'], ); expect('name1', copy.name); @@ -83,6 +96,7 @@ void main() { expect(startTime, copy.startTime); expect('hash1', copy.deviceAppHash); expect(true, copy.inForeground); + expect(['screen1'], copy.viewNames); }); }); } diff --git a/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart b/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart index f72eb2470b..b0eaa0fe71 100644 --- a/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart +++ b/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart @@ -5,6 +5,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:sentry/sentry.dart'; +import '../navigation/sentry_navigator_observer.dart'; import '../sentry_flutter_options.dart'; typedef WidgetBindingGetter = WidgetsBinding? Function(); @@ -47,6 +48,11 @@ class FlutterEnricherEventProcessor implements EventProcessor { app: _getApp(event.contexts.app), ); + final app = contexts.app; + if (app != null) { + contexts.app = _appWithCurrentRouteViewName(app); + } + // Flutter has a lot of Accessibility Settings available and exposes them contexts['accessibility'] = _getAccessibilityContext(); @@ -237,4 +243,15 @@ class FlutterEnricherEventProcessor implements EventProcessor { inForeground: inForeground, ); } + + SentryApp _appWithCurrentRouteViewName(SentryApp app) { + final currentRouteName = SentryNavigatorObserver.currentRouteName; + if (currentRouteName != null) { + final viewNames = app.viewNames ?? []; + viewNames.add(currentRouteName); + return app.copyWith(viewNames: viewNames); + } else { + return app; + } + } } diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 51c4b0fd07..bfe1c0c6a2 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -1,6 +1,8 @@ import 'package:flutter/widgets.dart'; +import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; +import '../event_processor/flutter_enricher_event_processor.dart'; import '../native/sentry_native.dart'; /// This key must be used so that the web interface displays the events nicely @@ -22,6 +24,9 @@ typedef AdditionalInfoExtractor = Map? Function( /// The [RouteSettings] is null if a developer has not specified any /// RouteSettings. /// +/// The current route name will also be set to [SentryEvent] +/// `contexts.app.view_names` by [FlutterEnricherEventProcessor]. +/// /// [SentryNavigatorObserver] must be added to the [navigation observer](https://api.flutter.dev/flutter/material/MaterialApp/navigatorObservers.html) of /// your used app. This is an example for [MaterialApp](https://api.flutter.dev/flutter/material/MaterialApp/navigatorObservers.html), /// but the integration for [CupertinoApp](https://api.flutter.dev/flutter/cupertino/CupertinoApp/navigatorObservers.html) @@ -84,11 +89,17 @@ class SentryNavigatorObserver extends RouteObserver> { ISentrySpan? _transaction; + static String? _currentRouteName; + + @internal + static String? get currentRouteName => _currentRouteName; + @override void didPush(Route route, Route? previousRoute) { super.didPush(route, previousRoute); - _setCurrentRoute(route); + _setCurrentRouteName(route); + _setCurrentRouteNameAsTransaction(route); _addBreadcrumb( type: 'didPush', @@ -104,7 +115,9 @@ class SentryNavigatorObserver extends RouteObserver> { void didReplace({Route? newRoute, Route? oldRoute}) { super.didReplace(newRoute: newRoute, oldRoute: oldRoute); - _setCurrentRoute(newRoute); + _setCurrentRouteName(newRoute); + _setCurrentRouteNameAsTransaction(newRoute); + _addBreadcrumb( type: 'didReplace', from: oldRoute?.settings, @@ -116,7 +129,9 @@ class SentryNavigatorObserver extends RouteObserver> { void didPop(Route route, Route? previousRoute) { super.didPop(route, previousRoute); - _setCurrentRoute(previousRoute); + _setCurrentRouteName(previousRoute); + _setCurrentRouteNameAsTransaction(previousRoute); + _addBreadcrumb( type: 'didPop', from: route.settings, @@ -147,7 +162,11 @@ class SentryNavigatorObserver extends RouteObserver> { ?.name; } - Future _setCurrentRoute(Route? route) async { + Future _setCurrentRouteName(Route? route) async { + _currentRouteName = _getRouteName(route); + } + + Future _setCurrentRouteNameAsTransaction(Route? route) async { final name = _getRouteName(route); if (name == null) { return; diff --git a/flutter/test/event_processor/flutter_enricher_event_processor_test.dart b/flutter/test/event_processor/flutter_enricher_event_processor_test.dart index bfccb8c773..4e748b2934 100644 --- a/flutter/test/event_processor/flutter_enricher_event_processor_test.dart +++ b/flutter/test/event_processor/flutter_enricher_event_processor_test.dart @@ -322,6 +322,24 @@ void main() { .length; expect(ioEnricherCount, 1); }); + + testWidgets('adds SentryNavigatorObserver.currentRouteName as app.screen', + (tester) async { + final observer = SentryNavigatorObserver(); + final route = + fixture.route(RouteSettings(name: 'fixture-currentRouteName')); + observer.didPush(route, null); + + final eventWithContextsApp = + SentryEvent(contexts: Contexts(app: SentryApp())); + + final enricher = fixture.getSut( + binding: () => tester.binding, + ); + final event = await enricher.apply(eventWithContextsApp); + + expect(event?.contexts.app?.viewNames, ['fixture-currentRouteName']); + }); }); } @@ -342,6 +360,11 @@ class Fixture { )..reportPackages = reportPackages; return FlutterEnricherEventProcessor(options); } + + PageRoute route(RouteSettings? settings) => PageRouteBuilder( + pageBuilder: (_, __, ___) => Container(), + settings: settings, + ); } void loadTestPackage() { diff --git a/flutter/test/sentry_navigator_observer_test.dart b/flutter/test/sentry_navigator_observer_test.dart index 8ef61b1398..c49588ab85 100644 --- a/flutter/test/sentry_navigator_observer_test.dart +++ b/flutter/test/sentry_navigator_observer_test.dart @@ -367,6 +367,73 @@ void main() { expect(scope.span, span); }); }); + + test('didPush sets current route name', () { + const name = 'Current Route'; + final currentRoute = route(RouteSettings(name: name)); + + const op = 'navigation'; + final hub = _MockHub(); + final span = getMockSentryTracer(name: name); + when(span.context).thenReturn(SentrySpanContext(operation: op)); + _whenAnyStart(hub, span); + + final sut = fixture.getSut( + hub: hub, + autoFinishAfter: Duration(seconds: 5), + ); + + sut.didPush(currentRoute, null); + + expect(SentryNavigatorObserver.currentRouteName, 'Current Route'); + }); + + test('didReplace sets new route name', () { + const oldRouteName = 'Old Route'; + final oldRoute = route(RouteSettings(name: oldRouteName)); + const newRouteName = 'New Route'; + final newRoute = route(RouteSettings(name: newRouteName)); + + const op = 'navigation'; + final hub = _MockHub(); + final span = getMockSentryTracer(name: oldRouteName); + when(span.context).thenReturn(SentrySpanContext(operation: op)); + _whenAnyStart(hub, span); + + final sut = fixture.getSut( + hub: hub, + autoFinishAfter: Duration(seconds: 5), + ); + + sut.didPush(oldRoute, null); + sut.didReplace(newRoute: newRoute, oldRoute: oldRoute); + + expect(SentryNavigatorObserver.currentRouteName, 'New Route'); + }); + + test('popRoute sets previous route name', () { + const oldRouteName = 'Old Route'; + final oldRoute = route(RouteSettings(name: oldRouteName)); + const newRouteName = 'New Route'; + final newRoute = route(RouteSettings(name: newRouteName)); + + const op = 'navigation'; + final hub = _MockHub(); + final span = getMockSentryTracer(name: oldRouteName); + when(span.context).thenReturn(SentrySpanContext(operation: op)); + when(span.status).thenReturn(null); + _whenAnyStart(hub, span); + + final sut = fixture.getSut( + hub: hub, + autoFinishAfter: Duration(seconds: 5), + ); + + sut.didPush(oldRoute, null); + sut.didPop(newRoute, oldRoute); + + expect(SentryNavigatorObserver.currentRouteName, 'Old Route'); + }); }); group('RouteObserverBreadcrumb', () {