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

[FEATURE] add server reflection support for grpc server #1181

Open
vtolstov opened this issue Feb 10, 2020 · 8 comments
Open

[FEATURE] add server reflection support for grpc server #1181

vtolstov opened this issue Feb 10, 2020 · 8 comments

Comments

@vtolstov
Copy link
Member

@vtolstov vtolstov commented Feb 10, 2020

Is your feature request related to a problem? Please describe.
Add ability to use with micro grpc service standalone grpc tools like web ui or grpc curl

Describe the solution you'd like
https://github.com/grpc/grpc-go/blob/master/Documentation/server-reflection-tutorial.md#enable-server-reflection

Additional context
simple change, needs to investigate drawbacks

@vtolstov

This comment has been minimized.

Copy link
Member Author

@vtolstov vtolstov commented Feb 10, 2020

not so simple as looks before:
as we don't have standard grpc and have micro, we need to implement reflection server by providing
https://github.com/grpc/grpc-go/blob/master/reflection/grpc_reflection_v1alpha/reflection.proto
and have only one rpc for it
rpc ServerReflectionInfo(stream ServerReflectionRequest) returns (stream ServerReflectionResponse);
}

if we fill response with valid data all stuff works fine with it

@vtolstov

This comment has been minimized.

Copy link
Member Author

@vtolstov vtolstov commented Feb 10, 2020

@asim

This comment has been minimized.

Copy link
Member

@asim asim commented Feb 10, 2020

Now that's interesting. I did not know that we could hack around the problem with this. An interesting solution. How much effort to do you think it is to add this by default to all services? We could add in server/grpc where we do extraction, have a reflection.go file for it?

@vtolstov

This comment has been minimized.

Copy link
Member Author

@vtolstov vtolstov commented Feb 11, 2020

I think that this is must be option to server. Because it adds additional handler to it. And also may be used to abuse it or something bad. Mostly we can add reflection.go file and slightly modify it for our use-case

@asim

This comment has been minimized.

Copy link
Member

@asim asim commented Feb 11, 2020

So Reflection() option in grpc server package to enable it?

@vtolstov

This comment has been minimized.

Copy link
Member Author

@vtolstov vtolstov commented Feb 11, 2020

yes, probably

@heisonyee

This comment has been minimized.

Copy link

@heisonyee heisonyee commented Feb 21, 2020

Consider adding an option for exposing grpc.Server, it will help us do more about things, such as registering prometheus, reflection, health etc..

@vtolstov

This comment has been minimized.

Copy link
Member Author

@vtolstov vtolstov commented Feb 21, 2020

we don't plan to leak abstraction and expose grpc.Server

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.