opentracing-contrib / java-spring-rabbitmq

OpenTracing RabbitMQ instrumentation
Apache License 2.0
19 stars 21 forks source link

Upgrade to spring boot 2.1+ #17

Closed ask4gilles closed 5 years ago

sudr commented 5 years ago

I'll take a stab at this.

ask4gilles commented 5 years ago

Thanks! You will see that there is a bunch of imports to change, then it the IT tests which are failing...

sudr commented 5 years ago

One of the test failure issues is due to a difference in what appears to be a change in Spring Framework 5.1. The proxy object in SimpleMessageListenerContainer is no longer a JDK dynamic proxy and therefore the advice chain that has the RabbitMqReceiveTracingInterceptor is not being invoked causing the receive span not to be populated in the tracer.

I'd say this requires more in-depth research. When I use Spring Boot 2.0.8 dependencies I get the current behavior where the proxy is a JDK dynamic proxy as expected and the test passes.

I was able to resolve some additional test failures due to changes in spring-rabbit behavior and new functionality. All tests now pass against Spring Boot 2.0.8 dependencies. Perhaps we can change this issue to be an update to Spring Boot 2.0+ while we research the 2.1+ changes related to the proxy.

wuyupengwoaini commented 5 years ago

Now springboot 1.x is still in use in most case,so could java-spring-rabbitmq creates a branch to continue to support springboot 1.x

sudr commented 5 years ago

I would agree that if there is consensus that we align with the Spring Boot 1.5 EOL timeline which is currently set for August 1st 2019.

ask4gilles commented 5 years ago

Thanks for the detailed explanation. I've created a separate milestone for it.

sudr commented 5 years ago

I've submitted PR #19 for the Spring Boot 2.1+ upgrade that resolves all the test failures. I hope we can release 1.0.0 soon and move 0.1.1 into maintenance mode/branch.

ask4gilles commented 5 years ago

fixed by https://github.com/opentracing-contrib/java-spring-rabbitmq/pull/19

I hope we can release 1.0.0 soon and move 0.1.1 into maintenance mode/branch. @sudr @wuyupengwoaini I've created a separate branch to maintain spring boot 1.5.x and merged this PR into master.