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

Support AsyncRestTemplate #124

Closed
MarcialRosales opened this issue Jan 20, 2016 · 14 comments · Fixed by #155
Closed

Support AsyncRestTemplate #124

MarcialRosales opened this issue Jan 20, 2016 · 14 comments · Fixed by #155
Assignees

Comments

@MarcialRosales
Copy link

It appears there is no support for AsyncRestTemplate. Sleuth has no way of intercepting the client's async requests to inject the corresponding headers. Therefore, it sends the requests without them. And of course, the responses do not carry them either and as a consequence Sleuth cannot propagate those headers into the thread running the ListenableFuture's callback.

I am using a sample project to demonstrate it : https://github.com/pivotalcsoemea/distributed-tracing-demo/blob/master/gateway/src/main/java/io/pivotal/demo/gateway/AsyncController.java#L51

I have noticed that when the inner Restful request (https://github.com/pivotalcsoemea/distributed-tracing-demo/blob/master/gateway/src/main/java/io/pivotal/demo/gateway/AsyncController.java#L72) is invoked, Sleuth logs the following statement (I have included the two previous statements for reference):

2016-01-20 18:38:00.464  INFO 97307 --- [nio-8080-exec-1] io.pivotal.demo.gateway.AsyncController  : Opening trade bob 100.0 @ 07d05a11-9483-4481-8876-70fb5ccc98e8

2016-01-20 18:38:00.799  WARN 97307 --- [/O dispatcher 1] io.pivotal.demo.gateway.AsyncController  : No Span avaliable in applySpread method

2016-01-20 18:38:00.820  WARN 97307 --- [nio-8080-exec-2] o.s.cloud.sleuth.util.ExceptionUtils     : Trace client warn: thread http-nio-8080-exec-2 tried to start a new Span with parent MilliSpan(begin=1453311480819, end=0, name=null, traceId=07d05a11-9483-4481-8876-70fb5ccc98e8, parents=[], spanId=07d05a11-9483-4481-8876-70fb5ccc98e8, remote=true, exportable=true, annotations={}, processId=null, timelineAnnotations=[]), but there is already a currentSpan MilliSpan(begin=1453311480404, end=0, name=http/async/open, traceId=07d05a11-9483-4481-8876-70fb5ccc98e8, parents=[], spanId=07d05a11-9483-4481-8876-70fb5ccc98e8, remote=false, exportable=false, annotations={account=bob}, processId=null, timelineAnnotations=[])
@marcingrzejszczak
Copy link
Contributor

Here you have an example how to set up AsyncRestTemplate to make everything work properly - https://github.com/spring-cloud-samples/brewery/blob/master/brewing/src/main/java/io/spring/cloud/samples/brewery/bottling/BottlingConfiguration.java#L30

Ping us back once you confirm that it's working and we'll close this issue.

@marcingrzejszczak
Copy link
Contributor

Ping?

@marcingrzejszczak
Copy link
Contributor

Closing due to lack of feedback

@MarcialRosales
Copy link
Author

Hi Marcin, first of all I am really sorry for not getting back to you on time. I have tried your changes and it does not fix the issue. The downstream services do not find any X-Trace-Id in the request header. When the downstream service logs their spans, they don't have any parent trace-id hence their trace-id is different to the one originated by the caller service.
You can see the changes:
https://github.com/pdeemea/distributed-tracing-demo/blob/master/gateway/src/main/java/io/pivotal/demo/gateway/GatewayConfiguration.java#L22

@marcingrzejszczak
Copy link
Contributor

I'll try your example and see what's going on...

@marcingrzejszczak
Copy link
Contributor

Thanks for filing this issue - it turned out that actually there is a problem :)

In the Brewery it was working because I was using @LoadBalanced RestTemplate inside an AsyncRestTemplate. In your scenario there was a typical plain old RestTemplate. You can check the latest snapshots if things are working properly (but they should since I've checked your example).

BTW you're using M4 and there is already M5. Some concepts have changed so you'll have to do some minor refactoring:

TraceManager -> Tracer
adding annotations -> logEvent(...)
adding binaryAnnotation -> tag(...)

@MarcialRosales
Copy link
Author

Thanks a lot ! I take that if I migrate to M5, my sample should work. I am working on another account now but as soon as I have a moment I will migrate and back to you if I have any issues.

@tg44
Copy link

tg44 commented Apr 13, 2016

I'm new in Spring, and it's support tools (like dependency version handling). How can I get this working?
My restTemplate sends the correct headers, but my asyncRestTemplate doesn't. I'm on SpringBoot 1.3.3, I have Trace, and I haven't got TraceManager in my classpath, so I think this is in a correct version. Did all the things mentioned upper. Am I doing something wrong, or it's still not perfectly working?

@marcingrzejszczak
Copy link
Contributor

@tg44 can you post your sample app so that we can check it out?

@tg44
Copy link

tg44 commented Apr 14, 2016

Declarations.
So the UserServiceNotAsync sends the correct headers, but the ProductService not.
(Start order is reg-address-user-prod-web, you can check localhost:1050/products and localhost:1050/users and localhost:1020/trace and localhost:1030/trace)

@marcingrzejszczak
Copy link
Contributor

Hi! Can you check if the issue is there with current version in master?

@marcingrzejszczak
Copy link
Contributor

Ok I'm looking at your code and I think that I found the issue.

You register your own AsyncRestTemplate right (https://github.com/TeamWanari/spring-ms-playground/blob/master/web/src/main/java/com/wanari/ms/web/WebService.java#L46) ? But you're not wrapping it in any trace related wrapper like we do https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebAsyncClientAutoConfiguration.java#L93 . So no wonder it doesn't work for your async implementation.

Can you check how we're registering the async rest template and do it accordingly?

I guess it would look sth like this:

@Bean   
public AsyncRestTemplate yourAsyncRestTemplate(ClientHttpRequestFactory factory, @LoadBalanced RestTemplate restTemplate, Tracer tracer) {
        return new TraceAsyncRestTemplate(wrapper, restTemplate, tracer);
    }

Just ensure with a debugger that factory is instanceof TraceAsyncClientHttpRequestFactoryWrapper.

@tg44
Copy link

tg44 commented Apr 15, 2016

Everything works fine, there was a missuse on my side. (just use @Autowired and don't use new :) )

@MarcialRosales
Copy link
Author

Thanks a lot and sorry for not getting back to you earlier. I am involved in a critical issue very recently and I have not had time to look at this.

On 15 Apr 2016, at 17:48, Gergő Törcsvári notifications@github.com wrote:

Everything works fine, there was a missuse on my side. (just use @Autowired https://github.com/Autowired and don't use new :) )


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub #124 (comment)

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.

4 participants