Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Den-creator opened this issue Mar 19, 2024 · 21 comments
Open

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

Den-creator opened this issue Mar 19, 2024 · 21 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@Den-creator
Copy link

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...

@Den-creator Den-creator added the question Further information is requested label Mar 19, 2024
@fuzzybinary
Copy link
Collaborator

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.

@fuzzybinary fuzzybinary added the enhancement New feature or request label Mar 19, 2024
@Den-creator
Copy link
Author

@fuzzybinary thank you for you reply !

@dballance
Copy link
Contributor

dballance commented Apr 22, 2024

@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
Copy link
Collaborator

@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
Copy link
Contributor

dballance commented Apr 22, 2024

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Author

Cool. Thank you !

fuzzybinary added a commit that referenced this issue Jun 24, 2024
Under certain circumstances, Flutter can create two FlutterEngines which each have their own method channels. If this happens, we could end up in a situation where RUM and Logs were not properly attached to a MethodChannel, resulting in error messages and lost calls.

We're removing the `DatadogRumPlugin` and `DatadogLogsPlugin` singletons and replacing them with instances to avoid this.  Both plugins will attach to existing Datadog Logs and RUM instances during initialization if they have already been inititalized.

However, this breaks our current mapper implementation. Since Datadog expects the mapper during initialization, and holds it during the life of the application, we have to move the mappers to a companion object of the `*Plugin` classes to avoid issues with multiple or disconnected `MethodChannels`.

This potentially also helps us with multiple isolate tracking, but is just the first step of that.

refs: #596 #580 RUM-491 RUM-4438
@RustamG
Copy link

RustamG commented Jul 23, 2024

Hi @fuzzybinary!
Is there anything we can do to make logging working in a background isolate? I'm getting the same error as #580 (comment):

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
Copy link
Collaborator

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
Copy link

RustamG commented Jul 24, 2024

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
Copy link

RustamG commented Jul 24, 2024

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 added a commit that referenced this issue Jul 24, 2024
When creating a logger from a background isolate, Flutter would throw because WidgetsFlutterBinding wasn't initialized. This is specifically because we were adding a callback handler, which is not supported from background isolates.

Add checks to both logs and RUM and prevent initialization of things we know won't work from background isolates.

This is not full support for background isolates but usable as a work around.

refs: #580
@fuzzybinary
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Collaborator

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

@dballance
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants