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

Support reflection #79

Closed
VMois opened this issue Nov 4, 2017 · 26 comments
Closed

Support reflection #79

VMois opened this issue Nov 4, 2017 · 26 comments

Comments

@VMois
Copy link

VMois commented Nov 4, 2017

I want to expose proto file for clients through HTTP as REST endpoint. I want to call this endpoint and get a raw proto file. Can I do this with gRPC.serveHTTP or not? Maybe you can advise some better ways to share a proto file with clients. Thanks!

@nicolasnoble nicolasnoble changed the title How can I expose proto file through HTTP using gRPC? Support reflection May 17, 2018
@kashifmin
Copy link

@VMois @lange
Have you been able to achieve this yet? We are looking to do something similar.

@VMois
Copy link
Author

VMois commented Apr 25, 2019

@kashifmin No, also gave up.

@stefanobaghino-da
Copy link

Does this ticket aim to track the support of reflection on the server, client, or both? Is it already somehow possible to invoke the reflection service from Node.js clients?

@nicolasnoble
Copy link
Member

We don't have any reflection support at the moment, neither from the client or the server. That being said, the ticket is mainly aimed at tracking server side reflection. Client side should be doable manually using the reflection protocol.

@nicolasnoble
Copy link
Member

Thinking about it more, I realized that I just said something stupid: both client and server are doable with no change in the core packages. In fact, we probably should build these as new packages.

@stefanobaghino-da
Copy link

Yeah, I'm studying how the whole thing works. Seems like client-side support is easily attainable by generating the code for the reflection protocol. Do you think this could be simply made into its own package without any addition?

Server-side instead would need a full implementation of the protocol, right?

I'd say it's probably better to have two different packages, one for clients and one for servers. Does this make sense?

@nicolasnoble
Copy link
Member

Yes, this makes sense. Both should be doable without any addition.

@stefanobaghino-da
Copy link

Regarding the server, what do you mean "without addition"? I assumed you would have had to somehow implement the reflection protocol.

@nicolasnoble
Copy link
Member

AFAIK, the reflection protocol is just a normal RPC. So you don't need to do anything special in the existing grpc code. You can simply add the appropriate RPC route for it to a server object taken as a constructor argument for instance.

@dinfer
Copy link

dinfer commented Mar 25, 2020

any progress?

@ghost
Copy link

ghost commented Dec 18, 2020

Reflection really creates barrier for entry. Do we have any chance yet?

@riskgod
Copy link

riskgod commented Apr 7, 2021

Seems still not support, hmmmmm

@kilianc
Copy link

kilianc commented Jun 9, 2021

any progress?

@aikoven
Copy link

aikoven commented Jul 3, 2021

I built a library on top of grpc-js that has a module that adds Server Reflection support. It should not be hard to adapt to raw grpc-js.

@smolijar
Copy link
Contributor

Great job @aikoven, I ported the solution to be usable with a raw grpc server: grpc-server-reflection

smolijar added a commit to smolijar/grpc that referenced this issue Jan 12, 2022
Though not yet part of the official implementation, there is a working solution for the gRPC as discussed in grpc/grpc-node#79 (comment)
@gawsoftpl
Copy link
Contributor

gawsoftpl commented Feb 2, 2022

For client you can use package: https://github.com/gawsoftpl/grpc-js-reflection-api-client. Working with @grpc/grpc-js

@jtimmons
Copy link
Contributor

jtimmons commented Oct 8, 2023

there hasn't been a lot of movement on this thread in a while but I'd be curious on the maintainers thoughts on this as well since I currently maintain another implementation of a gRPC reflection server within the NestJS framework (nestjs-grpc-reflection)

the main difference to the implementation from @aikoven / @smolijar is that it doesn't rely on a compilation step and instead purely uses proto-loader to introspect the proto files at runtime. If the maintainers would be interested in having a canonical reflection implementation in this monorepo similar to grpc-health-check, I wouldn't mind breaking out the core logic so that it can be used with multiple frontends (grpc-node, nestjs, etc) and then contributing that here

@murgatroid99
Copy link
Member

The intention was to use the structures defined in this gRFC. proto-loader implements that spec, so as far as I know, the service objects used to register services actually contain all of the necessary reflection information. Some minor modifications would be needed on the server to expose those objects, but once that's done, it should be possible to make a reflection service implementation that references the server object, and just pulls out everything it needs.

@jtimmons
Copy link
Contributor

jtimmons commented Oct 9, 2023

yes, absolutely - in fact we use those *Definition types in our reflection service to do most of the heavy lifting. What I'm wondering now is whether it fits with the goals of this project to include a reflection implementation or whether that's outside of the scope and should be kept separate? If it's in scope, I'd be happy to try and pull out what I have already and contribute it here

to give a sense of what the reflection library currently does on top of proto-loader (aside from the obvious request/response formatting):

  1. Symbol Indexing: Most of the reflection API's methods are lookups and we don't want to traverse the entire proto AST to find the symbol each time, so we do one big traversal up front to build an index and then just use that for lookup operations
  2. File Dependency Linkage. The FileDescriptorProto objects output by protoLoader don't contain links to other files that they depend on so we need to take a pass to manually link those. Arguably, this could be done in proto-loader instead
  3. Transitive Dependency Traversal. The reflection spec requires that you return both a file and everything that it depends on, so we need to traverse the file dependency graph and return all files sourced from the one that we're dealing with

@murgatroid99
Copy link
Member

Yes, I would consider it valuable for us to publish a reflection library that provides the service. We would really want to have a gRFC to determine what the right API for this is before publishing it. It doesn't necessarily need to be long and involved, the important thing is a description of the API that the library will expose and the reasoning for any non-obvious choices. For example, this gRFC is the one that defined the proto-loader library.

The points about proto-loader not outputting all of the reflection information are a surprise to me. I think the best path forward for those parts is to include that functionality in proto-loader itself, and I would be happy to work with you to accomplish that.

I do want to note one concern about the symbol indexing point: grpc-js already allows services to be added to or removed from a running server, so that's something we'll have to be careful of here. But I'm sure we could accomplish the same goals with a slight tweak, such as doing a slow traversal the first time information is accessed about a service and then caching that information for faster access later.

@jtimmons
Copy link
Contributor

okay great, I can put together a proposal for the new package then and create an issue to track some of that missing file descriptor info for proto-loader to keep that discussion separate

@jtimmons
Copy link
Contributor

@murgatroid99 - opened both of them in #2595 and grpc/proposal#397 so should be able to continue the conversation there

@jtimmons
Copy link
Contributor

jtimmons commented Dec 9, 2023

update for this thread: #2613 was merged in and @murgatroid99 published the new @grpc/reflection package last week: https://www.npmjs.com/package/@grpc/reflection

@Farenheith
Copy link

Farenheith commented Dec 16, 2023

@jtimmons that's really great! Thank you very much for the good work!
I've just tested this package using grpc-js-reflection-api-client to generate the client and it worked as intended!

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

No branches or pull requests

14 participants