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

#585 offer manual shutdown of jaeger tracer #586

Conversation

supersven
Copy link
Contributor

Which problem is this PR solving?

Sven Tennie added 2 commits February 8, 2019 17:13
Do not register a JaegerTracer shutdown hook in Glassfish/Payara as it
caused exceptions: It is executed after the container is shutdown and
the class loader might not be available anymore.

Signed-off-by: Sven Tennie <[email protected]>
When this flag is `true`, no shutdown hook is registered at the JVM.
This is useful in environments that do not support this kind of hooks
or when the user wants to control the closing of JaegerTracer by
oneself.

Signed-off-by: Sven Tennie <[email protected]>
@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #586 into master will decrease coverage by 0.22%.
The diff coverage is 44.44%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #586      +/-   ##
============================================
- Coverage      89.5%   89.27%   -0.23%     
- Complexity      539      540       +1     
============================================
  Files            68       68              
  Lines          1944     1949       +5     
  Branches        250      251       +1     
============================================
  Hits           1740     1740              
- Misses          130      133       +3     
- Partials         74       76       +2
Impacted Files Coverage Δ Complexity Δ
...n/java/io/jaegertracing/internal/JaegerTracer.java 89.01% <44.44%> (-1.67%) 26 <1> (+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 f5f6ad1...9fdc4c1. Read the comment docs.

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

kinda dirty, but I don't mind.

Copy link
Collaborator

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

Looks OK for a first version. If we ever need to make this smarter, we can create a specific method that determines whether or not to enable the hook.

@jpkrohling jpkrohling merged commit c88d448 into jaegertracing:master Feb 13, 2019
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.

3 participants