feat(microservices): add support for grpc reflection api

[jtimmons]

trafficstars

PR Checklist

Please check if your PR fulfills the following requirements:

• [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• [ ] Tests for the changes have been added (for bug fixes / features)
• [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• [ ] Bugfix
• [x] Feature
• [ ] Code style update (formatting, local variables)
• [ ] Refactoring (no functional changes, no api changes)
• [ ] Build related changes
• [ ] CI related changes
• [ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Automatically add support for the gRPC reflection API if the user has the @grpc/reflection package installed

Does this PR introduce a breaking change?

• [ ] Yes
• [x] No

Other information

I currently maintain a Nest Module package (nestjs-grpc-reflection) which adds this capability to a user's app, but opening this PR as a discussion point for whether this makes sense to add to NestJS directly!

The gRPC server reflection API exposes information about a gRPC package so that clients can introspect it, this is gaining in popularity in developer tooling such as Postman, grpcui, ezy and so on as it removes the need for developers to manually manage proto file definitions and can instead rely on their client to fetch the correct definition automatically

[coveralls]

Pull Request Test Coverage Report for Build ac32cf05-85fc-4042-afa2-a4bcb1a35160

Details

• 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.03%) to 92.135%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/microservices/server/server-grpc.ts	0	2	0.0%
<!--	Total:	0	2

Files with Coverage Reduction	New Missed Lines	%
packages/microservices/server/server-grpc.ts	1	91.39%
<!--	Total:	1

Totals
Change from base Build 2bf2e4c8-5da9-4dd1-8f93-3b733029afc6:	-0.03%
Covered Lines:	6736
Relevant Lines:	7311

---

💛 - Coveralls

[ssilve1989]

👍 looking forward to seeing this functionality released!

[ssilve1989]

@kamilmysliwiec any chance this can be reviewed?

[kamilmysliwiec]

You can declare a new custom strategy that inherits from the ServerGrpc class and overrides one method (I'm assuming loadProto) to register gRPC reflection API. If we want to make it even simpler, I'd be happy to approve a PR that adds one extra method called, let's say, onPreLoadDefinition where anyone could hook any logic they need (before calling grpcPackage.loadPackageDefinition).

I see no need for lazily loading yet another library when this should be fairly simple to achieve without making any changes to the codebase.

[jtimmons]

You can declare a new custom strategy that inherits from the ServerGrpc class and overrides one method (I'm assuming loadProto) to register gRPC reflection API. If we want to make it even simpler, I'd be happy to approve a PR that adds one extra method called, let's say, onPreLoadDefinition where anyone could hook any logic they need (before calling grpcPackage.loadPackageDefinition).

I see no need for lazily loading yet another library when this should be fairly simple to achieve without making any changes to the codebase.

sure, I can take a pass at one of those options and add to the documentation as a recipe? My thinking in integrating it more deeply into the ServerGrpc microservice strategy though is that:

1. The reflection spec and @grpc/reflection package are part of the gRPC node ecosystem alongside the server and proto-loader that we're using already so it's more a feature of that ecosystem than something standalone
2. Reflection as a whole is beginning to gain some good traction in gRPC community as it is a big improvement to the developer experience in local testing, for type/client generation of a live schema, etc

imo there's almost no reason to run a gRPC service without reflection these days so given the utility of it and it being central to the grpc-node ecosystem, it might make sense to try and make as frictionless as possible for people to enable?

[kamilmysliwiec]

But this means adding yet another peer dependency to the @nestjs/microservices package which already has way too many of them.

I agree that @grpc/reflection package has several benefits and using it makes a lot of sense for most gRPC applications. What I'm trying to say is that we should rather look for an alternative API that would let users install this dependency themselves and register it through a dedicated API (instead of doing this internally within the @nestjs/microservices pkg under a flag).

[jtimmons]

yep, that makes sense to me! So between the options we have, making a custom strategy feels a bit heavy-handed and isn't very composable with other changes like this in the future so I'm looking more into how we can inject the reflection service into the existing strategy. Feels like there's two paths here:

1. We can add an event handler like the onPreLoadDefinition that you mentioned into our GrpcOptions. It feels like maybe the wrong place to include this since it's not configuration but the plus is that it'd be easy for people to integrate with their existing services

export const grpcClientOptions: GrpcOptions = {
  transport: Transport.GRPC,
  options: {
    ...
    onPreLoadDefinition: (pkg, server) => {
      new ReflectionService(pkg).addToServer(server);
    }
  },
}

2. We could do the same on the raw ServerGrpc class and then have people load that as if it were a custom strategy. This is a bit more cumbersome for users but feels maybe a little more correct? The usage could look something like this:

  const grpcServer = ServerGrpc(grpcClientOptions.options);
  grpcServer.onPreLoadDefinition((pkg, server) => {
    const reflection = new ReflectionService(pkg);
    reflection.addToServer(server);
  });

  app.connectMicroservice({ strategy: grpcServer });

any preference between these directions or some other path I missed here @kamilmysliwiec?

[kamilmysliwiec]

Let's pursue the number 1 option (easy to integrate with existing services and compatible with the regular gRPC microservice creation flow - with no need to explicitly import the server class)

[jtimmons]

Let's pursue the number 1 option (easy to integrate with existing services and compatible with the regular gRPC microservice creation flow - with no need to explicitly import the server class)

great, just pushed those changes! Added a onLoadPackageDefinition hook to the GrpcOptions class and an example of usage to the grpc sample

[kamilmysliwiec]

LGTM

[jtimmons]

@kamilmysliwiec a little late on this but finally put a PR up to document this new field's usage in nestjs/docs.nestjs.com#3027