opentelemetry-demo icon indicating copy to clipboard operation
opentelemetry-demo copied to clipboard

Building protobuf assets

Open fsolleza opened this issue 6 months ago • 2 comments

The protobuf assets for some services are in the src directory for that service (e.g., src/accountingservice/genproto) . Applying a change in pb/demo.proto requires copying this file or using protoc to generate the required files in each src/Xservice directory before invoking docker compose build... or similar (I think). By contrast, the other services' Dockerfiles handle the copy and build of protobuf assets.

The following services (all golang) need to compile pb/demo.proto using protoc into their respective directories before docker compose build...:

  1. accountingservice
  2. checkoutservice
  3. productcatalogservice

The following service (c++) requires copying pb/demo.proto to src/currencyservice/proto/demo.proto

  1. currencyservice

Is there a reason for this? I have a fork that fixes this behavior but thought I would check first.

Use Github Discussions.

fsolleza avatar Feb 09 '24 13:02 fsolleza

We have a make target that will generate the protobuf assets or copy the demo.proto file for all services as required. Each language has their own way on how to do this, so instead of needing to know all of them, we created a single script that does it for all fo them.

make generate-protobuf will set up protobuf assets for all services. If you don't have the proper tooling (ie: protoc) it will gracefully move to the next service attempting each one.

We took this approach to allow people to make simple code changes, and then do a docker compose build ... without needing to generate the protobuf assets first.

If you feel like there is a better way, and you developed one in your fork, I would be interested to learn more.

puckpuck avatar Feb 09 '24 14:02 puckpuck

Thanks! Here's the scenario I was dealing with that led to my changes

  1. Make change to pb/demo.proto
  2. make start and realize that some things break
  3. make generate-protobuf and realize that I need tooling for the different builds: copy into directory for many services, protoc + plugins for golang, npm, etc.
  4. install all of them and build successfully

My setup makes changes to the Dockerfiles of the 4 services noted above so that a change in pb/demo.proto doesnt require all the different builds. Of course, this means that a new service for example, will need to build protobuf assets in their new Dockerfile, but at least they won't need to build all assets manually. This also doesn't take into account the fact that if a service uses a changed definition in pb/demo.proto, then that service would need some code updates.

Further, it makes the deployment a bit more consistent. At the moment, with a change in pb/demo.proto, the four services above need make generate-protobuf while others do not (along with all the tooling necessary).

This doesn't replace make generate-protobuf. It makes the make start and friends friendlier to small changes to pb/demo.proto.

The changes in my fork can be found here. Let me know if these are sensible changes.

https://github.com/open-telemetry/opentelemetry-demo/commit/dfaf53fe2f99bd8287fff6dc193a739e73766caf

fsolleza avatar Feb 09 '24 14:02 fsolleza

I think you should submit a PR from that fork :D

We will likely need some new entries in .gitignore so that people who run make generate-protobuf don't have any changes that need to be committed.

puckpuck avatar Feb 14 '24 01:02 puckpuck

this was fixed with #1386

puckpuck avatar Mar 20 '24 19:03 puckpuck