Filter mechanism for server transport creation and deletion

[zhangkun83]

To support a per-connection throttling mechanism used inside google, gRPC needs to allow user to add filters to the server that:

1. Gets called when a server transport is created and handshake-completed, and has access to transport-specific attributes such as client address and identity.
2. Adds per-transport attributes to ServerCall attributes.
3. Gets called when a server transport is terminated.

I discussed a few options with @ejona86:

ServerInterceptor

The filter returns a ServerInterceptor when transport is created. The interceptor is transport-scoped, unlike ordinary interceptors that are service-scoped.

interface ServerTransportFilter {
  ServerInterceptor transportCreated(SomePublicServerTransportType transport);
  void transportTerminated(SomePublicServerTransportType transport);
}

Pros:

1. Doesn't touch transport API (ServerTransport, ServerStream) which are internal.
2. Re-uses ServerInterceptor for purposes that ServerInterceptor is designed for (modifying ServerCall attributes etc).

Cons:

1. Changes the scope of ServerInterceptor. @ejona86 thinks bonding ServerInterceptor to a transport is seemingly generic but actually an ad-hoc solution for a particular problem. I don't necessarily agree though.

Decorating transport

The filter returns a decorated ServerListener at registration. Through a few more layers of wrapping, the filter would be able to decorate ServerListener.transportCreated(),ServerTransportListener.transportTerminated() and ServerStream.attributes().

Pros:

1. Minimal API change

Cons:

1. Heavily coupled with transport API which is internal.
2. Duplicates the existing interceptor mechanism, but on ServerStream instead of ServerCall. It would be ideal to just decorate ServerCall, which is what ServerInterceptor does. ServerInterceptor currently is per-service, but our use case need it to be per-connection.
3. Even more layers of decoration than the interceptors. The filter can be annoying to write.

Ad-hoc

Just for our original requirements, the filter would return per-transport attributes that are to be merged into ServerCall attributes.

interface ServerTransportFilter {
  Attributes transportCreated(SomePublicServerTransportType transport);
  void transportTerminated(SomePublicServerTransportType transport);
}

It probably won't work in this form, because the attributes such as peer security identity may not be available immediately after the transport is created, if the security handshake has not completed yet. On the other hand, the first two options can work around this situation by reading the attributes only when the first stream is created.

[zhangkun83]

Had a discussion with @louiscryan and @ejona86. We agree that the decoration pattern exposes too much of the transport API, thus it's undesirable. Instead, we consider using Context as the vehicle of propagating information from transport to calls, as well as the way of the filter to listen to transport termination. The interface would look this:

interface ServerTransportFilter {
  Context transportCreated(Attributes transportAttrs);
}

1. The Context returned by the filter is used as the transport Context. Stream Contexts, which are attached before calling the service handlers, inherit the transport Context. This forms a data path from the filter to the server interceptors and service handlers.
2. The filter can return a CancellableContext and listen to its cancellation. The context will be cancelled when the transport is terminated. This is how the filter listens to transport termination.
3. If the filter cancels the context before returning it, we don't want to shut down the transport. It doesn't seem to be the right interface for early-shutdown of transports, because it cannot signal whether it's a soft shutdown or a hard one (shutdownNow).

This interface is very small and doesn't expose transport API.

TBD:

1. If stream Contexts inherit the transport Context directly, every new (or closing) stream Context will register (or deregister) cancellation listener on the transport Context, which creates a hot spot on the listener registry of the transport Context. We need to make sure the data structure of the registry is efficient enough at adding and removing items, or make stream Contexts inherit a fork of the transport Context so that they don't register the listeners on it.
2. The filter will be called in ServerListener.transportCreated(ServerTransport). We need a way for transports to pass Attributes to the filter in this method. We could either add a getAttributes() to ServerTransport, or add a Attributes argument to transportCreated(). I lean to the latter, because it makes a strong point that the attributes are immutable.
3. Currently transport-specific information e.g. client IP and SSLSesion are propagated via ServerCall.attributes(). If transport filter uses Context to pass down information, ServerInterceptor will have two sources of information, which is abundant and could be confusing. We may consider deleting ServerCall.attributes() (thus ServerStream.attributes()) and move its contents to Context. @lukaszx0 since you added attributes(), I would like to hear your opinion.
4. ServerListener.transportCreated() has to be called after all handshakes (e.g., TLS) are done, so that the necessary information can be available to the attributes. I don't know if it's so now.

[zhangkun83]

@lukaszx0 I would like to hear your opinion on removing ServerCall.attributes() and instead passing everything in the Context.

[lukaszx0]

@zhangkun83 sorry for missing original mention.

I like the idea of using context for everything and removing ServerCall attributes. Even now, when I have fairly good knowledge of internals in this particular area I am sometimes confused if stuff is either in context or call attributes. Consolidating them into one place - context - would make things simpler.

Looping in @jhump who might have opinions on the main issue discussed here.

[jhump]

Haven't thought about it very deeply. My initial reaction is "no strong opinion". Using context instead of call attributes seems fine to me.

[zhangkun83]

@ejona86 @louiscryan and I had a further discussion about this issue. We feel that the Context-based interface earlier has two issues:

1. Obscure interface: Using Context cancellation to signal transport termination is implicit and somehow obscure.
2. Context abuse: Context is for passing data that is either consumed by application or propagated downstream. Data generated by the filter is not necessarily consumed by application or propagated downstream. It seems like abusing the Context.

Our consensus is that we keep ServerCall.attributes(), use it to pass data from the filter to interceptors, and call out transport termination explicitly. We have agreed on the following interface:

abstract class ServerTransportFilter {
  // Called after all handshake (e.g., SSL) is done.
  // transportAttrs: contains original transport information, such as
  // socket address and SSL context.
  // return: will be merged into ServerCall.attrs
  Attributes transportReady(Attributes transportAttrs) {
    return transportAttrs;
  }

  // attrs: contains all the data returned by transportReady of the same filter
  void transportTerminated(Attributes attrs);
}

@lukaszx0 WRT Attributes vs. Context, we have drawn a clear line between them. I hope this addresses your confusion.

• Context: for passing data that is either consumed by application or propagated downstream. E.g., deadline, tracing context, end-user credentials.
• Attributes: for passing data from transport to ServerInterceptor, but not necessarily meant to be consumed by application or propagated downstream. Typically per-connection information such as client address, SSL context etc.

ServerInterceptor is the one that consumes Attributes and produces Context.

[zhangkun83]

Reopening this issue to track the experimental status of ServerTransportFilter.

[ejona86]

Created #2577 for experimental tracking.

[rmichela]

I can't figure out how ServerTransportFilter is useful in its current form, since it provides so little information about the server transport from which to make any meaningful decision. In its current implementation, the only information I can learn about the transport is the client's remote address.

The only use I can see for a ServerTransportFilter is as a way to note that a connection starts and stops existing. Perhaps that is the only purpose.

[rmichela]

I've written a ServerTransportFilter that attaches a unique UUID to each transport session, and uses that UUID to announce transport lifecycle events.

When combined with a server interceptor, you can use the session UUID to write session aware gRPC services.

[ejona86]

@rmichela, you should be very careful with that session stuff. gRPC does not provide that feature in its API on purpose, because gRPC does not provide those guarantees. For example, L7 load balancers will demolish such transport=session assumptions. gRPC may also disconnect the transport whenever it wishes, which could disturb a login flow.

[rmichela]

Very true. Sessions in RPC always come with a few caveats. If your load balancer is not sticky, most bets are off unless you are persisting session in some kind of data store.

gRPC dropping and reconnecting the transport is something i hadn't considered. Perhaps the session ID should be established by a client interceptor. Even with a client interceptor, session persistence would be needed in the absence of a sticky session.