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

Use consistent API schema between @datadog/mobile-react-navigation and @datadog/browser-rum #552

Open
leggomuhgreggo opened this issue Nov 8, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@leggomuhgreggo
Copy link

leggomuhgreggo commented Nov 8, 2023

Is your feature request related to a problem? Please describe.

Currently there is a significant amount of divergence between the config schema + method names in @datadog/mobile-react-navigation and @datadog/browser-rum — and this creates extra implementation / maintenance overhead for users who are working on cross-platform expo apps (Web, iOS, Android).

For example: every time there's an update to any of the datadog RUM packages, where config has changed, we need to review the configuration and figure out which entries are still valid between the different RUM client packages.

Describe the solution you'd like

It would be great if the configuration schema + method names were the same between the RUM clients — and where they are not the same, it would be great if the differentiation were clearer.

I've outlined the differences here:

API Mismatch Summary

Native- vs web-specific config

  • only relevant to particular platform <-- perfectly legit, but ideally distinct from possibly similar sounding options
    • eg nativeCrashReportEnabled
  • seemingly missing feature
    • eg version (recently added!)

Overlapping config

  • same key / same value <-- this is the dream 😄
  • different key / same value <-- requires some aliasing, but not too bad
    • eg service vs serviceName
  • same or slightly different key / slightly different value 🤕
    • eg firstPartyHosts vs allowedTracingOrigins or siteParameter vs site
  • different key / same value, BUT unclear if features actually overlap 🤔
    • eg trackErrors vs forwardErrorsToLogs

Misc Other Issues

  • Method mismatch - names but also sync vs async
  • Companion libraries eg @datadog/browser-logs @datadog/mobile-react-navigation
  • CI / SourceMap orchestration
  • constructor vs init

In particular, it'd be cool to align the schema for these properties

  • service vs serviceName
  • firstPartyHosts vs allowedTracingUrls

Describe alternatives you've considered

Initially I tried to create a single abstraction for RUM client config, but I realized that the properties changed too much and weren't safe to assume as equivalent.

So now I have a subset of core properties that are shared: env service applicationId clientToken version

And for everything else I have platform-specific config objects, so my abstraction looks approximately like this:

export const initDatadog = async () => {
  await customDatadog.init(
    {
      applicationId: process.env.DD_RUM_APPLICATION_ID,
      clientToken: process.env.DD_RUM_CLIENT_TOKEN,
      env: process.env.DD_RUM_ENVIRONMENT,
      serviceName: process.env.DD_SERVICE_NAME, // RN alias of `service`
      version: process.env.APP_VERSION_DATADOG,
    },
    {
      rumNative: {
        firstPartyHosts: TRACING_ORIGINS,
        sessionSamplingRate: 90,
        resourceTracingSamplingRate: 90,
        telemetrySampleRate: 90,
      },
      rumWeb: {
        allowedTracingUrls: TRACING_ORIGINS,
        sampleRate: 90,
        sessionReplaySampleRate: 90,
        telemetrySampleRate: 90,
      },
    },
  );
};

Thanks!

@leggomuhgreggo leggomuhgreggo added the enhancement New feature or request label Nov 8, 2023
@louiszawadzki
Copy link
Contributor

Hi @leggomuhgreggo, thanks for reaching out!

This is indeed something we're aware of, we're trying to have our web and mobile APIs match as much as possible, but there are indeed some of the cases you mentioned where they differ.

Bringing full support for react-native-web is high in our backlog and will very likely improve this as we'll expose a single interface for both RN and web, I can let you know when it is ready.

I'll also raise this internally so we can see which improvements we can make in the short term.

Finally, I can maybe share some insights on some discrepancies to help you understand better the differences.

firstPartyHosts (mobile) vs allowedTracingOrigins (web): allowedTracingOrigins includes the host of your website by default, but not firstPartyHosts - since we cannot guess it on mobile. So if you copy from web to mobile, be sure to add the host of your website to your firstPartyHosts as well if that is relevant. Yet I also agree that the naming is very different when the concept is quite close, so this could be improved.

trackErrors (mobile) vs forwardErrorsToLogs (web): trackErrors enables you to report JS errors and console.error calls in React Native, to both RUM and Logs. forwardErrorsToLogs enables you to forward console.error calls in the web to Logs.

I also believe the version and site attribute is available in both browser and RN SDKs: https://docs.datadoghq.com/real_user_monitoring/browser/#configuration

I hope this clarifies a few things, let me know if you have further questions on this topic :)

@leggomuhgreggo
Copy link
Author

Thank you for the thoughtful response!

Bringing full support for react-native-web is high in our backlog and will very likely improve this as we'll expose a single interface for both RN and web, I can let you know when it is ready.

Oh nice!!!

Finally, I can maybe share some insights on some discrepancies to help you understand better the differences ...I hope this clarifies a few things

Thanks for this — I didn't fully appreciate some of those nuances.

Sincerely appreciate the attention / consideration @louiszawadzki

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

No branches or pull requests

2 participants