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

1.16 docker: query's JAEGER_AGENT_PORT #1986

Closed
naseemkullah opened this issue Dec 19, 2019 · 14 comments
Closed

1.16 docker: query's JAEGER_AGENT_PORT #1986

naseemkullah opened this issue Dec 19, 2019 · 14 comments

Comments

@naseemkullah
Copy link
Member

Requirement - what kind of business use case are you trying to solve?

Query outputs:
"error":"cannot obtain reporter config from env: cannot parse env var JAEGER_AGENT_PORT=udp://10.90.32.41:5775

Problem - what in Jaeger blocks you from solving the requirement?

on a docker environment, or Kubernetes
there's an unfortunate clash in the names: Kubernetes/Docker populates some env vars automatically, such as JAEGER_AGENT_PORT, and the tracer client in the jaeger-query is now configure automatically via env vars, one of which is JAEGER_AGENT_PORT, which expects an integer with the agent's port

Proposal - what do you suggest to solve the problem or improve the existing situation?

a workaround is to explicitly set JAEGER_AGENT_PORT to 6831

@ghost ghost added the needs-triage label Dec 19, 2019
@jpkrohling
Copy link
Contributor

jpkrohling commented Dec 20, 2019

Note that this happens only when the Jaeger instance is called jaeger, which is arguably a very common name for the Jaeger instance:

One possible solution is to ignore the env var value if the value is not an integer, without breaking the agent because of it.

Question: do we want to address this in the Jaeger Go Client, or in the Jaeger Query, which consumes the Jaeger Client? I'd vote for the Jaeger Go Client, as it would prevent the same issue from happening in other projects (like the case with Loki linked above).

@naseemkullah
Copy link
Member Author

naseemkullah commented Dec 20, 2019

Note that this happens only when the Jaeger instance is called jaeger, which is arguably a very common name for the Jaeger instance:

One possible solution is to ignore the env var value if the value is not an integer, without breaking the agent because of it.

Question: do we want to address this in the Jaeger Go Client, or in the Jaeger Query, which consumes the Jaeger Client? I'd vote for the Jaeger Go Client, as it would prevent the same issue from happening in other projects (like the case with Loki linked above).

Sounds good, will query eventually use opentelemetry client? If so I imagine the fix would be propagated.

@jpkrohling
Copy link
Contributor

will query eventually use opentelemetry client?

It might, but we haven't talked about it yet.

@seilorjunior
Copy link

Do you have any news about that?

@jpkrohling
Copy link
Contributor

@JuniorCtba are you facing this problem with a specific client (Java, Go)? If so, a workaround is to explicitly set the env var for the process, as mentioned in the issue description.

But to answer your question: we haven't moved yet but your contribution would be most welcome! For the Go client, here's the relevant code:

https://github.com/jaegertracing/jaeger-client-go/blob/0be28c34dabf7d342aa028c3a08bcbdc2b8832cb/config/config_env.go#L192-L201

IMO, we should just skip the else part of the code I linked above.

For the Java client, it would be around here:

https://github.com/jaegertracing/jaeger-client-java/blob/2f3655cc66cf37edf418fd4ecb6c0ad8587b2c75/jaeger-core/src/main/java/io/jaegertracing/Configuration.java#L705

@seilorjunior
Copy link

Thanks @jpkrohling i am using as docker.
It works with the env.

@bboreham
Copy link

First, let me mention the other workaround of setting enableServiceLinks: false in your Kubernetes manifests.

Now I would like to nit-pick:

Note that this happens only when the Jaeger instance is called jaeger

It happens when you have a Kubernetes Service named jaeger-agent in the same namespace.

@koalalorenzo
Copy link

koalalorenzo commented Feb 8, 2021

I am running into the same issue right now. Renaming seems very much like a temporary solution. Any other ideas?
I am deploying it using the latest jaeger official helm chart in Google Cloud (GKE), and the latest prometheus chart.

@jpkrohling
Copy link
Contributor

The naming clash will happen given the "right'' conditions. I don't think we ever received a PR to simply ignore the value of the env var in case it's not a valid number. If you are open to sending this PR, we can certainly consider incorporating it in the next minor version.

That said, the workaround is to override the env var with your own value, like 6831 or set enableServiceLinks: false as @bboreham mentioned.

@koalalorenzo
Copy link

There is no option for enableServiceLinks in the Helm Chart :( https://github.com/jaegertracing/helm-charts/blob/master/charts/jaeger/values.yaml

@jpkrohling
Copy link
Contributor

You should then really open an issue in that repository, hopefully followed by a PR ;-)

@koalalorenzo
Copy link

That is a good idea! I will do so, but I opened PRs there that never got merged reviewed 😢
Will you help reviewing or will I waste time again if I open a PR?

@koalalorenzo
Copy link

@jpkrohling PR added, now we wait 🤞

@jpkrohling
Copy link
Contributor

That is a good idea! I will do so, but I opened PRs there that never got merged reviewed

This shouldn't happen. Ping me if it get stuck again, no matter the PR.

Will you help reviewing or will I waste time again if I open a PR?

Helm charts aren't my specialty, but I can bug people to review your PR :-)

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 a pull request may close this issue.

7 participants