Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Update to okhttp v4.2.2 #666

Closed
adriangonz opened this issue Oct 28, 2019 · 8 comments
Closed

Update to okhttp v4.2.2 #666

adriangonz opened this issue Oct 28, 2019 · 8 comments

Comments

@adriangonz
Copy link

Requirement - what kind of business use case are you trying to solve?

Jaeger is currently using version v3.9.0 of com.squareup.okhttp3:okhttp, which is a few versions behind the latest v4.2.2. It would be useful to update it to the latest version.

Problem - what in Jaeger blocks you from solving the requirement?

Jaeger is pointing to v3.9.0, so in order to use the latest version, Jaeger would need to change its required version to v4.2.2. Before updating though, there are a couple of things that should be considered. Both are derived from the fact that from v4.x onwards, okhttp uses Kotlin.

  • One of the dependencies of jaeger-zipkin, zipkin-junit, depends on mockwebserver which is also part of the com.squareup.okhttp3 group. However, zipkin-junit currently points to v3.14.4 which is incompatible with v4.2.2 (because of the move to Kotlin). I've submitted a PR with this change to zipkin-junit, Update mockwebserver openzipkin/zipkin#2889.
  • Updating to v4.2.2 adds a new dependency on Kotlin. This can be considered a problem. However, if Jaeger keeps using okhttp it would be good to stay on the latest version.

On the other hand, v4.x remains source- and binary-compatible with v3.x, therefore there shouldn't be any need of extra changes on Jaeger.

Proposal - what do you suggest to solve the problem or improve the existing situation?

Once openzipkin/zipkin#2889 is accepted, we should be able to bump the version on the root build.gradle:

ext.okhttpVersion = getProperty('okhttpVersion','3.9.0')

No extra changes should be required.

@yurishkuro
Copy link
Member

I don't think we want kotlin dependency at runtime. Zipkin moved away from okhttp for that reason iirc.

@adriangonz
Copy link
Author

I agree @yurishkuro, it's an important point to consider. If not updating, is the plan then to move away from okhttp?

@yurishkuro
Copy link
Member

There is no specific plan yet. Do you have any thoughts?

@adriangonz
Copy link
Author

I think it's a tricky one. If we don't add the Kotlin dependency, then Jaeger will get stuck in the v3.x branch of okhttp. Therefore, the only alternative to adding Kotlin seems to move to something else.

This was also the reasoning for the Zipkin project to move to Armeria: openzipkin/zipkin#2646.

@yurishkuro
Copy link
Member

That PR is for Zipkin backend, but this is Jaeger client lib. We don't need a full blown server framework here, only some fairly basic HTTP functionality.

@adriangonz
Copy link
Author

I agree, the scope between Zipkin and jaeger-client is completely different. I used it as an example because, regardless of scope, I think that the same reasoning still applies.

The options moving forward seem to be these three:

  • Update to okhttp v4.x and add the Kotlin dependency. It's still not clear how much overhead this adds. It also depends a lot on how much traction Kotlin gets in the community (i.e. are more mainstream libraries going to move towards Kotlin?).
  • Move to some other HTTP requests library. Since jaeger-client just requires a library which makes HTTP requests, moving to a different one than okhttp shouldn't be too hard. However, before choosing this route, it is still worth considering if more libraries are going to move to Kotlin and if the feature set of other libraries is good enough to replace okhttp.
  • Get locked into okhttp v3.x. This is the default option if we don't take any action. At some point, okhttp will stop updating this branch though. Therefore, it seems the worse out of the three and I personally don't think that should be considered.

@yurishkuro
Copy link
Member

I don't know much about kotlin, but I assume that similar to Scala it brings with it some sort of stdlib, which will bloat the size of the client distribution. We might also need to share it's stdlib to avoid dependencies hell for users. So I view it almost as unattractive option as staying on okhttp3 (which at the moment is not terrible, it's not that old).

@adriangonz
Copy link
Author

Fixed with #674

Thanks for your help @yurishkuro @pavolloffay !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants