In order to not forget about this I'm creating this issue - this is regarding on whether we should use a ServiceLoader approach or a manual static registration (such as OT GlobalTracer.register(Tracer tracer))
About the ServiceLoader approach:
About manual registration:
I am not sure what "ServiceLoader approach" means for OpenFoo - just defining something like TracerFactory interface?
-1 to introducing global tracer, it received a lot of deserved criticism, I don't think it's worth it.
One issue with a ServiceLoader is it is not possible to override once it has been set. I believe we have OpenTracing users who need to do this.... I will check.
For now, let's go with ServiceLoader.
The GlobalTracer serves a different purpose than does the SPI mechanism.
The SPI mechanism is responsible for:
TracerFactory, if a TracerFactory has been registered via META-INF/services.Tracer, if a Tracer has been registered via META-INF/services.The SPI mechanism is used to create new instances.
The GlobalTracer is responsible for:
Tracer instance.The GlobalTracer is effectively a "Tracer registry", and is used to register a single instance of a Tracer that can thereafter be retrieved by any caller.
The TracerResolver uses both the SPI and GlobalTracer. The TracerResolver first uses the SPI mechanism to locate either a TracerFactory and/or a Tracer, and then attempts to instantiate them, in order of priority. Once a successful instantiation is accomplished, that instance is registered with GlobalTracer.
Let's investigate what would happen if we were to remove the GlobalTracer altogether:
Use of the TracerResolver, or any direct use of the SPI mechanism, would lead to the creation of different Tracer instances each time. Users of OpenTracing would thus be forced to implement some kind of "caching" mechanism, where the first instance obtained from the SPI mechanism could be registered. And, well, that would be their own version of GlobalTracer.
Therefore, I believe the GlobalTracer is an invaluable utility in the OpenTracing API.
Concerns regarding class loading complexity do not really apply to the GlobalTracer. The reason is simply due to the fact that, if there were such class loader complexities, the OpenTracing API would have to be present in the "special class loader" anyway for the Tracer instance to be instantiable by any mechanism (SPI, or direct invocation). If it's possible to instantiate the Tracer in the "special class loader", then the OpenTracing API is guaranteed to be present in the "special class loader". If the API is present, then the GlobalTracer is present too (reasonably speaking). Hence, if there is a class loading complexity issue, then one would simply not be able to instantiate a Tracer, making the downstream problem of "not being able to register this tracer" with the GlobalTracer a moot point.
@SevaSafris your arguments are valid, but I am still not convinced of the need for GT, at least in the main API. There are many situations when a global registry is not needed because the application framework has a different mechanism for that (e.g. Spring). By providing GT in the main API we create murky situation where a Spring user is unsure if they should use GT or inject tracer as a bean. And in general, global singleton is not the best design pattern, so it encourages bad practices where instrumentation libraries start depending on it instead of designing themselves to accept the tracer as a parameter.
There _is_ an issue with instrumentation relying on this, which forces the application developer to use it. We should define guidelines to instrumentation developers:
We need to have a review process for instrumentation listed in the Registry. and ensure best practices are being followed. I suggest this checklist is part of it.
With the new approach, we will have multiple objects: tracer, metrics, tagger, logger, etc.
Can we have a wrapper object which contains a reference to all of these, which users can pass as a reference, and be the object that is registered globally?
@pavolloffay would you like to take a crack at this?
Technically speaking, the GlobalTracer is not a part of the main API. It is a part of the opentracing-util artifact, which is not a linked dependency from opentracing-api. To include it in the runtime, one would have to explicitly include it in the dependency graph.
I believe the right approach to answer the question: "Should we keep the GlobalTracer around?" is to identify the most common use-cases, erring on the side of KISS (Keep It Simple, Stupid).
Will applications need some pattern to keep a reference of a Tracer? I believe the answer to this is: yes. And, a "KISS solution" to this is the design of the GlobalTracer itself.
Will there ever be a situation where a JVM will have more that 1 Tracer reference? I believe the answer to this question is: no. Hence, a global singleton / static reference is perfectly apt and the most "KISS solution".
If applications are using Spring, then their "bean patterns" should work on top of GlobalTracer.
The GlobalTracer can be more "KISS" than it is right now. Currently, the GlobalTracer has this "locking pattern", where, once a reference to a Tracer has been registered, the reference is permanent, as there is no way to replace it with another reference later.
What is the purpose of this complexity?
I consider the GlobalTracer as a "TracerRegistry". And, a "TracerRegistry" does not itself need to inherit from Tracer. I believe there is value to providing an simplistic standard "TracerRegistry" solution. If you believe such a solution does not belong to the opentracing-util artifact, perhaps it would be better placed in the opentracing-tracerresolver. Because, without a GlobalTracer or TracerRegistry, the TracerResolver would need to fill the gap by itself.
GlobalOpenConsensus object for now, which contains a reference to every OpenConsensus component (tracer, metrics, context propagation).@SevaSafris I believe this is acceptable, but please let me know if you see any hiccups. Obviously, this means that SpecialAgent will now be responsible for loading all of OpenConsensus, not just the tracer component.
I have started looking into this. I definitely agree that GlobalTracer is a useful primitive singleton.
It's useful when:
web.xmlBad OpenTracing designs:
The question is whether we want to provide just a registry or registry of singletons. If we can assume app startup is always controlled - used to init the registry we can go without singletons.
@SevaSafris could you please have a look at https://github.com/bogdandrutu/openconsensus/pull/218?
Adding TODOs
Tracer, tagger and metrics have been moved to OpenTelemetry
Most helpful comment
I have started looking into this. I definitely agree that
GlobalTraceris a useful primitive singleton.It's useful when:
web.xmlBad OpenTracing designs:
The question is whether we want to provide just a registry or registry of singletons. If we can assume app startup is always controlled - used to init the registry we can go without singletons.