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

OkHttp interceptor doesn't see "known hosts" list when OkHttp is initialized before the SDK #513

Closed
erawhctim opened this issue Mar 2, 2021 · 2 comments
Assignees
Labels
bug Something isn't working fixed
Milestone

Comments

@erawhctim
Copy link
Contributor

erawhctim commented Mar 2, 2021

Thanks for taking the time for reporting an issue!

Describe what happened
When OkHttp is initialized before the DD SDK and one of the TracingInterceptors is used, any "known hosts" set in the SDK Configuration aren't picked up by the interceptor(s) and those requests ultimately aren't traced in APM. This happens with the newer, non-deprecated Interceptor constructors that don't take in a list of known hosts (e.g. here).

Steps to reproduce the issue:
I've modified the sample app in my own fork to reproduce this issue (commit here):

  • Create an OkHttpClient with a TracedRequestListener and no known hosts provided.
  • Set known hosts in the DD SDK Configuration
  • Initialize the OkHttpClient before the DD SDK
  • Add the expected hostname as a known host (e.g. "unsplash.com" in this case)
  • Run the sample, go to the image loader and choose Picasso as the load type
  • Load image; If you debug within TracingInterceptor you'll notice that tracing is not enabled and FirstPartyHostDetector.knownHosts is empty

This can be avoided by using the deprecated Interceptor constructor(s) that take in the list of known hosts when constructing the OkHttpClient, but I'd rather not rely on deprecated functionality if it will be removed in the future.
EDIT: This doesn't seem to work any more with v1.8.0 🤔 however, providing the known hosts into the Interceptor used to work in prior SDK versions.

Describe what you expected:

  • I expect to be able to construct my OkHttpClient earlier than the DD SDK and still have APM/RUM tracing work properly.
    • (we initialize OkHttp earlier than any of our dependencies since its used by almost everything).
  • If the DD SDK must be initialized first, I need an initialization callback to be notified when the SDK is finished initializing (see issue Callback when SDK is fully initialized?  #511).
  • If neither of those are possible, should the old/deprecated Interceptor constructor become un-deprecated? What's the best path forward here?

Additional context

  • Android OS version: Android 11
  • Device Model: Galaxy S10
  • Datadog SDK version: 1.8.0
  • Versions of any other relevant dependencies (OkHttp, …): Whatever is set in the sample app
  • Proguard configuration: Whatever is set in the sample app
  • Gradle Plugins: Whatever is set in the sample app
@erawhctim erawhctim added the bug Something isn't working label Mar 2, 2021
@xgouchet xgouchet self-assigned this Mar 4, 2021
@xgouchet
Copy link
Collaborator

xgouchet commented Mar 4, 2021

Hello @erawhctim , and thanks for reporting this issue. It's been fixed and will be available in version 1.8.1 (which should be available soon). Let us know if upgrading your dependency solves this issue

@erawhctim
Copy link
Contributor Author

Great, thank you!

@xgouchet xgouchet closed this as completed Mar 9, 2021
@xgouchet xgouchet added the fixed label May 10, 2021
@xgouchet xgouchet added this to the 1.8.1 milestone May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed
Projects
None yet
Development

No branches or pull requests

2 participants