dd-sdk-flutter icon indicating copy to clipboard operation
dd-sdk-flutter copied to clipboard

RUM loses current view after showModalBottomSheet(useRootNavigator: true) closes

Open tshedor opened this issue 11 months ago • 5 comments

Describe the bug

👋 hope y'all are doing well.

RUM no longer tracks the current view after a await showModalBottomSheet(useRootNavigator: true) is popped from the context.

I tried to build a workaround by storing the current RUM view and reapplying it after the invocation returns, but I don't think the current view is exposed publicly.

The error logs are reported on subsequent RUM actions, such as button taps. The error logs stop reporting on a new navigation event (push, go, pushReplacement) as RUM is seemingly restored to a current view.

Reproduction steps

This modifies the simple example:

app.dart becomes:
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2022-Present Datadog, Inc.

import 'package:datadog_flutter_plugin/datadog_flutter_plugin.dart';
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';
import 'package:graphql_flutter/graphql_flutter.dart';

import 'main_screen.dart';
import 'screens/crash_screen.dart';
import 'screens/graph_ql_screen.dart';
import 'screens/network_screen.dart';

final navigationKey = GlobalKey<NavigatorState>();

class MyApp extends StatelessWidget {
  final GraphQLClient graphQLClient;

  MyApp({Key? key, required this.graphQLClient}) : super(key: key);

  final router = GoRouter(
    observers: [DatadogNavigationObserver(datadogSdk: DatadogSdk.instance)],
    initialLocation: '/home',
    navigatorKey: navigationKey,
    routes: [
      StatefulShellRoute.indexedStack(
        parentNavigatorKey: navigationKey,
        builder:
            (BuildContext context, GoRouterState state, StatefulNavigationShell navigationShell) {
          // Return the widget that implements the custom shell (in this case
          // using a BottomNavigationBar). The StatefulNavigationShell is passed
          // to be able access the state of the shell and to navigate to other
          // branches in a stateful way.
          return ScaffoldWithNavBar(navigationShell: navigationShell);
        },
        // #enddocregion configuration-builder
        // #docregion configuration-branches
        branches: <StatefulShellBranch>[
          // The route branch for the first tab of the bottom navigation bar.
          StatefulShellBranch(
            observers: [DatadogNavigationObserver(datadogSdk: DatadogSdk.instance)],
            routes: <RouteBase>[
              GoRoute(
                path: '/home',
                builder: (context, state) {
                  return const MyHomePage(title: 'Home');
                },
              ),
            ],
            // To enable preloading of the initial locations of branches, pass
            // 'true' for the parameter `preload` (false is default).
          ),
          // #enddocregion configuration-branches

          // The route branch for the second tab of the bottom navigation bar.
          StatefulShellBranch(
            observers: [DatadogNavigationObserver(datadogSdk: DatadogSdk.instance)],
            // It's not necessary to provide a navigatorKey if it isn't also
            // needed elsewhere. If not provided, a default key will be used.
            routes: <RouteBase>[
              GoRoute(
                path: '/graphql',
                builder: (context, state) {
                  return const GraphQlScreen();
                },
              ),
            ],
          ),
        ],
      ),
    ],
  );

  @override
  Widget build(BuildContext context) {
    return GraphQLProvider(
      client: ValueNotifier<GraphQLClient>(graphQLClient),
      child: RumUserActionDetector(
        rum: DatadogSdk.instance.rum,
        child: MaterialApp.router(
          title: 'Flutter Demo',
          theme: ThemeData.from(
            colorScheme: ColorScheme.fromSwatch(primarySwatch: Colors.deepPurple),
          ),
          routerConfig: router,
        ),
      ),
    );
  }
}

/// Builds the "shell" for the app by building a Scaffold with a
/// BottomNavigationBar, where [child] is placed in the body of the Scaffold.
class ScaffoldWithNavBar extends StatelessWidget {
  /// Constructs an [ScaffoldWithNavBar].
  const ScaffoldWithNavBar({
    required this.navigationShell,
    Key? key,
  }) : super(key: key ?? const ValueKey<String>('ScaffoldWithNavBar'));

  /// The navigation shell and container for the branch Navigators.
  final StatefulNavigationShell navigationShell;

  // #docregion configuration-custom-shell
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      // The StatefulNavigationShell from the associated StatefulShellRoute is
      // directly passed as the body of the Scaffold.
      body: navigationShell,
      bottomNavigationBar: BottomNavigationBar(
        // Here, the items of BottomNavigationBar are hard coded. In a real
        // world scenario, the items would most likely be generated from the
        // branches of the shell route, which can be fetched using
        // `navigationShell.route.branches`.
        items: const <BottomNavigationBarItem>[
          BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Section A'),
          BottomNavigationBarItem(icon: Icon(Icons.work), label: 'Section B'),
        ],
        currentIndex: navigationShell.currentIndex,
        // Navigate to the current location of the branch at the provided index
        // when tapping an item in the BottomNavigationBar.
        onTap: (int index) => navigationShell.goBranch(index),
      ),
    );
  }
  // #enddocregion configuration-custom-shell

  /// NOTE: For a slightly more sophisticated branch switching, change the onTap
  /// handler on the BottomNavigationBar above to the following:
  /// `onTap: (int index) => _onTap(context, index),`
  // ignore: unused_element
  void _onTap(BuildContext context, int index) {
    // When navigating to a new branch, it's recommended to use the goBranch
    // method, as doing so makes sure the last navigation state of the
    // Navigator for the branch is restored.
    navigationShell.goBranch(
      index,
      // A common pattern when using bottom navigation bars is to support
      // navigating to the initial location when tapping the item that is
      // already active. This example demonstrates how to support this behavior,
      // using the initialLocation parameter of goBranch.
      initialLocation: index == navigationShell.currentIndex,
    );
  }
}

The go_router code was borrowed from the stateful shell example.

pubspec.yaml becomes (this unpins go_router, which is 7 major versions behind stable)
name: test_app
description: A new Flutter project.
publish_to: "none"
version: 1.0.0+1

environment:
  sdk: ">=3.3.0 <4.0.0"

dependencies:
  flutter:
    sdk: flutter
  flutter_web_plugins:
    sdk: flutter

  cupertino_icons: ^1.0.2
  datadog_flutter_plugin:
  datadog_tracking_http_client:
  datadog_webview_tracking:
  datadog_gql_link:

  go_router:
  path_provider: ^2.1.0
  flutter_dotenv: ^5.1.0
  transparent_image: ^2.0.1
  graphql_flutter: ^5.1.2

dev_dependencies:
  flutter_test:
    sdk: flutter
  flutter_lints: ">= 1.0.0"

flutter:
  uses-material-design: true

  assets:
    - .env
named_screen.dart becomes:
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2023-Present Datadog, Inc.
import 'package:datadog_flutter_plugin/datadog_flutter_plugin.dart';
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

import 'unnamed_screen.dart';

class NamedScreen extends StatefulWidget {
  const NamedScreen({Key? key}) : super(key: key);

  @override
  State<NamedScreen> createState() => _NamedScreenState();
}

class _NamedScreenState extends State<NamedScreen> with RouteAware, DatadogRouteAwareMixin {
  void _onNextScreen() {
    Navigator.of(context).push(MaterialPageRoute(builder: (context) => const UnnamedScreen()));
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('Unnamed Screen')),
      body: Column(
        crossAxisAlignment: CrossAxisAlignment.center,
        mainAxisAlignment: MainAxisAlignment.center,
        children: [
          ElevatedButton(
            onPressed: () async {
              await showModalBottomSheet(
                context: context,
                routeSettings: const RouteSettings(name: 'Bottomsheet Dialog'),
                useRootNavigator: true,
                builder: (context) {
                  return SafeArea(
                    child: Row(
                      children: [
                        IconButton(
                            key: const Key('REJECT BY ICON'),
                            icon: const Icon(Icons.close),
                            onPressed: () {
                              DatadogSdk.instance.rum?.addAction(
                                RumActionType.tap,
                                'Datadog test',
                              );
                            }),
                        IconButton(
                          key: const Key('ACCEPT BY ICON KEY'),
                          icon: const Icon(Icons.done),
                          tooltip: 'MODAL ACCEPT BY ICON',
                          onPressed: () => {
                            context.pop(),
                          },
                        )
                      ],
                    ),
                  );
                },
              );
            },
            child: const Text('Open Bottom Sheet'),
          ),
          ElevatedButton(
            key: const Key('REJECT BY ICON'),
            onPressed: () {
              DatadogSdk.instance.rum?.addAction(
                RumActionType.tap,
                'Datadog test',
              );
            },
            child: Text('Subsequent Event'),
          ),
        ],
      ),
    );
  }
}
  1. Replace simple_example files with the code above
  2. Run dart pub upgrade to update go_router
  3. Run app
  4. Tap "Named Test"
  5. Tap "Open Bottom Sheet"
  6. Close bottom sheet
  7. Tap "Subsequent Event" and observe logs

SDK logs

flutter: [DATADOG SDK] 🐶 → 11:47:43.490 ⚠️ RUMAddUserActionCommand(time: 2025-01-15 19:47:43 +0000, attributes: [:], canStartBackgroundView: true, isUserInteraction: true, instrumentation: DatadogRUM.InstrumentationType.manual, actionType: DatadogRUM.RUMActionType.tap, name: "Button(Subsequent Event)", missedEventType: Optional(DatadogRUM.SessionEndedMetric.MissedEventType.action)) was detected, but no view is active. To track views automatically, configure
`RUM.Configuration.uiKitViewsPredicate` or use `.trackRUMView()` modifier in SwiftUI. You can also track views manually
with `RUMMonitor.shared().startView()` and `RUMMonitor.shared().stopView()`.
flutter: [DATADOG SDK] 🐶 → 11:47:43.507 ⚠️ RUMAddUserActionCommand(time: 2025-01-15 19:47:43 +0000, attributes: [:], canStartBackgroundView: true, isUserInteraction: true, instrumentation: DatadogRUM.InstrumentationType.manual, actionType: DatadogRUM.RUMActionType.tap, name: "Datadog test", missedEventType: Optional(DatadogRUM.SessionEndedMetric.MissedEventType.action)) was detected, but no view is active. To track views automatically, configure
`RUM.Configuration.uiKitViewsPredicate` or use `.trackRUMView()` modifier in SwiftUI. You can also track views manually
with `RUMMonitor.shared().startView()` and `RUMMonitor.shared().stopView()`.

Expected behavior

Maintain tracking of the current view so that RUM events are attached to a view

Affected SDK versions

2.10.0

Latest working SDK version

No response

Did you confirm if the latest SDK version fixes the bug?

Yes

Flutter Version

3.27.2

Setup Type

Flutter iOS with go_router (14.6.3)

Device Information

iOS 18.1, online, simulator iPhone 16 Pro

Other relevant information

useRootNavigator is used for showing bottom sheets over tabs when using go_router's StatefulShellRoutes.

RumUserActionDetector is not used.

go_router parent navigation keys are used so that all routes communicate through a single, accessible key

tshedor avatar Jan 15 '25 19:01 tshedor

Hi @tshedor thanks for the detailed report!

I'll take a look, but my guess here is that the bottom sheet isn't calling into the NavigationObserver as part of a route change, but I'll see if I can confirm that.

fuzzybinary avatar Jan 15 '25 19:01 fuzzybinary

@fuzzybinary Thanks for digging into it. That's an interesting idea...although it would be odd for first-party features to be ignored like that. I am using multiple observers that reference the same Datadog SDK since each stateful branch is, AFAIK, its own version of a go router. I don't know if this is advised use, but it is reporting the routes accurately to RUM

tshedor avatar Jan 16 '25 19:01 tshedor

@tshedor I think part of this is useRootNavigator. It looks like (to me) that the root navigator keeps a separate history stack.

We use the route and previousRoute parameters in DatadogNavigationObserver to figure out where you're popping back to, since things like replace or popping multiple pages is possible. The problem is, since the root navigator has it's own history stack, the previous route is not what you expect in either the push or pop cases.

In the example you provided, when we push the bottom sheet, the previousRoute is not the NamedScreen, it's an unnamed route specific to the root navigator. Therefore, when you pop, we get that same unnamed route, not NamedScreen.

I'll have to think about how to solve this in a general case since Datadog doesn't really have a concept of multiple routers / navigators or multiple views being active at the moment, and I'm not sure we're told which navigator is performing the route change, but I'll look into it.

One (obviously not great) workaround is to restart the view manually once the bottomsheet closes:

await showModalBottomSheet(
  // ...
).then((_) {
  DatadogSdk.instance.rum?.startView('NamedScreen');
});

fuzzybinary avatar Jan 21 '25 17:01 fuzzybinary

@fuzzybinary thanks for the write-up. That all makes sense, especially since it would be unusual to track multiple routers. Does RUM keep track of open modals? This would effectively be a modal route on top of the existing view. I'll do the manual start view for now. Thanks again for thinking through this.

tshedor avatar Jan 23 '25 02:01 tshedor

@tshedor RUM currently doesn't track the current "view stack" (for lack of a better term) mostly because I was worried about the fragility of a separate mechanism on top of what we're being told by the NavigationObserver.

That said, given the situation with the root navigator, it may be necessary for us to keep some sort of state in the navigation observer, and at least detect use of the root navigator. I'll keep this bug open for now to see if it's something we can support.

fuzzybinary avatar Jan 23 '25 13:01 fuzzybinary