Support for subclassing instrumentation Tracers in custom distributions

[trask]

Use some kind of lookup (SPI) to instantiate the instrumentation Tracer singletons so that they can be subclassed in custom distributions.

This will enable people to capture additional attributes from standard Request/Response type objects, e.g. #566 (comment), #776 (comment)

A similar hook has proven useful in Brave per @anuraaga 👍.

[pavolloffay]

I can work on this, but first I would like to clarify what would be the best approach.

Would each instrumentation define SPI to get its tracer or just an SPI to load the BaseTracer? Ideally the tracer subclasses should access typed request/response objects.

[trask]

Great!

My first thought would be SPI lookup for each tracer, e.g. io.opentelemetry.instrumentation.auto.grpc.server.GrpcServerTracer, to see if there's a subclass available to use instead.

It's a bit odd to use SPI lookup for a (non-abstract) class though.

And a complication is that we'll need to inject the subclass as a "helper class", and any other classes that it brings along with it (e.g. anonymous inner classes).

So maybe a separate SPI makes sense for loading these kinds of customizations, that can return the normal tracer class name, the subclass to use instead, and a list of helper classes to inject along with the subclass. We could load all of these SPI classes at agent init (they can live in AgentClassLoader) and build a map with an instrumentation API on top of the map that all the instrumentation use to inject extra helper classes and instantiate their tracers (which will happen in the user class loader).

[pavolloffay]

It's a bit odd to use SPI lookup for a (non-abstract) class though.

Yes I wanted to avoid doing this, but I cannot think of a better approach. So we will define an SPI for all tracer classes. We can add support gradually, one by one.

[trask]

Oh, there's a second proposal above that wasn't clear. E.g. create a single SPI interface something like:

interface CustomTracer {
  String getTracerClassName();
  String getCustomTracerClassName();
  List<String> getHelperClassNames();
}

And use ServiceLoader to load as many of these as are present during agent init and build a map keyed on tracer class name.

And add a method to instrumentation api, e.g.:

static <T> createTracer(Class<T> clazz)

that will check for one of these custom tracers and first, inject the helper classes, and instantiate and return custom tracer instead

[pavolloffay]

I get it now, do you think there can be any perf overhead at agent start time?

[trask]

always 😄, though I think this is probably a small worry relative to other agent init work

[pavolloffay]

@iNikem @anuraaga any opinions on this?

If no I would like to start working on @trask's proposal from #778 (comment)

[iNikem]

I like the idea of having SPI for overriding Tracers. I like the idea of having just one SPI to override any tracer and not one per tracer. I am not sure that using tracer class name is the best way to identify tracer to be overridden. We may try and see how this will come out :)

I wonder, how will this work with tracers hierarchies? E.g. this will not allow overriding HttpClientTracer.

Also, should we define extension points for tracers? E.g. we have more or less standard onRequest and onConnection, but they are not standardised. And I don't particularly like them :S

[anuraaga]

+1 to having one SPI to override any tracer rather than per-tracer, it seems a bit simpler to me. But seems like a good approach to me.

Also, should we define extension points for tracers? E.g. we have more or less standard onRequest and onConnection, but they are not standardised. And I don't particularly like them :S

+10

[trask]

This will enable people to capture additional attributes from standard Request/Response type objects, e.g. #566 (comment), #776 (comment)

Closing, we are going a different direction to provide this functionality (#2596)