Trace information is missing when exception is thrown from JmsListeners methods

[m-grzesiak]

Describe the bug
When an exception is thrown from a method annotated with @JmsListener all trace information is lost when it reaches the any implementation of error handler.

Tested on Spring Cloud Sleuth 2.2.3.RELEASE version.

It looks like the cause may be the same as in #1659 and #1660.

[artembilan]

I wonder how org.springframework.jms.listener.MessageListenerContainer is instrumented.
Unlike Spring AMQP and its org.springframework.amqp.rabbit.listener.AbstractMessageListenerContainer there is no Advice feature in Spring JMS.

My point is that even if we merge spring-projects/spring-amqp#1287 one day, it's approach is not going to help here. On the other hand we can't merge that fix yet because it is a big breaking change in behavior. And from here I crave to know how it is instrumented for @JmsListener, so we may have some idea how to fix it back in Spring AMQP to avoid a breaking change. And at the same time we would have aligned solution for all those @...Listener impls with their LisntenerContainer APIs.
For example we also have a @KafkaListener and there is its own LisntenerContainer behind the scene and it also comes without Advice feature. So, do we have the same missed traces issue when exception happens from the @KafkaListener?

Thanks for understanding!

[artembilan]

Sorry, Spring Kafka provides proxy feature for listener. It comes as a feature of the ContainerProperties:

	/**
	 * Set a chain of listener {@link Advice}s; must not be null or have null elements.
	 * @param adviceChain the adviceChain to set.
	 * @since 2.5.6
	 */
	public void setAdviceChain(Advice... adviceChain) {

Well, any way I think you do advising only of the listener method anyway, not including error handling: https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-brave/src/main/java/org/springframework/cloud/sleuth/brave/instrument/messaging/SleuthKafkaAspect.java#L103.

Therefore what you are asking is just out of support at the moment everywhere.

[garyrussell]

For spring-kafka we added the ability to decorate the consumer/producer spring-projects/spring-kafka#1520

We could do something similar for Channel in Spring AMQP so the interception can occur at a much lower level in the stack.

[garyrussell]

However, Channel has a huge API so that might be overkill.

Since we already wrap Channel with a proxy, we could add a callback setBeforePublish(Consumer<BasicProperties> decorator) and setAfterReceive(Consumer<BasicProperties> decorator) which would allow sleuth to modify headers as needed right before we publish or imediately after we receive.

[artembilan]

Here is what we have for JMS: https://github.com/spring-cloud/spring-cloud-sleuth/blob/master/spring-cloud-sleuth-brave/src/main/java/org/springframework/jms/config/TracingJmsListenerEndpointRegistry.java#L226. So, probably the result is the same - the error handling is not included into a tracing context.

[jonatan-ivanov]

I took a quick look at this, as it was kind of expected, it seems this is caused by the same thing, the error handling happens after the Span is closed, I created a sample project for it using the JMS Example: https://github.com/jonatan-ivanov/sleuth-gh-1663

This is where the tracing instrumentation happens: TracingJmsListenerEndpointRegistry.java#L274-L284
And this is where the error handling takes place: AbstractMessageListenerContainer.java#L923-L931

[jonatan-ivanov]

I'm closing this in favor of spring-projects/spring-framework#26532