Remove existing OpenTelemetry Collector code #2828
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2828 +/- ##
==========================================
- Coverage 95.94% 95.93% -0.02%
==========================================
Files 221 221
Lines 9696 9689 -7
==========================================
- Hits 9303 9295 -8
- Misses 325 326 +1
Partials 68 68
Continue to review full report at Codecov.
|
|
Are these components going to be migrated to https://github.com/jaegertracing/jaeger-otelcol? The main reason for having the code here was for the ease of development. Many times changes in the core Jaeger components were required. |
|
There were a number of places that these "AddOtelFlags" methods were added: https://github.com/jaegertracing/jaeger/blob/master/cmd/ingester/app/flags.go#L92 Should we clean those up as well? |
|
From what I understand, we'll first adapt this repository here to have multiple go modules, and then move the contents of jaeger-otelcol to this repo here. The goal is to avoid the circular dependency between otelcol and Jaeger, while still being able to have the binaries being built as part of this single repository. @joe-elliott is working on the reorganization, and @kevinearls will work on moving things here. Some of the components removed by this PR will get a new life after this reorganization. |
|
For context, the decision to remove this here right now also has to do with #2781, as we can't bump Thrift to 0.14 in the otelcol modules as it depends on a Zipkin Thrift repository that is still on 0.13. Unfortunately, Thrift 0.14 is incompatible with 0.13, as it adds |
Done! |
The same problem might pop up after in OTEL collector as well. Better would be to bump zipkin to thrift 0.14 as well. |
Indeed, it will affect that as well. |
There was a problem hiding this comment.
Apologies, found a few more otel references that should be cleaned up after approving:
https://github.com/jaegertracing/jaeger/blob/master/.gitignore#L37
https://github.com/jaegertracing/jaeger/blob/master/Makefile#L3
https://github.com/jaegertracing/jaeger/blob/master/Makefile#L177
https://github.com/jaegertracing/jaeger/blob/master/scripts/upload-all-docker-images.sh#L33
https://github.com/jaegertracing/jaeger/blob/master/scripts/upload-to-registry.sh#L84
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
mergify
bot
dismissed
joe-elliott’s stale review
Pull request has been modified.
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
| } | ||
|
|
||
| // AddOTELFlags adds flags that are exposed by OTEL collector | ||
| func AddOTELFlags(flags *flag.FlagSet) { | ||
| flags.String(collectorHostPort, "", "Comma-separated string representing host:port of a static list of collectors to connect to directly") |
There was a problem hiding this comment.
I wonder why these were considered specific to OTEL in the first place
There was a problem hiding this comment.
Not all flags were added to OTEL distros in other words some flags made sense only for legacy Jaeger.
|
I'm merging with two approvals, any left overs can be removed in a follow-up PR. |
The current OpenTelemetry code isn't representative anymore of where we want to go for Jaeger v2. In order to reduce confusion to users, we discussed during the last weekly meeting about removing this code. Exporters that were created for this are still available in earlier tags and can be recovered from there when needed.
Signed-off-by: Juraci Paixão Kröhling [email protected]