Extending tracing scope for @RabbitListener

[jonatan-ivanov]

Fixes spring-cloud/spring-cloud-sleuth#1660

It seems TracingRabbitListenerAdvice (the proxy in this class) only wraps the invocation of the listener not the invocation AND the error handling. This change modifies the target of the proxy so that the advice wraps the invocation and the error handling (see the referenced issue).

Please notice that this also changes the order of measurements of micrometer and brave. Before the change micrometer's instrumentation included brave's instrumentation. With this change the order will be inverted, brave's instrumentation wraps micrometer's instrumentation.

[garyrussell]

Please notice that this also changes the order of measurements ...

I need to think some more, but I am not sure we can make such a change in a patch release, we probably need an option to opt-in to the new behavior.

It might also break existing user advices.

I agree this makes more sense, but the existing behavior has been there for 10+ years (before my time even).

[artembilan]

See my comment here: https://github.com/spring-cloud/spring-cloud-sleuth/issues/1663#issuecomment-748196310

[artembilan]

See one more my comment: https://github.com/spring-cloud/spring-cloud-sleuth/issues/1663#issuecomment-748202467.

It looks like all the @...Listener abstractions suffer the same problem and the fix you propose here is not aligned with a general approach picked up for such a @...Listener API.

We probably still may consider this fix but as a common (or similar) solution for all the support @...Listener instrumentations.

Does it make sense or what ma I missing?

Thanks

[jonatan-ivanov]

@garyrussell @artembilan Yepp, I agree both of you, this is totally why I was seeking for feedback. :)

I haven't really looked into the details into the other, possibly related issues, but seemingly they share the same characteristics and probably the root cause too (error handling is not included in the tracing scope), let me list them here:

• https://github.com/spring-cloud/spring-cloud-sleuth/issues/1660
• https://github.com/spring-cloud/spring-cloud-sleuth/issues/1663
• https://github.com/spring-cloud/spring-cloud-sleuth/issues/1659
• https://github.com/spring-cloud/spring-cloud-sleuth/issues/1729

Could somebody confirm this: Based on this, no matter what common or not common solution will we came up with to fix this, it still would be a breaking change since the fix is including the error handling in the scope, right?

[garyrussell]

Sorry, I didn't realize there were multiple issues; my comments on the the JMS issue are for RabbitMQ.

[garyrussell]

Workaround suggestion here: https://github.com/spring-cloud/spring-cloud-sleuth/issues/1660#issuecomment-749148062

We can consider moving the scope of the proxy in 2.4.

[artembilan]

Superessed via https://github.com/spring-projects/spring-amqp/pull/1500