Copy jaegerTranslator and change it to use jaeger-idl mode

[Nabil-Salah]

Which problem is this PR solving?

• part of Migrate proto-gen/thrift-gen to jaeger-idl #6494

Description of the changes

• added jaeger translator internal/jptrace/translator

How was this change tested?

• go test ./internal/jptrace/translator/

Checklist

• I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
• I have signed all commits
• I have added unit tests for the new functionality
• I have run lint and test steps successfully
  • for jaeger: make lint test
  • for jaeger-ui: npm run lint and npm run test

[yurishkuro]

ah, crap

error: at least one *_test.go file must be in all directories with go files so that they are counted for code coverage
       if no tests are possible for a package (e.g. it only defines types), create empty_test.go

don't think this can be merged on its own, need to make full migration to jaeger-idl as part of the same PR.

[codecov]

Codecov Report

Attention: Patch coverage is 86.94158% with 266 lines in your changes missing coverage. Please review.

Project coverage is 93.74%. Comparing base (3cfff2d) to head (f88b9d6).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
...internal/receiver/jaegerreceiver/trace_receiver.go	61.01%	67 Missing and 25 partials ⚠️
...ernal/receiver/jaegerreceiver/testutil/testutil.go	60.86%	34 Missing and 2 partials ⚠️
...al/jptrace/translator/goldendataset/metrics_gen.go	81.11%	27 Missing ⚠️
...ternal/jptrace/translator/jaegerproto_to_traces.go	92.77%	16 Missing and 8 partials ⚠️
...ernal/jptrace/translator/jaegerthrift_to_traces.go	84.13%	14 Missing and 9 partials ⚠️
...trace/translator/goldendataset/traces_generator.go	75.00%	14 Missing and 6 partials ⚠️
...jptrace/translator/goldendataset/span_generator.go	96.35%	8 Missing and 5 partials ⚠️
.../jaeger/internal/receiver/jaegerreceiver/config.go	86.30%	7 Missing and 3 partials ⚠️
...trace/translator/goldendataset/pict_metrics_gen.go	93.05%	4 Missing and 1 partial ⚠️
...ranslator/tracetranslator/protospan_translation.go	0.00%	5 Missing ⚠️
... and 4 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6595      +/-   ##
==========================================
- Coverage   96.23%   93.74%   -2.50%     
==========================================
  Files         377      393      +16     
  Lines       21421    23444    +2023     
==========================================
+ Hits        20614    21977    +1363     
- Misses        614     1213     +599     
- Partials      193      254      +61     

Flag	Coverage Δ
badger_v1	12.21% <51.61%> (+1.58%)	⬆️
badger_v2	5.32% <52.21%> (+2.53%)	⬆️
cassandra-4.x-v1-manual	17.28% <51.61%> (+0.63%)	⬆️
cassandra-4.x-v2-auto	?
cassandra-4.x-v2-manual	?
cassandra-5.x-v1-manual	17.28% <51.61%> (+0.63%)	⬆️
cassandra-5.x-v2-auto	?
cassandra-5.x-v2-manual	?
elasticsearch-6.x-v1	20.95% <51.61%> (+0.52%)	⬆️
elasticsearch-7.x-v1	21.02% <51.61%> (+0.50%)	⬆️
elasticsearch-8.x-v1	21.17% <51.61%> (+0.50%)	⬆️
elasticsearch-8.x-v2	5.32% <52.21%> (+2.54%)	⬆️
grpc_v1	13.49% <51.61%> (+1.29%)	⬆️
grpc_v2	10.66% <52.21%> (+1.61%)	⬆️
kafka-3.x-v1	11.60% <39.78%> (+1.24%)	⬆️
kafka-3.x-v2	5.32% <52.21%> (+2.53%)	⬆️
memory_v2	5.32% <52.21%> (+2.53%)	⬆️
opensearch-1.x-v1	21.06% <51.61%> (+0.51%)	⬆️
opensearch-2.x-v1	21.06% <51.61%> (+0.51%)	⬆️
opensearch-2.x-v2	5.32% <52.21%> (+2.54%)	⬆️
tailsampling-processor	0.41% <0.00%> (-0.10%)	⬇️
unittests	92.66% <86.59%> (-2.43%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Nabil-Salah]

@yurishkuro
after i tried the full-migration it didn't work because OTEL jaegerreceiver uses jaeger/proto-gen/api_v2 which now uses jaeger-idl

[yurishkuro]

can we copy jaegerreceiver into cmd/jaeger/internal/receiver/jaegerreceiver?

Basically we need to break the circular dependency loop, I don't see another way other than copying. We will probably also need to copy kafka receiver / producer.

Zipkin receiver should be upgraded in OTEL upstream to de-couple. Worse case copy it too.

[yurishkuro]

Suggestion - if you have a proof of concept that copying breaks the dependency hell, then I would suggest putting a separate PR with the copy of those modules as is, and then a separate PR (this one) with swapping the models across the code base.

[Nabil-Salah]

okay i'll try copying with keeping module swap and look if it compiles and runs well and give you screenshots will use make lint test this can give proof of concept

and after that if it works I'll make the second pr and then come back here as you stated

[Nabil-Salah]

@yurishkuro
it looks promising it compiles well and tests work

make lint

make test

I only copied jaegerreceiver

I think I can start now with a copy of those modules

[yurishkuro]

ok, but why is CI all broken? Do the binaries actually run, or are we getting that duplicate registration error?

[yurishkuro]

maybe you didn't push, but I would expect to see a change in cmd/jaeger/internal/components where we swap OTEL components for the local copies.

[Nabil-Salah]

yes this what happened
i changed cmd/jaeger/internal/components yes

for ci it test will fail because of duplicate registration error but it fixes when I manually update model.pb.go but I didn't push the edit I think it has to be fixed from the .proto file

[Nabil-Salah]

@yurishkuro
Do you think I should continue with making another pr for the OTEL modules?
to copy jaegerreceiver and translator

[yurishkuro]

Do you think I should continue with making another pr for the OTEL modules?

I think I prefer to continue with this PR first by adding more copied modules until we get to a green build.

I don't think you should be changing model.pb.go. However, there is a possible trick we could use if we get stuck - to delete the generated code and replace it with aliases to the corresponding jaeger-idl types. That's a completely alternative approach that we can try, it may not even require copying OTEL modules, since it will indirectly repoint everything.

[yurishkuro]

That's a completely alternative approach that we can try, it may not even require copying OTEL modules, since it will indirectly repoint everything.

Maybe it's worth giving it a try in another PR before you spend more time on copying and renaming. Aliases could be a much easier path.

[yurishkuro]

Do you mean try making aliase from jaeger models to jaeger-idl models in all jaeger model files without any import replacement?

Yes. Basically hollow-out model.pb.go and proto-gen/api_v2/*.go to only contain aliases to types / interfaces (also delete extensions to model, since we moved them to idl), and see if everything else just works.

[Nabil-Salah]

okay will give it a try