Java Invocation correlation: Span.invocation(...)

[codefromthecrypt]

Current state

It is quite normal to have access to Java invocation metadata, such as the method invoked and Java parameters. This is easily the case in AOP, such as AspectJ, but it is also readily available in RPC frameworks like Spring WebMVC, Jersey and Armeria. Here's an example of retrofit from a tweet by @swankjesse:

 class InvocationLogger implements Interceptor {
   @Override public Response intercept(Chain chain) throws IOException {
     Request request = chain.request();
     Invocation invocation = request.tag(Invocation.class);
     if (invocation != null) {
       System.out.printf("%s.%s %s%n",
           invocation.method().getDeclaringClass().getSimpleName(),
           invocation.method().getName(), invocation.arguments());
     }
     return chain.proceed(request);
   }
 }

https://square.github.io/retrofit/2.x/retrofit/index.html?retrofit2/Invocation.html

This design is about how to get data like this into the span such that correlation or tagging is possible.

Current state and motivation

Right now, we add tags like "mvc.controller.class" and "mvc.controller.method" on a per-framework basis. We have separate tag names for this in webmvc, jaxrs and jersey, for example. However, we might consider how useful separate tag names for the invocation class and method are? For example.. how important is it that this class is a MVC or Jersey one? Would it not be easier to just look up "invocation.class" and find anything related to it?

Moreover, we have code directly correlated with an upstream or downstream method. For example, the direct or once removed descendant of something like Retrofit would be the OkHttp interceptor. Having the ability to correlate at "finish time", a remote handler with a Java invocation could have value. For example, you could have metrics that correlate these together even if the span tags only exist on the parent or grandparent. How?

Solve via Span.invocation(...)

One how could be to add a new method to Span: Span.invocation(...) This allows the MutableSpan, used by the FinishedSpanHandler, to see data including Class<?> and Method<?>, where relevant, regardless of the tagging policy. Remote requests like HttpRequest` could have an optional property of the invocation for correlation purposes. As everything is optional and doesn't effect Java interfaces, no one would break.

Someone could add a InvocationParser to ensure every span tagged with their invocation includes coherent data; by not deleting the old tags, and not forcing new ones, no one would break.

Third party finished span handlers such as metrics or tracing backends with richer models could see invocation parameters as they would also exist on MutableSpan even if ignored by tagging.

Why not propagate it?

We could choose to propagate the invocation data downstream in the trace context. This would be trickier due to increased size of the context and the fact that these data are not visible during context construction. For example, a local Java method, it is likely you can see the invocation parameters. A client call, you could also.

Server side is trickier because of the layering of the abstraction. We'd want to correlate a server call with its first downstream handler, but a wire-level server, such as netty, exists below a higher abstraction, such as WebFlux. This means it starts and creates its context well before it could know its eventual handler. Similar to the request route, it might not know until its already serviced the request! Hence propagation, wouldn't work in cases like this.

[codefromthecrypt]

cc @kojilin @jplock @marcingrzejszczak @nicmunroe

[anuraaga]

I think currently this doesn't model an invocation since class / method are just the definition. I guess the okhttp class must also have an object array of params.

Also might be better to use objects instead of class / method. In frameworks like gRPC, an RPC implementation, while often tied to a class, doesn't have to be, it could even just be a lambda.

I had to change a parameter in Armeria's doc service from class to object for this reason. In gRPC, the MethodDescriptor serves the purpose of the class / method.

[codefromthecrypt]

Whether or not we can read the parameters, is it not about the invocation of a java method? We do use the word invocation to describe the trace context at the point of invocation of that caused the span. Does it matter strictly if we can read parameters or not when describing the context?

[codefromthecrypt]

regardless good point to ensure we can handle lambda based @anuraaga. This should be in a unit test and drive the model requirements. I suspect aspectj etc also have some considerations for this and we can look into their modeling in addition to grpc.

That said, I think minimally we should consider the existing class/method requirements we have primarily as these are concretely in our code.

[anuraaga]

Whether or not we can read the parameters, is it not about the invocation of a java method?

Don't think I understand the question. But invoking a Java method requires both the definition (Class / Method) and parameters. There is only one method in an app but there are infinite invocations.

Without parameters, I would call it something like appMethod or javaMethod rather than an invocation.

[codefromthecrypt]

Don't think I understand the question. But invoking a Java method requires both the definition (Class / Method) and parameters. There is only one method in an app but there are infinite invocations.

of the enclosed invocation, we may or may not chose to or be able to capture the parameters. We don't control this completely as what's exposed is framework specific. The parts that we can read are still the invocation, even if not all of it is visible at tagging time. Further I don't really expect people to tag all parameters.

[codefromthecrypt]

PS sorry I did write Span.invocation(Class<?> clazz, Method<?> method) which was not my intention to say that would be the final signature.. rather to get thoughts going. I think I caused confusion by putting a signature out there.. I'll change to just say Span.invocation()

[anuraaga]

Thanks for clarifying - I missed that intention indeed.

Also, how does this relate to #999? Will we still have RpcRequest?

[codefromthecrypt]

Also, how does this relate to #999? Will we still have RpcRequest?

RpcRequest is not always associated with a Java method (ex incoming proto service)

   * The unqualified, case-sensitive method name. Prefer the name defined in IDL to any mapped
   * {@link Method#getName() Java method name}.

So, I would expect us to likely need an Invocation type of some sort which would be a correlation property of RpcRequest (and possibly HttpRequest)..

[anuraaga]

Cool - that also solves my question about lambda, such cases may not have a Java invocation attached, but instead could have IDL information in the RpcRequest.

Adding Java invocation as in this issue seems good to me