Upgrade okhttp3 3.14.9 => 4.9.3

[evantorrie]

As title says, this upgrades the underlying okhttp3 library from 3.14.9 to 4.9.3.

[jcchavezs]

Ping @shakuzen

On Tue, Jan 18, 2022, 18:34 ET @.***> wrote:

---

You can view, comment on, or merge this pull request online at:

https://github.com/openzipkin/zipkin-reporter-java/pull/205 Commit Summary

• 0cb52e4 https://github.com/openzipkin/zipkin-reporter-java/pull/205/commits/0cb52e427459cd616f426a1b2592714f79e1d617 Upgrade okhttp3 3.14.9 => 4.9.3

File Changes

(1 file https://github.com/openzipkin/zipkin-reporter-java/pull/205/files)

• M pom.xml https://github.com/openzipkin/zipkin-reporter-java/pull/205/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8 (2)

Patch Links:

• https://github.com/openzipkin/zipkin-reporter-java/pull/205.patch
• https://github.com/openzipkin/zipkin-reporter-java/pull/205.diff

— Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin-reporter-java/pull/205, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAWLP5CVETBFMO2RPJDUWWQDVANCNFSM5MHYZHQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

[shakuzen]

OkHttp 4.x requires the Kotlin standard library, which I believe is the reason we have remained on the 3.x version.

[jcchavezs]

Any particular reason for this upgrade @evantorrie?

On Thu, Jan 20, 2022, 03:56 Tommy Ludwig @.***> wrote:

OkHttp 4.x requires the Kotlin standard library, which I believe is the reason we have remained on the 3.x version.

— Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin-reporter-java/pull/205#issuecomment-1017070130, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYAXLGTRP3CWSV77W5A3UW52V3ANCNFSM5MHYZHQA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented. Message ID: @.*** com>

[evantorrie]

@jcchavezs Reason I proposed this is that the OpenTelemetry java project has two independent "trace" exporter subprojects, both of which I am trying to include in my own application. One is the jaeger trace exporter, the other the zipkin trace exporter, but they depend on conflicting versions of com.squareup.okhttp3:okhttp as shown.

[WARNING] 
Dependency convergence error for com.squareup.okhttp3:okhttp:3.14.9 paths to dependency are:
+-com.yahoo.tachyon.graviton:graviton_common:2.5.1-SNAPSHOT
  +-io.opentelemetry:opentelemetry-exporter-zipkin:1.10.0
    +-io.zipkin.reporter2:zipkin-sender-okhttp3:2.16.3
      +-com.squareup.okhttp3:okhttp:3.14.9
and
+-com.yahoo.tachyon.graviton:graviton_common:2.5.1-SNAPSHOT
  +-io.opentelemetry:opentelemetry-exporter-jaeger:1.10.0
    +-io.opentelemetry:opentelemetry-exporter-otlp-common:1.10.0
      +-com.squareup.okhttp3:okhttp:4.9.3

I was unaware of the dependency issues re Kotlin standard library, so I can probably work around this if I must, but it does seem like other popular opensource projects are dependent on the 4.x version.

[shakuzen]

See previous discussions around this in https://github.com/openzipkin/zipkin-reporter-java/pull/195. We would either need to support okhttp 3.x and 4.x in the same module (verified with an integration test) or make a new okhttp4 module, which would have a maintenance burden of copy-and-pasting code from the existing okhttp3 module.

[tokuhirom]

Note: OkHttp3 3.x was already End-of-life. https://square.github.io/okhttp/security/security/

[codefromthecrypt]

The dependencies here are to support existing applications, and often this conflicts with formal support life statements. As a few mentioned above, changing the okhttp3 code to depend on okhttp 4 changes the nature of the dependencies and will break those apps. The way forward would be to make an okhttp4 module instead, even if there is a large amount of copy/paste. This sort of thing is a reality in instrumentation and why you also see older dependencies in other places.

[codefromthecrypt]

So, basically I agree exactly what tommy said.. it is likely this can be solved with dependency management (declare a version override or resolution in the build). This is because in the case of senders we don't set things to provided scope. This is to avoid someone always having to declare a version. However, since we don't you may not get the version you want (hence this issue).

In looking at this closer, I think it would make more sense to test okhttp3's compat with 4 than make a copy/paste module, and document more clearly that the version indicated is a convenience version and this implies overrides are needed as time passes.

[codefromthecrypt]

TL;DR; I've softened on this because we have a similar library version update to communicate for thrift on the pending release.

I'll add a PR with an update to 4.x and a test that proves v3 still works (via invoker). I'll also add into the README how to downgrade the library version to 3.x.

Specifically, 3.12.13 still worked with JRE 7 and corresponding android versions, so we'll keep our bytecode level on this project 1.7 even after switching to 4.x (java 8). I'll add a comment why in the pom to this effect as it would otherwise be confusing.

The main fallout I think will be in the https://github.com/openzipkin/brave-example repo, where old library instrumentation will need to override the version, to set it back to 3. I'm ok handling that personally.

Thanks for your patience!

[codefromthecrypt]

https://github.com/openzipkin/zipkin-reporter-java/pull/223 fixes this