Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated tutorial to latest versions #3

Conversation

jpkrohling
Copy link
Contributor

This PR updates the scenarios to work with the latest changes to be introduced by @SevaSafris on the tutorial this is based on (yurishkuro/opentracing-tutorial#30).

This should be merged whenever that one is merged, to avoid the scenarios from being broken.

@SevaSafris, could you take a quick look at this one and see if this matches with your changes?

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@safris
Copy link

safris commented Oct 16, 2018

@jpkrohling, the changes to scenarios seem aligned to the work I did with the opentracing-tutorial. One comment though: tracer.close() still needs to be called. I ended up putting the JaegerTracer init in a try-resource block.

@jpkrohling
Copy link
Contributor Author

One comment though: tracer.close() still needs to be called. I ended up putting the JaegerTracer init in a try-resource block.

Even with 0.32.0? I think this shouldn't be necessary on that version, as I implemented a JVM shutdown hook that automatically closes the tracer instances.

@jpkrohling
Copy link
Contributor Author

This is now ready to be merged. @BenHall would you be able to review/merge this PR?

@jpkrohling jpkrohling force-pushed the Update-Scenarios-based-on-tutorial branch from 305da27 to 4f08ce3 Compare October 19, 2018 08:03
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling force-pushed the Update-Scenarios-based-on-tutorial branch from 4f08ce3 to 5f8bf25 Compare October 19, 2018 08:05
@BenHall BenHall merged commit 5a7a7cd into katacoda-scenarios:master Oct 19, 2018
@BenHall
Copy link
Contributor

BenHall commented Oct 19, 2018

Thanks for the updates!

@safris
Copy link

safris commented Oct 19, 2018

One comment though: tracer.close() still needs to be called. I ended up putting the JaegerTracer init in a try-resource block.

Even with 0.32.0? I think this shouldn't be necessary on that version, as I implemented a JVM shutdown hook that automatically closes the tracer instances.

Oh, I was not aware of 0.32.0. Should I also then upgrade the java-opentracing-tutorial to 0.32.0? This will resolve the concern re code coupling to Jaeger.

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

Successfully merging this pull request may close these issues.

3 participants