On Wed, Feb 17, 2021 at 6:17 PM Philippe Laflamme <[email protected]> wrote:
> This is 100% the behavior you want; anything else leads to madness. >> Interceptors inject themselves to the boundary because the application and >> the gRPC library and allow you to do "whatever you want" with the API. This >> produces a layering, and to preserve the layering "who calls who" *must* be >> the *exact* opposite for callbacks as normal method calls. >> > > I believe this can make sense in some cases, but I'm not sure that's the > case in general. I believe there are cases where you want the interceptor > to be at the same "logical" place on both paths. For example, a tracing > interceptor wants to be first on the outbound path so create a new span and > wants to be first on the callback path to make it the "active span" again. > As far as I can tell, it is not possible to have this setup with the > current API. > No, it really doesn't. What if there is a retrying interceptor? How about fault injection? How about binary logging? From our own experience of creating a tracing interceptor, I will strongly assert that it should not be split as you suggest. Initially it was unclear, but as time has gone on it has only been more obvious that it needs to be a single point. Otherwise most other features have to have custom tracing support to make everything cooperate. I completely agree there are competing requirements for where best to place it. But if you do split it as you suggest, it makes the data fuzzy and inconsistent. If you need low-level events when something happens, we do have an API for that <https://grpc.github.io/grpc-java/javadoc/io/grpc/CallOptions.html#withStreamTracerFactory-io.grpc.ClientStreamTracer.Factory->. But it is a "read-only" API. It is primarily for stats-gathering. It is run on arbitrary threads at important times, so it must execute quickly. The Interceptor+StreamTracer model uses two components that each have their own consistent view of what occurs, and discrepancies are dealt with when the results are combined. Note that a single RPC may have multiple streams created for it. Furthermore, the ServerInterceptor doesn't behave like this, i.e.: the > calls to ServerCall.Listener are in the same order as the calls to > interceptCall. So on that point, perhaps some small documentation updates > would help people figure this out. > That is factually incorrect. Rerun whatever test made you come to that decision. Hmm, right, my example was incomplete. What I meant to say was that the > same interceptor cannot do this kind of setup for both the outbound and > callback paths. > It can, by intercepting both the Call+Listener. But generally it is nonsensical to do so, since in one case it would call the library and in the other the application. Again, taking the tracing interceptor as an example, I believe you would > want something like this: > > void onMessage(...) { > try(activateSpan(span)) { super.onMessage(...) } > } > It took us a while to realize this the hard way, but you don't actually want to use the span for the callbacks. That can produce an infinite cascading of spans, as completing some work can cause other work to begin. You may want to propagate the *initial* span though, the span that was active before being replaced with `span`. The easiest way to realize "this isn't what you want" is to consider a simpler world where we could use blocking APIs everywhere: doRpc1(); doRpc2(); In that situation the two RPCs are related only by whatever span the code was called with. Now consider this non-blocking, callback-based approach: doRpc1().whenComplete(::doRpc2); You don't actually want doRpc2 within rpc1's span. The "best" span to use is the one active when whenComplete() is called, which here would be the same span for rpc1 and rpc2. Depending on the specific APIs in play that may be difficult to plumb, so sometimes you may have to accept the active span when doRpc1() was called. Maybe it still isn't obvious. Consider how ridiculous the results would be if this were made async but all the spans chained: for (int i = 0; i < 100; i++) doRpc(); Ignore the fact that you could do those RPCs in parallel. There's lots of cases where you need results from one RPC before starting another RPC and that still shouldn't impact the spans. I believe you want the tracing to be setup before all other interceptors on > both paths. For example, so that log statements related to that request are > attached to the same span. > I think different people may come to different decisions depending on what is important to them. I honestly don't know what log statements an interceptor would do such that I'd care about the span, with the exception of a "log every RPC" interceptor. -- You received this message because you are subscribed to the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/CA%2B4M1oO_UK20R3oAXoVWW0kXBCXByq-M7K%3DThJHGozG7dp4oyg%40mail.gmail.com.
smime.p7s
Description: S/MIME Cryptographic Signature
