Add close() method to io.opentracing.Tracer

[safris]

The Tracer interface does not define a close() method. Implementors of the Tracer interface, however, most often implement their own close() method to finalize traces, and to release resources. Since io.opentracing.Tracer.close() is not defined, application code that utilizes Tracer(s) must import the vendor-specific implementation in order for the close() method to be available. This effectively makes the application code coupled to the vendor, since the vendor-specific import is required.

By declaring a close() method in the io.opentracing.Tracer interface, vendor coupling can be avoided.

The close() method can be declared with a noop default, thus allowing for backward compatibility with all implementors of io.opentracing.Tracer that do not implement a close() method.

[jpkrohling]

The close() method can be declared with a noop default

IIRC, this library is still on Java 6, so, no default implementation for interfaces are available :/

[safris]

I just checked the io.opentracing:parent:pom, and it has compiler source and target set to 1.7.

[jpkrohling]

There seems to be an override here: https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/pom.xml#L32

[safris]

That override had eluded me. The override is itself a bad practice. So how should this move forward? 1) Declare the close() method without a default: This would force some vendors to have to implement the close() method if their Tracer(s) don't already have it implemented. 2) Remove the override: The class bytecode versions across the submodules are not consistent, which has the potential to lead to frustrating situations for vendors and app developers (this being one of them). I cannot comment on the reason for the override, as there is no comment in the pom where the override is set. Is this a path worth following? It's out of the scope of this issue, so a new issue would have to be created. 3) Do nothing: Keep the jdk1.6 override, and do not declare a Tracer.close(). This would force application code to be coupled to vendor implementations of the Tracer interface.

[yurishkuro]

This issue has come up a number of times before. Maybe worth searching for history for the arguments.

Personally I am supportive of adding close(). While the reasons against raised previously are still valid, it is clear that community needs this mechanism to release the tracer and its resources. I believe we have close() in one of the languages (Python?) and there have been no complaints from implementors.

However, backwards compatibility is an issue.

[yurishkuro]

Btw, option 4 to the list above - declare optional interface for closing and cast the tracer.

[safris]

Yes, this is an Option 4. However, this option is a hack that would further bury the primary issues (not having a close() on Tracer, and the jdk1.6 override). I believe Option 4 would be to incur a mindful technical debt.

[carlosalberto]

This was being addressed in #250 - maybe worth checking the arguments there (at the time we had decided to not add this method and put if in a FAQ, which... never happened).

Personally I'd recommend we 1) do not override the Java version (to avoid confusion) and stick to Java 1.6 (using 1.7 or newer would require another discussion IHMO), and 2) if we add this close() method, add it directly to the Tracer interface.

[yurishkuro]

I am going to close this as a dup of #250, please continue discussion on close() there. Regarding dropping 1.6 - a separate issue altogether. We had a similar 1.6 discussion https://github.com/jaegertracing/jaeger-client-java/issues/511 and did not arrive at an agreement, and that's just a single implementation of the API.