api, core: add ClientCallTracer APIs for tracing components with capability to expose tracer properties to external frameworks #6081
Conversation
…e callbacks in call events, populate tracer attributes from ClientCallTracer instances.
…ullLifecycleTracer to avoid name collision, changed ClientInterceptor implementation in CensusTracingModule to use ClientCallTracer, modified tests to accommodate with new apis.
…acer.Factory to ClientCalls.
voidzcy
force-pushed
the
feature/add_new_client_call_tracer_api
branch
from
605a2ba
to
e7317d5
Compare
| @@ -279,4 +279,14 @@ public void setMessageCompression(boolean enabled) { | |||
| public Attributes getAttributes() { | |||
| return Attributes.EMPTY; | |||
| } | |||
|
|
|||
| /** | |||
| * Returns tracer specific properties (if any) attached by tracing components. | |||
There was a problem hiding this comment.
Add a link to ClientCallTracer.
Also say that this is available as soon as the ClientCall is created.
| * | ||
| * @param builder receiver of tracer information. | ||
| */ | ||
| public void getTracerAttributes(Attributes.Builder builder) {} |
There was a problem hiding this comment.
Define an annotation like in https://github.com/grpc/grpc-java/blob/master/api/src/main/java/io/grpc/EquivalentAddressGroup.java#L142 . Annotate this argument, ClientCall.getTracerAttributes() and the attribute keys with this annotation will allow the user to associate them more easily.
|
|
||
| @VisibleForTesting | ||
| static final Attributes.Key<TraceId> TRACE_ID_ATTRIBUTES_KEY = Attributes.Key | ||
| .create("census-trace-id"); |
There was a problem hiding this comment.
We need to expose those keys to our internal framework. If we had moved Census integration to a separate artifact, we would simply expose those keys from a public class in that artifact. For now, we have to keep it internal. Maybe we can add a public CensusInternalAccessor class that exposes these keys.
There was a problem hiding this comment.
io.grpc package does not have Census dependency, where should this accessor class go?
| * | ||
| * @param status the result of the remote call. | ||
| */ | ||
| public void callEnded(Status status) {} |
There was a problem hiding this comment.
By looking at these tracking methods, I realize that they essentially overlap with the functionality of ClientInterceptor. I question myself whether it's really necessary to have such a large API.
Alternatively, ClientCallTracer could have only getTracerAttributes(). We don't need the new setter on ManagedChannelBuilder, instead we add withClientCallTracer() on CallOptions (possibly deprecate withStreamTracerFactory(). Keep using ClientInterceptor in CensusTracingModule, and let the interceptor to set ClientCallTracer for new calls, and grab the Span from ClientCall.getTracerAttributes() right after creating the call.
That looks like a much smaller change. Also ClientInterceptor probably has less impact on performance (because you don't iterate on the list of tracers on each callback). Sorry for @voidzcy that I came up this idea only after you made the code changes.
There was a problem hiding this comment.
That's ok, no worries.
But, does that solve ordering constraint of ClientInterceptors (assuming in the future we move Census modules out of core and no longer enforce Census interceptors run last)?
Framework interceptor needs to check two places to get the Span. One is from ClientCall#getTracerAttributes() for the case when Census interceptor runs first, and the other is from callOptions passed to ClientInterceptor#interceptCall() for the case when framework interceptor runs first.
There was a problem hiding this comment.
Framework interceptors would get the Span only from ClientCall#getTracerAttributes(). As soon as the Framework interceptors get a ClientCall from next.newCall(...), it can be sure the ClientCallTracer from the Census interceptor has been passed to the ClientCallImpl, and getTracerAttributes() can be immediately called, no matter which interceptor is run first.
There was a problem hiding this comment.
Oh, you are right. next.newCall(...) runs all the way down to ClientCallImpl. The Span will be there.
There was a problem hiding this comment.
I will have a separate PR to make the changes.
|
Closing in favor of #6090. |
New API class
ClientCallTracer/ClientCallTracer.Factory.ClientCallTracertracesClientCallevents and allow callers to populate tracer properties throughClientCallTracer#getTracerAttributes(...). Those attributes are further populated up toClientCallinterface for external access.Users attach
ClientCallTracer.Factoryinstances toChannelviaManagedChannelBuilder#clientCallTracerFactories(...). , which creates and attach a newClientCallTracerinstance to each newClientCallit creates.ClientCallImplinvokes callback methods defined inClientCallTracerat each call events.Implementation of #6080.
CensusTracingModuleis the first usage of this API. It transits fromClientInterceptortoClientCallTracerto give access to Census SpanId and TraceId to external users/frameworks.