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

Expose SDK Sidecar GRPC Server as HTTP+JSON #240

Closed
markmandel opened this issue Jun 4, 2018 · 8 comments
Closed

Expose SDK Sidecar GRPC Server as HTTP+JSON #240

markmandel opened this issue Jun 4, 2018 · 8 comments
Assignees
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Milestone

Comments

@markmandel
Copy link
Collaborator

Overview

Provide a REST / HTTP+JSON interface to the SDK gRPC Sidecar service.

Motivation

  1. The tooling around C++ on Windows is basically a deal breaker. It involved installing 7 things and deviated quite considerably from the tooling around installing on Linux/MacOS
  2. gRPC on Unreal and Unity tends to be difficult to impossible. But both can do HTTP+JSON. This will make integration with these commercial engines much easier.
  3. Any other language that gRPC doesn't support out of the box, this provides an easy to use alternative.

Design

I see that there are two options, each with it's own pros and cons:

Envoy Proxy

  • Pro: It's less tightly integrated with the SDK process.
  • Pro: it has a lot of development and backing behind it.
  • Con: It would be a separate process / separate Pod, which adds complexity.
  • Pro: It also exposes metrics around usage and consumption. They may come in handy down the line.

We could do local development with a docker run command that runs both Envoy and the SDK in local mode.

gRPC Gateway

  • Pro: It looks like we can run this in the same process as the gRPC server. This means we can keep the single binary for the sidecar as well as for local development.
  • Pro; it's simpler. It's built for just the one purpose that we want.
  • Con: I'm not sure that the grpc-gateway has as much contribution / backing at Envoy.
    (but looks like releases are fairly frequent)
  • Question: How well does the grpc-gateway support streaming rpcs?

Research

Envoy

grpc-gateway

@markmandel markmandel added kind/design Proposal discussing new features / fixes and how they should be implemented area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Jun 4, 2018
@markmandel
Copy link
Collaborator Author

@Kuqd I believe you've got some experience with grpc-gateway. What are your thoughts?

@markmandel markmandel added the kind/feature New features for Agones label Jun 4, 2018
@cyriltovena
Copy link
Collaborator

My thoughts regarding grpc-gateway:

  • it can run in a sidecar also, it basically connect as a client to your grpc server so both works in or out of the process. (which means you also get metrics if you have some on your grpc server)
  • it's pretty quick to implement, I've got some piece of code ready to go.
  • Stream is implemented over newline-delimited JSON streams, so if you adapt your client you also get streams but not bidirectional. (no idea how envoy works on this subject )
  • it can generate swagger-spec which is good for documentation but also for generating client code and all the goodness that comes with the swagger ecosystem. (not the case with envoy)
  • It's declarative, so you add more info to your proto files and the code is generated, this also means you get documentation for developer that want to create a custom client, they just have to look at either the proto file or the swagger generated. (not the case with envoy)
  • you have possibility to extend/hook via code into the gateway to make it fit your needs (not the case with envoy)

On our project we've stopped using it because we've dropped grpc and not vise-versa.

I don't have any experience with envoy so I can't give feedback, but I've heard it's good.

I would start with grpc-gateway and if required improve via a POC using envoy. If we have more time I'm happy to see someone take a shot at envoy to compare the end result before taking the decision. As @EricFortin would say:

we should timebox it

For me the client gen is neat, and I feel like with grpc-gateway we keep that.

@markmandel
Copy link
Collaborator Author

markmandel commented Jun 4, 2018

@Kuqd I think you have basically 100% echoed my thoughts.

It's also far better documented than Envoy in this regard as well.

grpc-gateway it is!

@markmandel markmandel self-assigned this Jun 4, 2018
@markmandel markmandel added this to the 0.3.0 milestone Jun 4, 2018
@markmandel
Copy link
Collaborator Author

Copy pasted from discussion on chat for posterity:

Cyril Tovena [10:14 AM]
:stuck_out_tongue: (edited)
now the question is do you want to go with sidecar in the pod or in-process

Mark Mandel [10:15 AM]
I think in process
it's far simpler

Cyril Tovena [10:15 AM]
ok should it be on by default ?

Mark Mandel [10:15 AM]
so that's a good question

Eric Fortin [10:15 AM]
I agree with in process being better

Mark Mandel [10:15 AM]
start with yes?

Eric Fortin [10:15 AM]
I wanted to put my thought in GH but can do so here

Mark Mandel [10:16 AM]
or start with it always on, and then we can tighten it down
yeah true - probably better on github, then there is a record
(or I can copy paste)
yeah, I worried about the additional resources/complexity of adding _another_ sidecar
which is then a proxy to a proxy... it feels a little messy
and the local SDK server would maybe need to shift to being a `docker-compose` setup maybe? it was not as clean as I would like

Eric Fortin [10:18 AM]
yeah docker dependency for day to day scares me, because Windows.

Mark Mandel [10:18 AM]
yeah, whereas having it in process means we can still compile a .exe that can be executed

Eric Fortin [10:18 AM]
exactly

Mark Mandel [10:19 AM]
:thumbsup:  I think we are all in furious agreement.

Eric Fortin [10:19 AM]
Again haha

Cyril Tovena [10:20 AM]
@Mark Mandel I had one issue with the gateway it was related to the swagger-gen and it got fixed but it was a good 2month to wait
that's my only downside
but since it's google you might be able to speed up thing if required

Mark Mandel [10:21 AM]
that is true. I have abilities to bug people :smile:

Cyril Tovena [10:21 AM]
that's a good skill !

@markmandel
Copy link
Collaborator Author

Work happening over here: https://github.com/markmandel/agones/tree/feature/http+json

@cyriltovena
Copy link
Collaborator

cyriltovena commented Jun 8, 2018

Reviewed your code and I have 2 points.

1- Some directions for swagger scheme:

https://github.com/grpc-ecosystem/grpc-gateway/blob/a5b66c16bab6156b1cc525e76159bdff463123ba/examples/proto/examplepb/a_bit_of_everything.proto#L28

2- there is no graceful shutdown on sigterm sigkill, which happens if kubernetes wants to kill or reschedule a pod, in our case if we are unhealthy it will happen, not a biggie but it’s good practice to not leave dangling goroutine. May be for later.

One side effect could be that code after serve grpc or http won’t be reached if forcefully killed. Right now it’s just log but even that is helping developers to understand what happens, later it could be resources cleanup or signaling controller or sending to events sink.

👏

PS: typo here https://github.com/markmandel/agones/blob/6af84cef36ebba2f4e7a3518485e992fe27eb7f8/sdk.proto#L31

@cyriltovena
Copy link
Collaborator

cyriltovena commented Jun 8, 2018

Also I doubt swagger will generate a client that support the stream in health GRPC method, you could

  • duplicate the health method (with a different name) without the stream and implement differently
  • remove the (fancy) stream.
  • implement a custom http client for each language.

@cyriltovena
Copy link
Collaborator

There is a PR for go swagger-api/swagger-codegen#8199

markmandel added a commit to markmandel/agones that referenced this issue Jun 12, 2018
This also replaces the C++ build image we have from the gRPC docker hub,
with one that we manage ourselves, since getting updates in a timely
matter was getting difficult.

The C++ base will likely go away once the transition to REST based SDK,
so the base may end up going away.

This is prerequisite for googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 12, 2018
This also replaces the C++ build image we have from the gRPC docker hub,
with one that we manage ourselves, since getting updates in a timely
matter was getting difficult.

The C++ base will likely go away once the transition to REST based SDK,
so the base may end up going away.

This is prerequisite for googleforgames#240
markmandel added a commit that referenced this issue Jun 13, 2018
This also replaces the C++ build image we have from the gRPC docker hub,
with one that we manage ourselves, since getting updates in a timely
matter was getting difficult.

The C++ base will likely go away once the transition to REST based SDK,
so the base may end up going away.

This is prerequisite for #240
markmandel added a commit to markmandel/agones that referenced this issue Jun 13, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 13, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 15, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 15, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 15, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
cyriltovena pushed a commit to cyriltovena/agones that referenced this issue Jun 17, 2018
This also replaces the C++ build image we have from the gRPC docker hub,
with one that we manage ourselves, since getting updates in a timely
matter was getting difficult.

The C++ base will likely go away once the transition to REST based SDK,
so the base may end up going away.

This is prerequisite for googleforgames#240
markmandel added a commit to markmandel/agones that referenced this issue Jun 17, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes googleforgames#240
markmandel added a commit that referenced this issue Jun 18, 2018
Implement grpc-gateway in front of the gRPC based sdk-server, so that
it can be access via HTTP+JSON.

This includes documentation and a swagger/openapi specificiation.

This also has been implemented such that the sdk-server is still a single
binary, and as such, the HTTP+JSON interface can still be used for local
development.

Closes #240
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Projects
None yet
Development

No branches or pull requests

2 participants