Skip to content

RUM-13441: [Cronet] Migrating okHttp to new instrumentation API#3180

Merged
satween merged 5 commits intodevelopfrom
tvaleev/feature/RUM-13441-Okhttp-instrumentation
Apr 16, 2026
Merged

RUM-13441: [Cronet] Migrating okHttp to new instrumentation API#3180
satween merged 5 commits intodevelopfrom
tvaleev/feature/RUM-13441-Okhttp-instrumentation

Conversation

@satween
Copy link
Copy Markdown
Contributor

@satween satween commented Feb 16, 2026

What does this PR do?

  • Moves OkHttp instrumentation to the new networking-library agnostic approach.
  • Introduces a new plugin-based instrumentation API for OkHttp network monitoring, replacing the legacy DatadogInterceptor/TracingInterceptor/DatadogEventListener
    approach (all classes kept for the backward compatibility)
    with a single unified entry point:
OkHttpClient.Builder()
  .configureDatadogInstrumentation(
      rumInstrumentationConfiguration = RumNetworkInstrumentationConfiguration(),
      apmInstrumentationConfiguration = ApmNetworkInstrumentationConfiguration(tracedHosts)
  )
  .build()

same as for Cronet

CronetEngine.Builder()
  .configureDatadogInstrumentation(
      rumInstrumentationConfiguration = RumNetworkInstrumentationConfiguration(),
      apmInstrumentationConfiguration = ApmNetworkInstrumentationConfiguration(tracedHosts)
  )
  .build()

Motivation

  • Moving instrumentation logic into library-agnostic approach
  • Simplifying and unifying instrumentation configuration for all networking libraries
  • Making Datadog instrumentation configuration latest in the configuration chain

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@satween satween requested a review from a team as a code owner February 16, 2026 22:10
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 57939dbb75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@satween satween force-pushed the tvaleev/feature/RUM-13974-APM-DNI branch from 8f048a3 to 91ec6c5 Compare February 17, 2026 03:18
@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 57939db to 404f86e Compare February 17, 2026 03:23
@satween satween force-pushed the tvaleev/feature/RUM-13974-APM-DNI branch 6 times, most recently from cb8e557 to 62046e7 Compare February 20, 2026 17:52
@satween satween requested a review from a team as a code owner February 26, 2026 09:44
@satween satween force-pushed the tvaleev/feature/RUM-13974-APM-DNI branch 2 times, most recently from d4a5e23 to f3b2a1e Compare February 27, 2026 10:55
Base automatically changed from tvaleev/feature/RUM-13974-APM-DNI to develop February 27, 2026 13:30
@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 404f86e to bc7f3d2 Compare March 2, 2026 16:44
@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from bc7f3d2 to 9e872b7 Compare March 11, 2026 13:47
@0xnm
Copy link
Copy Markdown
Member

0xnm commented Mar 12, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9e872b76aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 9e872b7 to 175ce9d Compare March 24, 2026 16:20
@satween
Copy link
Copy Markdown
Contributor Author

satween commented Mar 24, 2026

@codex review

@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 175ce9d to 3e13750 Compare March 24, 2026 16:42
@satween satween requested review from 0xnm and kikoveiga March 24, 2026 16:42
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 175ce9ddbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 3e13750 to c200358 Compare March 25, 2026 10:45
@datadog-prod-us1-5

This comment has been minimized.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 25, 2026

Codecov Report

❌ Patch coverage is 90.79903% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.00%. Comparing base (a1fc7c4) to head (5d0a67d).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
.../datadog/android/okhttp/OkHttpIntegrationPlugin.kt 84.75% 5 Missing and 4 partials ⚠️
...okhttp/internal/ApmInstrumentationOkHttpAdapter.kt 70.97% 7 Missing and 2 partials ⚠️
...roid/rum/internal/net/RumNetworkInstrumentation.kt 50.00% 0 Missing and 3 partials ⚠️
.../datadog/android/cronet/CronetIntegrationPlugin.kt 92.11% 1 Missing and 2 partials ⚠️
...okhttp/internal/RumInstrumentationOkHttpAdapter.kt 92.68% 1 Missing and 2 partials ⚠️
...adog/android/okhttp/internal/OkHttpResponseInfo.kt 75.00% 0 Missing and 2 partials ⚠️
...oid/okhttp/internal/RequestTracingStateRegistry.kt 95.12% 0 Missing and 2 partials ⚠️
...mResourceAttributesProviderCompatibilityAdapter.kt 50.00% 0 Missing and 2 partials ⚠️
...id/trace/ApmNetworkInstrumentationConfiguration.kt 66.67% 0 Missing and 1 partial ⚠️
...ndroid/trace/internal/ApmNetworkInstrumentation.kt 75.00% 1 Missing ⚠️
... and 3 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3180      +/-   ##
===========================================
+ Coverage    71.98%   72.00%   +0.02%     
===========================================
  Files          948      956       +8     
  Lines        34952    35286     +334     
  Branches      5804     5858      +54     
===========================================
+ Hits         25158    25406     +248     
- Misses        8224     8271      +47     
- Partials      1570     1609      +39     
Files with missing lines Coverage Δ
.../android/trace/internal/net/RequestTracingState.kt 100.00% <100.00%> (ø)
...datadog/android/trace/internal/net/TraceContext.kt 100.00% <ø> (ø)
...g/android/cronet/internal/CronetRequestCallback.kt 87.27% <100.00%> (ø)
...onet/internal/CronetRequestFinishedInfoListener.kt 90.00% <ø> (-5.00%) ⬇️
.../android/okhttp/internal/CompositeEventListener.kt 100.00% <100.00%> (ø)
...tadog/android/okhttp/internal/OkHttpRequestInfo.kt 83.78% <100.00%> (+2.70%) ⬆️
...d/okhttp/internal/RegistryTrackingEventListener.kt 100.00% <100.00%> (ø)
...id/okhttp/internal/graphql/OkHttpGraphQLAdapter.kt 65.96% <100.00%> (ø)
...id/trace/ApmNetworkInstrumentationConfiguration.kt 73.42% <66.67%> (-1.28%) ⬇️
...ndroid/trace/internal/ApmNetworkInstrumentation.kt 60.98% <75.00%> (-0.50%) ⬇️
... and 11 more

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from c200358 to daf6d71 Compare April 7, 2026 13:21
@satween satween requested review from 0xnm and kikoveiga April 7, 2026 13:22
@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 09b1282 to 2a39a00 Compare April 8, 2026 14:20
registry: RequestTracingStateRegistry,
rumNetworkInstrumentation: RumNetworkInstrumentation?
) {
val userEventListenerFactory: EventListener.Factory = build().eventListenerFactory
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that we will build OkHttpClient twice? OkHttpClient.Builder.build() here and then somewhere else as well? Won't it leak resources with such throwaway OkHttpClient?

Copy link
Copy Markdown
Contributor Author

@satween satween Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is the trade-off we have to pay for the convenience of the public API we provide for the user.
The alternatives is less stable imo, as there is only two options:

  1. Reflection-based approach
  2. Overriding the customer's event listener with a separate method in the OkHttpIntegrationPlugin

I believe this approach is way less impactful.

Regarding the resources - as builded client will be never used - I guess we are safe

@satween satween force-pushed the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch from 2a39a00 to c562706 Compare April 14, 2026 12:21

private fun OkHttpClient.Builder.withMethodUrlAndBodyReplacementInterceptor(
url: String,
body: okhttp3.RequestBody
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nit

Suggested change
body: okhttp3.RequestBody
body: RequestBody

@satween satween merged commit 927fe56 into develop Apr 16, 2026
27 checks passed
@satween satween deleted the tvaleev/feature/RUM-13441-Okhttp-instrumentation branch April 16, 2026 08:49
@ExperimentalRumApi
@ExperimentalTraceApi
@Suppress("PackageNameVisibility")
internal fun OkHttpClient.Builder.configureDatadogInstrumentation(
Copy link
Copy Markdown
Contributor

@hamorillo hamorillo Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Sample App is using this method but now it's not visible externally, so SampleApp doesn't compile.

e: file:...dd-sdk-android/sample/kotlin/src/main/kotlin/com/datadog/android/sample/SampleApplication.kt:104:10 Cannot access 'configureDatadogInstrumentation': it is internal in 'com.datadog.android.okhttp'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants