Consider what to do about OkHttp switching to Kotlin

[codefromthecrypt]

Currently, our elasticsearch module and server tests use OkHttp.. most important to end users is the elasticsearch module. I switched the version and it broke api a little

--- a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/client/HttpCall.java
+++ b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/client/HttpCall.java
@@ -132,7 +132,7 @@ public final class HttpCall<V> extends Call.Base<V> {

   public static <V> V parseResponse(Response response, BodyConverter<V> bodyConverter)
     throws IOException {
-    if (!HttpHeaders.hasBody(response)) {
+    if (!HttpHeaders.promisesBody(response)) {
       if (response.isSuccessful()) {
         return null;
       } else {

More importantly, this adds yet another 2MiB as that's the size of the kotlin standard library. Tests, we can use anything, but we should be considerate about the actual server code.

asking @openzipkin/core Java maintainers an opinion on this. I suspect it is either continue and not upgrade to OkHttp 4, upgrade to OkHttp 4 and accept the kotlin dep, or switch to Armeria and remember that we have some integrations downstream like zipkin-aws signatures which would need to be redone if we chose that path.

[anuraaga]

Happy to help migrate things to Armeria

[codefromthecrypt]

@anuraaga if you are up to the port, I'd review it. Warning: there will be work in zipkin-aws even if not that bad and likely easier than what was done in zipkin-gcp

[vietj]

indeed I just hit that issue when trying to run brave-instrumentation-http-tests with zipkin-junit that consume conflicting versions of OkHttp.

[anuraaga]

Started hacking on this today, think it'll be fairly straightforward.

[vietj]

after this can we expect a release after ?

[codefromthecrypt]

after this can we expect a release after ?

technically this will be true won't it :P

[vietj]

I'm tired, soon holidays :-)

[codefromthecrypt]

ps the api break is our fault as it is an internal api. I can tentatively remove that to decouple things

[anuraaga]

Realized it's premature to close this issue until zipkin-aws is migrated, starting that now.

[codefromthecrypt]

we have a pending pull request on zipkin-aws, now. thanks. I don't think there's any decision left as we've already made it, so closing this.

[codefromthecrypt]

I realize that if someone glances at this, it might seem there was only one factor into switching libraries. It would have been a mistake to only consider this.

OkHttp is an extremely elegant library that solves problems well. Especially coupled with its buddies okio moshi and wire, it is very efficient.

What isn't on this issue were several other chats around the bundling of concerns that made a transition sensible:

• interest and hands: without @anuraaga nothing likely would change
• mutual stake: if we didn't already use Armeria, this might not have happened
• digging issue: if we didn't have people continuing to raise issues about https endpoints, this might not have happened. Ex #1611 and several variants with pings on chat

Despite attempts to be very transparent, sometimes issues are raised all over the place, and discussions don't get copied everywhere. We probably weren't careful enough to elaborate here that this, if anything was a combination of above and timing of hands to do the work.

I think I can speak for everyone in saying we are happy with all the help we've had from OkHttp and the change here does not imply a change from elsewhere like in Brave and Reporter where we put OkHttp forward in most examples. I tried to touch on this in release notes, but forgot to put something here https://github.com/openzipkin/zipkin/releases/tag/2.16.2

Especially in hindsight, I would recommend projects to think carefully about any transition away from OkHttp, and certainly consider using more of it. Its design is so thoughtful and we really had an easy few years depending on it. It is very efficient in practice, and has been remarkably stable. I would only expect such stability to continue.

[codefromthecrypt]

fyi @vietj zipkin-junit still uses okhttp. I checked with v4.1 and it seems to work fine with no compile breaks. If you have any issue with zipkin-junit please open another issue. There's no plan to change that off okhttp. The only thing changed in this issue are internals of our elasticsearch driver.