Add a new submodule to build a client bundle. #601

carlosalberto · 2019-03-07T00:33:34Z

Have a bundled Client artifact (fat jar)

Addresses #599 - I'm wondering if this name would go well with the rest of the artifacts ;)

Let me know if this could be of help.

Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

codecov · 2019-03-11T23:29:07Z

Codecov Report

Merging #601 into master will decrease coverage by 0.25%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #601      +/-   ##
============================================
- Coverage     89.53%   89.27%   -0.26%     
+ Complexity      542      540       -2     
============================================
  Files            68       68              
  Lines          1949     1949              
  Branches        251      251              
============================================
- Hits           1745     1740       -5     
- Misses          129      133       +4     
- Partials         75       76       +1

Impacted Files	Coverage Δ	Complexity Δ
...rtracing/internal/reporters/CompositeReporter.java	`71.42% <0%> (-28.58%)`	`6% <0%> (-1%)`
...gertracing/internal/reporters/LoggingReporter.java	`81.81% <0%> (-9.1%)`	`4% <0%> (-1%)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df3f205...3a401e0. Read the comment docs.

Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

objectiser · 2019-03-12T09:47:21Z

jaeger-client/build.gradle

@@ -7,3 +9,28 @@ dependencies {

    testCompile group: 'junit', name: 'junit', version: junitVersion
 }
+
+/* Shaded the same libraries as jaeger-thrift,
+ * leaving the slf4j dependency in place, as a *requirement*


This comment seems out of date now.

Although not sure whether the slf4j-api should be shaded, as there is no slf4j logging impl as part of this uber jar, so would need to pick it up from the app's dependencies. Not sure what the impact of shading the api would have on picking up an impl?

Yeah, I was wondering about this myself later on (whether shading it or not). So I think I will update the code to not shade it at all.

objectiser · 2019-03-12T09:51:47Z

jaeger-client/build.gradle

+    relocate 'okio'             , 'io.jaegertracing.vendor.okio'
+    relocate 'org.apache'       , 'io.jaegertracing.vendor.org.apache'
+    relocate 'org.slf4j'        , 'io.jaegertracing.vendor.org.slf4j'
+    classifier 'bundle'


Would prefer another name - bundle makes me think OSGi 😄 , but not sure what might be better. Could be something simple like all - but will let others comment.

all sounds about right to me.

objectiser · 2019-03-12T09:53:35Z

jaeger-client/build.gradle

+    }
+}
+
+tasks.check.dependsOn tasks.shadowJar


This wasn't required in the thrift usecase - was the shadowJar task not being triggered?

shadowJar is only being triggered in the thrift case when jaeger-crossdock is being built. Based on what you wrote, it sounds like we should do the same here? :)

Not sure it should be dependent on jaeger-crossdock build - ideally it should just be built when the non-shaded artifact is being built.

Oh, then this line should stay, as it builds the shaded artifact right after the non-shaded one was built.

* Use a 'all' classifier. * Do not shade slf4j. Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

carlosalberto · 2019-03-13T01:14:09Z

hey @objectiser @yurishkuro

I've updated the PR to reflect the latest feedback. Let me know ;)

objectiser · 2019-03-13T09:56:32Z

@carlosalberto looks good - my only concern is that I believe the qualified jar dependency would still have a transient dependency on the non-shaded dependencies.

If the main use of the jar is to physically download the jar and include it with a javaagent, then this would not be an issue. But if someone includes the dependency in their app's pom.xml, this would still pull in the transient dependencies. I guess they could always exclude them - but not ideal.

What do others think?

carlosalberto · 2019-03-20T03:31:52Z

Hey hey @objectiser

Trying to re-take this conversation:

If the main use of the jar is to physically download the jar and include it with a javaagent, then this would not be an issue.

I think this is the exact use scenario, yes - to download and use it directly (would also work when doing Cassandra server side instrumentation, where you would simply put this JAR in the Cassandra bin path).

objectiser · 2019-03-20T08:35:24Z

@carlosalberto Agree it meets that use case - just worried whether some users would expect to use it as a pom dependency.

@jpkrohling Thoughts? Would documenting this in the readme be enough?

jpkrohling · 2019-03-20T09:12:14Z

@carlosalberto would you please list the contents of your local Maven repository for the affected artifact like, tree ~/.m2/repository/io/jaegertracing/jaeger-client/?

If you are producing both a "slim" and a "fat" jar, I'd be interested in seeing the mvn dependency:tree from an application consuming each of them. If you haven't tested that yet, I can do it myself. I think I still have something I used for #526 and related.

Add a new submodule to build a client bundle.

238c6d3

Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

carlosalberto force-pushed the client-bundle branch from 22247f8 to 238c6d3 Compare March 7, 2019 00:37

Build the jaeger-client fat jar in the same module.

afc58cf

Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

Shade slf4j.

ed9e405

Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

carlosalberto mentioned this pull request Mar 11, 2019

Create a jar with the dependencies for easy deployment #599

Closed

objectiser reviewed Mar 12, 2019

View reviewed changes

Update the build.

3a401e0

* Use a 'all' classifier. * Do not shade slf4j. Signed-off-by: Carlos Alberto Cortez <calberto.cortez@gmail.com>

yurishkuro closed this in 8ba71a6 Jan 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new submodule to build a client bundle. #601

Add a new submodule to build a client bundle. #601

carlosalberto commented Mar 7, 2019

codecov bot commented Mar 11, 2019 •

edited

Loading

objectiser Mar 12, 2019

objectiser Mar 12, 2019

carlosalberto Mar 12, 2019

objectiser Mar 12, 2019

carlosalberto Mar 12, 2019

objectiser Mar 12, 2019

carlosalberto Mar 12, 2019

objectiser Mar 12, 2019 •

edited

Loading

carlosalberto Mar 12, 2019

carlosalberto commented Mar 13, 2019

objectiser commented Mar 13, 2019

carlosalberto commented Mar 20, 2019

objectiser commented Mar 20, 2019

jpkrohling commented Mar 20, 2019

Add a new submodule to build a client bundle. #601

Add a new submodule to build a client bundle. #601

Conversation

carlosalberto commented Mar 7, 2019

Have a bundled Client artifact (fat jar)

codecov bot commented Mar 11, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

objectiser Mar 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlosalberto commented Mar 13, 2019

objectiser commented Mar 13, 2019

carlosalberto commented Mar 20, 2019

objectiser commented Mar 20, 2019

jpkrohling commented Mar 20, 2019

codecov bot commented Mar 11, 2019 •

edited

Loading

objectiser Mar 12, 2019 •

edited

Loading