Add method middleware via gateway-specific method annotations

[shilkin]

Hello! I need to have a reverse proxy with custom middleware for some http handlres. I hope I implemented it in a generic manner. My version of generated code is backward compatible. May be it would be usefull for other developers. Thanks!

Unit tests +
Documentation +
Backward compatibility +

[tmc]

@shilkin Thank you for your contribution. This is quite interesting. Can you please sign the Google CLA?

This merits some wider discussion and better documentation but in general I'm open to the approach.

A few questions that immediately come to mind:

• what is the line of responsibility between handlers such as this and grpc interceptors?
• what other types of method options do we anticipate? should those influence any design decisions here? (a recent example is specifying additional possible http return codes in swagger gen)
• can/should we support this at the service level? file level?

[tmc]

you should document this difference and spell 'Middlware' correctly.

Perhaps instead with would introduce a more general WithOptions call or extend Register{{$svc.GetName}}Handler to support variadic options functions.

[tmc]

this is grammatically kind of awkward.

[tmc]

We should document this in a distinct section and keep the original example simple.

[tmc]

this file might more appropriately be named something more general in the context of attaching additional annotations to methods.

[shilkin]

The most appropriate is 'options' but this name is busy and deprecated.
I can move this extension to options.proto
What can you suggest?

Anyway we can declare different options in different files.

[shilkin]

First of all excuse me for my english:-)

what is the line of responsibility between handlers such as this and grpc interceptors?

If I undestand you correctly we speaking about things like this or this.
Grpc interceptors are designed for working on side of grpc-server, my handlres are actually http handlers and they responsible for processing http requests on side of gateway. For example, I want to declare should gateway check session for partical route or not. Current design of registering function allows me to do it only for the whole muх, as it's shown in examples (allowCORS). But I want to customize it for every single http handler.

what other types of method options do we anticipate? should those influence any design decisions here? (a recent example is specifying additional possible http return codes in swagger gen)

Well quick answer on the first question: I don't know:-) But we can use more complex data structure for method option (instead of simple string). This structure may be a point of extension.

can/should we support this at the service level? file level?

I think we shouldn't. I prefer to think about exactly method options. If some day we will need to extend service or file options, we will have to declare a new structures.
Speaking about http middlewares on service layer. We can do it on service layer without any code generation. All we need is to wrap our mux with another http handler (allowCORS again). So I see no reason to extend service options at the moment. Current design of generated registering functions is quite simple and flexible enough.

[shilkin]

@tmc, @arun0009, I refactored method options. Now it's a structure and we can extend it. I have to change readme again:-)

[arun0009]

@shilkin : Thanks for making this generic. I tried to run the instructions you have in README, I think you need to change
--go_out=Moptions/middleware.proto=github.com/grpc-ecosystem/grpc-gateway/options
to
--go_out=Moptions/method.proto=github.com/grpc-ecosystem/grpc-gateway/options

[shilkin]

@tmc Please, look at current versoin.

[achew22]

@shilkin, thanks for sending in your thoughts! I'm excited to see new ideas propagating through the grpc ecosystem. I have a couple of questions about this change as well. I spent some time thinking about your change in the context of a project I'm working on and had a few questions.

1. In my experience the ordering of options in protos is nondeterministic. In the arrangement you have right now it may not be possible to always get the same ordering of middleware from protoc to protoc. What do you think about the option having a repeated instead of having multiple instances of the option? I think the ordering of a repeated is also nondeterministic but it is less nondeterministic than options (it breaks more tests when this gets changed but it is still a thing allowed by the spec).
2. How do you intend to handle different interceptor requirements for different environments? For example, I have tracing in production and disabled in development mode. How do you envision maintaining that functionality?
3. Code generation is good, but explicitness is also a virtue. Putting the middleware in the .proto file feels like shifting an important part of code (as opposed to API definition) into the service definition.

I really like the idea of having a better story for middleware, but I wonder if we can make a more idiomatic go interface for doing it. For example, in gRPC, grpc.NewServer takes an optional set of ServerOptions. These provide a very flexible point fo extending the gRPC server.

As a strawman example of what that might look like. What do you think about something like

func session(h runtime.HandlerFunc) runtime.HandlerFunc {
  return func(w http.ResponseWriter, r *http.Request, p map[string]string) {
    // get ssid from cookie and check if session is valid
    h(w, r, p)
  }
}

func ratelimit(h runtime.HandlerFunc) runtime.HandlerFunc {
  return func(w http.ResponseWriter, r *http.Request, p map[string]string) {
    // check custom rate limit for this handler
    h(w, r, p) 
  }
 }

func main() error {
 ctx := context.Background()
 ctx, cancel := context.WithCancel(ctx)
 defer cancel()

 mux := runtime.NewServeMux()
 clientOpts := []grpc.DialOption{grpc.WithInsecure()}
 httpOptions := []runtime.HttpOption{
    runtime.Middleware{session},
    runtime.Middleware{ratelimit},
  }

 err := gw.RegisterYourServiceHandlerFromEndpoint(ctx, mux, *echoEndpoint, clientOpts, httpOptions)
 if err != nil {
   return err
 }

 http.ListenAndServe(":8080", mux)
 return nil
}

One of the things I like about this is that it gives us an obvious path forward for HTTP extensibility. You could use this not just for middleware but for getting monitoring hooks into the internals of the gRPC-gateway runtime.

[achew22]

@shilkin, with the merging of #336, I'm going to close this PR. The functionality you were looking for should now be possible 🎆 YAY!