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

WFLY-13530 Expose JAX-RS Resources to gRPC clients #326

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ronsigal
Copy link
Contributor

@ronsigal ronsigal commented Jul 22, 2020

Expose JAX-RS resources as gRPC services.

@jamezp jamezp marked this pull request as draft July 22, 2020 22:13

=== Hard Requirements

* A gRPC server will be exposed on its own port.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC is also new to me so I am trying to perform some research for the security aspects but it seems later on HTTP/2 - wouldn't that mean it can come over the existing Undertow connectors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @darranl,

Well, that's a good question. The Java gRPC framework runs on Netty, so I supposed that we would need to hook it into Netty. When I communicated with Flavia, I posed the issue in those terms. I can go back and see what she says about using the current Undertow.

-Ron

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronsigal if some discussions have already happened lets try and get ongoing discussions into maybe a more open thread somewhere - I think gRPC has a lot of implications for the server as a whole.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sure. Until recently, I was just trying to figure out what was even possible. In fact, mostly I was just looking at protobuf as a transport layer until Bilge mention gRPC at the F2F meeting. So the previouse discussion has mostly been me spewing a bunch of stuff at the RESTEasy folks (sorry, RESTEasy folks). But now that it's exposed to the light of day, it's probably time to be more open. How would you suggest proceeding?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to link this from another conversation @jimma linked grpc/grpc-java#4738. If that can become part of the grpc-java project that would allow us to use that instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jamezp, and thank you @jimma for investigating that project.

It seems that they're doing the inverse of what I've been working on. That is, they're catching a gRPC request, dispatching it to a servlet, constructing a gRPC request, and dispatching it to a gRPC server. It sounds at first like there's some redundancy there, but probably not. What I'm doing is subclassing a gRPC class to catch a gRPC request, constructing an HttpServletRequest and an HttpServletResponse, and dispatching it to a JAX-RS resource by way of a RESTEasy servlet.

It seems that, given one of their servlets, we could dispatch either to a gRPC service or a JAX-RS "service" (resource).

This is going to take some investigation.

I've just pushed some experimental code to https://github.com/ronsigal/grpc-java. In particular, there are three classes in https://github.com/ronsigal/grpc-java/tree/grpc-jaxrs/examples/src/main/java/io/grpc/examples/helloworld:

  1. HelloWorldServerJAXRS is a modification of the gRPC server HelloWorldServer.java.
  2. JAXRSForwarderBuilder builds an instance of
  3. JAXRSForwarder, which constructs an getHttpServletRequest and an getHttpServletRequest and passes them to RESTEasy.

It's very preliminary.

* language independence

With this feature request we propose to expose JAX-RS services as gRPC services,
accessible by gRPC clients.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all protocol related RFEs there is a client side and a server side, for security we generally need to consider both but are we able to say providing any client is outside the scope of this RFE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'm just thinking of a way to expose JAX-RS resources to gRPC clients. gRPC already has its own mechanism for creating clients.

* speed
* language independence

With this feature request we propose to expose JAX-RS services as gRPC services,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there are two areas:

  1. Exposing gRPC services.
  2. Forwarding from gRPC services to JAX-RS endpoints.

Wouldn't it make sense for this to be two RFEs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another good point. I would say 1 is a whole other feature. The discussion started with https://issues.redhat.com/browse/PRODMGT-1811 "JBoss EAP integration with grpc for remote EJB/Remoting/IIOP", and then https://issues.redhat.com/browse/WFLY-11034 "Integration with grpc (high performance, open-source universal RPC framework)". The RESTful part was split off by Brad Maxwell in https://issues.redhat.com/browse/WFLY-13530 "REST Integration with grpc (high performance, open-source universal RPC framework)". That's all I'm addressing for now.

With this feature request we propose to expose JAX-RS services as gRPC services,
accessible by gRPC clients.

Given a gRPC service definition like
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kind of feature requests we see seem to be asking about gRPC in terms of using gRPC for existing deployments, it that going to trigger a need for generation of these definitions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another great point. Yeah, to create a gRPC client, there would have to be a protobuf definition (barring something magical, I guess). I was working on a translator that would take a Java class and translate it into a protobuf definition. It works, but only for a very limited set of classes, for now. Also, I was thinking at the protubuf level, which doesn't involve methods. So, for now, I'm thinking that the protobuf definition would be created manually. I think that automating it would be an interesting project, though.


=== Other Interested Projects

* Elytron ?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will definitely be required so we need to make sure the security integration is a part of the requirements. What I want to avoid is something where we end up with some temporary security solution where we retrospectively need to add Elytron integration and than have two solutions in play whilst one is deprecated.

It may be we can eliminate the client side for now although if invocations originating within a server are required we probably should not.

