Opentelemetry-java: GlobalTracer implementation

Created on 11 Apr 2019  路  12Comments  路  Source: open-telemetry/opentelemetry-java

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:

  • Defacto standard for SPI in Java
  • Always works within the specific context of a classloader. Different classloader hierarchies sometimes make this behave kind of 'strange' (complicated, unexpected).

About manual registration:

  • Inflexible, but simple and easy to reason about.
Agreed Feedback Requested

Most helpful comment

I have started looking into this. I definitely agree that GlobalTracer is a useful primitive singleton.

It's useful when:

  • working in a non-CDI environment - e.g. tracing servlet filter registered via web.xml
  • a fallback mechanism when initializing instrumentation - e.g. instrumentation provides a default constructor

Bad OpenTracing designs:

  • missing API to reset tracer - e.g. useful when a tracer needs to be wrapped, testing
  • missing API for unwrapping - sometimes it's needed to get the concrete implementation. E.g. Java JPA unwrap methods.

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.

All 12 comments

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:

  1. Instantiating a new TracerFactory, if a TracerFactory has been registered via META-INF/services.
  2. Instantiating a new 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:

  1. Providing a static registry for a 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:

  • Always accept a reference to a tracer at construction.
  • Fall back to global tracer if no tracer reference is provided.

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.

Conclusion:

  • We will go with a GlobalOpenConsensus object for now, which contains a reference to every OpenConsensus component (tracer, metrics, context propagation).

TODO

  • How can we best provide guidance to users who do not want to use this?
  • How can we provide guidance to instrumentation developers so that they do not force it on their users?

@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:

  • working in a non-CDI environment - e.g. tracing servlet filter registered via web.xml
  • a fallback mechanism when initializing instrumentation - e.g. instrumentation provides a default constructor

Bad OpenTracing designs:

  • missing API to reset tracer - e.g. useful when a tracer needs to be wrapped, testing
  • missing API for unwrapping - sometimes it's needed to get the concrete implementation. E.g. Java JPA unwrap methods.

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

  • [x] Add global registry for Metrics and tags
  • [ ] move resolution code to provider interfaces and change interfaces to abstract classes

Tracer, tagger and metrics have been moved to OpenTelemetry

Was this page helpful?
0 / 5 - 0 ratings