[RUM-9902] Isolating implementation layer from integration modules#2773
Conversation
…ndencies from wear app
…d tracing builder
…raicer depeendencies
jonathanmos
left a comment
There was a problem hiding this comment.
Nice pr! I've left a few comments
…k version RUM-9902 - fix detekt and tests
[RUM 9903] Updating opentelemetry version
0xnm
left a comment
There was a problem hiding this comment.
I left some questions/comments, but overall lgtm.
This reverts commit 4cfff41 # Conflicts: # integrations/dd-sdk-android-okhttp-otel/src/main/kotlin/com/datadog/android/okhttp/otel/OkHttpExt.kt # integrations/dd-sdk-android-okhttp-otel/src/test/kotlin/com/datadog/android/okhttp/otel/OkHttpExtTest.kt
Co-authored-by: Nikita Ogorodnikov <[email protected]>
Co-authored-by: Nikita Ogorodnikov <[email protected]>
…g-unused-code' into tvaleev/feature/RUM-9902-removing-unused-code
…/android/okhttp/trace/TracingInterceptor.kt Co-authored-by: Nikita Ogorodnikov <[email protected]>
…g-unused-code' into tvaleev/feature/RUM-9902-removing-unused-code
There was a problem hiding this comment.
LGTM. I left two comments, one of them, regarding traceRateLimit, should be addressed before merging this PR.
Also I have a concern about SAMPLE_KEEP -> USER_KEEP change for the sampling priority. We need to check with APM spec if it may have any impact on the intake processing (e.g. metrics computation) or trace propagation.
Maybe it has impact only for the tracer -> agent -> intake setup, and since we are in the agent-less mode and setup is tracer -> intake, this doesn't change anything for us, but still it is better to double-check.
This can be done after merging PR.
What does this PR do?
The goal of this PR is to isolate the implementation from the API specification. The Datadog tracing integration is provided by the dd-trace-java library, which we moved into the dd-android-trace-internal module in a previous change.
We are now trying to finally eliminate
opentracingdependencies and are facing the following issues:opentracingAPI specification is used as an internal layer for module communication. For example, thedd-android-sdk-trace-httpmodule uses the following objects fromopentracing:Span,Scope, etc. The diagram below displays the existing architecture:As you might have noticed, there are a lot of modules that are aware of the specification classes and are bound to a specific implementation of the
opentracingspecification, such asAndroidTracing.A lot of downcasting logic can be found inside the integration module:
To solve the described issues and decouple the implementation from the internal and external specifications, we provide the following architecture:
This PR mainly includes:
dd-sdk-android-trace-apimodule was added to provide the internal API specification for trace functionality.dd-android-sdk-tracemodule is now the only point that knows both the specification and the implementation.opentracingAPI specification was replaced with thedd-sdk-android-trace-apifor all modules that used theopentracingspecification previously.4.All
apidependencies were replaced withimplementationdependencies for all modules except fordd-sdk-android-trace-api.TL;DR:
In this pull request, we isolated the specific implementation from the specification and removed all of the downcasting logic from the integration modules. Now, the only place where the implementation meets the specification is the dd-sdk-android-trace module.
Motivation
Otel requires API 26 or desugaring support
Most of the files in the
dd-sdk-android-trace-internaldirectory are written in Java because it adopts thedd-trace-javadirectory. Therefore, some Kotlin features, such as the internal modifier, will not work if they are made public. Additionally,dd-trace-javawas initially designed for a server-side agent and contains specific methods that are not applicable to Android, which could confuse customers.Additional Notes
Note that there is still dependency on opentracing and internal module mostly for tests. I decided to remove them in a separate PR to reduce amount of changes.
Review checklist (to be filled by reviewers)