grpc-swift icon indicating copy to clipboard operation
grpc-swift copied to clipboard

Channel level interceptors

Open jongarate opened this issue 3 years ago • 12 comments

Description

The current interceptor implementation sits on top of each service client. This forces the implementation on the client side of the gRPC service to arrange a factory with an interceptor instance per available service method.

When the backend supports multiple services (which is usually the case), forces the development on the client side to define interceptors per-service/per-method, resulting in a lot of boilerplate code and cumbersomeness.

Suggested solution

Just like in Android, it would be helpful to have an option to define interceptors at the Channel level, so that at least basic exchange data can be logged/observed (e.g. Headers, etc...) by hooking to a single point in the whole architecture.

jongarate avatar Mar 15 '21 15:03 jongarate

This is quite cumbersome at the moment and worth improving.

Adding it at the channel-level has a few difficulties, mainly that it's not easy to resolve which interceptors to use when they exist at the channel level and in the interceptor factory. Should the interceptors from the factory override or be added to those defined on the channel?

Another approach might be to change the generated interceptor factory: it could provide a generic factory method for 'default' interceptors and default the implementation of each factory method to call the default. That way you'd only have to implement one method per service and you retain full control over configuring interceptors on a per-method basis.

glbrntt avatar Mar 15 '21 17:03 glbrntt

To my view, channel-level interceptors would sit higher architecturally speaking than the method specific ones, which could be further extended in more detail in the method-level ones if found necessary. They should essentially coexist as their scope and purpose is quite different.

Providing a generic factory for default interceptors would certainly be an improvement, and I guess more feasible in the short run. However, it would still require attaching them to each client explicitly, which hurts maintainability in the long run.

jongarate avatar Mar 15 '21 17:03 jongarate

For base factory, temp solution. Resolving unnecessary recreations for same requests. U may improve it for your self.

typealias InterceptorEntity = (SwiftProtobuf.Message
                               & SwiftProtobuf._MessageImplementationBase
                               & SwiftProtobuf._ProtoNameProviding)

class GPRCBaseInterceptorFactory {
    private final class InterceptorsLocator {
        private var interceptors = [String: [Any]]()
        
        func register<T: InterceptorEntity, D: InterceptorEntity>(_ interceptors: [ClientInterceptor<T, D>]) {
            let description = String(describing: T.self) + String(describing: D.self)
            self.interceptors[description] = interceptors
        }
        
        func resolve<T: InterceptorEntity, D: InterceptorEntity>() -> [ClientInterceptor<T, D>]? {
            let description = String(describing: T.self) + String(describing: D.self)
            return interceptors[description] as? [ClientInterceptor<T, D>]
        }
    }

    
    //MARK: Internal properties
    private static let locator = InterceptorsLocator()
    private let creators: [InterceptorCreator]
    
    //MARK: Initialization
    required init(creators: [InterceptorCreator]) {
        self.creators = creators
    }
    
    func getInterceptors<T: InterceptorEntity, D: InterceptorEntity>() -> [ClientInterceptor<T, D>] {
        guard let interceptors: [ClientInterceptor<T, D>]  = Self.locator.resolve() else {
            let newInterceptors: [ClientInterceptor<T, D>] = creators.map { $0.create() }
            Self.locator.register(newInterceptors)
            return getInterceptors()
        }
        return interceptors
    }
}

VorobyevS avatar Mar 24 '21 20:03 VorobyevS

Still, as I mentioned, this requires feeding the interceptor manually for each service's client method, which is what the whole point of discussion is about.

jongarate avatar Apr 09 '21 08:04 jongarate

thankfully, there are plenty of pre-existing examples from java-grpc to tonic (rust-grpc) on how to architect this. This would be huge currently have a section at the bottom of each file I copy & past authentication interceptors into. It's actually quite shocking this hasn't been a more popular issue given the universality of authentication.

lacasaprivata2 avatar Jul 05 '21 03:07 lacasaprivata2

Indeed it would be very helpful for everyone.

XabierGoros avatar Jul 05 '21 06:07 XabierGoros

I'm so surprised that this isn't implemented.

bartekpacia avatar Oct 06 '22 15:10 bartekpacia

Just ran across this issue on the server side. In several situations, registering an interceptors at the Channel level is strongly preferred to adding interceptors for every single method. A single registration point will reduce both developer errors and reduces the testing burden.

Three use cases that come to mind:

  • Enhanced error logging. Append the origin address to the request headers for function level error logging. This information is not in the HPACKHeaders.
  • Blacklist. Compare incoming request addresses against a blacklist to block known malicious requests. [There are alternative mechanisms for implementing this, but they add architectural complexity which may not be desirable.]
  • Authentication / Authorization. Though not my preferred mechanism, a channel interceptor would allow authorization / authentication checks to be centralized. In my hypothetical implementation, I'd use a channel interceptor to verify that a malicious request didn't attempt to set authorization requirements. Then I'd use a function interceptor to specify the access requirements. Then I'd use a channel interceptor to perform the authentication / authorization check.

Jerry-Carter avatar Oct 31 '22 22:10 Jerry-Carter

+1

cassianomonteiro avatar Apr 10 '23 15:04 cassianomonteiro

For a newbie in swift, how do I get this working? I was able to do this simply in Dart, but can't find a way to do it with swift.

I read through various related issues in this repo regarding this. In current state, do I have to create the boilerplate for every single method?

Thank you 🙏🏽

Edit: found the partial solution here: https://github.com/grpc/grpc-swift/issues/1567#issuecomment-1425469302. Reduces boilerplate at least a little bit.

talksik avatar Dec 10 '23 22:12 talksik

For a newbie in swift, how do I get this working? I was able to do this simply in Dart, but can't find a way to do it with swift.

I read through various related issues in this repo regarding this. In current state, do I have to create the boilerplate for every single method?

You do, yes. This will be addressed in the next major version.

glbrntt avatar Dec 11 '23 09:12 glbrntt

Thank you @glbrntt !

talksik avatar Dec 11 '23 19:12 talksik