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

version command prints jaeger errors #7931

Closed
06kellyjac opened this issue Feb 15, 2022 · 18 comments · Fixed by #16573
Closed

version command prints jaeger errors #7931

06kellyjac opened this issue Feb 15, 2022 · 18 comments · Fixed by #16573
Assignees
Labels
bug Something isn't working exporter/coralogix help wanted Extra attention is needed

Comments

@06kellyjac
Copy link

Describe the bug

version command prints jaeger errors

./otelcontribcol --version
2022/02/15 09:48:33 proto: duplicate proto type registered: jaeger.api_v2.PostSpansRequest
2022/02/15 09:48:33 proto: duplicate proto type registered: jaeger.api_v2.PostSpansResponse
otelcontribcol version latest

Steps to reproduce

build v0.44.0 and run --version

What did you expect to see?

no warnings

@06kellyjac 06kellyjac added the bug Something isn't working label Feb 15, 2022
@jpkrohling jpkrohling self-assigned this Feb 15, 2022
@jpkrohling
Copy link
Member

This is caused by both the Jaeger receiver and the Coralogix exporter registering the same types:

This is a transitive dependency of the coralogix exporter:
https://github.com/coralogix/opentelemetry-cx-protobuf-api/blob/d2a5d0ecf53ef7d38129bc756009169c4c32167f/collector.pb.go#L182-L186

@oded-dd @ofirshmuel, any reason you are not using your own namespaces for that?

@ofirshmuel
Copy link
Contributor

Hi @jpkrohling , we are investigating this issue.

@mx-psi
Copy link
Member

mx-psi commented Jul 15, 2022

@ofirshmuel I think this is still happening, do you have any updates?

@frzifus
Copy link
Member

frzifus commented Jul 27, 2022

seems to be still a thing in dc19b91. @ofirshmuel are you going to continue your pr? otherwise i would like to try.

@jpkrohling jpkrohling added the comp:jaeger Jaeger related issues label Jul 29, 2022
@jpkrohling jpkrohling assigned frzifus and unassigned jpkrohling Jul 29, 2022
@frzifus
Copy link
Member

frzifus commented Jul 30, 2022

This is caused by both the Jaeger receiver and the Coralogix exporter registering the same types

I am surprised that it this exporter workes. 🙅. There are two definitions of jaeger.api_v2.PostSpansRequest, jaeger and coralogix. For me it looks like carologix is adding the message Metadata it the request.

+ message Metadata {
+     string ApplicationName = 1;
+     string SubsystemName = 2;
+ }

message PostSpansRequest {
    Batch batch = 1 [
        (gogoproto.nullable) = false
    ];
+     Metadata metadata = 2;
}

As far as I understand, this leads to a few problems:

  1. it is not clear which of the two messages is registered first. Seems to be decided at each program start. As a result we can not make sure that Metadata will be a part of PostSpansRequest.
  2. If the jaeger would get extended in the future this could cause issues there too.
  3. Maybe in the future the type registration will follow a similar approach as other register methods and will end with a panic in case of a double registration.

Any recommendations how to solve this?

cc @oded-dd @ofirshmuel @jpkrohling

@github-actions github-actions bot added the help wanted Extra attention is needed label Jul 30, 2022
@oded-dd
Copy link
Contributor

oded-dd commented Jul 31, 2022

@frzifus thank you for your help. Adding @povilasv to the thread. He'll probably also attend it soon, and could give you all the insights you need.

@frzifus
Copy link
Member

frzifus commented Aug 31, 2022

@oded-dd @povilasv any updates? :)

@povilasv
Copy link
Contributor

@oded-dd @povilasv any updates? :)

Hey I'm currently working on this, the fix should be there soon 😃

@github-actions
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

@github-actions github-actions bot added the Stale label Nov 10, 2022
@mx-psi mx-psi removed the Stale label Nov 10, 2022
@mx-psi
Copy link
Member

mx-psi commented Nov 10, 2022

Is this still a thing @povilasv? Or did #13844 fix it?

@frzifus
Copy link
Member

frzifus commented Nov 10, 2022

@mx-psi yes its still a thing. d397c98

2022/11/10 11:29:59 proto: duplicate proto type registered: jaeger.api_v2.PostSpansRequest
2022/11/10 11:29:59 proto: duplicate proto type registered: jaeger.api_v2.PostSpansResponse

@github-actions
Copy link
Contributor

Pinging code owners: @oded-dd @ofirshmuel. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mx-psi mx-psi removed the comp:jaeger Jaeger related issues label Nov 10, 2022
@mx-psi
Copy link
Member

mx-psi commented Nov 10, 2022

Okay, I have labeled this as exporter/coralogix instead of Jaeger, AIUI the fix needs to be done in the Coralogix exporter.

@jpkrohling
Copy link
Member

I'm in favor of disabling the coralogix exporter until this is fixed. The component owners had enough time to fix this already.

@mx-psi
Copy link
Member

mx-psi commented Nov 23, 2022

Let me ping them one last time and send an email to the email on @oded-dd's bio:

@oded-dd @ofirshmuel if you don't actively fix issues from your exporter your component may get excluded from the default build, making it harder to be used. Please take a look at the issue as soon as you can.

If there is no answer, let's bring this up on the next Collector SIG

@joaorzz
Copy link

joaorzz commented Nov 30, 2022

I've been having that problem using the 0.66.0 version. Still no solution?

@mx-psi
Copy link
Member

mx-psi commented Nov 30, 2022

We got no answer here or in the email, thus I added this to today's SIG agenda. cc @oded-dd @ofirshmuel @jpkrohling

@povilasv
Copy link
Contributor

povilasv commented Dec 1, 2022

Hey! Sorry somehow missed the ping on this issue. Working on the fix in #16573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working exporter/coralogix help wanted Extra attention is needed
Projects
None yet
8 participants