TracingFilter and ServletFilterSpanDecorator

[williamokano]

Hi, in this line https://github.com/opentracing-contrib/java-spring-web/blob/master/opentracing-spring-web-starter/src/main/java/io/opentracing/contrib/spring/web/starter/ServerTracingAutoConfiguration.java#L87 the lib is checking for some ServletFilterSpanDecorator decorators. Yesterday I had to create a custom decorator for me and I noticed that if I register my custom decorator, the STANDARD_TAGS is no longer registered due to this checking.

Would not be better if instead of manually checking and injecting we could not simply register it as a Bean and register all the beans?

The problem is that I don't to lose this default behavior, and if I register my custom bean I also have to define standard_tags as a bean to not lose it. So I have do to do something like this:

@Configuration
class DecoratorsConfig {
  @Bean
  public ServletFilterSpanDecorator myCustomDecorator() {
    return new MyCustomerServletFilterSpanDecorator();
  }
  
  @Bean
  public ServletFilterSpanDecorator defaultDecorator() { // this is actually undesired for me.
    return ServletFilterSpanDecorator.STANDARD_TAGS;
  }
}

If someone just don't want to use it, somehow, it could be annotated as @ConditionalOnProperty and be disabled through a property.

I didn't make a PR because I want to hear you guys before to understand this behavior.

[geoand]

Hello,

That sounds reasonable to me, but @pavolloffay is the opentracing expert here :)

[pavolloffay]

If somebody whats to provide custom decorators it's his responsibility to provide all the decorators, that is the current deal.

Your proposal makes sense, I think most people just want to add more stuff in the decorators rather than omitting standard tags. If we do this change we should do it globally for all decorators we have.

[whiskeysierra]

There should still be the ability to completely override the defaults because some tags in the standard are not desired (e.g. http.url exposing path parameters).

[geoand]

@whiskeysierra would you be interested in contributing that?

[whiskeysierra]

I'd just leave it as it is, tbh. The current behavior is a) already established and b) allows for all scenarios (combine with standard and completely configure it by hand).