Support OpenTracing 0.32.0 and 0.33.0 #623

pavolloffay · 2019-05-06T14:41:58Z

Signed-off-by: Pavol Loffay [email protected]

codecov · 2019-05-06T14:47:50Z

Codecov Report

Merging #623 into master will decrease coverage by 0.24%.
The diff coverage is 77.77%.

@@             Coverage Diff              @@
##             master     #623      +/-   ##
============================================
- Coverage     89.62%   89.37%   -0.25%     
+ Complexity      564      562       -2     
============================================
  Files            69       69              
  Lines          2063     2071       +8     
  Branches        262      263       +1     
============================================
+ Hits           1849     1851       +2     
- Misses          134      138       +4     
- Partials         80       82       +2

Impacted Files	Coverage Δ	Complexity Δ
...n/java/io/jaegertracing/internal/JaegerTracer.java	`88.92% <77.77%> (-0.05%)`	`26 <0> (ø)`
...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 0d4c675...74af1c8. Read the comment docs.

pavolloffay · 2019-05-06T15:20:34Z

wait tracerresolver has to be updated too

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java

bltavares · 2019-06-19T21:03:58Z

I've recently found the error where the library is instrumented using opentracing 0.33 (on io.pedestal.log) could not use the 0.35 version of Jaeger.

It failed at runtime, when JaegerTracer tried to call .activeSpan on a scopeManager [here]

Stack trace

:via
              [{:type java.lang.NoSuchMethodError
                :message "io.opentracing.ScopeManager.activeSpan()Lio/opentracing/Span;"
                :at [io.jaegertracing.internal.JaegerTracer activeSpan "JaegerTracer.java" 194]}]
              :trace
              [[io.jaegertracing.internal.JaegerTracer activeSpan "JaegerTracer.java" 194]
               [io.opentracing.util.GlobalTracer activeSpan "GlobalTracer.java" 148]
               [io.pedestal.log$eval33020$fn__33030 invoke "log.clj" 845]

This class has been modified, but the method haven't. Could the error have been caused by version conflicts of the interface on the classpath, or is this something should be tackled by the bump as well?

(I didn't have time yet to test it with this PR code tho, sorry about that)

objectiser · 2019-06-21T14:01:00Z

@bltavares Just tried calling activeSpan() via the GlobalTracer, using Jaeger 0.35.2 and OpenTracing 0.33, and it worked fine - could you double check the jaeger and OT dependencies being used?

bltavares · 2019-06-21T19:39:15Z

@objectiser It seems that it is loading another version of opentracing from pedestal. My bad, I've looked only on GitHub for the source of the lib, using the latest code instead of the version.

It fails with Opentracing 0.31.0 and Jaeger 0.35.5. I'll post the dep tree for sake of completeness.

dep tree

+- io.pedestal:pedestal.log:jar:0.5.5:compile
[INFO] |  |  +- (org.clojure:clojure:jar:1.9.0:compile - omitted for conflict with 1.10.0)
[INFO] |  |  +- (org.slf4j:slf4j-api:jar:1.7.25:compile - omitted for duplicate)
[INFO] |  |  +- (io.dropwizard.metrics:metrics-core:jar:4.0.2:compile - omitted for conflict with 3.2.3)
[INFO] |  |  +- io.dropwizard.metrics:metrics-jmx:jar:4.0.2:compile
[INFO] |  |  |  +- (io.dropwizard.metrics:metrics-core:jar:4.0.2:compile - omitted for conflict with 3.2.3)
[INFO] |  |  |  \- (org.slf4j:slf4j-api:jar:1.7.25:compile - omitted for duplicate)
[INFO] |  |  +- io.opentracing:opentracing-api:jar:0.31.0:compile
[INFO] |  |  \- io.opentracing:opentracing-util:jar:0.31.0:compile
[INFO] |  |     +- (io.opentracing:opentracing-api:jar:0.31.0:compile - omitted for duplicate)
[INFO] |  |     \- io.opentracing:opentracing-noop:jar:0.31.0:compile
[INFO] |  |        \- (io.opentracing:opentracing-api:jar:0.31.0:compile - omitted for duplicate)
[INFO] |  +- io.jaegertracing:jaeger-client:jar:0.35.5:compile
[INFO] |  |  +- io.jaegertracing:jaeger-thrift:jar:0.35.5:compile
[INFO] |  |  |  +- (io.jaegertracing:jaeger-core:jar:0.35.5:compile - omitted for duplicate)
[INFO] |  |  |  +- (org.slf4j:slf4j-api:jar:1.7.25:compile - omitted for duplicate)
[INFO] |  |  |  +- (org.apache.thrift:libthrift:jar:0.12.0:compile - omitted for conflict with 0.10.0)
[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.35.5:compile
[INFO] |  |  |  +- (io.opentracing:opentracing-api:jar:0.32.0:compile - omitted for conflict with 0.31.0)
[INFO] |  |  |  +- (io.opentracing:opentracing-util:jar:0.32.0:compile - omitted for conflict with 0.31.0)
[INFO] |  |  |  +- (com.google.code.gson:gson:jar:2.8.2:compile - omitted for conflict with 2.3.1)
[INFO] |  |  |  \- (org.slf4j:slf4j-api:jar:1.7.25:compile - omitted for duplicate)
[INFO] |  |  \- io.jaegertracing:jaeger-tracerresolver:jar:0.35.5:compile
[INFO] |  |     +- (io.jaegertracing:jaeger-core:jar:0.35.5:compile - omitted for duplicate)
[INFO] |  |     \- io.opentracing.contrib:opentracing-tracerresolver:jar:0.1.6:compile
[INFO] |  |        \- (io.opentracing:opentracing-api:jar:0.32.0:compile - omitted for conflict with 0.31.0)

Signed-off-by: Pavol Loffay <[email protected]>

yurishkuro · 2019-07-30T23:26:49Z

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java

+        @Override
+        public void close() {
+          wrapped.close();
+          if (finishSpanOnClose) {


nit: could short-circuit this at the top of the method:

if (!finishSpanOnClose) { return scopeManager.activate(start()); }

objectiser · 2019-07-31T09:57:16Z

build.gradle

 ext.guavaVersion = getProperty('guavaVersion','18.0')
 ext.apacheThriftVersion = getProperty('apacheThriftVersion','0.12.0')
 ext.jerseyVersion = getProperty('jerseyVersion','2.22.2')
 ext.slf4jVersion = getProperty('slf4jVersion','1.7.25')
 ext.gsonVersion = getProperty('gsonVersion','2.8.2')
-ext.tracerResolverVersion = getProperty('tracerResolverVersion','0.1.6')
+ext.tracerResolverVersion = getProperty('tracerResolverVersion','0.1.7')


Version 0.1.8 has been released with a couple of changes (adding module name and supporting classloader parameter).

Signed-off-by: Pavol Loffay <[email protected]>

objectiser · 2019-07-31T12:53:28Z

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java

    @Deprecated
-    public JaegerSpan startManual() {
+    // @Override keep compatibility with 0.32.0
+    public Span startManual() {


Should this remain JaegerSpan startManual()?

No, It has to be Span now as it does not override the API anymore.

objectiser · 2019-07-31T12:54:56Z

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java

+        }
+
+        // @Override keep compatibility with 0.32.0
+        public Span span() {


Should this return JaegerSpan?

See the previous message, we cannot return covariant type as the method is not overridden.

objectiser · 2019-07-31T12:57:42Z

jaeger-core/src/test/java/io/jaegertracing/internal/ActiveSpanTest.java

@@ -89,25 +94,30 @@ public void testActiveSpanAutoReference() {
  }

  @Test
+  @SuppressWarnings("deprecation")


Should this test be here now? Isn't it covered in the 0.32 tests?

I am fine keeping it here too. Here it uses the direct type whereas in the 0.32 test it uses old interface.

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay requested a review from yurishkuro May 6, 2019 14:42

yurishkuro reviewed May 6, 2019

View reviewed changes

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerTracer.java Show resolved Hide resolved

yurishkuro mentioned this pull request Jul 29, 2019

Release 1.0.0 #639

Closed

pavolloffay force-pushed the 0.33.0 branch from 4f801bc to 9a24ace Compare July 30, 2019 13:35

Support opentracing 0.32.0 and 0.33.0

ffa1856

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay force-pushed the 0.33.0 branch from 9a24ace to ffa1856 Compare July 30, 2019 13:39

unify messages

1151c25

Signed-off-by: Pavol Loffay <[email protected]>

yurishkuro reviewed Jul 30, 2019

View reviewed changes

yurishkuro approved these changes Jul 30, 2019

View reviewed changes

objectiser reviewed Jul 31, 2019

View reviewed changes

pavolloffay added 3 commits July 31, 2019 14:05

Test 0.32.0

d26cbb8

Signed-off-by: Pavol Loffay <[email protected]>

Simplify gradle

b2d3772

Signed-off-by: Pavol Loffay <[email protected]>

remove line

57dd12a

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay changed the title ~~Bump OpenTracing to 0.33.0~~ Support OpenTracing 0.32.0 and 0.33.0 Jul 31, 2019

objectiser reviewed Jul 31, 2019

View reviewed changes

objectiser approved these changes Jul 31, 2019

View reviewed changes

Add one more test

74af1c8

Signed-off-by: Pavol Loffay <[email protected]>

pavolloffay merged commit cf9f993 into jaegertracing:master Jul 31, 2019

pavolloffay mentioned this pull request Aug 2, 2019

Preparing release 1.0.0 #642

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support OpenTracing 0.32.0 and 0.33.0 #623

Support OpenTracing 0.32.0 and 0.33.0 #623

pavolloffay commented May 6, 2019

codecov bot commented May 6, 2019 •

edited

Loading

pavolloffay commented May 6, 2019

bltavares commented Jun 19, 2019

objectiser commented Jun 21, 2019

bltavares commented Jun 21, 2019

yurishkuro Jul 30, 2019 •

edited

Loading

objectiser Jul 31, 2019

objectiser Jul 31, 2019

pavolloffay Jul 31, 2019

objectiser Jul 31, 2019

pavolloffay Jul 31, 2019 •

edited

Loading

objectiser Jul 31, 2019

pavolloffay Jul 31, 2019

Support OpenTracing 0.32.0 and 0.33.0 #623

Support OpenTracing 0.32.0 and 0.33.0 #623

Conversation

pavolloffay commented May 6, 2019

codecov bot commented May 6, 2019 • edited Loading

Codecov Report

pavolloffay commented May 6, 2019

bltavares commented Jun 19, 2019

objectiser commented Jun 21, 2019

bltavares commented Jun 21, 2019

yurishkuro Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

objectiser Jul 31, 2019

Choose a reason for hiding this comment

objectiser Jul 31, 2019

Choose a reason for hiding this comment

pavolloffay Jul 31, 2019

Choose a reason for hiding this comment

objectiser Jul 31, 2019

Choose a reason for hiding this comment

pavolloffay Jul 31, 2019 • edited Loading

Choose a reason for hiding this comment

objectiser Jul 31, 2019

Choose a reason for hiding this comment

pavolloffay Jul 31, 2019

Choose a reason for hiding this comment

codecov bot commented May 6, 2019 •

edited

Loading

yurishkuro Jul 30, 2019 •

edited

Loading

pavolloffay Jul 31, 2019 •

edited

Loading