Minimize third-party dependencies

[jpkrohling]

See opentracing/opentracing-java#296 (comment)
Related: opentracing/opentracing-java#300
Related: #84

This library depends on some third-party libraries, which might reduce the places where Jaeger would run. Ideally, we would ship only with that is absolutely required.

This task might be done as a standalone task, as part of the Performance tests, or as part of the Android tasks.

[gsoria]

Can I work on this issue?

[jpkrohling]

It's yours! The first step is to list all the dependencies that are brought when a client application depends on the jaeger-client artifact. If you are not familiar with Jaeger and OpenTracing, here's a couple of resources that might help:

http://opentracing.io/
https://www.jaegertracing.io/
https://www.katacoda.com/courses/opentracing or https://github.com/yurishkuro/opentracing-tutorial/tree/master/java

If you need any assistance, do let me know, either here or Gitter. Good luck!

[gsoria]

First I created a simple java project and analyzed the dependencies with:

mvn dependency:tree > dependencies.log

The output of this command was:

[INFO] java.io.jaegertracing.examples:maven-example:jar:0.0.1-SNAPSHOT
[INFO] \- io.jaegertracing:jaeger-client:jar:0.31.0:compile
[INFO]    +- io.jaegertracing:jaeger-thrift:jar:0.31.0:compile
[INFO]    |  +- org.slf4j:slf4j-api:jar:1.7.25:compile
[INFO]    |  +- org.apache.thrift:libthrift:jar:0.11.0:compile
[INFO]    |  |  +- org.apache.httpcomponents:httpclient:jar:4.4.1:compile
[INFO]    |  |  |  +- commons-logging:commons-logging:jar:1.2:compile
[INFO]    |  |  |  \- commons-codec:commons-codec:jar:1.9:compile
[INFO]    |  |  \- org.apache.httpcomponents:httpcore:jar:4.4.1:compile
[INFO]    |  \- com.squareup.okhttp3:okhttp:jar:3.9.0:compile
[INFO]    |     \- com.squareup.okio:okio:jar:1.13.0:compile
[INFO]    +- io.jaegertracing:jaeger-core:jar:0.31.0:compile
[INFO]    |  +- io.opentracing:opentracing-api:jar:0.31.0:compile
[INFO]    |  +- io.opentracing:opentracing-util:jar:0.31.0:compile
[INFO]    |  |  \- io.opentracing:opentracing-noop:jar:0.31.0:compile
[INFO]    |  \- com.google.code.gson:gson:jar:2.8.2:compile
[INFO]    \- io.jaegertracing:jaeger-tracerresolver:jar:0.31.0:compile
[INFO]       \- io.opentracing.contrib:opentracing-tracerresolver:jar:0.1.4:compile

I wanted to compare the resolution of dependencies with an android application, so I created this android project

Then I analyzed the dependencies with:

gradle app:dependencies

The output of this command was:

+--- io.jaegertracing:jaeger-client:0.31.0
|    +--- io.jaegertracing:jaeger-thrift:0.31.0
|    |    +--- io.jaegertracing:jaeger-core:0.31.0
|    |    |    +--- io.opentracing:opentracing-api:0.31.0
|    |    |    +--- io.opentracing:opentracing-util:0.31.0
|    |    |    |    +--- io.opentracing:opentracing-api:0.31.0
|    |    |    |    \--- io.opentracing:opentracing-noop:0.31.0
|    |    |    |         \--- io.opentracing:opentracing-api:0.31.0
|    |    |    +--- com.google.code.gson:gson:2.8.2
|    |    |    \--- org.slf4j:slf4j-api:1.7.25
|    |    +--- org.slf4j:slf4j-api:1.7.25
|    |    +--- org.apache.thrift:libthrift:0.11.0
|    |    |    +--- org.slf4j:slf4j-api:1.7.12 -> 1.7.25
|    |    |    +--- org.apache.httpcomponents:httpclient:4.4.1
|    |    |    |    +--- org.apache.httpcomponents:httpcore:4.4.1
|    |    |    |    +--- commons-logging:commons-logging:1.2
|    |    |    |    \--- commons-codec:commons-codec:1.9
|    |    |    \--- org.apache.httpcomponents:httpcore:4.4.1
|    |    \--- com.squareup.okhttp3:okhttp:3.9.0
|    |         \--- com.squareup.okio:okio:1.13.0
|    +--- io.jaegertracing:jaeger-core:0.31.0 (*)
|    \--- io.jaegertracing:jaeger-tracerresolver:0.31.0
|         +--- io.jaegertracing:jaeger-core:0.31.0 (*)
|         \--- io.opentracing.contrib:opentracing-tracerresolver:0.1.4
|              \--- io.opentracing:opentracing-api:0.31.0

The dependencies are the same in maven java project and android with gradle, and the list of the non duplicates dependencies are:

jaeger-client:jar	0.31.0
jaeger-thrift:jar	0.31.0
slf4j-api:jar	1.7.25
libthrift:jar	0.11.0
httpclient:jar	4.4.1
commons-logging:jar	1.2
commons-codec:jar	1.9
httpcore:jar	4.4.1
okhttp:jar	3.9.0
okio:jar	1.13.0
jaeger-core:jar	0.31.0
opentracing-api:jar	0.31.0
opentracing-util:jar	0.31.0
opentracing-noop:jar	0.31.0
gson:jar	2.8.2
jaeger-tracerresolver:jar	0.31.0
opentracing-tracerresolver:jar	0.1.4

I need some advice about the next step, especially with the ideas related to change thrift and the possible protobuf serialization.

[jpkrohling]

This is really great work, @gsoria! Some of these dependencies are transitive dependencies, like com.squareup.okio:okio:1.13.0. If we eliminate our dependency on com.squareup.okhttp3:okhttp:3.9.0, the one to okio will go away as well.

Based on your report, it looks like we have a dependency only on the following external artifacts:

• com.squareup.okhttp3:okhttp:3.9.0
• org.apache.thrift:libthrift:0.11.0
• com.google.code.gson:gson:2.8.2

I would say that the dependency on org.slf4j:slf4j-api:1.7.25 is acceptable and the ones on OpenTracing artifacts are desirable.

The next step then is to see how dependent are we on those three artifacts and how easy it would be to remove that. For instance, I have the impression that the GSON library is used only in one place. So, I think it would make sense to have a simple report, listing the places where each library is used and your personal impression on how easy would it be to replace them.

[gsoria]

Thanks @jpkrohling, I will be working on that!

[gsoria]

I have reviewed the codebase and these are my findings:

• com.squareup.okhttp3:okhttp:3.9.0

The library okhttp is used in HttpSender class (and its test, HttpSenderTest).to perform a POST to send a Span.
If we want to eliminate this dependency and the transitive dependencies, we can replace the implementation of HttpSender using the JDK Standard API (java.net), for example extending the class io.jaegertracing.internal.utils.Http, to add a makePostRequest.

Another option can be using httpclient, because it is in fact a transitive dependency of org.apache.thrift:libthrift.

• com.google.code.gson:gson:2.8.2

This dependency is used for the following scenarios:

A. To serialize a Map to Json, in converters related to Zipkin:

V2SpanConverter
ThriftSpanConverter

B. To deserialize the JSON payload of the response in:

HttpSamplingManager
gson.fromJson(json, SamplingStrategyResponse.class);

HttpBaggageRestrictionManagerProxy
return gson.fromJson(json, LIST_TYPE);

I think the serialization from scenario A can be done manually, but in the scenario B, considering that the response of the request is encoded in JSON, it is best to keep a parser.

• org.apache.thrift:libthrift:0.11.0

Is used in these classes:
ThriftSenderBase
ThriftUdpTransport

Test
InMemorySpanServerHandler
ThriftSenderBaseTest

And many classes automatically generated by thrift in jaeger-thrift/src/main/gen-java.

I think this dependency is the library on which we depend the most, because changing it would imply redefining the transport and the data model for both the client and the backend side.

[jpkrohling]

Thanks @gsoria, I think your assessment is spot on. We could leave the decision about replacing the okhttp dependency to the actual internship, as we would need to measure the performance impact of such change. If we are using it only in a couple of places, we could even consider using HttpUrlConnection instead of relying on Apache HTTP Client, especially considering that we might move from thrift to gRPC in the future, so, we wouldn't be pulling Apache HTTP client transitively.

About GSON: we are bringing about 230K to deserialize the JSON payloads. This may be a fair price to pay, as Java 6 has no standard JSON APIs, but I think this payload may be actually very simple. It would be nice to see what are the other alternatives here. How do the payloads look like? Are they simple JSONs? If so, we may be able to get away with a simple tokenizer, even if the performance isn't that good, given that these places are not in the critical path.

[gsoria]

Thanks @jpkrohling I will be analyzing the alternatives related to gson.

[jpkrohling]

@gsoria , please record your contribution (if you haven't done so yet) on Outreachy's website before the deadline, which is tomorrow Tuesday, October 30, 2018 4pm UTC. Thanks!

https://www.outreachy.org/december-2018-march-2019-outreachy-internships/communities/cncf-tracing/research-the-gaps-in-tracing-web-clients/contributions/

[gsoria]

@jpkrohling I already did it, thank you!

[t-8ch]

Could it be that the httpclient and httpcore libraries from Apache are not actually used (transitively) and could be excluded from the transitive dependencies of libthrift?

[yurishkuro]

We only use thrift for encoding the payload, not for http. So yes, if it has transitive deps we may be able to exclude them.