search

DataDog / dd-sdk-android

Datadog SDK for Android (Compatible with Kotlin and Java)
Apache License 2.0
150 stars 59 forks source link

`DdRumContentProvider` added to manifest despite not using RUM feature #1751

Open lwasyl opened 10 months ago

lwasyl commented 10 months ago

Describe what happened After updating to 2.3.0 (from 1.19.3) I noticed that a provider was added to the apk: com.datadog.android.rum.DdRumContentProvider. The only two DD dependencies that I added to the project are com.datadoghq:dd-sdk-android-trace and com.datadoghq:dd-sdk-android-okhttp

I'm reporting this as a bug since the 1.x -> 2.x migration document suggests that

All relevant products (RUM, Trace, Logs, etc.) are now extracted into different modules. That allows you to integrate only what is needed into your application.

Since I'm not using RUM, I don't expect any RUM classes being pulled into the project.

If this is not a bug, then I'd ask for clarification whether it's safe to explicitly remove this provider even though it's added by the library automatically

Additional context

xgouchet commented 10 months ago

Hi @lwasyl and thanks for this question.

This is not a bug, but mostly a side effect of using the com.datadoghq:dd-sdk-android-okhttp dependency, which provides OkHttp integration for both RUM and Trace features.

The DdRumContentProvider is mostly a no-op provider that simply measures the timestamp and reason for the. process startup. It's then used by RUM features, but you can safely remove it from your build if you need to.

lwasyl commented 10 months ago

Thank you, it wasn't clear to me that okhttp is a common artifact that bundles integration related to okhttp for all features :+1:

In that case I might suggest to expose the RUM dependency as api in the okhttp, since right now removing the attribute is tricky as IDE doesn't see RUM classes. However I see that RUM classes are part of public API of e.g. DatadogInterceptor so exposing them would make sense overall.

Would you consider separating artifacts for okhttp-tracing and okhttp-rum integrations separately? TracingInterceptor doesn't seem to depend on RUM by itself

xgouchet commented 10 months ago

In that case I might suggest to expose the RUM dependency as api in the okhttp, since right now removing the attribute is tricky as IDE doesn't see RUM classes. However I see that RUM classes are part of public API of e.g. DatadogInterceptor so exposing them would make sense overall.

That's a good point, I'll add it to our backlog indeed

Would you consider separating artifacts for okhttp-tracing and okhttp-rum integrations separately? TracingInterceptor doesn't seem to depend on RUM by itself

That's another solution, which goes in a direction that we think is too verbose, especially since the RumInterceptor relis on the TracingInterceptor, so those two are kinda coupled.