From e28408b3be7b2afc70383c3f3b3838c12136428f Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Mon, 18 Sep 2017 10:21:34 -0700 Subject: [PATCH 1/8] C# Interceptors Proposal Draft --- L12-csharp-interceptors.md | 213 +++++++++++++++++++++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 L12-csharp-interceptors.md diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md new file mode 100644 index 000000000..a4708e9f7 --- /dev/null +++ b/L12-csharp-interceptors.md @@ -0,0 +1,213 @@ +Title +---- +* Author(s): mehrdada +* Approver: jtattermusch +* Status: Draft +* Implemented in: C# +* Last updated: September 18, 2017 +* Discussion at: (filled after thread exists) + +## Abstract + +This proposal introduces client and server interceptor APIs in gRPC C#. + +The API provides the facility to register certain functions to run when an RPC +is invoked on the client or the server side. These functions can be chained and +composed flexibly as users wish. + +## Background + +gRPC C# does not have an interceptor API. Functionality can be partially +simulated on the client side by extending the `CallInvoker` class. However, +this has some limitations in composition and the ability to decouple +`CallInvoker` from each other. On the server side, similar or replacement +functionality was basically non-existent. + + +### Related Proposals: + +* [Ruby Client and Server + Interceptors](https://github.com/grpc/proposal/pull/34) proposal + +## Proposal + +This proposal consists of two independent pieces of infrastructure for server +and client interceptors, both of which are rooted in the new +`Grpc.Core.Interceptors` namespace in `Grpc.Core` assembly. + +### Client Interceptors + +Client interceptors derive from the new abstract base class +`Grpc.Core.Interceptors.ClientInterceptor`. This abstract class defines five +different hooks for various RPC types and invocations, namely +`BlockingUnaryCall`, `AsyncUnaryCall`, `AsyncClientStreamingCall`, +`AsyncServerStreamingCall`, and `AsyncDuplexStreamingCall`. The signature of +these methods correspond to each of their counterparts in `CallInvoker`, with +the notable distinction that each take an additional argument `next`, which is +a delegate that invokes the next step in the interceptor chain or the actual +underlying `CallInvoker` handler for the final interceptor in the chain: + +```csharp +TResponse BlockingUnaryCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, TResponse> next); +AsyncUnaryCall AsyncUnaryCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, AsyncUnaryCall> next); +AsyncServerStreamingCall AsyncServerStreamingCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, AsyncServerStreamingCall> next); +AsyncClientStreamingCall AsyncClientStreamingCall(Method method, string host, CallOptions options, Func, string, CallOptions, AsyncClientStreamingCall> next); +AsyncDuplexStreamingCall AsyncDuplexStreamingCall(Method method, string host, CallOptions options, Func, string, CallOptions, AsyncDuplexStreamingCall> next); +``` + +In the general case, the interceptors are not obligated to invoke `next` once. +They might choose to not continue with the call chain and terminate it +immediately by returning their own desired return value, or potentially call +`next` more than once, to simulate retries, transactions, or other similar +potential use cases. In fact, the ability to take an explicit `next` argument +is the primary rationale for needing a special-purpose interceptor machinery on +the client side, since `CallInvoker` is capable of doing a lot for us. +However, chaining `CallInvoker`s require each one to explicitly hold a +reference to the next `CallInvoker` in the chain. The `ClientInterceptor` +design, however, abstracts away the next function from the object and passes it +as a continuation to each interceptor hook. An internal `CallInvoker` +implementation chains interceptors together and ultimately with the underlying +`CallInvoker` and `Channel` objects. All of the functionality described so far +could have been implemented as an external library, and does not change the +non-intercepted code path at all. In addition to hook functions to be +overridden by the client interceptor implementations, the class provides a few +`static` methods that lets the interceptor register additional hooks to execute +code when certain events happen in asynchronous and streaming calls. These +helper static functions are the only reason this facility could not have been +implemented by an external library: they require access to the internal +constructors of `AsyncUnaryCall`, +`AsyncServerStreamingCall`, and so on: + +```csharp +protected static AsyncUnaryCall Intercept(AsyncUnaryCall call, + Func, TResponse> response = null, + Func, Metadata> responseHeaders = null, + Func, Func> getStatus = null, + Func, Func> getTrailers = null, + Func dispose = null); + +protected static AsyncServerStreamingCall Intercept(AsyncServerStreamingCall call, + Func, IAsyncStreamReader> responseStream = null, + Func, Metadata> responseHeaders = null, + Func, Func> getStatus = null, + Func, Func> getTrailers = null, + Func dispose = null); + +protected static AsyncClientStreamingCall Intercept(AsyncClientStreamingCall call, + Func, IClientStreamWriter> requestStream = null, + Func, TResponse> response = null, + Func, Metadata> responseHeaders = null, + Func, Func> getStatus = null, + Func, Func> getTrailers = null, + Func dispose = null); + +protected static AsyncDuplexStreamingCall Intercept(AsyncDuplexStreamingCall call, + Func, IClientStreamWriter> requestStream = null, + Func, IAsyncStreamReader> responseStream = null, + Func, Metadata> responseHeaders = null, + Func, Func> getStatus = null, + Func, Func> getTrailers = null, + Func dispose = null); +``` + +Client interceptors are registered on a `Channel` or `CallInvoker` via +extension methods named `Intercept` defined in `ChannelExtensions` and +`CallInvokerExtensions`. While they fundamentally operate on an underlying +`CallInvoker` and not a `Channel` directly, the ones that take a `Channel` as +an argument serve as a syntactic sugar for ease of construction in user code: +they simply create an underlying `DefaultCallInvoker` and route calls through +it. Since `Intercept` is the only way to register interceptors on a +`CallInvoker`, the `InterceptingCallInvoker` that chains interceptor and +exposes a `CallInvoker` can remain an internal class. + +An alternative design registering interceptors directly on a `Channel` +returning an "intercepted `Channel`" object was considered but ultimately +dismissed, despite having an advantage of 100% compatibility with anywhere a +`Channel` would have been used, since it added complexity to the `Channel` +object and potentially slowing down the fast code path where no interceptor is +registered. When invoking RPCs in user code, `CallInvoker` is sufficiently +close to a `Channel` for constructing client objects and using local variable +type inference (`var`), the actual user code would look identical in most +cases: + +```csharp +var interceptedChannel = channel.Intercept(interceptorObject); +// interceptedChannel type is really `CallInvoker`, but since +// both `CallInvoker` and `Channel` can be passed to the generated +// client code, the code looks similar. +var greeterClient = new GreeterClient(interceptedChannel); +``` + +A higher-level, more user-friendly, canonical base class for common interceptor +functionality can be provided by deriving from `ClientInterceptor` and adding +unified hooks (e.g. `BeginCall`, `EndCall`) that can be overridden once but +operate on all types of RPC invocations. + +In order to carry satellite data across interceptors and `CallInvoker`s, +this proposal suggests `CallOptions` to be amended with a `Tag` property +and a `WithTag` helped method to register a custom arbitrary object, or +a dictionary of values. This is the only other piece of client interceptors +that requires being implemented from within the `Grpc.Core` assembly due +to `internal` access requirements. + + +### Server Interceptors + +Server-side interceptors derive from the new `ServerInterceptor` class +implemented in `Grpc.Core.Interceptors` namespace. Server interceptors are +registered on individual service definitions as opposed to an entire server, +though it might be sensible to provide a syntactic sugar for adding an +interceptor chain to all services served by a `Server` instance. + +In particular, an instance of a class derived from `ServerInterceptor` is +registered via an extension method for class `ServerServiceDefinition`. This +method is defined as an extension method to decouple the server interceptor +machinery from the core gRPC infrastructure as much as possible. This +extension method wraps all of the existing handlers in the service definition +with ones intercepted by the passed interceptor object. + +```csharp +Server server = new Server +{ + Services = { Greeter.BindService(new GreeterImpl()).Intercept(new LogInterceptor()) }, + Ports = { new ServerPort("localhost", Port, ServerCredentials.Insecure) } +}; +server.Start(); +``` + +Since support for this handler substitution was needed from the +`ServerServiceDefinition` class itself, a general internal method +`SubstituteHandlers` is added to actually apply the mapping given the wrapping +logic as a delegate and return a new service definition object with the new +handlers. Beyond this, and slight modification to the definition of server +call handlers, no special support from the core gRPC machinery was needed, and +the fast path without any registered interceptor remains intact. + +Similar to client interceptors, server interceptors can override four functions +`UnaryServerHandler`, `ClientStreamingServerHandler`, +`ServerStreamingServerHandler`, and `DuplexStreamingServerHandler`. Likewise, +they all take a `next` argument that invokes the next interceptor in the chain +or the actual RPC handler at the end of the chain: + +```csharp +async Task UnaryServerHandler(TRequest request, ServerCallContext context, UnaryServerMethod next); +async Task ClientStreamingServerHandler(IAsyncStreamReader requestStream, ServerCallContext context, ClientStreamingServerMethod next); +async Task ServerStreamingServerHandler(TRequest request, IServerStreamWriter responseStream, ServerCallContext context, ServerStreamingServerMethod next); +async Task DuplexStreamingServerHandler(IAsyncStreamReader requestStream, IServerStreamWriter responseStream, ServerCallContext context, DuplexStreamingServerMethod next); +``` + +In order to carry satellite data between interceptors and the handler, adding a +`Tag` property to `ServerCallContext` may be justified. A more high-level +canonical derived class from `ServerInterceptor` for providing most common +functionalities can be done here as well. + +## Rationale + +The design goals where flexibility, decoupling and non-disruption to existing +code, and keeping the fast path fast and the design trade-offs made intend to +accomplish these goals. Some of these decisions were discussed above. + + +## Implementation + +[C# Interceptor Support Pull Request](https://github.com/grpc/grpc/pull/12613) From d9eb02886325d609192760733383f086ce2b42ee Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Mon, 18 Sep 2017 23:42:36 -0700 Subject: [PATCH 2/8] Add discussion link and update status to In Review --- L12-csharp-interceptors.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index a4708e9f7..47cf8cec7 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -2,10 +2,10 @@ Title ---- * Author(s): mehrdada * Approver: jtattermusch -* Status: Draft +* Status: In Review * Implemented in: C# -* Last updated: September 18, 2017 -* Discussion at: (filled after thread exists) +* Last updated: September 19, 2017 +* Discussion at: https://groups.google.com/d/topic/grpc-io/GKcnFC3oxhk/discussion ## Abstract From d186c6d411a45df029ac787ea0b862cda2b2ce3a Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Tue, 19 Sep 2017 10:30:48 -0700 Subject: [PATCH 3/8] Amend the proposal to address satellite data --- L12-csharp-interceptors.md | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index 47cf8cec7..fddb8dfa5 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -143,12 +143,12 @@ functionality can be provided by deriving from `ClientInterceptor` and adding unified hooks (e.g. `BeginCall`, `EndCall`) that can be overridden once but operate on all types of RPC invocations. -In order to carry satellite data across interceptors and `CallInvoker`s, -this proposal suggests `CallOptions` to be amended with a `Tag` property -and a `WithTag` helped method to register a custom arbitrary object, or -a dictionary of values. This is the only other piece of client interceptors -that requires being implemented from within the `Grpc.Core` assembly due -to `internal` access requirements. +In order to carry satellite data across interceptors and `CallInvoker`s, this +proposal suggests `CallOptions` to be amended with an `Items` dictionary and a +`WithItems` helper method to register a custom dictionary which would hold +shared state across the interceptors and `CallInvoker`s. This is the only other +piece of client interceptors that requires being implemented within the +`Grpc.Core` since it needs adding a member to `CallOptions` structure. ### Server Interceptors @@ -196,10 +196,9 @@ async Task ServerStreamingServerHandler(TRequest request, I async Task DuplexStreamingServerHandler(IAsyncStreamReader requestStream, IServerStreamWriter responseStream, ServerCallContext context, DuplexStreamingServerMethod next); ``` -In order to carry satellite data between interceptors and the handler, adding a -`Tag` property to `ServerCallContext` may be justified. A more high-level -canonical derived class from `ServerInterceptor` for providing most common -functionalities can be done here as well. +In order to carry satellite data between interceptors and the handler, this +proposal suggests adding an `Items` dictionary to `ServerCallContext` to hold +arbitrary state. ## Rationale From 4f02bcccd06da2263529cd7257378c4ddec819a6 Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Mon, 16 Oct 2017 10:43:01 -0700 Subject: [PATCH 4/8] Add more related proposal links --- L12-csharp-interceptors.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index fddb8dfa5..5d1f23a98 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -4,7 +4,7 @@ Title * Approver: jtattermusch * Status: In Review * Implemented in: C# -* Last updated: September 19, 2017 +* Last updated: October 16, 2017 * Discussion at: https://groups.google.com/d/topic/grpc-io/GKcnFC3oxhk/discussion ## Abstract @@ -24,10 +24,13 @@ this has some limitations in composition and the ability to decouple functionality was basically non-existent. -### Related Proposals: +### Related Proposals: * [Ruby Client and Server Interceptors](https://github.com/grpc/proposal/pull/34) proposal +* [Node Client Interceptors](https://github.com/grpc/proposal/pull/14) proposal +* [Python Client and Server + Interceptors](https://github.com/grpc/proposal/pull/39) proposal ## Proposal From ce36180b11f7bcb9854197e38e5c07dbbd7c3e3c Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Mon, 16 Oct 2017 12:15:24 -0700 Subject: [PATCH 5/8] Revise the C# interceptor proposal - Unify `ClientInterceptor` and `ServerInterceptor` classes into one `Interceptor` base class, so that implementing a single class to serve on both sides becomes possible. - Update the signature of the client interceptor hooks, taking a context object instead of the contextual data in-line. Also, rename the `next` delegate to `continuation` for clarity. - Update the server-side interceptor machinery to reflect the new design, which is able to intercept the call before reading the first request and express interest in further interception if necessary. - Formatting and clarifications. --- L12-csharp-interceptors.md | 292 ++++++++++++++++++++----------------- 1 file changed, 160 insertions(+), 132 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index 5d1f23a98..26b224ee5 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -18,10 +18,10 @@ composed flexibly as users wish. ## Background gRPC C# does not have an interceptor API. Functionality can be partially -simulated on the client side by extending the `CallInvoker` class. However, -this has some limitations in composition and the ability to decouple -`CallInvoker` from each other. On the server side, similar or replacement -functionality was basically non-existent. +simulated on the client side by extending the `CallInvoker` class. However, this +has some limitations in composition and the ability to decouple `CallInvoker` +from each other. On the server side, similar or replacement functionality was +basically non-existent. ### Related Proposals: @@ -34,104 +34,110 @@ functionality was basically non-existent. ## Proposal -This proposal consists of two independent pieces of infrastructure for server -and client interceptors, both of which are rooted in the new -`Grpc.Core.Interceptors` namespace in `Grpc.Core` assembly. +This proposal consists of two pieces of infrastructure for server and client +interceptors, both of which are placed in the new `Grpc.Core.Interceptors` +namespace in `Grpc.Core` assembly. + +While client and server interceptors use different hooks to intercept on RPC +invocations, they share the abstract base class `Interceptor` without any +abstract methods. The rationale for this is it enables a single class to act +both as a server interceptor and a client interceptor. An alternative +considered were using an `interface` instead of an abstract base class, but were +ruled out due to potential future versioning issues (adding a method to an +interface would be a breaking API change, whereas adding an additional method to +an abstract base should be fine as long as it is not marked `abstract`). Another +alternative that was ruled out was to separate it out to `ClientInterceptor` and +`ServerInterceptor` classes. This may have made each class look slightly more +lightweight, but realistically, it would be just as easy to implement just the +client methods and not the server methods on an interceptor. ### Client Interceptors Client interceptors derive from the new abstract base class -`Grpc.Core.Interceptors.ClientInterceptor`. This abstract class defines five -different hooks for various RPC types and invocations, namely +`Grpc.Core.Interceptors.Interceptor`. This abstract class defines five +different virtual methods to hook into various RPC invocation types, namely `BlockingUnaryCall`, `AsyncUnaryCall`, `AsyncClientStreamingCall`, -`AsyncServerStreamingCall`, and `AsyncDuplexStreamingCall`. The signature of -these methods correspond to each of their counterparts in `CallInvoker`, with -the notable distinction that each take an additional argument `next`, which is -a delegate that invokes the next step in the interceptor chain or the actual -underlying `CallInvoker` handler for the final interceptor in the chain: +`AsyncServerStreamingCall`, and `AsyncDuplexStreamingCall`. + +The request-unary interceptor hooks take a request value as their first +argument, followed by the common arguments for all of the methods: + +1. A `context` argument of a new type `ClientInterceptorContext` that encapsulates the context of the call. An older alternative + design used a similar signature to the corresponding method in the + `CallInvoker` class, but that would limit the ability to add new contextual + information to pass along without changing the signature and thus breaking + API compatibility. + +2. A `continuation` argument, which is a delegate that invokes the next step in + the interceptor chain or the actual underlying `CallInvoker` handler for the + final interceptor in the chain and returns a value of the same type as the + interceptor method. For unary requests, it takes the `request` value as its + first argument, followed by the `context` to proceed the invocation with. The + interceptor method is allowed to pass in a new or modified `context` along + and that is what the RPC continues with. For client-streaming RPCs, only the + `context` argument should be passed along to the `continuation`. ```csharp -TResponse BlockingUnaryCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, TResponse> next); -AsyncUnaryCall AsyncUnaryCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, AsyncUnaryCall> next); -AsyncServerStreamingCall AsyncServerStreamingCall(Method method, string host, CallOptions options, TRequest request, Func, string, CallOptions, TRequest, AsyncServerStreamingCall> next); -AsyncClientStreamingCall AsyncClientStreamingCall(Method method, string host, CallOptions options, Func, string, CallOptions, AsyncClientStreamingCall> next); -AsyncDuplexStreamingCall AsyncDuplexStreamingCall(Method method, string host, CallOptions options, Func, string, CallOptions, AsyncDuplexStreamingCall> next); +TResponse BlockingUnaryCall( + TRequest request, + ClientInterceptorContext context, + BlockingUnaryCallContinuation continuation) + +AsyncUnaryCall AsyncUnaryCall( + TRequest request, + ClientInterceptorContext context, + AsyncUnaryCallContinuation continuation) + +AsyncServerStreamingCall AsyncServerStreamingCall( + TRequest request, + ClientInterceptorContext context, + AsyncServerStreamingCallContinuation continuation) + +AsyncClientStreamingCall AsyncClientStreamingCall( + ClientInterceptorContext context, + AsyncClientStreamingCallContinuation continuation) +``` +AsyncDuplexStreamingCall AsyncDuplexStreamingCall( + ClientInterceptorContext context, + AsyncDuplexStreamingCallContinuation continuation) ``` -In the general case, the interceptors are not obligated to invoke `next` once. -They might choose to not continue with the call chain and terminate it -immediately by returning their own desired return value, or potentially call -`next` more than once, to simulate retries, transactions, or other similar -potential use cases. In fact, the ability to take an explicit `next` argument -is the primary rationale for needing a special-purpose interceptor machinery on -the client side, since `CallInvoker` is capable of doing a lot for us. -However, chaining `CallInvoker`s require each one to explicitly hold a -reference to the next `CallInvoker` in the chain. The `ClientInterceptor` +In the general case, the interceptors are not obligated to invoke `continuation` +just once. They might choose to not continue with the call chain and terminate +it immediately by returning their own desired return value, or potentially call +`continuation` more than once, to simulate retries, transactions, or other +similar potential use cases. In fact, the ability to take an explicit +`continuation` argument is the primary rationale for needing a special-purpose +interceptor machinery on the client side, since `CallInvoker` is capable of +doing a lot for us. That said, chaining `CallInvoker`s require each one to +explicitly hold a reference to the next `CallInvoker` in the chain. This design, however, abstracts away the next function from the object and passes it as a continuation to each interceptor hook. An internal `CallInvoker` implementation chains interceptors together and ultimately with the underlying `CallInvoker` and `Channel` objects. All of the functionality described so far could have been implemented as an external library, and does not change the -non-intercepted code path at all. In addition to hook functions to be -overridden by the client interceptor implementations, the class provides a few -`static` methods that lets the interceptor register additional hooks to execute -code when certain events happen in asynchronous and streaming calls. These -helper static functions are the only reason this facility could not have been -implemented by an external library: they require access to the internal -constructors of `AsyncUnaryCall`, -`AsyncServerStreamingCall`, and so on: +non-intercepted code path at all, therefore should not have any negative +performance impact when no interceptor is used. -```csharp -protected static AsyncUnaryCall Intercept(AsyncUnaryCall call, - Func, TResponse> response = null, - Func, Metadata> responseHeaders = null, - Func, Func> getStatus = null, - Func, Func> getTrailers = null, - Func dispose = null); - -protected static AsyncServerStreamingCall Intercept(AsyncServerStreamingCall call, - Func, IAsyncStreamReader> responseStream = null, - Func, Metadata> responseHeaders = null, - Func, Func> getStatus = null, - Func, Func> getTrailers = null, - Func dispose = null); - -protected static AsyncClientStreamingCall Intercept(AsyncClientStreamingCall call, - Func, IClientStreamWriter> requestStream = null, - Func, TResponse> response = null, - Func, Metadata> responseHeaders = null, - Func, Func> getStatus = null, - Func, Func> getTrailers = null, - Func dispose = null); - -protected static AsyncDuplexStreamingCall Intercept(AsyncDuplexStreamingCall call, - Func, IClientStreamWriter> requestStream = null, - Func, IAsyncStreamReader> responseStream = null, - Func, Metadata> responseHeaders = null, - Func, Func> getStatus = null, - Func, Func> getTrailers = null, - Func dispose = null); -``` - -Client interceptors are registered on a `Channel` or `CallInvoker` via -extension methods named `Intercept` defined in `ChannelExtensions` and +Client interceptors are registered on a `Channel` or `CallInvoker` via extension +methods named `Intercept` defined in `ChannelExtensions` and `CallInvokerExtensions`. While they fundamentally operate on an underlying -`CallInvoker` and not a `Channel` directly, the ones that take a `Channel` as -an argument serve as a syntactic sugar for ease of construction in user code: -they simply create an underlying `DefaultCallInvoker` and route calls through -it. Since `Intercept` is the only way to register interceptors on a -`CallInvoker`, the `InterceptingCallInvoker` that chains interceptor and -exposes a `CallInvoker` can remain an internal class. - -An alternative design registering interceptors directly on a `Channel` -returning an "intercepted `Channel`" object was considered but ultimately -dismissed, despite having an advantage of 100% compatibility with anywhere a -`Channel` would have been used, since it added complexity to the `Channel` -object and potentially slowing down the fast code path where no interceptor is -registered. When invoking RPCs in user code, `CallInvoker` is sufficiently -close to a `Channel` for constructing client objects and using local variable -type inference (`var`), the actual user code would look identical in most -cases: +`CallInvoker` and not a `Channel` directly, the ones that take a `Channel` as an +argument serve as syntactic sugar for more ergonomy in application code and they +simply create an underlying `DefaultCallInvoker` and route calls through it. +Since `Intercept` is the only way to register interceptors on a `CallInvoker`, +the `InterceptingCallInvoker` that chains interceptor and exposes a +`CallInvoker` can remain a `private` class. + +An alternative design registering interceptors directly on a `Channel` returning +an "intercepted `Channel`" object was considered but ultimately dismissed, +despite having an advantage of 100% compatibility with anywhere a `Channel` +would have been used, since it added complexity to the `Channel` object and +potentially slowing down the fast code path where no interceptor is registered. +When invoking RPCs in user code, `CallInvoker` is sufficiently close to a +`Channel` for constructing client objects and using local variable type +inference (`var`), the actual user code would look identical in most cases: ```csharp var interceptedChannel = channel.Intercept(interceptorObject); @@ -142,72 +148,94 @@ var greeterClient = new GreeterClient(interceptedChannel); ``` A higher-level, more user-friendly, canonical base class for common interceptor -functionality can be provided by deriving from `ClientInterceptor` and adding -unified hooks (e.g. `BeginCall`, `EndCall`) that can be overridden once but -operate on all types of RPC invocations. - -In order to carry satellite data across interceptors and `CallInvoker`s, this -proposal suggests `CallOptions` to be amended with an `Items` dictionary and a -`WithItems` helper method to register a custom dictionary which would hold -shared state across the interceptors and `CallInvoker`s. This is the only other -piece of client interceptors that requires being implemented within the -`Grpc.Core` since it needs adding a member to `CallOptions` structure. +functionality can be provided by deriving from `Interceptor` and adding unified +hooks (e.g. `BeginCall`, `EndCall`) that can be overridden once but operate on +all types of RPC invocations. ### Server Interceptors -Server-side interceptors derive from the new `ServerInterceptor` class -implemented in `Grpc.Core.Interceptors` namespace. Server interceptors are -registered on individual service definitions as opposed to an entire server, -though it might be sensible to provide a syntactic sugar for adding an -interceptor chain to all services served by a `Server` instance. - -In particular, an instance of a class derived from `ServerInterceptor` is -registered via an extension method for class `ServerServiceDefinition`. This -method is defined as an extension method to decouple the server interceptor -machinery from the core gRPC infrastructure as much as possible. This -extension method wraps all of the existing handlers in the service definition -with ones intercepted by the passed interceptor object. +Server-side interceptors derive from the new `Interceptor` class implemented in +`Grpc.Core.Interceptors` namespace. Server interceptors are registered on +individual service definitions as opposed to an entire server, though it might +be sensible to provide a mechanism for adding an interceptor chain to all +services served by a `Server` instance in the future. + +In particular, an instance of a class derived from `Interceptor` is registered +on a `ServerServiceDefinition` instance via its new `Intercept` method, which +returns a new `ServerServiceDefinition` whose handlers are intercepted through +the given interceptor. An alternate design was considered in which `Intercept` +was an extension method on `ServerServiceDefinition` and reached out to an +internal `SubstituteHandlers` method on the object for minimal disruption to +`ServerServiceDefinition`. However, the benefits of decoupling it was unclear, +since the class defining that extension method would have needed to access the +internals of `ServerServiceDefinition` nevertheless. ```csharp Server server = new Server { - Services = { Greeter.BindService(new GreeterImpl()).Intercept(new LogInterceptor()) }, - Ports = { new ServerPort("localhost", Port, ServerCredentials.Insecure) } + Services = { Greeter.BindService(new GreeterImpl()).Intercept(new LogInterceptor()) }, + Ports = { new ServerPort("localhost", Port, ServerCredentials.Insecure) } }; server.Start(); ``` -Since support for this handler substitution was needed from the -`ServerServiceDefinition` class itself, a general internal method -`SubstituteHandlers` is added to actually apply the mapping given the wrapping -logic as a delegate and return a new service definition object with the new -handlers. Beyond this, and slight modification to the definition of server -call handlers, no special support from the core gRPC machinery was needed, and -the fast path without any registered interceptor remains intact. +The handler substitution process operates on each of the four RPC handler types, +and registers the interceptor on them. The RPC handler dispatch code is changed +so that if at least one interceptor is registered on the handler, the +interceptor chain is invoked and is given the call context. This singular +check, i.e. whether or not at least one interceptor is registered for a handler, +is the only additional work needed be done on the fast path, where no +interceptor is registered, so the performance impact should be minimal and +contained to one additional `interceptor == null` check for each RPC invocation. +If an interceptor chain exists, it is then given control and is allowed to +substitute the call handler with an arbitrary continuation, which is invoked +instead of the handler and can intercept the call during its full duration. The +first interceptor invocation is also allowed to return the original handler +intact, indicating it not being interested to do any additional processing or +observation on the call, or throw an exception and terminate the RPC +immediately. Throwing an exception from an interceptor is semantically +equivalent to throwing an exception from a server handler. + +The method signatures for the server side interceptor hooks are as follows: -Similar to client interceptors, server interceptors can override four functions -`UnaryServerHandler`, `ClientStreamingServerHandler`, -`ServerStreamingServerHandler`, and `DuplexStreamingServerHandler`. Likewise, -they all take a `next` argument that invokes the next interceptor in the chain -or the actual RPC handler at the end of the chain: +```csharp +// THandler is one of: +// Grpc.Core.UnaryServerMethod, +// Grpc.Core.ClientStreamingServerMethod, +// Grpc.Core.ServerStreamingServerMethod, or +// Grpc.Core.DuplexStreamingServerMethod, +// depending on the RPC type. +delegate Task ServerHandlerInterceptor(ServerCallContext context, THandler handler); + +ServerHandlerInterceptor> GetUnaryServerHandlerInterceptor() +ServerHandlerInterceptor> GetServerStreamingServerHandlerInterceptor() +ServerHandlerInterceptor> GetClientStreamingServerHandlerInterceptor() +ServerHandlerInterceptor> GetDuplexStreamingServerHandlerInterceptor() +``` + +By default, the implementation is a no-op: ```csharp -async Task UnaryServerHandler(TRequest request, ServerCallContext context, UnaryServerMethod next); -async Task ClientStreamingServerHandler(IAsyncStreamReader requestStream, ServerCallContext context, ClientStreamingServerMethod next); -async Task ServerStreamingServerHandler(TRequest request, IServerStreamWriter responseStream, ServerCallContext context, ServerStreamingServerMethod next); -async Task DuplexStreamingServerHandler(IAsyncStreamReader requestStream, IServerStreamWriter responseStream, ServerCallContext context, DuplexStreamingServerMethod next); +return (context, handler) => Task.FromResult(handler); ``` -In order to carry satellite data between interceptors and the handler, this -proposal suggests adding an `Items` dictionary to `ServerCallContext` to hold -arbitrary state. +The design in which all four server interceptor methods were combined into one +was considered, but dismissed in favor of this one because eventually, the +interceptor hook needed to return an object of the same type as the handler +passed to it, and it would have needed to reflect over it to reconstruct the +type. Instead, under this design, such unification is still an option, simply +by deriving a class that implements all four functions and calls a shared method +to minimize the code written, should that style fits that particular application +better. + ## Rationale -The design goals where flexibility, decoupling and non-disruption to existing -code, and keeping the fast path fast and the design trade-offs made intend to -accomplish these goals. Some of these decisions were discussed above. +A primary design goal is not breaking the existing API at all. In addition, +flexibility, decoupling and non-disruption to existing code, and keeping the +"fast path", where no interceptor is registered, fast. Some design trade-offs to +accomplish these goals were discussed in-line with the decision. ## Implementation From 3acdfd7d6bc9e6c3a7dbe0af266643aec9f633b1 Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Wed, 6 Jun 2018 09:11:43 -0700 Subject: [PATCH 6/8] Update the proposal to match the final implementation --- L12-csharp-interceptors.md | 380 +++++++++++++++++++++++++------------ 1 file changed, 256 insertions(+), 124 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index 26b224ee5..850514cf2 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -4,7 +4,7 @@ Title * Approver: jtattermusch * Status: In Review * Implemented in: C# -* Last updated: October 16, 2017 +* Last updated: June 6, 2018 * Discussion at: https://groups.google.com/d/topic/grpc-io/GKcnFC3oxhk/discussion ## Abstract @@ -18,10 +18,10 @@ composed flexibly as users wish. ## Background gRPC C# does not have an interceptor API. Functionality can be partially -simulated on the client side by extending the `CallInvoker` class. However, this -has some limitations in composition and the ability to decouple `CallInvoker` -from each other. On the server side, similar or replacement functionality was -basically non-existent. +simulated on the client side by extending the `CallInvoker` class. However, +this has some limitations in composition and the ability to decouple +`CallInvoker` from each other. On the server side, similar or replacement +functionality is effectively non-existent. ### Related Proposals: @@ -40,69 +40,89 @@ namespace in `Grpc.Core` assembly. While client and server interceptors use different hooks to intercept on RPC invocations, they share the abstract base class `Interceptor` without any -abstract methods. The rationale for this is it enables a single class to act -both as a server interceptor and a client interceptor. An alternative -considered were using an `interface` instead of an abstract base class, but were -ruled out due to potential future versioning issues (adding a method to an -interface would be a breaking API change, whereas adding an additional method to -an abstract base should be fine as long as it is not marked `abstract`). Another -alternative that was ruled out was to separate it out to `ClientInterceptor` and -`ServerInterceptor` classes. This may have made each class look slightly more -lightweight, but realistically, it would be just as easy to implement just the -client methods and not the server methods on an interceptor. +abstract methods. An alternative was initially considered where two separate +base classes supported client and server interceptors. The rationale for +settling on a single base class was to enable a single class to be able to get +registered both as a server interceptor and a client interceptor, making +libraries providing generic interceptors nicer to use, and the cost of having +several additional virtual methods in the same class was minimal should one +want to implement only one side of the interceptor. Another alternative +considered was using an `interface` instead of an abstract base class, but that +was ruled out since interfaces are less amenable to non-breaking changes than +abstract classes across future versions (adding a method to an interface would +be a breaking change for existing users, whereas adding an additional method to +an abstract base class should be fine as long as it is not marked `abstract`). + +### Abstract Base Class + +All interceptors must derive, directly or indirectly, from the abstract base +class `Grpc.Core.Interceptors.Interceptor` and can choose to override zero or +more of its methods. ### Client Interceptors -Client interceptors derive from the new abstract base class -`Grpc.Core.Interceptors.Interceptor`. This abstract class defines five -different virtual methods to hook into various RPC invocation types, namely -`BlockingUnaryCall`, `AsyncUnaryCall`, `AsyncClientStreamingCall`, -`AsyncServerStreamingCall`, and `AsyncDuplexStreamingCall`. - -The request-unary interceptor hooks take a request value as their first -argument, followed by the common arguments for all of the methods: - -1. A `context` argument of a new type `ClientInterceptorContext` that encapsulates the context of the call. An older alternative - design used a similar signature to the corresponding method in the - `CallInvoker` class, but that would limit the ability to add new contextual - information to pass along without changing the signature and thus breaking - API compatibility. - -2. A `continuation` argument, which is a delegate that invokes the next step in - the interceptor chain or the actual underlying `CallInvoker` handler for the - final interceptor in the chain and returns a value of the same type as the - interceptor method. For unary requests, it takes the `request` value as its - first argument, followed by the `context` to proceed the invocation with. The - interceptor method is allowed to pass in a new or modified `context` along - and that is what the RPC continues with. For client-streaming RPCs, only the - `context` argument should be passed along to the `continuation`. +The `Interceptor` class defines the following virtual methods to hook into RPC +invocations of various types on the client side: ```csharp -TResponse BlockingUnaryCall( - TRequest request, - ClientInterceptorContext context, - BlockingUnaryCallContinuation continuation) - -AsyncUnaryCall AsyncUnaryCall( - TRequest request, - ClientInterceptorContext context, - AsyncUnaryCallContinuation continuation) - -AsyncServerStreamingCall AsyncServerStreamingCall( - TRequest request, - ClientInterceptorContext context, - AsyncServerStreamingCallContinuation continuation) - -AsyncClientStreamingCall AsyncClientStreamingCall( - ClientInterceptorContext context, - AsyncClientStreamingCallContinuation continuation) -``` -AsyncDuplexStreamingCall AsyncDuplexStreamingCall( - ClientInterceptorContext context, - AsyncDuplexStreamingCallContinuation continuation) +TResponse BlockingUnaryCall(TRequest request, ClientInterceptorContext context, BlockingUnaryCallContinuation continuation); +AsyncUnaryCall AsyncUnaryCall(TRequest request, ClientInterceptorContext context, AsyncUnaryCallContinuation continuation); +AsyncServerStreamingCall AsyncServerStreamingCall(TRequest request, ClientInterceptorContext context, AsyncServerStreamingCallContinuation continuation); +AsyncClientStreamingCall AsyncClientStreamingCall(ClientInterceptorContext context, AsyncClientStreamingCallContinuation continuation); +AsyncDuplexStreamingCall AsyncDuplexStreamingCall(ClientInterceptorContext context, AsyncDuplexStreamingCallContinuation continuation); ``` +Four of these methods correspond to the asynchronous invocations of the four +different possible RPC types supported by gRPC: unary, server-streaming, +client-streaming, and duplex-streaming; the fifth, `BlockingUnaryCall` is +provided for performance reasons for non-asynchronous unary-unary invocations. +The API is designed to directly correspond to the methods supported by +`CallInvoker`. An interceptor implementation interested in intercepting each +RPC type needs to override and customize the implementation of each of the +respective methods. Once again, blocking and asynchronous code paths for unary +invocations are separate and both need to be implemented and overriding one +does not automatically apply to the other. + +Of the aformentioned methods, the ones that intercpet request-unary RPCs take +an instance of the unary request object. All methods take a context class of +type `Grpc.Core.Interceptors.ClientInterceptorContext` and a `continuation` +delegate to invoke to continue with the RPC chain. + +#### The `ClientInterceptorContext` class + +A new class, `Grpc.Core.Interceptors.ClientInterceptorContext` is introduced +to encapsulate the details of the invocation, namely the following properties, +but is flexible to support the addition of new properties in the future: + +- `Method` of type `Method` representing the current + method to be invoked. +- `Host`, a string representing the host name of the current invocation. +- `Options`, the `CallOptions` for the current invocation. + +A `context` instance is passed to the interceptor to describe the invocation. +The interceptor is free to propagate the same object or create its own context +object and pass it to the next continuation to control how the invocation is +going to proceed. + +An alternative was considered to pass such properties in-line as arguments +to the interceptor and have the interceptor invoke the continuation listing +them as arguments, but in addition to usability hurdles due to verboseness, +having a class makes adding properties more seamless and future-proof than +adding a parameter to method signatures, thus that alternative was +eliminated. + +#### The `continuation` delegate + +The invocation-side interceptors get control over an RPC invocation and can +read and optionally modify aspects of the invocation they are interested in. +Such properties are passed via the request value argument for the unary RPCs +and the context object specified above. In order to proceed with the +execution of the RPC, the interceptor can choose to call the `continuation` +and pass it a new request instance (for request-unary RPCs only) and a context +instance. The interceptor is allowed to invoke `continuation` zero (useful +for a caching interceptor, for example) or more times (e.g. useful for a retry +interceptor) and return a value as it sees fit. + In the general case, the interceptors are not obligated to invoke `continuation` just once. They might choose to not continue with the call chain and terminate it immediately by returning their own desired return value, or potentially call @@ -120,6 +140,16 @@ could have been implemented as an external library, and does not change the non-intercepted code path at all, therefore should not have any negative performance impact when no interceptor is used. +#### Return values + +The return value of each method is of the same type as the corresponding +method in the `CallInvoker` object for that RPC type and is documented in +that class. For asynchronous and streaming invocations, intercepting the +entire call would entail returning a custom instance of the `Async***Call` +value that wraps the value returned from `continuation`. + +#### Registration + Client interceptors are registered on a `Channel` or `CallInvoker` via extension methods named `Intercept` defined in `ChannelExtensions` and `CallInvokerExtensions`. While they fundamentally operate on an underlying @@ -150,26 +180,132 @@ var greeterClient = new GreeterClient(interceptedChannel); A higher-level, more user-friendly, canonical base class for common interceptor functionality can be provided by deriving from `Interceptor` and adding unified hooks (e.g. `BeginCall`, `EndCall`) that can be overridden once but operate on -all types of RPC invocations. +all types of RPC invocations. Additionally, a pipeline-builder style of +registration can be added in the future. Since this gRFC does not preclude +addition of those, and in the interest of not adding APIs that would be set +in stone, the scope of this gRFC is limited to the core interceptor hooks and +external libraries can provide such functionality for now. + +#### Order of execution + +Registering an interceptor on a `Channel`-like object is to be thought of as +treating the underlying object as a black box and intercept methods before +they are handed over to the underlying object. Thus, intercepting an +intercepted channel will result in the last interceptor being run first: + +```csharp +var chan = newChannel(); +var interceptedOnce = chan.Intercept(interceptor1); +var interceptedTwice = interceptedOnce.Intercept(interceptor2); +// interceptor2 will take control first, and calling continuation +// from interceptor2 will invoke interceptor1. continuation passed +// to interceptor1 will invoke the call invoker which invokes +// the channel. +``` + +However, a helper API is provided to register multiple interceptors +at once on a `Channel` or `CallInvoker`, in which case, the order +of execution is in the order listed. + +```csharp +var interceptedChannel = chan.Intercept(interceptor1, interceptor2); +// interceptor1 will take control before interceptor2. +``` + +#### Simple client interceptor example + +```csharp +// Invokes the specified callback before each RPC gets invoked. +class CallbackInterceptor : Interceptor +{ + readonly Action callback; + public CallbackInterceptor(Action callback) + { + this.callback = GrpcPreconditions.CheckNotNull(callback, nameof(callback)); + } + public override TResponse BlockingUnaryCall(TRequest request, ClientInterceptorContext context, BlockingUnaryCallContinuation continuation) + { + callback(); + return continuation(request, context); + } + public override AsyncUnaryCall AsyncUnaryCall(TRequest request, ClientInterceptorContext context, AsyncUnaryCallContinuation continuation) + { + callback(); + return continuation(request, context); + } + public override AsyncServerStreamingCall AsyncServerStreamingCall(TRequest request, ClientInterceptorContext context, AsyncServerStreamingCallContinuation continuation) + { + callback(); + return continuation(request, context); + } + public override AsyncClientStreamingCall AsyncClientStreamingCall(ClientInterceptorContext context, AsyncClientStreamingCallContinuation continuation) + { + callback(); + return continuation(context); + } + public override AsyncDuplexStreamingCall AsyncDuplexStreamingCall(ClientInterceptorContext context, AsyncDuplexStreamingCallContinuation continuation) + { + callback(); + return continuation(context); + } +} +``` ### Server Interceptors -Server-side interceptors derive from the new `Interceptor` class implemented in -`Grpc.Core.Interceptors` namespace. Server interceptors are registered on -individual service definitions as opposed to an entire server, though it might -be sensible to provide a mechanism for adding an interceptor chain to all -services served by a `Server` instance in the future. - -In particular, an instance of a class derived from `Interceptor` is registered -on a `ServerServiceDefinition` instance via its new `Intercept` method, which -returns a new `ServerServiceDefinition` whose handlers are intercepted through -the given interceptor. An alternate design was considered in which `Intercept` -was an extension method on `ServerServiceDefinition` and reached out to an -internal `SubstituteHandlers` method on the object for minimal disruption to -`ServerServiceDefinition`. However, the benefits of decoupling it was unclear, -since the class defining that extension method would have needed to access the -internals of `ServerServiceDefinition` nevertheless. +Server-side interceptors derive from the `Interceptor` class and optionally +override the four relevant methods to intercepting different kinds of +incoming RPCs. All of these methods are expected to return a `Task` object +and thus can be asynchronous in nature. + +```csharp +Task UnaryServerHandler(TRequest request, ServerCallContext context, UnaryServerMethod continuation); +Task ClientStreamingServerHandler(IAsyncStreamReader requestStream, ServerCallContext context, ClientStreamingServerMethod continuation); +Task ServerStreamingServerHandler(TRequest request, IServerStreamWriter responseStream, ServerCallContext context, ServerStreamingServerMethod continuation); +Task DuplexStreamingServerHandler(IAsyncStreamReader requestStream, IServerStreamWriter responseStream, ServerCallContext context, DuplexStreamingServerMethod continuation); +``` + +Of these four, the two that return unary values are expected to return a +a `TResponse` value asynchronously (via `Task`) whereas the +response-streaming RPCs are expected to write the return values to the +`responseStream` stream. Additionally, for unary requests, the request +value is passed as an argument to the intercepting method directly, +whereas for client-streaming RPCs, they should be read from the +`requestStream` given to the interceptor. + +#### The RPC context + +The interceptor is passed a `context` object representing the +context of the current RPC. This is the same object passed to +RPC handlers and the interceptor is in a position to treat +itself as if it were the actual RPC handler. + +#### The `continuation` delegate + +A `continuation` delegate is passed to the server-side interceptor +method that matches the signature of the actual RPC handler (depending +on the RPC kind), and its purpose is to carry-on with RPC processing. + +The `continuation` takes the same parameters as the interceptor method, +with the exception of the `continuation` itself, and returns a value of +the same type as the intercepting method. + +#### Intercepting requests and responses + +The intercepting method can choose to inspect and modify +the request value as it passes it to the `continuation` (for the +request-unary RPCs), optionally inspect and modify the response +value returned from `continuation` invocation (for response-unary RPCs). +Additionally, it can do sophisticated interception over the lifecycle +of streaming RPCs by wrapping the `requestStream` (for client-streaming +RPCs) and/or `responseStream` (for server-streaming RPCs). + +#### Registration + +Server interceptors are registered on `ServerServiceDefinition` objects +via the `Intercept` extension method provided by `ServerSeviceDefinition` +method. ```csharp Server server = new Server @@ -180,63 +316,59 @@ Server server = new Server server.Start(); ``` -The handler substitution process operates on each of the four RPC handler types, -and registers the interceptor on them. The RPC handler dispatch code is changed -so that if at least one interceptor is registered on the handler, the -interceptor chain is invoked and is given the call context. This singular -check, i.e. whether or not at least one interceptor is registered for a handler, -is the only additional work needed be done on the fast path, where no -interceptor is registered, so the performance impact should be minimal and -contained to one additional `interceptor == null` check for each RPC invocation. -If an interceptor chain exists, it is then given control and is allowed to -substitute the call handler with an arbitrary continuation, which is invoked -instead of the handler and can intercept the call during its full duration. The -first interceptor invocation is also allowed to return the original handler -intact, indicating it not being interested to do any additional processing or -observation on the call, or throw an exception and terminate the RPC -immediately. Throwing an exception from an interceptor is semantically -equivalent to throwing an exception from a server handler. - -The method signatures for the server side interceptor hooks are as follows: +#### Order of execution + +Server interceptors operate identically to the client interceptors in +terms of order of execution. Please consult the section above for details. -```csharp -// THandler is one of: -// Grpc.Core.UnaryServerMethod, -// Grpc.Core.ClientStreamingServerMethod, -// Grpc.Core.ServerStreamingServerMethod, or -// Grpc.Core.DuplexStreamingServerMethod, -// depending on the RPC type. -delegate Task ServerHandlerInterceptor(ServerCallContext context, THandler handler); - -ServerHandlerInterceptor> GetUnaryServerHandlerInterceptor() -ServerHandlerInterceptor> GetServerStreamingServerHandlerInterceptor() -ServerHandlerInterceptor> GetClientStreamingServerHandlerInterceptor() -ServerHandlerInterceptor> GetDuplexStreamingServerHandlerInterceptor() -``` -By default, the implementation is a no-op: +#### Simple server-side interceptor example ```csharp -return (context, handler) => Task.FromResult(handler); +// The interceptor invokes the registered callback on every RPC +// and passes the context value of the current call to it +class ServerCallContextInterceptor : Interceptor +{ + readonly Action interceptor; + + public ServerCallContextInterceptor(Action interceptor) + { + GrpcPreconditions.CheckNotNull(interceptor, nameof(interceptor)); + this.interceptor = interceptor; + } + + public override Task UnaryServerHandler(TRequest request, ServerCallContext context, UnaryServerMethod continuation) + { + interceptor(context); + return continuation(request, context); + } + + public override Task ClientStreamingServerHandler(IAsyncStreamReader requestStream, ServerCallContext context, ClientStreamingServerMethod continuation) + { + interceptor(context); + return continuation(requestStream, context); + } + + public override Task ServerStreamingServerHandler(TRequest request, IServerStreamWriter responseStream, ServerCallContext context, ServerStreamingServerMethod continuation) + { + interceptor(context); + return continuation(request, responseStream, context); + } + + public override Task DuplexStreamingServerHandler(IAsyncStreamReader requestStream, IServerStreamWriter responseStream, ServerCallContext context, DuplexStreamingServerMethod continuation) + { + interceptor(context); + return continuation(requestStream, responseStream, context); + } +} ``` -The design in which all four server interceptor methods were combined into one -was considered, but dismissed in favor of this one because eventually, the -interceptor hook needed to return an object of the same type as the handler -passed to it, and it would have needed to reflect over it to reconstruct the -type. Instead, under this design, such unification is still an option, simply -by deriving a class that implements all four functions and calls a shared method -to minimize the code written, should that style fits that particular application -better. - - ## Rationale A primary design goal is not breaking the existing API at all. In addition, flexibility, decoupling and non-disruption to existing code, and keeping the -"fast path", where no interceptor is registered, fast. Some design trade-offs to -accomplish these goals were discussed in-line with the decision. - +"fast path", where no interceptor is registered, fast. Some design trade-offs +to accomplish these goals were discussed in-line with the decision. ## Implementation From 0532b1f0b20ca205ae073b17da1cf67f8c295aac Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Thu, 7 Jun 2018 08:13:18 -0700 Subject: [PATCH 7/8] fix typo: intercepts --- L12-csharp-interceptors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index 850514cf2..92ac390a3 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -83,7 +83,7 @@ respective methods. Once again, blocking and asynchronous code paths for unary invocations are separate and both need to be implemented and overriding one does not automatically apply to the other. -Of the aformentioned methods, the ones that intercpet request-unary RPCs take +Of the aformentioned methods, the ones that intercept request-unary RPCs take an instance of the unary request object. All methods take a context class of type `Grpc.Core.Interceptors.ClientInterceptorContext` and a `continuation` delegate to invoke to continue with the RPC chain. From a5726d75a88125b7692796137f4c08ac8146c7a9 Mon Sep 17 00:00:00 2001 From: Mehrdad Afshari Date: Thu, 7 Jun 2018 08:16:51 -0700 Subject: [PATCH 8/8] Update status to Approved --- L12-csharp-interceptors.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/L12-csharp-interceptors.md b/L12-csharp-interceptors.md index 92ac390a3..8c379d846 100644 --- a/L12-csharp-interceptors.md +++ b/L12-csharp-interceptors.md @@ -2,9 +2,9 @@ Title ---- * Author(s): mehrdada * Approver: jtattermusch -* Status: In Review +* Status: Approved * Implemented in: C# -* Last updated: June 6, 2018 +* Last updated: June 7, 2018 * Discussion at: https://groups.google.com/d/topic/grpc-io/GKcnFC3oxhk/discussion ## Abstract