grpc-ecosystem / go-grpc-middleware

Golang gRPC Middlewares: interceptor chaining, auth, logging, retries and more.
Apache License 2.0
6.26k stars 690 forks source link

v2: Replace our chaining of interceptors with newest gRPC ones. #342

Closed bwplotka closed 3 years ago

bwplotka commented 3 years ago

See: https://godoc.org/google.golang.org/grpc#ChainUnaryInterceptor

Blocked on upgrade PR: https://github.com/grpc-ecosystem/go-grpc-middleware/pull/321

Pulled from https://github.com/grpc-ecosystem/go-grpc-middleware/issues/275 for visibility.

Blocker for v2.

curiousleo commented 3 years ago

Looks like this has been resolved by #385 and can be closed?

johanbrandhorst commented 3 years ago

Sounds good to me!

bwplotka commented 3 years ago

I haven't seen this fix 😱 Amazing, thanks! Super cool, this brings us almost ready for v2 release (: Thank you so much!

ecordell commented 1 year ago

grpc's ChainUnaryInterceptor returns a ServerOption, not an interceptor, so it can't be used to build chains that are also interceptors (to be included in other chains).

We use this behavior in spicedb to associate default interceptors with service implementations:

https://github.com/authzed/spicedb/blob/500d9dcbf376a545a96c0d0b62340e7f1c653e85/internal/services/v1/relationships.go#L58-L68

which are then included in bigger interceptor chains for actually serving, based on startup options, etc.

It seems pretty handy to have func(...Interceptor) Interceptor in this library - any chance we could bring it back for v2? Or do you think it would be better to see if grpc-go could have an option to change the return type of the Chain fns?

(We could refactor a bit to avoid this need, but these chain helpers seemed generally useful).

johanbrandhorst commented 1 year ago

Hi Evan. I think we're trying to remove as much API as possible at this point, since we're cutting a new major. Do you think you could refactor your code or re-implement the old code in your code base? If we get more requests for this we can always add it later.