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

instrumenting RabbitMQ via spring-amqp #508

Closed
jonathan-lo opened this issue Oct 3, 2017 · 14 comments

Comments

Projects
None yet
5 participants
@jonathan-lo
Copy link
Contributor

commented Oct 3, 2017

There's been some interest at my work with instrumenting rabbit interactions that are made using spring-amqp. @adriancole mentioned that there's been previous requests for this instrumentation channel e.g. openzipkin/zipkin#1428.

I can have a look at this if there's sufficient interest. Since spring-amqp has some nice hooks into the consume / produce lifecycle, this looks feasible.

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2017

The plan of attack I'm thinking is:
producer via RabbitTemplate - use the MessagePostProcessor hook to inject trace headers / annotate
consumer via RabbitListener - use SimpleRabbitListenerContainerFactory's adviceChain which allows us to specify an aspect to extract the trace headers and handle appropriately

@adriancole

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2017

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 12, 2017

Hmm you're right, whilst RabbitTemplate and RabbitListener is from spring-rabbit, the extension points are from the spring-amqp maven dep, so instrumentation-spring-amqp sounds reasonable.

Will have a play around with the integration tests, at the moment it seems the easiest path would be to add rabbitmq service to ci.

@tnsubramaniam

This comment has been minimized.

Copy link

commented Nov 14, 2017

We at General Motors have a need for this for integrating our legacy apps with Cloud Foundry deployed Microservices. I vote yes

@liangxp

This comment has been minimized.

Copy link

commented Nov 22, 2017

I also want to trace rabbitMQ Message

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2018

should we close this now that @adriancole is working on a generic spring-messaging instrumentation?

@adriancole

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2018

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2018

I see. My spike is using spring amqp's abstractions. So the value add here could be digging the exchange / routing key out of the MessageProperties

@adriancole

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2018

your spike looks good. only question I have really is if there is another way apart from aop to handle the consumer side. I'm guessing instrumenting with spring-amqp (or spring-rabbit) could have an advantage of decoupling the incompatible rabbit 4.x from 5.x series as well master...jonathan-lo:master

cc @shakuzen

@skastenholz

This comment has been minimized.

Copy link
Contributor

commented Jan 22, 2018

I would like to contribute a different way of implementing the listener advice skastenholz@0a721a6:

final Message message = (Message) methodInvocation.getArguments()[1];
final TraceContextOrSamplingFlags extracted = extractor.extract(message.getMessageProperties());
final Span span = tracer.nextSpan(extracted).kind(Span.Kind.CONSUMER).start();
try (SpanInScope ws = tracer.withSpanInScope(span)) {
  return methodInvocation.proceed();`
} finally {
  span.finish();
}

This turned out to work on my project and follows https://github.com/openzipkin/brave/tree/master/brave#setting-a-span-in-scope-manually.

What do you think?

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2018

@skastenholz Yep for sure my next step was to clean the implementation to pretty much the same as what you have there :)

So in my mind there's a couple more things to explore:

  • a non aop solution as @adriancole mentioned above
  • annotating extra info from publisher confirms?
@skastenholz

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2018

@adriancole Regarding a non-aop approach for the AMQP listener I do not see any option after double-checking https://docs.spring.io/spring-amqp/reference/htmlsingle/#containerAttributes. Does this work for you or do you have an alternative in mind?

@adriancole

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2018

@jonathan-lo

This comment has been minimized.

Copy link
Contributor Author

commented Jan 31, 2018

I'll PR.

From memory a wrapping delegate was non trivial since the most commonly used case for consumer side was annotation driven.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.