grpc / grpc-java

The Java gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/java/
Apache License 2.0
11.39k stars 3.83k forks source link

Create @GrpcClass annotation processor hook for generated source files. #3173

Open daicoden opened 7 years ago

daicoden commented 7 years ago

To write an annotation processor that further augments the generated source from the grpc protoc plugin is a little difficult.

Currently, I process @Generated annotations and search for value ~= /gRpc/.

As an external example, looking at the new grpc-dagger in 2.11 (google/dagger#647), the additional dagger code generation is triggered off of an annotation applied to the implementing class. It would be nice to have all the code generation based off of the gprc classes. Right now it feels like a two step process. Add the proto, generate the grpc stub, implement the stub, add the processor annotation, compile again for the additional code.

WDYT?

carl-mastrangelo commented 7 years ago

I'm not familiar with annotation processors, what would adding a special annotation do that would ease this process?

Also, as an alternative, why not generate your own stubs (as a protoc plugin) if you want to extend them?

daicoden commented 7 years ago

I'm trying to use the grpc generated output so I can use other libraries which are based off of that, dagger-grpc is the only external use case I have right now... but I've written some additional generation that might be useful to others. Do you think most users will write their own protoc plugin for stub generation?

For the annotation processor, I'm not an expert either, but you basically create an annotation which tags elements, the processor can retrieve those elements, and the processor generates additional code. I can find the classes, for my own code generation, by scanning and name matching. However, if you envision annotation processing as a way to extend grpc functionality, it would be nice to have some hook for annotation processors to avoid the two cycle compile dance described above. The scanning and name matching strategy seems to be frowned upon, and I think technically the annotation processor plugin is supposed to provide its own marker annotation.

Other ideas:

carl-mastrangelo commented 7 years ago

two cycle compile dance described above.

I don't understand, why are there two compilation steps?

Add the proto, generate the grpc stub, implement the stub, add the processor annotation, compile again for the additional code.

Why do you have to implement the stub before processing annotations?

daicoden commented 7 years ago

I don't understand, why are there two compilation steps?

This comment is specifically for grpc-dagger, and I believe they do the multi-step because there's no way to get the grpc classes without the name scanning. I think additional annotation processors will follow this pattern too without a good way to get a handle on the grpc generated class.

To clarify, it's two compilation steps when adding new services. The steps for the grpc-dagger are:

1) add service to proto 2) compile 3) create class that implements ServiceImplBase 4) add @GrpcService(grpcClass = GrpcGeneratedClass.class) to the service implementation (this is the marker interface for the annotation processor). 5) compile again.

Full example of implementation.

...
@GrpcService(grpcClass = HealthCheckServiceGrpc.class) // Marker annotation for grpc-dagger processor
public class HealthCheckService extends HealthCheckServiceGrpc.HealthCheckServiceImplBase {
    @Inject public HealthCheckService(Set<HealthCheck> healthChecks) {
      this.healthChecks = healthChecks;
    }

    @Override
    public void getHealth(GetHealthRequest request, StreamObserver<GetHealthResponse> responseObserver) {
        responseObserver.onNext(GetHealthRequest.newBuilder().setSuccess(healthChecks.stream().allMatch(HealthCheck::isUp)).build());
        responseObserver.onCompleted();
    }
}

You could add in the packages manually before the first compile step, but that's not really going to fly as we rely on the IDE a lot.

I believe that the code generation in this example could be based on the HealthCheckServiceGrpc, without the implementation class needed.

carl-mastrangelo commented 7 years ago

It is not clear to me that this saves the second compilation step. Of the 5 steps you listed, none of them can be removed if there was a marker annotation on the *Grpc classes.

For your example code, I don't see how the annotation helps. You can tell the wrapper HealthCheckServiceGrpc class from the base class you extend.

And for that matter HealthCheckServiceGrpc is just a collection of other classes and method, it doesn't really have any functionality itself.

It doesn't sound like the solution you propose actually solves the problem you raise.

daicoden commented 7 years ago

Ah, sorry. For my use case, none of the code generated needs to know about the implementing class, and I suspect most extensions won't need the implementing class either. In the case of grpc-dagger it does use the implementing class, but none of the code it generates really needs to know about it except for 1 module, and if it were me, I would probably exclude that from the code generation and make that part of the implementation API...

I really appreciate your time. I'm often guilty for proposing a solution, so sorry about that. So just backing up a step, here's my problem:

I would like to wire up RPC dispatch differently than the current generated stub in a way that will be compatible with future API changes to GRPC. My preference would be to do this with an annotation generator.

And for that matter HealthCheckServiceGrpc is just a collection of other classes and method, it doesn't really have any functionality itself.

That's the crux of the issue! It has a bunch of metadata about how to wire up the service, and a wiring implementation. I want to get at the metadata in an annotation processor to generate a different wiring implementation. I hit a wall with annotation processors, and was modifying the c++ generator, but when I saw the grpc-dagger annotation processor, I opened this issue.

If possible, I'd like to generate custom dispatch code in an annotation processor. Even if I follow the grpc-dagger pattern, I still can't get access to the MethodType. With #2446 I'm unblocked, but thought it could be cleaned up with a code generation api. Something like this would be my dream:

...
@RpcMethod(
  requestType = com.example.HelloWorldRequest,
  responseType = com.example.HelloWorldResponse,
  methodType = io.grpc.MethodDescriptor.MethodType.UNARY
  name = generateFullMethodName("example.ExampleService", "helloWorld")
)
// Don't care any more about the method descriptor details, the annotation generator would be based purely off of @RpcMethod
public static final io.grpc.MethodDescriptor<com.example.HelloWorldRequest, comExample.HelloWorldResponse> METHOD_HELLO_WORLD =
      io.grpc.MethodDescriptor.<example.protos.Request, example.protos.Response>newBuilder()
          .setType(io.grpc.MethodDescriptor.MethodType.UNARY)
          .setFullMethodName(generateFullMethodName(
              "example.ExampleService", "helloWorld"))
          .setRequestMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
              com.example.HelloWorldRequest.getDefaultInstance()))
          .setResponseMarshaller(io.grpc.protobuf.ProtoUtils.marshaller(
              com.example.HelloWorldResponse.getDefaultInstance()))
          .build();

If that doesn't seem like a good idea, then is there a way to extend the c++ plugin? I copy pasted it, and started making tweaks, but I want to get changes - like when the protobuf descriptor was added to the grpc method descriptor.

cc @jhump @lukaszx0 who might have perspective on this request.

If the answer is to write my own c++ plugin, we can close this issue.

ejona86 commented 6 years ago

This was partially addressed in #4609. The RpcService still doesn't have an annotation, but it could follow in the same vein.

MostafaAmer-2 commented 3 years ago

@daicoden What plugin are you using to allow dagger to create the auto generated files upon compilation? I've referenced my issue on the Dagger project https://github.com/google/dagger/issues/2157. Your help is greatly appreciated.