Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split the resolver from the "factory" #30

Closed
jpkrohling opened this issue Aug 10, 2018 · 1 comment
Closed

Split the resolver from the "factory" #30

jpkrohling opened this issue Aug 10, 2018 · 1 comment

Comments

@jpkrohling
Copy link
Contributor

It's usually a good practice to have a loader (resolver, in this case) and a factory as different entities. This way, the loader can be extended or replaced and the factories would still work. This is specially important for an SPI, such as this one.

One concrete problem caused by the current approach is that one cannot easily provide a custom implementation of the TracerResolver, as current resolver (such as Jaeger's) extends the TracerResolver and exposes the tracer only via a protected method.

My proposal (pseudo code):

public interface TracerFactory {
    Tracer getTracer();
}

public class TracerResolver {
  public static Tracer resolve() {
    ServiceLoader<TracerFactory> factories = ServiceLoader.load(TracerFactory.class);
    // iterators, disambiguation, ...
    return factories.iterator().next().getTracer();
  }
}

// factory example, very similar to current TracerResolver subclasses
public class JaegerTracerFactory implements TracerFactory {
  public Tracer getTracer() {
    return Configuration.fromEnv().getTracer();
  }
}
@sjoerdtalsma
Copy link
Collaborator

I think your idea is an improvement over the current resolver, which was intentionally as simple as possible.

One concern:

  1. Could we introduce such a TracerFactory interface in a backward-compatible way?

Maybe let the current TracerResolver resolve both the new TracerFactory and the current TracerResolver subclasses?

I would like to create a release in which we document that people should implement a TracerFactory SPI but that allows them time to do so without existing instrumentations breaking by a simple java-tracerresolver library upgrade?

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

No branches or pull requests

2 participants