Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gRPC: per-client interceptors #21648

Closed
michalszynkiewicz opened this issue Nov 23, 2021 · 7 comments · Fixed by #21891
Closed

gRPC: per-client interceptors #21648

michalszynkiewicz opened this issue Nov 23, 2021 · 7 comments · Fixed by #21891
Assignees
Labels
area/grpc gRPC kind/enhancement New feature or request
Milestone

Comments

@michalszynkiewicz
Copy link
Member

Description

For gRPC services we now have @GlobalInterceptors and interceptors registered for specific servies.
We likely need something similar for clients, so that a user can attach an interceptor to a specific client

CC @cescoffier @mkouba

Implementation ideas

No response

@michalszynkiewicz michalszynkiewicz added the kind/enhancement New feature or request label Nov 23, 2021
@quarkus-bot quarkus-bot bot added the area/grpc gRPC label Nov 23, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 23, 2021

/cc @cescoffier

@mkouba
Copy link
Contributor

mkouba commented Nov 24, 2021

Hm, so currently any bean that implements ClientInterceptor is registered automatically? If so then we're quite inconsistent ;-)

Anyway, I think that we can reuse the @io.quarkus.grpc.GlobalInterceptor and possibly introduce the @RegisterClientInterceptor?

@cescoffier
Copy link
Member

Exactly, I found out that when implementing the metrics support.

@mkouba mkouba self-assigned this Dec 1, 2021
@mkouba mkouba added this to the 2.6 - main milestone Dec 2, 2021
mkouba added a commit to mkouba/quarkus that referenced this issue Dec 2, 2021
- resolves quarkusio#21648
- make the RegisterInterceptor annotaion repeatable
- refactor server interceptors
mkouba added a commit to mkouba/quarkus that referenced this issue Dec 2, 2021
- resolves quarkusio#21648
- make the RegisterInterceptor annotaion repeatable
- refactor server interceptors
mkouba added a commit to mkouba/quarkus that referenced this issue Dec 3, 2021
- resolves quarkusio#21648
- make the RegisterInterceptor annotaion repeatable
- refactor server interceptors
mkouba added a commit to mkouba/quarkus that referenced this issue Dec 6, 2021
- resolves quarkusio#21648
- make the RegisterInterceptor annotaion repeatable
- refactor server interceptors

Co-authored-by: Michał Szynkiewicz <[email protected]>
Co-authored-by: Clement Escoffier <[email protected]>
jeanphi-baconnais pushed a commit to jeanphi-baconnais/quarkus that referenced this issue Dec 7, 2021
- resolves quarkusio#21648
- make the RegisterInterceptor annotaion repeatable
- refactor server interceptors

Co-authored-by: Michał Szynkiewicz <[email protected]>
Co-authored-by: Clement Escoffier <[email protected]>
@sukolenvo
Copy link

Hi, I am trying to register client interceptor for a specific service. My bean graph is something like:

FooAuthClientInterceptor {
}

BeanA {
  @GrpcClient("foo")
  FooBlockingStub fooGrpcClient;
  
  @GrpcClient("bar")
  BarBlockingSub barGrpcClient;
}

BeanB {
  @GrpcClient("foo")
  FooBlockingStub grpcClient;
}

BeanC {
  @GrpcClient("foo")
  FooBlockingStub grpcClient;
}

Adding @RegisterClientInterceptor(FooAuthInterceptor.class) for every injection point seems a bit redundant (and could be easily forgotten for new services). Have you considered support for annotation for a type? Something like:

@RegisterFor("foo")
FooAuthClientInterceptor {
}

Alternatively if interceptor list could be set in GrpcClientConfiguration that would also be nit.

@cescoffier
Copy link
Member

You can register a global interceptor. See https://quarkus.io/guides/grpc-service-consumption#client-interceptors

@sukolenvo
Copy link

sukolenvo commented Mar 29, 2022

Yes, but isn't global interceptor will attach to @GrpcClient("bar") as well? Its a bit risky to mark Auth interceptor as a default one - means credentials might go to the external service that wasn't intended to get them.

@michalszynkiewicz
Copy link
Member Author

Yes it will.
You can work around it by delegating all interactions with a specific client to a single bean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/grpc gRPC kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants