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

[WIP] Add grpc-web support #207

Closed
wants to merge 1 commit into from
Closed

[WIP] Add grpc-web support #207

wants to merge 1 commit into from

Conversation

ST-DDT
Copy link
Collaborator

@ST-DDT ST-DDT commented Apr 3, 2019

Work in progress - Feedback appreciated

Adds #196

Usage simply add grpc-spring-boot-starter-web to your dependencies and the library will automatically provide web methods to access the grpc-services.

Calls

  • GET /grpc
    Lists all available grpc-methods
  • POST /grpc/{service}/{method}
    Execute the given grpc-method with the data that were send in the request body.
    If you want to send multiple requests (client streaming) you can do so by sending a list of inputs (json-array).

Features

  • Use any spring supported media format to send the data to the server and receive them.
    • You can define your own HttpMessageConverter if you want to add additional formats
  • Does not require a running grpc-server at all.
  • Also works as library without web-server (direct method invocation)
  • Input/Output highly configurable
  • Independent of the other grpc-spring-boot-starter projects

TODO

  • Add javadocs
  • Add documentation
  • Add tests

@ST-DDT ST-DDT added the enhancement A feature request or improvement label Apr 3, 2019
@ST-DDT ST-DDT self-assigned this Apr 3, 2019
@ST-DDT ST-DDT added this to the 2.4.0 milestone Apr 3, 2019
@yidongnan
Copy link
Collaborator

This looks very useful 🎉

I am not familiar with grpc web, I have the following questions.

  1. Does this meet the criteria of the grpc web protocol? PROTOCOL-WEB.md.
    e.g. Text-encoded (response) streams, content-type etc.
  2. Need to provide an example with grpc web client, like this helloworld
  3. In addition to calling the method directly, can it also provide a proxy way, similar to a gateway

@ST-DDT ST-DDT modified the milestones: 2.4.0, 2.5.0 May 17, 2019
@ST-DDT ST-DDT force-pushed the grpc-web-bridge branch from 6dd59b8 to a050ac2 Compare May 17, 2019 14:46
@ST-DDT ST-DDT force-pushed the grpc-web-bridge branch from a050ac2 to 6c7288b Compare June 23, 2019 22:59
@ST-DDT ST-DDT removed this from the 2.5.0 milestone Jul 4, 2019
@ST-DDT ST-DDT added this to the Future milestone Feb 23, 2020
@mattdkerr
Copy link
Contributor

mattdkerr commented Feb 23, 2020

Some docs on how to add Protobuf message converter could be useful, such as for when a message has a property of the Any type. I have this if you need a contribution.

@mattdkerr
Copy link
Contributor

This is a really interesting feature- it could be a good alternative for replacing a custom REST API alternative to a gRPC API. Are there specific caveats that should be known? I had also been looking at Go based solutions that can code-gen a web proxy from the gRPC “reflection” metadata endpoint, but likely would need to wrap that in a container of some kind for external configuration and reuse across services.

I’ve started reviewing but will likely take a bit longer later this week to fully grok and give decent feedback.

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Feb 23, 2020

Some docs on how to add Protobuf message converter could be useful, such as for when a message has a property of the Any type. I have this if you need a contribution.

I already wrote some HttpMessageConverters myself and I know how to register them.
The only thing I'm not happy about is the reflection usage to go from Class to Descriptor.
But if you already have the code, you could post it for others as a reference.

Are there specific caveats that should be known?

IIRC Streaming calls are currently not supported and I'm not sure how to implement them.
I thought about using some kind of ManyAny wrapper, but it's not really suitable for streaming and even less for long running calls.

I also have to consider other libraries in this area such as grpc-gateway for compatibility reasons.

I’ve started reviewing but will likely take a bit longer later this week to fully grok and give decent feedback.

Thanks, but your effort might end up in vain, because I thought about rewritting the code from scratch to support thinks like custom curls that are defined in the protobuf files.

This is a really interesting feature- it could be a good alternative for replacing a custom REST API alternative to a gRPC API.

Thanks you very much for the feedback. This is a very big feature and it didn't gather any attention, so my motivation to pushing this forward was low. If you think this is worthwhile, perhaps we can revive it.

@mattdkerr
Copy link
Contributor

I do think it’s worth exploring. Would it be packaged as a separate module?

Regarding streaming, I believe that web sockets is the best fit on the HTTP/2 side. This is even bigger to tackle, but could prove to be pretty interesting.

Both of these features may be more desirable separated from Spring, then wrapped up for Spring here. (Similar to things like Micrometer usage in Actuator.) Have you considered that sort of set up? I might be willing to contribute to this more actively.

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Feb 23, 2020

I do think it’s worth exploring. Would it be packaged as a separate module?

If you consider grpc-spring-boot-web-starter a separate module then yes.

Regarding streaming, I believe that web sockets is the best fit on the HTTP/2 side. This is even bigger to tackle, but could prove to be pretty interesting.

If we use HTTP/2 we could just use grpc. It should be possible to support HTTP1 as well. Or are websockets always HTTP2?

Both of these features may be more desirable separated from Spring, then wrapped up for Spring here. (Similar to things like Micrometer usage in Actuator.) Have you considered that sort of set up? I might be willing to contribute to this more actively.

I haven't thought about that yet.
Which features would you separate into that module?

  • protobuf-extension for urls, methods... -> reuse grpc-web/grpc-gateway's protos?
  • request url to grpc method mapping -> probably hard to implement framework agnostic or causes code duplication
  • request body parsing -> only a few lines in the HttpMessageConverter

@linarkou
Copy link

linarkou commented Jul 2, 2020

What about google.api.http support?
There is one Golang implementation : https://github.com/grpc-ecosystem/grpc-gateway
It seems more flexible for me...

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Jul 3, 2020

Yeah, support for those is planned, but I never had the time to fully implement it.
If you have an example of how to register custom/dynamic command mappings in spring, I will try to push this forward a bit.

@jbf154
Copy link

jbf154 commented Jul 16, 2020

FWIW, we're using grpc-spring-boot-starter for gRPC services and leverage Ambassador to handle the grpc-web encoding. These are server-streaming APIs and are long-lived. IIRC, client-streaming isn't officially supported yet (though there was an improbable grpc-web websocket proxy but it was labeled as not ready for production).

@ST-DDT ST-DDT mentioned this pull request May 17, 2021
@a-simeshin
Copy link
Contributor

Yeah, support for those is planned, but I never had the time to fully implement it.
If you have an example of how to register custom/dynamic command mappings in spring, I will try to push this forward a bit.

Very interesting functionality. Сould you share information, in what state is it at the moment? Is there any implementation plan?

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Sep 3, 2021

Very interesting functionality. Сould you share information, in what state is it at the moment?

Due to time constraints I haven't looked into it. I still haven't searched/found a way to register the web request mappings dynamically.

Is there any implementation plan?

The implementation is currently not scheduled for a specific release. Contributions are welcome.

@a-simeshin
Copy link
Contributor

a-simeshin commented Sep 3, 2021

Very interesting functionality. Сould you share information, in what state is it at the moment?

Due to time constraints I haven't looked into it. I still haven't searched/found a way to register the web request mappings dynamically.

Do I understand correctly that the problem is to dynamically determine by the REST input address which service and method should be called, while they are already in the context of the spring?

If I understood the problem correctly, could this be a possible solution?

@RequestMapping(value = "/grpc/{service}/{method}", method = RequestMethod.POST)
public void restRouter(@PathVariable("service") String service, @PathVariable("method") String method, @RequestBody Object body) {
     //Then load bean from context by service value and call method by method value?
}

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Sep 3, 2021

Due to time constraints I haven't looked into it. I still haven't searched/found a way to register the web request mappings dynamically.

Do I understand correctly that the problem is to dynamically determine by the REST input address which service and method should be called, while they are already in the context of the spring?

If I understood the problem correctly, could this be a possible solution?

@RequestMapping(value = "/grpc/{service}/{method}", method = RequestMethod.POST)
public void restRouter(@PathVariable("service") String service, @PathVariable("method") String method, @RequestBody Object body) {
     //Then load bean from context by service value and call method by method value?
}

Well, part of it.
The current implementation in this PR already does that (and it works for simple calls).
https://github.com/yidongnan/grpc-spring-boot-starter/pull/207/files#diff-b6cc19cda735dac0076cc592f22e7179c30af1eb91d61730dc3faec8bb671c20R150
Hovever, as requested above, the annotations/options on the grpc definition should allow for an arbitary path to the grpc calls. Including those without a well known prefix such as /grpc/.
That requires dynamic registration of mapping paths and that is blocking me to a certain extend.

