-
Notifications
You must be signed in to change notification settings - Fork 344
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
Add gRPC reflection and file descriptor set support #1086
Add gRPC reflection and file descriptor set support #1086
Conversation
🦋 Changeset detectedLatest commit: 557923a The changes in this PR will be included in the next version bump. This PR includes changesets to release 61 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some notes showing my thought process.
packages/handlers/grpc/package.json
Outdated
@@ -21,6 +21,7 @@ | |||
"fs-extra": "9.0.1", | |||
"graphql-scalars": "1.3.0", | |||
"graphql-compose": "7.21.4", | |||
"grpc-reflection-js": "^0.0.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See "Discussion Notes" in the PR regarding why this requires swapping in grpc
packages/handlers/grpc/src/index.ts
Outdated
import { load } from '@grpc/proto-loader'; | ||
PackageDefinition, | ||
} from 'grpc'; | ||
import { createPackageDefinition, load } from '@grpc/proto-loader'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createPackageDefinition
isn't exported by @grpc/proto-loader
.
What can we do to get this?
packages/handlers/grpc/src/index.ts
Outdated
services.map((service: string) => reflectionClient.fileContainingSymbol(service)) | ||
); | ||
serviceRoots.forEach((serviceRoot: Root) => { | ||
serviceRoot.name = this.config.serviceName || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, the first nested
object will have a name of ""
. We add the config.serviceName
to properly nest the objects for naming.
if (this.config.useReflection) { | ||
const grpcReflectionServer = this.config.endpoint; | ||
const reflectionClient = new grpcReflection.Client(grpcReflectionServer, creds); | ||
const services = (await reflectionClient.listServices()) as string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
services
will include the strings of all of the services exposed by gRPC reflection... including the grpc.reflection.v1alpha
ServerReflection
service. It's quite meta... GQL can then be used to forward the reflection.
responseType = camelCase( | ||
currentPath.split('.').join('_') + '_' + responseType.replace(reflectionPath, '') | ||
); | ||
requestType = camelCase(currentPath.split('.').join('_') + '_' + requestType.replace(reflectionPath, '')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stuff was tricky. The requestType
and responseType
properties under methods
are in different format when we use reflection.
586742b
to
0c34937
Compare
} | ||
const protoDefinition = await root.load(fileName as string, options); | ||
protoDefinition.resolveAll(); | ||
packageDefinition = await load(fileName as string, options); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the original protoFilePath
logic should still work
0c34937
to
c63eea0
Compare
251f0a5
to
967618d
Compare
@tatemz That is really COOL! Thank you! |
Correct. We are awaiting progress on grpc/grpc-node#1627. You can test it out by building the |
913cca1
to
ed6b0f5
Compare
9cb6252
to
8aa53d7
Compare
This change adds a new feature to the gRPC handler that allows graphql-mesh to automatically create a schema by querying the reflection endpoints on a gRPC service that exposes its reflection service. This feature is enabled by adding the grpc-reflection-js package. Additionally, this change replaces dependencies from @grpc/grpc-js with grpc as that is the package that is used with generated proto code. The grpc package works in conjunction with the grpc-reflection-js package.
This change reverts back to using @grpc/grpc-js instead of grpc since the grpc-reflection-js package no longer has conflicting implementation. See redhoyasa/grpc-reflection-js#3
This change syncs @grpc/grpc-js versions for compatibility with grpc-reflection-js.
This change adds a new grpc handler configuration property to specify either a binary-encoded or JSON file descriptor set.
This change fixes an issue where the gRPC reflection feature was not correctly adding gathered gRPC service root objects.
This change updates the grpc handler feature supporting reflection and file descriptor sets using the latest proto-loader api.
8aa53d7
to
a2365f2
Compare
Sorry I didn't keep track of this. Thanks for carrying the torch @ardatan ! 💯 |
Didn't work at all in the version ^0.14.9. Buffer.from does not accept json... |
Co-authored-by: Renovate Bot <[email protected]>
Overview
Note: This is not yet complete. See "Discussion Notes" below.
This change adds two new features to the gRPC handler.
Both of these features make it easier for
graphql-mesh
to automatically create a schema for gRPC.useReflection: boolean
This config option enables
graphql-mesh
to generate a schema by querying the gRPC reflection endpoints. This feature is enabled by thegrpc-reflection-js
package.descriptorSetFilePath: object | string
This config option enabled
graphql-mesh
to generate a schema by importing either a binary-encoded file descriptor set file or a JSON file descriptor set file. This config works just likeprotoFilePath
and can be a string or an object containing the file and proto loader options.Binary-encoded file descriptor sets can be created by using
protoc
with the--descriptor_set_out
option. Example:protoc -I . --descriptor_set_out=./my-descriptor-set.bin ./my-rpc.proto
JSON file descriptor sets can be created using
protobufjs/protobuf.js
.Todo
@grpc/grpc-js
grpc-reflection-js
version oncev0.1.0
has been published to NPMForkprotobuf.js
and exportcreatePackageDefinition
if support of protoset binary format? protobufjs/protobuf.js#1117 does not get answeredPointprotobuf.js
dependency to fork@grpc/proto-loader
if Add functions for loading and parsing binary-encoded or plain object file descriptor sets grpc/grpc-node#1635 is acceptedDiscussion Notes
@grpc/grpc-js
vsgrpc
This issue has been resolved by redhoyasa/grpc-reflection-js#3
There is a conflict when usinggrpc-reflection-js
. TheCredentialsChannel
type used by it is different than theCredentialsChannel
type used by@grpc/grpc-js
. This is why I swapped ingrpc
. However, on thegrpc
package README, it states that this package is deprecated (presumably in favor of@grpc/grpc-node
? 🤔 🤷♂️)I find this odd, because when using the JS
protoc
plugin, all of the generated gRPC server/client code uses thegrpc
package.I suppose until
protoc
updates it's output dependency, then usinggrpc
should be ok. Alternatively, we can submit a PR togrpc-reflection-js
to update that dependency to use@grpc/grpc-js
. Whew 😅createPackageDefinition
The use of
createPackageDefinition
is not recommended as discussed in grpc/grpc-node#1627. Instead grpc/grpc-node#1635 was raised.This change usescreatePackageDefinition
from@grpc/proto-loader
. There's only one problem... that function isn't exported.Follow these two issues to read more:
support of protoset binary format? protobufjs/protobuf.js#1117createPackageDefinition without doing file I/O? grpc/grpc-node#550To get this feature to work, edit the following files:What do you suggest we do to make this shippable?New Config
This change adds two new config properties to the
grpc
handler. To enable this feature, either setuseReflection
ordescriptorSetFilePath
Sample reflection
.meshrc.yaml
Sample binary-encoded file descriptor set
.meshrc.yaml
Sample JSON file descriptor set
.meshrc.yaml