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

Add custom gRPC interceptors support #7755

Closed
korthout opened this issue Sep 2, 2021 · 2 comments · Fixed by #7839
Closed

Add custom gRPC interceptors support #7755

korthout opened this issue Sep 2, 2021 · 2 comments · Fixed by #7839
Assignees
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior

Comments

@korthout
Copy link
Member

korthout commented Sep 2, 2021

Description

Introduce a new gateway configuration for gRPC interceptors.

When configured, it needs to:

  • load a jar with a custom io.grpc.ServerInterceptor implementation
  • load that class and bind it to the server builder (something like: serverBuilder.intercept(interceptor))
@korthout korthout added the kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. label Sep 2, 2021
@korthout korthout added this to Ready in Zeebe Sep 2, 2021
@npepinpe npepinpe moved this from Ready to Planned in Zeebe Sep 3, 2021
@korthout korthout added kind/feature Categorizes an issue or PR as a feature, i.e. new behavior and removed kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. labels Sep 6, 2021
@npepinpe
Copy link
Member

npepinpe commented Sep 10, 2021

Here's my planned breakdown, let me know what you think:

  • Extract the external JAR loading code into the util module. This means ExporterJarRepository -> ExternalJarRepository, ExporterJarClassLoader -> ExternalJarClassLoader, and ExporterJarLoadException -> ExternalJarLoadException. I did think about having a new module for this, and it might make sense in the future if we wanted this module to be used elsewhere outside of Zeebe, but as it is I think it's OK in the util module. Move external JAR loading utilities from the broker to the util module #7815
  • Introduce external interceptor loading into the Gateway, via a new Map<String, InterceptorCfg> interceptors configuration option which will be like the Map<String, ExporterCfg> exporters properties. The reason to use a different configuration option is to decouple it from the ExporterCfg, as we may later want different things. It could be that we introduce a shared ExternalServiceCfg, and have both classes just subclass that, since initially there is no way to configure the interceptor. In the same issue, I would also add the loading of the interceptor repository from the configuration (much like the ExporterRepository). Load arbitrary interceptors in the gateway #7839
  • Use the interceptor repository to instantiate the interceptors, and wrap any and all services with them. For now, assume instantiation means the interceptor is valid. Load arbitrary interceptors in the gateway #7839

If time permits, we could look into configuration of the interceptors. Without configuration, it means the interceptors are mostly pure functions, or extract whatever they can from the context (i.e.Context.current()) and the request metadata. This seems to be most interceptors that I could find on Github. However, some interesting ones, like the TracingInterceptor, require some configuration via a builder. To satisfy these constraints, we would need a way to configure interceptors, or instantiate them with some parameters. One option is:

  • Implement something like Use Spring's ApplicationContext to instantiate exporter arguments #7628 - this will make it easier to instantiate the user's ServerInterceptor bean according to the Spring Boot conventions. For now, I would actually only scope it to the Gateway/StandaloneGateway, and we can extend it to the Broker/StandaloneBroker later as a follow up, and apply it to the exporters. With this, we can now instantiate beans much as Spring would.
  • Instantiate the ServerInterceptor as a bean, with the args map literally being the arguments for the constructor of the bean. This is something Spring supports out of the box, so the BeanFactory from above should work with this. Another option is to take a page from Java's SPI mechanism, and instead load an InterceptorProvider from the JAR or class path. This provider can then take in the configuration options and any other context (such as the Query API interface for the client).

Personally, I would delegate interceptor configuration to the interceptor author. My reasoning here is that I would like to use the ServerInterceptor interface provided by the gRPC library as the interface we're loading from the JAR (or even from the class path), as imo this would make it easier to interoperate with the ecosystem of existing ServerInterceptor implementations.

At any rate, I bring up the configuration because as you can note from the last point, it might change what we load from the JAR - a provider, or the interceptor itself.

@npepinpe npepinpe moved this from Planned to In progress in Zeebe Sep 14, 2021
@npepinpe npepinpe self-assigned this Sep 14, 2021
@npepinpe
Copy link
Member

We will skip the configuration for now.

ghost pushed a commit that referenced this issue Sep 15, 2021
7815: Move external JAR loading utilities from the broker to the util module r=npepinpe a=npepinpe

## Description

This PR simply moves all external JAR loading utilities from the broker module to the util module. The utilities were previously used to load exporters, and will soon be used to load interceptors in the gateway. They were consequently renamed from `ExporterJar*` to `ExternalJar*`. No real logic changes were done, just moving things.

## Related issues

relates to #7755



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
@npepinpe npepinpe moved this from In progress to Review in progress in Zeebe Sep 16, 2021
@npepinpe npepinpe linked a pull request Sep 16, 2021 that will close this issue
9 tasks
@ghost ghost closed this as completed in ea10262 Sep 20, 2021
Zeebe automation moved this from Review in progress to Done Sep 20, 2021
@KerstinHebel KerstinHebel removed this from Done in Zeebe Mar 23, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes an issue or PR as a feature, i.e. new behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants