Ability to use a single interceptor for both unary and streaming RPCs

[jhump]

After #1801 is completed, it becomes possible for a stream interceptor to be used when processing unary RPCs.

This issue is a feature request for some new API that allows for exactly this. The existing API, for backwards compatibility, will continue to register a stream interceptor that is only used for streaming RPCs. But a new method/dial option could be added that allows for registering a stream interceptor that is used for unary RPCs and/or for both streaming and unary RPCs.

[dfawley]

I'd like to redesign our interceptor API. Please add your list of requirements/nice-to-haves in this bug and I'll compile them here:

Requirements:

• Single interceptor type for unary and streaming
  • If it's difficult to implement, provide an optional convenience wrapper to generate a stream-style interceptor from a single function (like how unary interceptors work today).
• Include access to ServiceDesc/FileDescriptorProto (#1526)
• Chainable / able to easily register multiple handlers

Nice to Haves:

• Single API for stats handlers and interceptors
• Global registration pattern
• Able to specify via CallOptions
• Function signature includes explicit context.Context (from @skipor)
• Same/similar type signature as RPC handler itself (Java does this)
• Include [de]serialization steps as interceptors to allow inserting interceptors before deserialization or after serialization.

This is currently just a quick list based on my memory at the moment. I intend to add to this as I run into more things.

~~This will be a breaking change (breaking only features marked "EXPERIMENTAL" in the docs), but we will make sure to leave the current version around for one or two releases to allow time for users to migrate. We will also be sure to make a gRFC proposal for this.~~ We will maintain the existing interceptors - two of the four were not marked as experimental and they are relied heavily enough upon that it is not feasible to remove them.

[skipor]

Nice to Have, for Interceptors redesign: extract context.Context from ServerStream and pass it as the first argument in stream interceptor. Now every context.Context wrapping requires also ServerStream wrapping, which is too complex. In my experience Context wrapping with value or timeout is a very common pattern, and it would be really nice to use it as everywhere else.

[devpro108]

@dfawley wondering any news on new interceptor design, when it will be available, looking forward to single interceptor for both unary and streams..

[dfawley]

For the moment, there are no plans to work on this, as there are just too many other, higher-priority things on our plate. However, when investigating how to support the xDS protocol for gRPC-LB v2 (ref), it's appearing that interceptors could play a prominent role. If that turns out to be the case, we may be able to do some or all of this work as part of that effort.

[devpro108]

Thank you @dfawley

[stale[bot]]

This issue is labeled as requiring an update from the reporter, and no update has been received after 7 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[jhump]

@dfawley, I think stale bot got it wrong on this one, too. I see this is "on hold", but probably should not be closed -- AFAICT, it is not pending any further input from the reporter(s).

[dfawley]

Yeah, it hit a lot of issues this morning. It doesn't seem to be listening to the "onlyLabels" setting (or we set it wrong).