specification icon indicating copy to clipboard operation
specification copied to clipboard

RFC: Closeable Tracer

Open austinlparker opened this issue 5 years ago • 7 comments

This is the tracking issue for the Closeable Tracer RFC. See: https://github.com/opentracing/specification/blob/master/rfc/closeable_tracer.md

Summary

The OpenTracing Tracer API is extended to add a Close method for Tracers. This is intended to provide a clean decoupling of the Tracer at the interface and concrete implementation level in a consistent manner.

Current Status

The RFC has been proposed; It now needs to be discussed and make sure we've caught all of our important cases so that implementation can begin.

Related Issues

https://github.com/opentracing/opentracing-java/issues/250

austinlparker avatar Oct 01 '18 16:10 austinlparker

I don't understand why the Tracer interface needs to be Closeable. Tracer implementations, I understand. But not the Tracer interface.

Isn't a Tracer only destructed by its creator? And its creator already has the concrete implementation, so it understands any additional abilities.

TracerImpl tracer = new TracerImpl();
// ...use tracer...
tracer.close();

or

try (TracerImpl tracer = new TracerImpl()) {
  // ...use tracer...
}

pauldraper avatar Oct 31 '18 22:10 pauldraper

The attached rfc describes the situations when you may need to close a tracer without knowing its implementation.

yurishkuro avatar Oct 31 '18 23:10 yurishkuro

There's a singular code example and it references an implementation.

The only plausible case for not having the concrete importation seems to be "reflection", thus making the proposal the equivalent of "being able to use reflected Tracers but without much reflection".

pauldraper avatar Oct 31 '18 23:10 pauldraper

The only plausible case for not having the concrete importation seems to be "reflection"

I'm sure I mentioned this elsewhere, but my concrete case was when loading the tracer via ServiceLoader in Java, possibly using a library such as the OT-contrib's java-tracerresolver. In this scenario, the developer does not know beforehand which concrete implementation will reside in the classpath.

Here's one place where I could make use of it:

https://github.com/wildfly/wildfly/blob/d290e3f7461fbda9fe61584f2ca8553a75122656/microprofile/opentracing-smallrye/src/main/java/org/wildfly/microprofile/opentracing/smallrye/TracerInitializer.java#L84-L97

jpkrohling avatar Nov 02 '18 08:11 jpkrohling

See https://en.wikipedia.org/wiki/Interface_(computing) sections: Software interfaces in object-oriented languages and Programming to the interface

Basically, the point is so that users of the interface do not need to know about the underlying implementation at all. I ran into this problem when I was evaluating 2 different opentracing compliant libraries in the same code base simultaneously and I was using dependency injection to choose between one or the other based on a configuration parameter. Normally with interfaces, the only place that needs to know about the concrete implementation is the DI container/assembler. But, because the opentracing interface doesn't specify some things (like close, traceId, spanId), I had to do type casting and checking at runtime to determine the implementation which dirtied up my nicely abstracted code.

It seems to me that this proposal (and some of the others I see open, like #123) would be a good addition to the interface. Perhaps it would be good to look at the current popular implementations and figure out what they all do similarly that could be added to the opentracing interface.

codeman9 avatar Nov 13 '18 20:11 codeman9

Yeah, I wonder if flush would also be a useful addition? I was working on a sample using DI earlier and ran into this problem with having to cast an instance in order to force a flush of the tracer (https://github.com/lightstep/lightstep-tracer-csharp/blob/b89d93951fa2cf7ffa1935f1a03a17f8f9d76cf1/examples/LightStep.CSharpDITestApp/Program.cs#L38)

austinlparker avatar Nov 14 '18 17:11 austinlparker

Some further comments that indicate the utility of this RFC - https://github.com/opentracing/opentracing-java/pull/329#issuecomment-462382683 https://github.com/opentracing/opentracing-java/pull/329#issuecomment-462389128

austinlparker avatar Feb 11 '19 16:02 austinlparker