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

Elastic APM Agent distributed tracing support #517

Open
AriPaaWun opened this issue Apr 11, 2023 · 8 comments
Open

Elastic APM Agent distributed tracing support #517

AriPaaWun opened this issue Apr 11, 2023 · 8 comments

Comments

@AriPaaWun
Copy link

This is a feature request.

It would be nice if HTTP Kit client would support Elastic APM distributed tracing or W3C Trace Context.

I spent a few hours trying to figure out how I could do a plugin for Elastic APM Agent, but in the end I couldn't see any feasible solution.

Maybe it would easier for people who understand http-kit better.

In the meanwhile, it is possible to add the traceparent header per application, but there are a lot of applications where I need to do this.

@ptaoussanis
Copy link
Member

@AriPaaWun Hi Ari,

Could you please describe, at a conceptual level at least, how you'd see a feature like this working in http-client?

What kind of API do you have in mind?

@AriPaaWun
Copy link
Author

Of course.

We are using Elastic APM agent to monitor our services. Part of this monitoring is request tracing. A request to our backend creates a transaction where all inner operations and outgoing requests (by http-client in most cases) are logged and visualised in our Elastic stack.

For the outgoing requests to show the full trace of the second and subsequent backends there must be a distributed tracing header in the call.
The Elastic APM Java agent supports many HTTP clients out of the box, but not HTTP Kit client.

Elastic APM agent does distributed tracing by adding the traceparent header to requests. For example see Apache HTTP Client instrumentation.

Elastic offers a Plugin API that supposedly can be used to instrument other HTTP clients not supported out-of-the-box. While I was looking into this, I ran into two problems.

  1. The Plugin API doesn't offer all the functionality that is used in the out-of-the-box instrumentations. I'm not sure if this is a problem, or can all the needed tracing functionality just be imported to create a plugin. Or maybe one would need to code the traceparent header generation from scratch or copy-paste it from apm-agent-java sources.
  2. HTTP Kit's HttpClient, Request and RequestConfig are quite closed. If they had more open API, maybe a third party could instrument them with Elastic APM agent Plugin API.

Easiest for me and other users of HTTP Kit and Elastic APM agent would be if HTTP Kit support was added to Elastic APM agent and we could just update to the newest versions.

@ptaoussanis
Copy link
Member

Hi Ari, thanks for the extra info.

Easiest for me and other users of HTTP Kit and Elastic APM agent would be if HTTP Kit support was added to Elastic APM agent and we could just update to the newest versions.

I'm sorry, I have no involvement in or familiarity with Elastic APM, so can't advise on that.

HTTP Kit's HttpClient, Request and RequestConfig are quite closed. If they had more open API, maybe a third party could instrument them with Elastic APM agent Plugin API.

Would it be possible to describe precisely how you'd need the http-kit API to change? What exactly would it need to be able to open/do/provide, that isn't currently the case?

As a first step, we don't need to know how the functionality should be implemented - just a clear understanding of what the desired functionality is.

The more clear you can be, the more likely it is that someone here will be able to provide feedback on feasibility and/or potentially assist with getting in the necessary support if it's indeed lacking.

E.g. when I call http-kit/request, I need it to take a new argument x which will then cause clearly-described behaviour y, etc.

Cheers!

@AriPaaWun
Copy link
Author

The Clojure API doesn't need to change, I think.

But the underlaying Java class(es) would need some changes for the instrumentation to be possible.

At the least it should be possible to read headers, method and proxy_url in RequestConfig. I suppose proxy_url contains the URL with protocol. Method, URL and protocol is needed to start a new span or a sub-step in the current trace.
I think URL and protocol could also be read from the String url in HttpClient.exec.

Headers from RequestConfig are needed so that the traceparent header can be appended before the request is executed.

The other instrumentations in apm-agent-java can set the header straight to that HTTP client's HttpRequest object:

    @Override
    public void setHeader(String headerName, String headerValue, HttpRequest request) {
        request.setHeader(headerName, headerValue);
    }

Above example from this RequestHeaderAccessor.

I couldn't find similar HttpRequest class in http-kit's code.

If the RequestConfig was readable, I guess that it could be replaced when entering HttpClient.exec. Replaced with a new instance that has headers with appended traceparent header.

Please keep in mind that my experience with this kind of instrumentation is very limited.

@bsless
Copy link
Collaborator

bsless commented Nov 13, 2023

Having given it some thought, I may have an idea, it also might coincide with needs for open telemetry or other metrics frameworks;
Add an interceptor interface with three methods: on request, on response, on error
On request takes the http request and returns some context which is passed to the response or error methods along with the respective response or exception.
The constructor APIs should accept one or more interceptors.
wdyt?

@AriPaaWun
Copy link
Author

Well, yes, that sounds like it could do the trick.

@jefimm
Copy link

jefimm commented Apr 4, 2024

any updates on this ?

@ptaoussanis
Copy link
Member

@bsless's idea above sounds broadly reasonable to me - would be open to seeing a PR in that direction, or concrete feedback advocating for another approach 👍

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

No branches or pull requests

4 participants