Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

RFC: Closeable Tracer #130

Open
austinlparker opened this issue Oct 1, 2018 · 7 comments
Open

RFC: Closeable Tracer #130

austinlparker opened this issue Oct 1, 2018 · 7 comments

Comments

@austinlparker
Copy link
Member

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

opentracing/opentracing-java#250

@pauldraper
Copy link

pauldraper commented Oct 31, 2018

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...
}

@yurishkuro
Copy link
Member

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

@pauldraper
Copy link

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".

@jpkrohling
Copy link

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

@codeman9
Copy link

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.

@austinlparker
Copy link
Member Author

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
Copy link
Member Author

Some further comments that indicate the utility of this RFC -
opentracing/opentracing-java#329 (comment)
opentracing/opentracing-java#329 (comment)

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

No branches or pull requests

5 participants