Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Support sending spans over HTTP #98

Closed
wkiser opened this issue Nov 15, 2017 · 17 comments
Closed

Support sending spans over HTTP #98

wkiser opened this issue Nov 15, 2017 · 17 comments

Comments

@wkiser
Copy link

wkiser commented Nov 15, 2017

Similar to the HttpSender in jaeger-java. It seems like it could be as simple as using THttpClient in place of TUDPTransport and connecting to the collector directly. I'm happy to help out with the implementation if needed.

Background: I have some applications that are generating huge spans (> 65k) because they are including full SQL and ES queries as logs in the trace. Right now we are manually truncating the queries, but my clients would like to be able to view them for debugging purposes.

@ProvoK
Copy link
Contributor

ProvoK commented Apr 24, 2018

It would be nice to have this possibility. It will enable to trace in context where is not possible to have an agent installed, for example a Lambda (AWS) function.
Is this in development?

EDIT: I'll be really glad to contribute as far as possible 😀

@yurishkuro
Copy link
Member

not aware of anyone working on it. Contributions are welcome.

@ProvoK
Copy link
Contributor

ProvoK commented Apr 26, 2018

Cool, I'll give a shot.
I'm new to Jaeger and OpenTracing at all, so I'll probably take some time to get right the code.

EDIT: actually working on it (https://github.com/ProvoK/jaeger-client-python/tree/send_spans_over_http)

@ProvoK
Copy link
Contributor

ProvoK commented May 20, 2018

Here I am, sorry for the delay.

After some investigation on the code with the help of @black-adder, I started a refactor to make this change in a clean way. So it's all still a WIP, and I would like to have some feedbacks

Work done [compare]

So, there are quite a lot of changes, because things were a bit entangled.

  1. I extracted the duty of sending batches from the Reporter class, which now delegates to a injected ( via __init__ ) Sender subclass (senders.py, new module). There is actually only one real implementation of the Sender, which is UDPSender, aka sending via a local agent.

    • senders.HTTPSender will be the agent-less sender.
  2. I removed local_agent_net.LocalAgentSender.request_sampling_strategy method, because actually it were mixing responsibilities: now LocalAgentSender really does the reporting (and nothing more) whilst LocalAgentHTTP is (still) responsible for the sampling.

  3. Changed config for

    • Injecting a Sender class to Reporter (UDPSender if config.local_agent_enabled True else HTTPSender. Idk if it's correct, could be done more explicitly with a config ad hoc parameter)
    • passing to RemoteControlledSampler a really only-sampling capabilities channel. Before it was a LocalAgentSender, but it used only its sampling capabilities!
    • Deleted _create_local_agent_channel which is now senders.UDPSender job

I fixed also test accordingly (else if there is a crossdock test that fail...I don't get what's wrong) Travis

Next steps

  1. Make CI full-green
  2. Implement Sender unit tests (?)
  3. I would like to rename/refactor local_agent_net module and it's classes names.
    • LocalAgentSender -> fine
    • LocalAgentHTTP -> RemoteSampler (or just something that indicates its sampling duties)
  4. Implement senders.HTTPSender.send
  5. Add end to end crossdocks agent-less tests
  6. Add documentation for this change

@yurishkuro
Copy link
Member

yurishkuro commented May 20, 2018

@ProvoK looks like a good start. I would prefer you did not do all changes in one humongous PR, break them apart in smaller logical units, e.g. refactoring first, then renaming, then adding http sender.

Small comment on the naming (but not for the first refactoring PR):

  • I would avoid the name "LocalAgent" since senders don't have a direct dependency on it being "local" or "agent".
  • LocalAgentHTTP -> RemoteSampler - we already have remote sampler, what you're renaming is actually a client / rpc abstraction. I am not sure it's even necessary as a separate class, because all of its functionality is just three lines making an HTTP call:
    http_client = tornado.httpclient.AsyncHTTPClient(
    defaults=dict(request_timeout=timeout))
    # Properly url encode the params
    url = url_concat(
    'http://%s:%d/sampling' % (self.agent_http_host, self.agent_http_port),
    [('service', service_name)])
    return http_client.fetch(url)

@ProvoK
Copy link
Contributor

ProvoK commented May 20, 2018

@yurishkuro Sure, I was thinking about splitting it too.

So I can just remove the actual HTTPSender (which is a placeholder) and just PR with the actual refactoring? Of course I'll have to fix the crossdock test that is failing.
About that, how can I debug that error a bit more? It just show this and nothing more

EDIT:
Probably is something I did trying to "fix" end to end tests.
https://github.com/ProvoK/jaeger-client-python/blob/4bcb81da6b7d083a2149cb5c576baf3676d70c79/crossdock/server/endtoend.py#L64-L100

@yurishkuro
Copy link
Member

Yes, putting a PR with refactoring is the best way forward, we can't really review another branch (no place to leave comments).

Ideally you should not need to change in the crossdock code, because if you do it means your change is not backwards compatible.

The best way to troubleshoot the errors is to run crossdock locally (make crossdock-fresh, I believe), and then look at the logs via make crossdock-logs.

@mstump
Copy link

mstump commented Aug 2, 2018

Is this effort dead?

@yurishkuro
Copy link
Member

no - #186

@r0fls
Copy link

r0fls commented Jul 25, 2019

@yurishkuro is #208 still being considered?

@ishrivatsa
Copy link

ishrivatsa commented Feb 23, 2020

Is this still going on ? I saw that there was a commit pertaining to http support which was reverted. I want to use tracing within Serverless functions like AWS Lambda or Azure functions. I cant use UDP for this. Could you please provide more info ?

@v-yzakapko
Copy link

Hey! Is there some news about it. I need to use tracing via HTTP and it will be good to use it with OT. Maybe somebody has any idea how to implement it using the https://github.com/open-telemetry/opentelemetry-python or combining opentelemetry and opentracing?

@yurishkuro
Copy link
Member

@ptrhck
Copy link

ptrhck commented Jul 29, 2020

This would be a great feature! Is there any other Python opentracing client that can send spans over HTTP to Jaeger collector?

@CiCi503
Copy link

CiCi503 commented Sep 29, 2020

Is this still going on ? I want to use tracing within Serverless functions too, I can not tracing via UDP.

@tx19980520
Copy link

Is this still going on ? I saw that there was a commit pertaining to http support which was reverted. I want to use tracing within Serverless functions like AWS Lambda or Azure functions. I cant use UDP for this. Could you please provide more info ?

Have you found an alternative solution now?

@ptrhck
Copy link

ptrhck commented Nov 23, 2020

You can use OpenTelemetry as the client: https://opentelemetry-python.readthedocs.io/en/stable/exporter/jaeger/jaeger.html

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants