-
Notifications
You must be signed in to change notification settings - Fork 155
Conversation
Ben Demaree seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Hey @bendemaree were you able to get it to work to send span/trace to agent running in docker all-in-one image running on a remote host. I am trying to get it to work but so far no luck. |
@bendemaree do you mind adding a unit test? |
Update: So I was able to get traces sent to remote agent, when agent was running in a VM ( via virtualbox) on same mac where client was. When Jaeger agent was on a different VM running elsewhere, I didn't get any traces and tcpdump kept showing UDP error, it could have been because of the our corp network, will debug it more to see if I can consistently reproduce the issue in a different network setup |
@bharat-p fyi, the main UDP sender was really not meant to be used over the network, the main assumption was that the agent would run on the same host and the UDP connection will be to 127.0.0.1 If you cannot make such deployment work, I think a better option would be to implement an HTTP sender. In fact, in that case you won't even need to run the agent, because we're adding HTTP endpoints to the collectors directly. The Java and Go clients already have HTTP-based reporters. |
@yurishkuro I'm happy to add a test, yes. Should I hold off? Like @bharat-p I had difficulties getting requests to actually hit the agent, even after the updated host. The localhost-only strategy is an impediment for me locally and in a potential deployment; I'd hope to have the Jaeger stack pulled in for local development with docker-compose and in a potential deployment, as a sidecar to a Kubernetes deployment (with N homogenous pods on a node talking to a Jaeger agent on that node). How much effort do you think adding an HTTP sender is? I can attempt adding that but I could use some high-level implementation guidance. |
@bendemaree Given that using another host doesn't work too well, I think it makes sense not to merge this at all. Could you explain though why you cannot run the agent as a side car on the same bare metal host? E.g. NewRelic is using the same mechanism as Jaeger: https://github.com/kubernetes/kubernetes/tree/master/examples/newrelic |
@yurishkuro Whoops, I messed up there; I see now that I agree; if this enables broken configuration it should just be closed. ISTM the issue for folks here (myself included) is running the agent as part of a local Docker stack in which |
Technically there are many options of getting spans out of process, e.g. gRPC or Kafka, but we want to balance it against adding more dependencies to the library. An HTTP sender can be implemented with the dependencies we already have, and it works better than UDP when the agent on the same host is not available (e.g. we have a similar use case when running some services in the cloud). |
@bendemaree / @yurishkuro Would you mind explaining how it works in K8S world ( I am not too familiar with it). How does a service running in K8S as container can reach agent ( which probably is running in another container) via |
@bharat-p you may want to take a look at https://github.com/jaegertracing/jaeger-kubernetes, and also open a ticket there if you have questions like ^^ (I'm not sure I know the correct answer as we're not running Jaeger on K8S internally). |
I'm syncing on this, k8s templates require you to configure the address to point to the agent/(or collector) as the agent/collector does not run on localhost. In production k8s/openshift templates we decided to run agent as https://kubernetes.io/docs/concepts/workloads/controllers/daemonset/ so all pods on the same node will send spans to the same agent. The agent address is not a cc @jpkrohling. |
Thanks @pavolloffay , so I guess this PR is still relevant as without it we won't be able to configure client to send trace info to agent address like |
@bharat-p +1. Can you add a unit test for this property? Then we can merge. |
Original PR was opened by @bendemaree , he will have to add UT for new config. If he's ok, I can open a new PR with his change + UT. |
oh, sorry - yeah, either way. |
I'll get a test up today! |
Any progress on that? |
@bendemaree if you are tied up with other things, let me know and I will open a new PR with UT to get this merged in. Thanks. |
Opened a new PR #51 with @bendemaree 's changes alsong with UT to test new config. |
Closing, done in #51 |
This allows setting the Jaeger agent host to something other than
localhost
in config. This scenario is also noted in #47 and this implementation follows the notes given there.Fixes #47