Add auto-config for the tracer resolver - #16
Conversation
|
@pavolloffay @malafeev Continuation of the conversation from previous PR. To address your comments from the previous PR:
No the tracer resolver does not register the tracer with the GlobalTracer.
There are two problems that need to be addressed, (1) how instrumentations access the tracer, and (2) how does the Tracer get configured in the first place. This PR is simply dealing with the second problem. As mentioned before, there are two ways the Tracer can be created/configured - either explicitly by the application, or via the |
|
Have created #17 to discuss the first problem (i.e. how instrumentations access the tracer). |
| @RunWith(SpringJUnit4ClassRunner.class) | ||
| public class TracerResolverConfigurationTest { | ||
|
|
||
| @Autowired(required=false) |
There was a problem hiding this comment.
No specific reason - can be removed.
| <version>0.0.1-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>opentracing-spring-cloud-tracerresolver</artifactId> |
There was a problem hiding this comment.
This should include starter in the name: we could name it:
opentracing-spring-cloud-tracerresolver-starter or opentracing-spring-cloud-starter-tracerresolver. It seems that the second is a convention in spring.
| <dependency> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-web</artifactId> | ||
| <version>${version.org.springframework.boot}</version> |
| <dependency> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-test</artifactId> | ||
| <version>${version.org.springframework.boot}</version> |
|
|
||
| @SpringBootTest | ||
| @RunWith(SpringJUnit4ClassRunner.class) | ||
| public class TracerResolverConfigurationTest { |
There was a problem hiding this comment.
To test this you have to make sure that instance provided by traceresolver is the one used by instrumentations.
There was a problem hiding this comment.
We had a similar problem in jaeger-k8s,
This solved the problem https://github.com/snowdrop/jaeger-opentracing/blob/master/spring-cloud-kubernetes-jaeger/src/main/java/org/springframework/cloud/kubernetes/jaeger/JaegerKubernetesAutoConfiguration.java#L49
I think it was failing on an error that there are two tracer beans although I'm not sure why it works here.
| import io.opentracing.contrib.tracerresolver.TracerResolver; | ||
|
|
||
| @Configuration | ||
| @ConditionalOnClass(value = {TracerResolver.class}) |
There was a problem hiding this comment.
So when there is no trace resolver it will use TracerAutoConfiguration (e.g. NoopTracer)?
There was a problem hiding this comment.
Although it should probably be conditional on no Tracer bean as well, in case one has been provided.
|
@pavolloffay Thanks for the comments - will revisit on my return next week if there is resolution as to whether this should be included in this repo. |
|
If we really want it here I would maybe prefer to have it in
|
|
Closing, done in opentracing-contrib/java-spring-web#30 |
|
@objectiser thanks for bringing this up |
2 participants
This PR extracts the tracer resolver auto-config from #15.