Lack of support for protobuf imports from other services

[tpiecora]

This issue seems to be related to, or possibly the same/similar as #201, though I did not have quite the same experience.

When trying to run truss on a protobuf file that imports a protobuf file from another service (which is on my $GOPATH) it only seems to be looking in the PWD:
cannot parse input: cannot find package "." in: /Users/.../go/src/.../.../my-service

I tried various things, even adding additional proto_path variables to the protoc function in truss. I can successfully use the protoc CLI myself and by adding the PWD and $GOPATH/src paths to proto_path, it is able to generate the pb.go file.

After digging into truss a bit, it doesn't appear to me that the way truss parses the pb.go file and generates the handlers that it can actually reference the imported protobuf file's messages in the code generation. With a bit of tinkering I was able to get it to output the handlers.go file but anywhere one of the function arguments was a struct generated from an imported protobuf message truss seemed unable to find the type, though it was able to provide a name for the argument.

Not having this functionality is a significant gap as it is somewhat expected that in a microservice architecture, services would be making RPC calls to each other. To work around this, anywhere that we want to use a message from another service we have to replicate the definition of that message in both services and now maintain two copies of the message, which is an unsustainable pattern.

Just for reference, per #201, we are generating our pb.go file alongside the proto file.
The imports are relative to $GOPATH/src.
They reside in different directories.
The go package (pb.go) for each imported proto file is also already generated when we create that service.

Any thoughts on how to proceed to implement this functionality, or has any progress been made on this issue?

[zaquestion]

replicate the definition of that message in both services and now maintain two copies of the message, which is an unsustainable pattern

This is precisely what we have been doing. Though we didn't have very many cases where this came up. There were some cases where it would been ideal to use the same enum between services and that even get particularly nasty when you need to convert between them.

We definitely would like to fix this issue, I believe @adamryman has a pretty clear idea of what needs to be done.

[zaquestion]

Getting this PR finished and merged is the first step. #239

[tpiecora]

If we can get a line of sight on what needs to be happen, we can help get it done.

Definitely not a fan of having to maintain multiple copies of message definitions. Let's get this solved.
@eriktate

[adamryman]

@tpiecora I am excited that someone wants the feature so much.

I will give a detailed response in the next 24 hours. I am a little under the weather, though I wanted to put a comment in here to let people know that I have been notified, and I will respond soon.

[adamryman]

Hi @tpiecora

I have created proposal #242 which will get truss another step closer to solving this issue.

Before more proposals are created, lets make sure we get a complete view of what you want.

Could you provide:

• 3 .proto files
  • 1 .proto file that contains a message definition you would like to be shared. (shared.proto)
  • 2 .proto files that each contains a service definition that also:
    • imports the .protofile that contains a shared message
    • uses that shared message in an rpc definition

Ideally, please provide these files in a sample repo, though a comment will do as well.

[tpiecora]

That is almost what I want.
If truss supported what you suggested, I would probably just try to make it work. Ideally, I would just have two proto files, one for each service. That way I can import the proto file from one service to use the messages in it. I'll try to provide as close to a real-world example as possible.

We have conversation-service and a msg-service, and the conversation service exposes an API that allows the sending of messages within the context of a specific conversation. The conversation service itself does not actually send any messages, but proxies those requests through to the msg service. So the conversation service would use the message definitions of the msg service because it can receive and potentially return them. The message service has no API that is consumed by anything but the conversation service.

conversation.proto:

syntax = "proto3";

package conversation;

import "github.com/myproject/another-service/msg.proto";

service Conversation {
...
    rpc SendMessage(MsgConvID) returns () {}
    rpc GetMessages(ConversationID) returns (Msgs) {}
...
}

message Conversation {
    string id = 1;
    repeated string users = 2;
    string status = 3;
    repeated Msg messages = 4;
}

message ConversationID {
    string id = 1;
}

message MsgConvID {
    string id = 1;
    msg.Msg message = 2;
}

message Msgs {
    repeated msg.Msg messages = 1;
}

msg.proto

syntax = "proto3";

package msg;

service Message {
  rpc CreateMessage(Msg) returns (MsgID) {}
}

message Msg {
  string id = 1;
  string conversation_id = 2;
  bytes body = 3;
  repeated MetaData meta_data = 4;
}

message MsgID {
  string id = 1;
}

message MetaData {
  string key = 1;
  string value = 2;
}

[tpiecora]

@adamryman Just wanted to follow up on this. Do you have what you need from me? I reviewed proposal #242. Is this all the work that is required to accomplish what is necessary to use the imported files?

[adamryman]

@tpiecora #242 will be needed in order to generalize this functionality. Though it is not the only step needed.

A few questions about your example.

• In msg.proto could you define MessageModel?
• In conversation.proto could you define ConversationID?

We are planning on using this example as our test case, to insure that this issue is resolved.

Features we will need to implement for this functionality:

• Read .proto imports recursively
  • Generate their respective .pb.go files using protoc-gen-go.
    • Use a map of filepaths to make sure we don't run protoc-gen-go more than we need to.
• svcdef parses and stores golang imports
• svcdef parses and stores imported input and output types to RPCs
• All places in gengokit where the .pb.go "pb" packages is imported will need to be updated to import all types that the service needs. In your example, the conversation service would need imports to both the conversation types and the msg types.

These step will become actual issues.

Thanks for your continued interest!

[tpiecora]

My apologies, I see when I was trying to clean up the examples I left out a couple things. I made some edits. Let me know if you need anything else for that.

[eriktate]

I created a proposal issue with some more detail around a potential use case for this kind of functionality: #245

[TyeMcQueen]

Our work around was to create a repo just for protobuf definitions and the code generated from them and then use that code from other repos where the business logic for each service is kept. This avoids any duplication of code. But it means the protobuf def'ns are not directly in the repo for the service that uses them.

It also means that some of the annoying hoop jumping so that dependencies get found is isolated to the one protobuf repo.

And, yes, this pattern is not useful when the protobuf def'ns are coming from unrelated teams.