Both TLS and authentication do seem to be defined for gRPC so Elytron integration here will be a requirement - this will have the advantage that a SecurityIdentity is established on arriving at the service and will be in use for any ongoing invocation from that service.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I totally defer to you and Farah, et al, regarding security.

Ah, so there's the scenario in which a JAX-RS resource makes a gRPC call. I didn't even think of that. My first reaction is that it would be yet another feature.

static class GreeterImpl extends GreeterGrpc.GreeterImplBase {

public void sayHello(HelloRequest req, StreamObserver<HelloReply> responseObserver) {
JAXRSForwarderBuilder builder = new JAXRSForwarderBuilder();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an illustration of how the service would be generated or would the developer be expected to implement the service to forward to JAX-RS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I think of it as an illustration. There's got to be a way of translating the gRPC invocation into something suitable for the target JAX-RS resource. Now, if the JAX-RS resource is written to match the gRPC service definition, then the translation could be automated. In fact, I imagine a simple default JAXRSForwarder that does that. But if the scenario involves a gRPC client for an existing JAX-RS resource, then the translation would be more complex. Now that you mention it, it might be interesting to think about automating the process, but it's not clear to me how feasible that would be.

For now, though, I'm just thinking about a way to make the translation process as easy as possible to define.

@darranl
Copy link
Contributor

darranl commented Jul 24, 2020

@ronsigal regarding discussions I don't know if maybe something on the wildfly-dev list would be a good starting point.

Starting to research gRPC myself it feels like the kind of thing where the general support / strategy within the application server should be defined, the individual subsystems such as JAX-RS and EJB which want to expose their existing deployments would then dynamically make their resources available through this. For areas such as security this would be provided consistently within the general support.

For gRPC initially if feels like it could have a good fit with CDI, I don't know how practical that would be and if it would cause a lot of considerations that may make it a better fit as a SmallRye project. On one side if that gets too complex it may be something that makes more sense as a SmallRye project to define how gRPC deployments are handled, on the other side unless the exposing of JAX-RS endpoints is 100% automated including the protobuf generation it sounds like a level of user deployment may be necessary anyway which may mean deployment handling is required.

I think the exposed socket is possibly less of an issue compared to the general strategy. Maybe it will be necessary to expose a separate server socket for now, I would have thought something like this could justify it's own subsystem which would mean it can be defined in it's own Galleon layer but that would mean as a subsystem it could follow a similar path the Remoting subsystem took i.e. exposing a port and once possible adding support to delegate through Undertow.

Regarding the other comments about how this could integrate with Undertow, the main motivation for gRPC seems to be the use of this binary protocol we probably should be cautious that we are not adding too many layers on our side that requests need to be translated thought otherwise we may be negating the benefits from the outset.

Recently the tasks I have been working through have involved a lot of DeploymentUnitProcessor refactoring to restore better collaboration between subsystems regarding how they share security policy information, so far it has been slow going and considering backwards compatibility there is still quite a long way to go. This is the reason for something like this I am interested in the overall architecture first so we can hopefully avoid this kind of retrospective refactoring as we need to enhance it further.

@ronsigal
Copy link
Contributor Author

Hey @darranl, I really appreciate the feedback.

  1. I'll put together an email for the wildfly-dev list. If you don't mind, I'll include your most recent post, which lays out a number of issues.

  2. If there is sufficient interest, then there's room for a lot of features. I've focused exclusively on JAX-RS because it's what I know best, but I would be happy to contribute to a group effort.

  3. When you say gRPC sounds like a good fit with CDI, are you suggesting injecting gRPC clients in the same way MicroProfile REST Clients can be injected? That is different than what I've been thinking about, but it makes sense as part of a larger discussion.

  4. Concerning the separate socket and a separate subsystem, again, I just proposed what I understand. One possibility is Undertow running on Netty, when that arrives.

  5. As for protobuf and gRPC, there is some discussion on https://issues.redhat.com/browse/WFLY-13530. I think there could be two separate features: 1) protobuf as a transport layer, and 2) exposing JAX-RS resources to gRPC. Originally, I thought 2 would be awkward at best, and I focused on 1 at first. I ran some tests and was able to get as much as a 15% speedup with invocations running over the internet to a WildFly server on openshift. Then, after Bilge mentioned gRPC at the F2F, I started thinking about it again. At first I tried to find a hook inside of gRPC to capture requests, but I just couldn't find anything. That's when I thought of extending gRPC classes, which is what I've proposed in the feature request.

@ronsigal
Copy link
Contributor Author

I've sent an email to the wildfly-dev mailing list.

@ronsigal ronsigal changed the title WFLY-13530 WFLY-13530 Expose JAX-RS Resources to gRPC clients Feb 21, 2022
@ronsigal ronsigal marked this pull request as ready for review June 1, 2022 13:14
Minor updates

WFLY-13530: update and expand on the original analysis

[WFLY-13530] Included new material, eliminate use of "JAX-RS"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants