DataDog / dd-sdk-flutter

Flutter bindings and tools for utilizing Datadog Mobile SDKs
Apache License 2.0
43 stars 42 forks source link

Is it not possible to use data dog from flutter isolate ? #580

Open Den-creator opened 6 months ago

Den-creator commented 6 months ago

Question

By calling DatadogSdk.instance.initialize in isolate I've receive such error:

[ERROR:flutter/runtime/dart_isolate.cc(1097)] Unhandled exception: 'package:flutter/src/services/platform_channel.dart': Failed assertion: line 542 pos 7: '_binaryMessenger != null || BindingBase.debugBindingType() != null': Cannot set the method call handler before the binary messenger has been initialized. This happens when you call setMethodCallHandler() before the WidgetsFlutterBinding has been initialized. You can fix this by either calling WidgetsFlutterBinding.ensureInitialized() before this or by passing a custom BinaryMessenger instance to MethodChannel().

0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61)

1 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)

2 MethodChannel.setMethodCallHandler (package:flutter/src/services/platform_channel.dart:542:7)

3 DatadogSdkMethodChannel.initialize (package:datadog_flutter_plugin/src/datadog_sdk_method_channel.dart:56:21)

4 DatadogSdk.initialize (package:datadog_flutter_plugin/datadog_flutter_plugin.dart:168:21)

But WidgetsFlutterBinding.ensureInitialized() can not be called in isolate...
fuzzybinary commented 6 months ago

Yes this is limitation of the Datadog SDK at the moment, you can only use it from the main isolate at the moment.

If you have a need for this, please raise it as a feature request. It's definitely doable we just haven't had any requests for the functionality.

Den-creator commented 6 months ago

@fuzzybinary thank you for you reply !

dballance commented 5 months ago

@fuzzybinary - We need this for our impl. We've been stuck back on the 3rd party plugin but need to upgrade now due to the Apple Privacy changes.

How can we request this change?

For clarity - we run a single FlutterEngine with two isolates. On Android - one isolate runs within a foreground service and runs even when the UI isolate is terminated. On iOS it's similar - but the runtime model for iOS means both isolates are always running.

We primarily log from one isolate (background) but that isolate is always the second isolate to be established.

I'm working now to upgrade to datadog_flutter_plugin. I don't remember the exact behavior we had last time we tried - but I vaguely remember it being that DD was already configured and wouldn't allow the second isolate to spin up with configuration.

This works fine with the older, 3rd party plugin (datadog_flutter). Both isolates will log appropriately. I'll report back if I can't get that working with datadog_flutter_plugin

fuzzybinary commented 5 months ago

@dballance Yes, I'll try to raise it in priority.

Because Datadog acts as a singleton, you should still only configure once from your primary isolate, but I'll look into ways you can communicate with that singleton from multiple isolates.

dballance commented 5 months ago

An update as I am working through the upgrade.

Atleast on Android - as long as I call DatadogSdk.instance.initialize in both isolates, I get logs from both. No obvious errors and logs do not indicate any DD SDK configuration errors.

This is required to ensure we can call DatadogSdk.instance.logs?.createLogger in both isolates. Should I be using the attachToExisting in the secondary isolate? We work similarly to the "app in app" in terms of how we're bootstrapped.

dballance commented 5 months ago

attachToExisting does seem to work for us. Will need to check on iOS to be sure (I think this is where the rub was last time). Hopefully I can get to that this afternoon and report back.

dballance commented 5 months ago

@fuzzybinary - finally got through this today. It seems attachToExisting - since we're just using logging - will work for us as it is.

One enhancement that would be nice - is wrapping any awaits for pending initialization into the the attachToExisting call. There is a race condition where a call to initialize may be in flight from one isolate, but not complete (somehow?), when the second isolate calls attachToExisting.

In my current impl to get this working, Isolate A calls initialize then starts Isolate B, which immediately tries to get the DD SDK running so it can attach logs. In this case, Isolate A awaits completion of the initialize call before it proceeds to "starting" Isolate B. Aside: Isolate A also starts Isolate B not via spawn but via a platform call to a host FlutterEngineGroup to start the FlutterEngine that runs Isolate B.

This seemed to work fine on Android - but on iOS the first call to attachToExisting from Isolate B always seemed to fail. I added in some recursion - call attachToExisting and if instance.logs is null, call it again until it returns an instance - and it seems to always attach on the second call here.

Could purely be some Swift shared resource thing, but would be nice to either return an obvious failure (throw or return a result that could be used here to understand that attachToExisting didn't find an "existing") AND potentially have the attachToExisting call wait on any pending initialize calls before returning.

May not be possible - but wanted to pass it along in case it was useful.

Thank you for your effort with datadog_flutter_plugin!

fuzzybinary commented 5 months ago

Hey @dballance

Thanks for all the excellent info. My guess here as to the source of your race condition is the wait on the platform channel to perform its operations for initialization. I'm not sure why attachToExisting returns null on that first try, but I'll look into it.

Likely the way I'll approach attaching from a new isolate and / or waiting for initialization is with a separate call, as attachToExisting is meant more for hybrid apps than for separate isolates. I'm thinking something like the following:

void startIsolateasync  {
  await Datadog.waitUntilInitialized(timeout: 1.0);
  var datadog = Datadog.attachIsolate()
}

This way, folks who don't immediately start all isolates don't have to perform the wait.

Let me know if you have other issues, and I'll let you know when I have a cleaner interface for this.

dballance commented 4 months ago

We've pushed into production after migrating with attachToExisting on our end and happily seeing logs from both iOS and Android across multiple isolates.

Know it's not ideal - but is working for us. Might still be issues in RUM or other parts of the SDK, but atleast for logging we're šŸŽ‰.

šŸ»

Den-creator commented 4 months ago

Cool. Thank you !

RustamG commented 2 months ago

Hi @fuzzybinary! Is there anything we can do to make logging working in a background isolate? I'm getting the same error as https://github.com/DataDog/dd-sdk-flutter/issues/580#issue-2194671381:

flutter: 'package:flutter/src/services/platform_channel.dart': Failed assertion: line 542 pos 7: '_binaryMessenger != null || BindingBase.debugBindingType() != null': Cannot set the method call handler before the binary messenger has been initialized. This happens when you call setMethodCallHandler() before the WidgetsFlutterBinding has been initialized. You can fix this by either calling WidgetsFlutterBinding.ensureInitialized() before this or by passing a custom BinaryMessenger instance to MethodChannel(). flutter: #0 _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61) flutter: #1 _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5) flutter: #2 MethodChannel.setMethodCallHandler (package:flutter/src/services/platform_channel.dart:542:7) flutter: #3 new DdLogsMethodChannel (package:datadog_flutter_plugin/src/logs/ddlogs_method_channel.dart:21:19) flutter: #4 DdLogsPlatform._instance (package:datadog_flutter_plugin/src/logs/ddlogs_platform_interface.dart:15:37) flutter: #5 DdLogsPlatform._instance (package:datadog_flutter_plugin/src/logs/ddlogs_platform_interface.dart) flutter: #6 DdLogsPlatform.instance (package:datadog_flutter_plugin/src/logs/ddlogs_platform_interface.dart:17:41) flutter: #7 DatadogLogging.createLogger (package:datadog_flutter_plugin/src/logs/ddlogs.dart:90:20) flutter: #8 isolateFunc (package:dd_with_isolates/main.dart:46:46)

Datadog version: 2.6.0 Flutter version: 3.16.4

Below is the minimal source code to reproduce the issue:


import 'dart:isolate';

import 'package:datadog_flutter_plugin/datadog_flutter_plugin.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

void main() async {
  WidgetsFlutterBinding.ensureInitialized();

  final clientToken = ''; // replace with yours
  final applicationId = ''; // replace with yours
  final env = ''; // replace with yours

  final configuration = DatadogConfiguration(
    clientToken: clientToken,
    env: env,
    site: DatadogSite.us5,
    nativeCrashReportEnabled: false,
    loggingConfiguration: DatadogLoggingConfiguration(),
    rumConfiguration: DatadogRumConfiguration(
      applicationId: applicationId,
    ),
  );

  await DatadogSdk.instance.initialize(configuration, TrackingConsent.granted);

  runApp(const MyApp());
}

void isolateFunc(RootIsolateToken rootIsolateToken) async {
  BackgroundIsolateBinaryMessenger.ensureInitialized(rootIsolateToken);

  await DatadogSdk.instance.attachToExisting(
    DatadogAttachConfiguration(
      detectLongTasks: false,
    ),
  );

  final logConfiguration = DatadogLoggerConfiguration(
    remoteLogThreshold: LogLevel.info,
    networkInfoEnabled: true,
    customConsoleLogFunction: null,
  );

  try {
    final logger = DatadogSdk.instance.logs!.createLogger(logConfiguration);
    logger.debug('test message from background isolate');
  } catch (e, st) {
    debugPrint(e.toString());
    debugPrint(st.toString());
  }
}

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatelessWidget {
  const MyHomePage({super.key});

  void _checkDatadogInIsolate() {
    final rootIsolateToken = RootIsolateToken.instance!;

    Isolate.spawn(isolateFunc, rootIsolateToken);
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
      ),
      body: Center(
        child: ElevatedButton(
          onPressed: _checkDatadogInIsolate,
          child: const Text('Spawn'),
        ),
      ),
    );
  }
}
fuzzybinary commented 2 months ago

Hi @RustamG ,

Adding official support still on my radar, but I was under the impression that attachToExisting was a usable workaround, and BackgroundIsolateBinaryMessenger.ensureInitialized should have initialized the BinaryMessenger to use that properly.

I'll take a look as soon as I have a chance.

RustamG commented 2 months ago

Thank you for reply.

Yep, as you can see in the example above, I have attachToExisting and BackgroundIsolateBinaryMessenger.ensureInitialized, but it doesn't help. Same error would be in case initialize is called instead of attachToExisting.

Is there any understanding on when this could be resolved? Or any ways to workaround in the meantime?

RustamG commented 2 months ago

I believe the error comes from this limitation: Unable to receive Platform Channel calls in background isolates. Currently native part calls Dart via method channel just to map the log event. As a quick solution is it possible to remove the setMethodCallHandler call if LogEventMapper is not set in the DatadogLoggingConfiguration?

fuzzybinary commented 2 months ago

Hey @RustamG ,

Yes that's the main issue -- I'm curious why @dballance didn't run into it. In reality, we should not attempt to set the callback handlers in background isolates ever. Here is a more complete fix.

As it says in the PR, this still isn't full background isolate support, but usable as a workaround for simple cases.

dballance commented 2 months ago

In my case, I'm not using Isolate.spawn. I am running two separate FlutterEngines in a shared FlutterEngineGroup.

A VERY high level overview is here: https://docs.flutter.dev/add-to-app/multiple-flutters#components

And a very simple example is here: https://github.com/flutter/samples/tree/main/add_to_app/multiple_flutters

Basically, I don't run into this issue and can use plugins from all isolates. There are a few caveats (not worth digging into those in this context, imo) but it's a fundamental difference from using Isolates for background tasks / processing (i.e. using Isolate.spawn).

Our app runs in the background, both on iOS and Android. Basically, on Android, we want some FlutterEngine to operate within an Android "Foreground Service", so that even if the main "app" is terminated (and thus the "UI Engine" as we call it) the background engine doing all non-UI work can continue to run. While not explicitly needed on iOS (due to runtime differences with Android), we do the same on iOS just to keep the overall setup simple.

Unrelated note, there is an issue I have in the our backlog where it seems DD logs quit working after some time on Android. It's likely something in this ballpark, but I hesitate to open an issue here until I can investigate our impl.

fuzzybinary commented 2 months ago

@dballance Did you update to the newest version includes this pr?

dballance commented 2 months ago

@dballance Did you update to the newest version includes this pr?

I have not - looks like it might resolve the unrelated note I added! We did merge 2.6.0 into staging via dependabot but I just haven't had time to look into it, as i've been out of office on paternity leave šŸ˜….

dballance commented 2 months ago

Will try to test in the next two weeks or so (still out part time) and let you know if it resolved our issue!

dballance commented 2 weeks ago

An update - I've upgraded, still potentially having some issues where logs are not updated for some android devices. However, I haven't had a chance to dive in and try to understand why. It is planned for our next release though, so I should have a better answer soon.

fuzzybinary commented 2 weeks ago

@dballance Keep me posted. If it turns out it's unrelated to background isolates (either related to multiple engines or something else entirely) let's open a separate ticket for better tracking.