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

Bump the opentracing-proto submodule to fix go bindings #90

Closed
gordallott opened this issue Feb 9, 2022 · 3 comments · Fixed by #91 or jaegertracing/jaeger#3525
Closed

Bump the opentracing-proto submodule to fix go bindings #90

gordallott opened this issue Feb 9, 2022 · 3 comments · Fixed by #91 or jaegertracing/jaeger#3525
Labels

Comments

@gordallott
Copy link

gordallott commented Feb 9, 2022

Describe the bug
The go bindings have been broken for a while because of upstream changes in open-telemetry/opentracing - they recently merged a PR that will fix golang bindings built from their .proto files

To Reproduce
Steps to reproduce the behavior:

  1. make proto
  2. oh no the golang proto files don't build

Expected behavior
The proto files to build

Screenshots
nope

Version (please complete the following information):

  • OS: linux
  • Jaeger version:
  • Deployment:

What troubleshooting steps did you try?

Additional context
the cause was the upstream proto files depending on generated go that didn't exist anywhere publicly, they now import the correct repository

The opentracing-proto submodule needs to be bumped to (at least) open-telemetry/opentelemetry-proto@0e254b5

I avoided just making a PR to do this because I'm unsure of any other changes that might affect this repository

@gordallott gordallott added the bug label Feb 9, 2022
@yurishkuro
Copy link
Member

Make proto works for me. You need to init/update submodule in your local repo. If you still can't build, please post the errors.

@gordallott
Copy link
Author

make proto works, it builds the files - the problem is it generates incorrect go code as detailed above. specifically the imports are wrong because of a mistake on part of open-telemetry/opentelemetry-proto

@yurishkuro
Copy link
Member

cc @albertteoh

I see that in the main Jaeger repo we have ./otel_proto_patch.sed that rewrites some of the parts in the file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants