-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TracingFilter and ServletFilterSpanDecorator #111
Comments
Hello, That sounds reasonable to me, but @pavolloffay is the opentracing expert here :) |
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. |
There should still be the ability to completely override the defaults because some tags in the standard are not desired (e.g. |
@whiskeysierra would you be interested in contributing that? |
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). |
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, theSTANDARD_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:
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.
The text was updated successfully, but these errors were encountered: