Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Close tracer on JVM shutdown #546

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

jpkrohling
Copy link
Collaborator

Signed-off-by: Juraci Paixão Kröhling [email protected]

Which problem is this PR solving?

Short description of the changes

  • Adds a shutdown hook to close the tracer before the JVM dies. I tested this with the same code as the reporter. When running the code without an explicit tracer.close(), I see this in the logs for the client:
12:35:51.066 [main] INFO io.jaegertracing.internal.reporters.LoggingReporter - Span reported: f9e218d52dc14279:f9e218d52dc14279:0:1 - say-hello

But nothing on the server side. Starting an all-in-one container with --log-level=debug confirms that nothing is received on that end.

With this PR applied, I do see the trace:

12:40:39.544 [main] INFO io.jaegertracing.internal.reporters.LoggingReporter - Span reported: 3ce5effda0536731:3ce5effda0536731:0:1 - say-hello

And on the server side:

{"level":"debug","ts":1536316839.6288192,"caller":"app/span_processor.go:104","msg":"Span written to the storage by the collector","trace-id":"3ce5effda0536731","span-id":"3ce5effda0536731"}

Signed-off-by: Juraci Paixão Kröhling <[email protected]>
@ghost ghost assigned jpkrohling Sep 7, 2018
@ghost ghost added the review label Sep 7, 2018
@codecov
Copy link

codecov bot commented Sep 7, 2018

Codecov Report

Merging #546 into master will increase coverage by 0.12%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #546      +/-   ##
============================================
+ Coverage     88.64%   88.76%   +0.12%     
- Complexity      517      519       +2     
============================================
  Files            67       67              
  Lines          1893     1896       +3     
  Branches        243      243              
============================================
+ Hits           1678     1683       +5     
+ Misses          141      138       -3     
- Partials         74       75       +1
Impacted Files Coverage Δ Complexity Δ
...n/java/io/jaegertracing/internal/JaegerTracer.java 90.42% <100%> (+0.11%) 24 <0> (ø) ⬇️
...rtracing/internal/reporters/CompositeReporter.java 71.42% <0%> (ø) 6% <0%> (+1%) ⬆️
...java/io/jaegertracing/zipkin/ZipkinV2Reporter.java 100% <0%> (+28.57%) 3% <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 8e9492f...41a9d10. Read the comment docs.

@jpkrohling jpkrohling merged commit 41a9d10 into jaegertracing:master Sep 7, 2018
@ghost ghost removed the review label Sep 7, 2018
@ackerleytng
Copy link

Thanks! Looking forward to the next release!

@@ -120,6 +120,14 @@ protected JaegerTracer(JaegerTracer.Builder builder) {
}
this.ipv4 = ipv4;
this.tags = Collections.unmodifiableMap(tags);

// register this tracer with a shutdown hook, to flush the spans before the VM shuts down
Runtime.getRuntime().addShutdownHook(new Thread() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what will this do to unit tests?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jaeger-client-java does not automatically flush when program exits
4 participants