-
-
Notifications
You must be signed in to change notification settings - Fork 407
Update lessons #30
Update lessons #30
Changes from 3 commits
82c9b53
15419f9
560464c
06b33b4
b748e2f
06c8847
77377a2
8af3052
ea77699
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,7 +64,7 @@ public class Hello { | |
} | ||
|
||
private void sayHello(String helloTo) { | ||
Span span = tracer.buildSpan("say-hello").startManual(); | ||
Span span = tracer.buildSpan("say-hello").start(); | ||
|
||
String helloStr = String.format("Hello, %s!", helloTo); | ||
System.out.println(helloStr); | ||
|
@@ -85,7 +85,7 @@ public class Hello { | |
We are using the following basic features of the OpenTracing API: | ||
* a `tracer` instance is used to create a span builder via `buildSpan()` | ||
* each `span` is given an _operation name_, `"say-hello"` in this case | ||
* builder is used to create a span via `startManual()` | ||
* builder is used to create a span via `start()` | ||
* each `span` must be finished by calling its `finish()` function | ||
* the start and end timestamps of the span will be captured automatically by the tracer implementation | ||
|
||
|
@@ -101,7 +101,7 @@ Our `pom.xml` already imports Jaeger: | |
<dependency> | ||
<groupId>com.uber.jaeger</groupId> | ||
<artifactId>jaeger-core</artifactId> | ||
<version>0.26.0</version> | ||
<version>0.27.0</version> | ||
</dependency> | ||
``` | ||
|
||
|
@@ -113,9 +113,9 @@ import com.uber.jaeger.Configuration.ReporterConfiguration; | |
import com.uber.jaeger.Configuration.SamplerConfiguration; | ||
|
||
public static com.uber.jaeger.Tracer initTracer(String service) { | ||
SamplerConfiguration samplerConfig = new SamplerConfiguration("const", 1); | ||
ReporterConfiguration reporterConfig = new ReporterConfiguration(true, null, null, null, null); | ||
Configuration config = new Configuration(service, samplerConfig, reporterConfig); | ||
SamplerConfiguration samplerConfig = new SamplerConfiguration().withType("const").withParam(1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about we use a simple There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jpkrohling, I just started learning OpenTracing, so I'm not familiar with "export the relevant env vars", yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. This is particular to Jaeger (not generic OpenTracing), and it might indeed confuse users. For reference, I'm talking about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you change this to use |
||
ReporterConfiguration reporterConfig = new ReporterConfiguration().withLogSpans(true); | ||
Configuration config = new Configuration(service).withSampler(samplerConfig).withReporter(reporterConfig); | ||
return (com.uber.jaeger.Tracer) config.getTracer(); | ||
} | ||
``` | ||
|
@@ -177,7 +177,7 @@ In the case of `Hello Bryan`, the string `"Bryan"` is a good candidate for a spa | |
to the whole span and not to a particular moment in time. We can record it like this: | ||
|
||
```java | ||
Span span = tracer.buildSpan("say-hello").startManual(); | ||
Span span = tracer.buildSpan("say-hello").start(); | ||
span.setTag("hello-to", helloTo); | ||
``` | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,9 @@ private Hello(Tracer tracer) { | |
} | ||
|
||
private void sayHello(String helloTo) { | ||
Span span = tracer.buildSpan("say-hello").startManual(); | ||
Span span = tracer.buildSpan("say-hello").start(); | ||
span.setTag("hello-to", helloTo); | ||
|
||
String helloStr = String.format("Hello, %s!", helloTo); | ||
span.log(ImmutableMap.of("event", "string-format", "value", helloStr)); | ||
|
||
|
@@ -32,8 +32,8 @@ public static void main(String[] args) { | |
throw new IllegalArgumentException("Expecting one argument"); | ||
} | ||
String helloTo = args[0]; | ||
Tracer tracer = Tracing.init("hello-world"); | ||
new Hello(tracer).sayHello(helloTo); | ||
tracer.close(); | ||
try (Tracer tracer = Tracing.init("hello-world")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the tracer closable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tracer is com.uber.jaeger.Tracer in this class, which implements the Closeable interface. io.opentracing.Tracer, however, does not extend the Closeable interface. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [off-topic] Would be a good improvement for OpenTracing :) |
||
new Hello(tracer).sayHello(helloTo); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ | |
import lib.Tracing; | ||
|
||
public class HelloActive { | ||
|
||
private final Tracer tracer; | ||
|
||
private HelloActive(Tracer tracer) { | ||
|
@@ -17,7 +17,7 @@ private HelloActive(Tracer tracer) { | |
private void sayHello(String helloTo) { | ||
try (Scope scope = tracer.buildSpan("say-hello").startActive(true)) { | ||
scope.span().setTag("hello-to", helloTo); | ||
|
||
String helloStr = formatString(helloTo); | ||
printHello(helloStr); | ||
} | ||
|
@@ -43,8 +43,8 @@ public static void main(String[] args) { | |
throw new IllegalArgumentException("Expecting one argument"); | ||
} | ||
String helloTo = args[0]; | ||
Tracer tracer = Tracing.init("hello-world"); | ||
new HelloActive(tracer).sayHello(helloTo); | ||
tracer.close(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll still need this until we release 0.31.1, which includes the JVM shutdown hook. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have been waiting for opentracing/opentracing-java#250 and opentracing/specification#128 to move forward with the Closeable spec, but it seems this will take a while. I've updated this pull request to upcast to JaegerTracer in a try-resource block. |
||
try (Tracer tracer = Tracing.init("hello-world")) { | ||
new HelloActive(tracer).sayHello(helloTo); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should change it to
io.jaegertracing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, ok. I didn't know this package has been moved to io.jaegertracing:jaeger-core. The java package names have changed, so imports and fully qualified class references would have to be updated too. I can update this pull request with these changes as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
I think you have noticed as well, but we are on 0.310. already, so, a version bump to that one would be good.