@a-simeshin
Copy link
Contributor

Well, part of it.
The current implementation in this PR already does that (and it works for simple calls).
https://github.com/yidongnan/grpc-spring-boot-starter/pull/207/files#diff-b6cc19cda735dac0076cc592f22e7179c30af1eb91d61730dc3faec8bb671c20R150
Hovever, as requested above, the annotations/options on the grpc definition should allow for an arbitary path to the grpc calls. Including those without a well known prefix such as /grpc/.
That requires dynamic registration of mapping paths and that is blocking me to a certain extend.

I want to express my gratitude for the idea and explanation.
Created a library similar to this one, but for the client, since colleagues from load testing do not want to leave their REST load generators (sad). Now they can create rest->grpc proxy adapters as a separate deployment in a couple of lines of code. The client was selected in order to maintain the integrity of the load tests.

Now I seem to understand what the problem exactly is with dynamic REST mapping.
Also, PojoFormat can be replaced with JsonFormat https://github.com/protocolbuffers/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Sep 4, 2021

Also, PojoFormat can be replaced with JsonFormat https://github.com/protocolbuffers/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java

Thanks for the hint. I originally intended the PojoFormat to allow "any Content-Type" to be translated into a "pojo"/map using spring's HttpMessageConverters and then pojo -> proto. Today I think its better/easier to reuse existing mappers (JsonFormat) for that. But I'm not sure whether we'll be able to push the conversion to a HttpMessageConverter or if we have to do that inside the controller method itself, as the former requires prior knowldedge of the expected data-type. So this somewhat dependents on the solution regarding the dynamic rest mapping.
AFAIK Bypassing spring's HttpMessageConverters without side effects on clients or other controller methods is non-trivial.

@jakub-spiewak
Copy link

Any progress?

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Apr 5, 2022

Any progress?

Unfortunately not. I currently don't have time to tackle this.
It works at it is just with limitations that are not suitable for a library release.

@linarkou
Copy link

linarkou commented Apr 20, 2022

@ST-DDT I just came up with an idea of creating new Servlet, configuring it and registering it using ServletRegistration.Dynamic like here.
What do you think?

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Apr 20, 2022

Sounds like an interesting approach, I'm not sure whether I can interweave this with normal spring-web.
/foobar/a -> web
/foobar/b -> grpc
/bar -> web
/foo -> grpc

Might be worth a try.

@linarkou
Copy link

@ST-DDT

Sounds like an interesting approach, I'm not sure whether I can interweave this with normal spring-web.
/foobar/a -> web
/foobar/b -> grpc
/bar -> web
/foo -> grpc

Might be worth a try.

I suppose you need to collect all the grpc mappings and register Serlvet only for them.
This small example works and covers your case.

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.web.servlet.ServletContextInitializer;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

import javax.servlet.*;
import javax.servlet.http.*;
import java.io.*;

@SpringBootApplication
@RestController
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class, args);
    }

    @GetMapping({"/foobar/a", "/bar"})
    public String hello() {
        return "spring-web";
    }

    @Configuration
    public static class ConfigureServlet implements ServletContextInitializer {

        @Override
        public void onStartup(ServletContext servletContext) {
            Servlet myServlet = new GrpcServlet();
            ServletRegistration.Dynamic serviceServlet = servletContext.addServlet("myServlet", myServlet);

            serviceServlet.addMapping("/foobar/b", "/foo");
            serviceServlet.setAsyncSupported(true);
            serviceServlet.setLoadOnStartup(1);
        }
    }

    public static class GrpcServlet extends HttpServlet {
        public void doGet(HttpServletRequest req, HttpServletResponse res) throws IOException {
            res.setContentType("text/html");
            PrintWriter pw = res.getWriter();
            pw.println("grpc");
            pw.close();
        }
    }
}

@ST-DDT
Copy link
Collaborator Author

ST-DDT commented Feb 11, 2023

@ST-DDT ST-DDT closed this Feb 11, 2023
@ST-DDT ST-DDT deleted the grpc-web-bridge branch February 11, 2023 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants