search

opentracing-contrib / java-spring-web

OpenTracing Spring Web instrumentation
Apache License 2.0
107 stars 59 forks source link

Fails when using Spring Webflux #59

Closed darrenhaken closed 6 years ago

darrenhaken commented 6 years ago

Are you aware this fails with Spring Webflux?

pavolloffay commented 6 years ago

No, thanks for reporting. Do you have any error logs/reproducer? Would you like to submit a fix for it?

darrenhaken commented 6 years ago

I can take a look at submitting a fix sure, I’ll open a PR soon

pavolloffay commented 6 years ago

@darrenhaken thanks :+1:

mdvorak commented 6 years ago

I believe that TracerAutoConfiguration and TracerRegisterAutoConfiguration does not belong in this library (java-spring-web) at all. Then this jar can be for webflux app excluded completely (and possibly new designed for reactive stack should be created).

However, for easier -starter integration, it I suggest adding @ConditionalOnClass. Its easy change on its own, but adding tests will be more time consuming.

Change is in commit f7fff3e35cbc397b1be90968e1d8441564afea02

pavolloffay commented 6 years ago

@mdvorak is this resolved by #68 ?

mdvorak commented 6 years ago

Only partially, I did add condition to new config class for client, but did not touch ServerTracingAutoConfiguration. So, add @ConditionalOnClass(Servlet.class) and you're done.

geoand commented 6 years ago

Sorry for being late to the party but I don't see any indication that the module supports Spring 5 yet, much less Webflux which is a non-Servlet Web framework of it's own.

Am I missing something obvious here?

mdvorak commented 6 years ago

@geoand Of course, same can be achieved by <exclusions> when referenced by ot-spring-cloud. Still, does not do any harm either.

pavolloffay commented 6 years ago

I have created #72. Webflux support will need a separate instrumentation for both client and server side.