Update lessons

[safris]

Hi @yurishkuro, I went through the lessons in this repository yesterday. The changes in this pull request identify the changes to the OpenTracing APIs since the lessons were written. I also updated run.sh to allow the lessons to run on jdk9+.

[safris]

Hi @yurishkuro and @jpkrohling, I pushed a commit that upgrades to io.jaegertracing:jaeger-core:0.31.0.

[safris]

Hi @yurishkuro and @jpkrohling, I caught a mistake in the work I had done to upgrade Jaeger to 0.31.0. The mistake was that the lessons stopped communicating with Jaeger that's running in Docker. The reason for this is due to the fact that in 0.31.0, it seems that the service provider for SenderFactory does not live in jaeger-core any more. It took a bit of digging, but I found ThriftSenderFactory in jaeger-thrift. This was elusive, because there were no compile-time or runtime errors to help me realize something was wrong. The reason I realized something was wrong is by noticing the "No suitable sender found. Using NoopSender, meaning that data will not be sent anywhere!" WARN message in the log. I have updated pom.xml and the README.md.

[yurishkuro]

@SevaSafris use jaeger-client instead of jaeger-core, the former pulls all needed dependencies

[yurishkuro]

if you're removing the calls to tracer.close(), you will not get clean traces, unless we implement JVM shutdown hook in Jaeger.

[safris]

@yurishkuro, regarding tracer.close(): The close method is defined in io.jaegertracing.internal.JaegerTracer, but it is not defined in io.opentracing.Tracer. If we keep the tracer.close() method call, then the Hello class becomes implementation specific to Jaeger as the OT vendor. Perhaps the close() method should be added to io.opentracing.Tracer?

[jpkrohling]

@yurishkuro said:

if you're removing the calls to tracer.close(), you will not get clean traces, unless we implement JVM shutdown hook in Jaeger.

I submitted a PR last week to add this. It should be available in the next version of the client.

@SevaSafris said:

Perhaps the close() method should be added to io.opentracing.Tracer?

I believe so too, but this should probably be discussed/suggested as an issue opened against https://github.com/opentracing/opentracing-java . Would you be interested in starting a discussion there about this?

[safris]

@yurishkuro, I submitted a new issue for discussion here.

[jpkrohling]

What's the status of this? I'd like to run through the Katacoda scenarios and see if anything is broken, as it uses this code here.

[jpkrohling]

This LGTM, but please let's not get it merged until at least Tuesday, as @yurishkuro is presenting on Monday and will use this repo. As the Katacoda scenarios use the same code base, this has potential to break the scenarios.

I'll try to get the scenarios up to date next week, in line with this PR, unless @SevaSafris wants to give it a shot as well. The repository is this: https://github.com/katacoda-scenarios/opentracing-scenarios

[safris]

@jpkrohling, apologies for the delayed response -- I'm just getting over a cold. Regarding the scenarios: I have my full focus on the java-agent right now, and won't be able to get around to the scenarios until next week perhaps.

[jpkrohling]

Don't worry! I'm starting to check the Katacoda scenarios based on this branch and will send a PR with the required fixes to that repo.

[jpkrohling]

Could we aim to get this merged by tomorrow, preferably today? I'm OK with the changes here, even though I'd prefer to have 0.32.0 instead of 0.31.0 and omit the tracer.close() calls, but those can be done at a follow up PR.

Once this here is merged, I'll ask @BenHall to merge katacoda-scenarios/opentracing-scenarios#3

[yurishkuro]

@jpkrohling you have merge writes here

[jpkrohling]

That's true :-) Merging!