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

Implement Sleuth interceptor for Apache HttpClient #685

Closed
metacubed opened this issue Aug 24, 2017 · 16 comments

Comments

@metacubed
Copy link

@metacubed metacubed commented Aug 24, 2017

Would it be possible to add an interceptor which instruments outgoing calls from an Apache HttpClient? It would follow the same pattern as the other interceptors in org.springframework.cloud.sleuth.instrument.web.client.

This interceptor would implement org.apache.http.HttpRequestInterceptor, and can be used as follows:

@Autowired
private TraceHttpClientInterceptor tracingInterceptor; // New class

...

org.apache.http.client.HttpClient httpClient =
    HttpClientBuilder.create()
                     .addInterceptorFirst(tracingInterceptor)
                     .build();
@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Aug 24, 2017

How about a contribution? :) You have almost everything sorted out so it would be great if you continued the work and wrote the missing implementation and wrote some tests to it. We will gladly accept such a contribution :)

@metacubed

This comment has been minimized.

Copy link
Author

@metacubed metacubed commented Aug 25, 2017

@marcingrzejszczak, for sure, I'll be happy to contribute. I just wanted to be sure that it fits into the overall project before starting.

@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Jan 21, 2018

With the #711 done, it's enough to wrap your builder with Brave components (https://github.com/openzipkin/brave/tree/master/instrumentation/httpclient). Just add the io.zipkin.brave:brave-instrumentation-httpclient dependency.

To enable tracing, create your client using TracingHttpClientBuilder.

httpclient = TracingHttpClientBuilder.create(tracing).build();

You can also use TracingCachingHttpClientBuilder if you depend on org.apache.httpcomponents:httpclient-cache

@marcingrzejszczak marcingrzejszczak added this to the 2.0.0.M6 milestone Jan 21, 2018
@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Jan 21, 2018

Where tracing would be the HttpTracing bean.

@Autowired HttpTracing tracing;

httpclient = TracingHttpClientBuilder.create(tracing).build();
@metacubed

This comment has been minimized.

Copy link
Author

@metacubed metacubed commented Jan 27, 2018

@marcingrzejszczak, yes, I saw the announcement just this week (I think). That and open-tracing compatibility are great news!

Once I got into implementation, I found that you couldn't really configure all available HttpClientBuilders in a post-process step (like the RestTemplate), since they become immutable. So I dropped the idea and just configured the HttpClients individually.

This solution bypasses that by asking you to use a custom builder. Your code still has to change at all points-of-use, but it is all neatly encapsulated.

@mukulb82

This comment has been minimized.

Copy link

@mukulb82 mukulb82 commented Oct 8, 2018

@marcingrzejszczak please share the complete example. it will really help to see the implementation. is the project on github? i have the same problem statement to use apache HTTP client for tracing in spring boot microservice and export traces to zipkin

@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Oct 8, 2018

@mukulb82 wherever you're using an HttpClient you have to use in the way I've presented above. You can read more about this here https://github.com/openzipkin/brave/tree/master/instrumentation/httpclient and here you have a test for that https://github.com/openzipkin/brave/blob/master/instrumentation/httpclient/src/test/java/brave/httpclient/ITTracingHttpClientBuilder.java#L44-L55

@mukulb82

This comment has been minimized.

Copy link

@mukulb82 mukulb82 commented Oct 8, 2018

okay. let me take a look. thanks

@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Oct 8, 2018

You can autowire it. Tracer and HttpTracing are available as beans.

@mukulb82

This comment has been minimized.

@mukulb82

This comment has been minimized.

Copy link

@mukulb82 mukulb82 commented Oct 8, 2018

ok, while autowiring these are not available. i tried doing the autowiring and i am being told

No qualifying bean of type 'brave.Tracing' available: expected at least 1 bean which qualifies as autowire candidate

@mukulb82

This comment has been minimized.

Copy link

@mukulb82 mukulb82 commented Oct 8, 2018

i am using - spring-beans-4.3.9.RELEASE.jar

@marcingrzejszczak

This comment has been minimized.

Copy link
Contributor

@marcingrzejszczak marcingrzejszczak commented Oct 8, 2018

I have no idea what you're doing. If you generate a project via start.spring.io then it will work (https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/autoconfig/TraceAutoConfiguration.java#L76). If you have questions about Brave, please go to Brave's GitHub project and ask for questions there or on Brave's Gitter channel.

@spring-cloud spring-cloud locked as resolved and limited conversation to collaborators Oct 8, 2018
@adriancole

This comment has been minimized.

Copy link
Contributor

@adriancole adriancole commented Oct 9, 2018

on gitter, both brave alone and also later sleuth were mentioned. I have no idea specifically what's of interest but if I create a diff for using httpclient I'll post it here.

@adriancole

This comment has been minimized.

Copy link
Contributor

@adriancole adriancole commented Oct 9, 2018

@adriancole

This comment has been minimized.

Copy link
Contributor

@adriancole adriancole commented Oct 9, 2018

https://github.com/openzipkin/brave-webmvc-example/compare/add-apachehc-tracing for brave standalone (just because I said I would :P )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.