DataDog / dd-sdk-reactnative

Datadog SDK for ReactNative
Apache License 2.0
114 stars 41 forks source link

Allow DD initialization from the native side using DdSdkConfiguration #653

Open ddiachkov opened 4 months ago

ddiachkov commented 4 months ago

Feature description

Hello! We have an app with a lot of native code that initializes before the JS side and runs in the background. The native code needs the ability to send logs to DD, and so the SDK must be initialized on application start.

Recently dd-sdk-reactnative added the new DdSdkNativeInitialization.initFromNative API to load configuration from JSON file. Unfortunately, it is a bit limiting since JSON configurations need to be stored in the repo, and it doesn't support environment variables or build environments.

Proposed solution

It would be nice to be able to init dd-sdk-reactnative with just DdSdkConfiguration instance. That would fix the limitations since we can generate it in the runtime using BuildConfig for tokens.

Here is the patch I ended up with:

diff --git a/node_modules/@datadog/mobile-react-native/android/src/main/kotlin/com/datadog/reactnative/DdSdkNativeInitialization.kt b/node_modules/@datadog/mobile-react-native/android/src/main/kotlin/com/datadog/reactnative/DdSdkNativeInitialization.kt
index 1b352a1..d6e6028 100644
--- a/node_modules/@datadog/mobile-react-native/android/src/main/kotlin/com/datadog/reactnative/DdSdkNativeInitialization.kt
+++ b/node_modules/@datadog/mobile-react-native/android/src/main/kotlin/com/datadog/reactnative/DdSdkNativeInitialization.kt
@@ -345,6 +345,24 @@ class DdSdkNativeInitialization internal constructor(
                 )
             }
         }
+
+        /**
+         * Initializes the Datadog React Native SDK using the provided configuration.
+         */
+        @JvmStatic
+        fun initFromNative(appContext: Context, ddSdkConfiguration: DdSdkConfiguration) {
+            val nativeInitialization = DdSdkNativeInitialization(appContext.applicationContext)
+            try {
+                nativeInitialization.initialize(ddSdkConfiguration)
+            } catch (@Suppress("TooGenericExceptionCaught") error: Exception) {
+                Log.w(
+                    DdSdkNativeInitialization::class.java.canonicalName,
+                    "Failed to initialize the Datadog SDK: $error"
+                )
+            }
+        }
     }
 }

Alternatively, making internal constructor public also works.

Other relevant information

For more context, this is how my whole setup works:

  1. On application start, I initialize a new DdSdkConfiguration with settings based on runtime / build environment.
  2. Then I initialize dd-sdk-reactnative via DdSdkNativeInitialization.initFromNative with DdSdkConfiguration.
  3. I also have a native module that provides access to the current DdSdkConfiguration to the JS side (it generates datadog-configuration.json from DdSdkConfiguration).
  4. When JS side starts, I initialize DatadogProvider like this:
const config = new FileBasedConfiguration({
    configuration: NativeModules.Telemetry.datadogConfiguration(),
});

<DatadogProvider configuration={config}>{children}</DatadogProvider>
marco-saia-datadog commented 4 months ago

Hey @ddiachkov, thank you for your great feedback!

I agree that we should allow passing a configuration to the native initialiser, as it can be useful in many different use cases.

We will evaluate this feature soon to work on a solid solution.

At first glance, I believe that we should allow you to retrieve the configuration from our SDK module itself, so that you can use it on the JS side if you have to. On the other hand, I would also consider refactoring the current implementation to avoid having to maintain two configurations, as it would be counterintuitive and possibly unsafe.

We will look into this and keep you updated on this thread.

Thank you again for your contribution!

kagrawal98 commented 3 weeks ago

+1 it will be really helpful to have this support added