Skip to content

Move Okhttp client initialization to background thread.#2824

Closed
Laimiux wants to merge 1 commit into
DataDog:developfrom
Laimiux:laimonas/lazy-okhttp-client
Closed

Move Okhttp client initialization to background thread.#2824
Laimiux wants to merge 1 commit into
DataDog:developfrom
Laimiux:laimonas/lazy-okhttp-client

Conversation

@Laimiux

@Laimiux Laimiux commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

Moving OkHttp Client initialization to the background thread.

Motivation

It's a standard practice to initialize the Okhttp client on the background thread. It's a pretty trivial thing to do since there already exists Call.Factory interface.

Additional Notes

#2809

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)

@Laimiux
Laimiux requested review from a team as code owners August 19, 2025 14:52
@0xnm

0xnm commented Aug 20, 2025

Copy link
Copy Markdown
Member

Hello @Laimiux! Thanks for your contribution.

Before moving forward, can you please tell what is the impact of the OkHttp initialization on the overall Datadog SDK initialization duration in your app?

As I mentioned here, for our sample app running on emulator it is around 1%.

For more context https://www.zacsweers.dev/dagger-party-tricks-deferred-okhttp-init/
OkHttpClient initialization can take upwards of 100ms on some Android devices in the wild due to its use of TrustManagerFactory. Initializing the Cache is also not free, as it creates an internal Executor and may incur some disk IO if you want to prepare a cache directory for it first.

Yes, but this article talks about generic setup, so it is a generic advice. Datadog SDK have a specific setup which doesn't use cache. SDK indeed is using TLS, so TrustManagerFactory will be invoked and the particular provider backing it up depends on the OEM (that is why its initialization time may vary), but on the chart you've posted here percentile is pretty much uniformed (so there are probably no spikes in the initialization duration for different OEMs due to runtime implementation issues).

@Laimiux

Laimiux commented Aug 20, 2025

Copy link
Copy Markdown
Contributor Author

Hello @Laimiux! Thanks for your contribution.

Before moving forward, can you please tell what is the impact of the OkHttp initialization on the overall Datadog SDK initialization duration in your app?

As I mentioned here, for our sample app running on emulator it is around 1%.

For more context https://www.zacsweers.dev/dagger-party-tricks-deferred-okhttp-init/
OkHttpClient initialization can take upwards of 100ms on some Android devices in the wild due to its use of TrustManagerFactory. Initializing the Cache is also not free, as it creates an internal Executor and may incur some disk IO if you want to prepare a cache directory for it first.

Yes, but this article talks about generic setup, so it is a generic advice. Datadog SDK have a specific setup which doesn't use cache. SDK indeed is using TLS, so TrustManagerFactory will be invoked and the particular provider backing it up depends on the OEM (that is why its initialization time may vary), but on the chart you've posted here percentile is pretty much uniformed (so there are probably no spikes in the initialization duration for different OEMs due to runtime implementation issues).

I see it take about ~9% of total initialization. While the other areas are bigger, this one is easy to address.

Screenshot 2025-08-20 at 7 59 54 AM Screenshot 2025-08-20 at 8 14 24 AM

@0xnm

0xnm commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hello @Laimiux. Thanks for your contribution.

Since we are using Gitlab CI, CI checks won't run from the forks, so I will pull your change and push it to the repo by myself and open a new PR.

@0xnm

0xnm commented Aug 21, 2025

Copy link
Copy Markdown
Member

Closing in favour of #2829.

@0xnm 0xnm closed this Aug 21, 2025
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.

4 participants