Skip to content
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

[featureflag] - use full demo protos #271

Merged
merged 6 commits into from
Aug 15, 2022

Conversation

puckpuck
Copy link
Contributor

@puckpuck puckpuck commented Aug 9, 2022

Unblocks #245

Changes

Uses a full copy of the demo.proto to build corresponding gRPC modules. Previously we were using a small subset of demo.proto without the package hipstershop declaration. There are several new files in this PR because we are now building the Erlang behavior modules for all the services part of the protobuf definition. This can be addressed in another PR to clean up the docker build process for this service.

For significant contributions please make sure you have completed the following items:

Does this PR require a changelog update?

  • Appropriate CHANGELOG.md updated for non-trivial changes

Copy link
Member

@tsloughter tsloughter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@puckpuck thanks! This looks good. All the extra modules is why I was waiting to do this -- and the fact the ffs_demo_pb module is now huge. But looking at this PR it seems that new module is only 7k lines and the old 2k, so not bad and not an issue.

I would suggest using a symlink to link to the top level demo.proto if possible -- I think Windows supports those now? -- otherwise I'll focus on making a change to the grpc server generator to support reading files from parent directories.

@puckpuck
Copy link
Contributor Author

I created #279 to track updating Dockerfile for this service

Copy link
Member

@austinlparker austinlparker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@cartersocha cartersocha merged commit e4cd8b1 into open-telemetry:main Aug 15, 2022
@puckpuck puckpuck deleted the ffs-fix-protos branch August 16, 2022 02:25
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants