[WIP] Bootify Spring tracing #740
Conversation
|
chiming in from gitter.. so at first I was "wow" this is easy. Second a little concerned actually. For example, there are a fair amount of libraries out there which will end up doubly instrumented by accident of us auto-configuring by default. Even if we change sleuth in a later version, it will pin sleuth 2.0.x to the version before this happened and I think that won't play out nice in support. I think this needs to be opt-in either by modularization (ex autoconfiguration buddy modules) or some other sort. |
|
cc some springy folks @jonathan-lo @marcingrzejszczak @zeagord @shakuzen @devinsba @llinder I think it is awesome to have this effort picked up. We need to tread carefully, but definitely we should tread! |
|
As I also said in Gitter, I do have second thoughts too. I am wondering if this is not overlapping with Sleuth for example. To create a spring-boot-instrumentation module where autoconfiguration for all spring things would be done (using conditionalOnClass for example) is a good idea IMHO. Or we could stay as it is and not add boot support, leaving this to other library (i.e sleuth) do all the work. |
|
PS especially with rest template we should be careful about double-instrumentation. The dragons... If we do something like this, I would suggest it as a "best efforts" and kick out to sleuth for anything serious. |
|
related is the playground app here: openzipkin/brave-example#25 |
|
At this point isn't sleuth auto configuration of brave. Why the duplicate effort? |
|
Some of the conversations have gone like this:
* Similarly to how we still support Spring 2.5, allow basics for those not
yet on Boot 2, or apps that may not upgrade for years.
* Some don't use sleuth even if we recommend it. Basics help reduce cost of
support and prevent us pushing people out of zipkin's ecosystem.
* Establish a pattern for things that may never be in sleuth (ex there are
things like camel which might be boot but not sleuth)
For example, if there is basic autoconfiguration, properties and otherwise
tightly coupled to the tracing component can be where the component is
defined. We do this in zipkin's server for example. We have buddy modules
for autoconfig to reduce the integration costs and coupling.
I don't expect autoconfig to all be usable in sleuth, for example, as it is
more advanced. Also, sleuth is more about molecules than atoms. For
example, there are interesting relationships between multiple components
which themselves require auto-configuration.
TL;DR; to the degree we pursue this...
I think we should do *only basics*, allowing those on custom spring boot
setups to not mess up basics, reducing cost of support on our channel
especially where english is second language.
We should define a pattern for remote instrumentation that isn't in sleuth,
or would make its dependency graph laborious. For example, cassandra and
camel are on different modules. There is some value in being able to drop
things in and not have to explain how to do autoconfiguration by hand.
Beyond this, especially to the "atoms not molecules" point, we recommend to
use sleuth for boot. For example, I'd expect no inter-relationships like
exist in sleuth to exist here. For example, no feign+ribbon+hystrix as it
is frankly not something we'd want to try to afford.
My 2p
|
|
another thing to consider
even if we don't configure instrumentation, configuring Tracing (naming
correctly setting up reporters and closing them etc) this would be a big
help. similar to brave-springbeans we could start with a copy/paste
removing thing which would be largely reusable also by sleuth. maybe this
isnt 80% of the way but at least I think it would reduce some of the
support cost.
ex a lot of the stuff here is undifferentiated and the code in sleuth to
pick which sender is also
https://github.com/openzipkin/brave-webmvc-example/pull/25/files
if we could at least get the bedding right, yeah that sounds quite a win.
|
|
I think a separate module containing the auto-configuration would be better to avoid two auto-configurations for the same thing ending up on the classpath and having to rely on ordering or dependencies on one another to avoid funkiness. The separate module should also help with migrating Sleuth auto-configuration to the Brave module while maintenance branches of Sleuth can still upgrade but not depend on the Brave auto-config module. I do wonder if we will be making new support issues for ourselves by having more ways to do about the same thing. It might cause confusion on when to use what, but to the degree that users have varied setups, I'm not sure how much we can avoid that while supporting them. |
|
So what I'm hearing is that this is primarily catering who are using boot 1...? IMHO we shouldn't need to be too smart about double instrumentation - it should be a case of adding instructions to use either sleuth or brave's autoconfig |
|
I think we agree that if we decide to autoconfigure, we will do it in a separate module. |
|
There will be dependency issues since the brave module will now depend on spring-boot and spring-cloud does not have dependencies that depend on spring-boot. If this is provided, sleuth won't be able to depend on it and will, therefore, duplicate auto-configuration. |
|
ping @dsyer |
|
My 2p is still start with one for the tracing things, like we have for
spring beans. We get a fair amount of support requests for basics like
setting up the tracer. People forget to close it and that costs a lot of
time. Let's start with that.. ex at least Tracing bean (likely also
HttpTracing and any future MessagingTracing) and the senders?
Then, the examples for those in ad-hoc or otherwise become small and
dependency mess won't be present.
|
|
this is nearing a year old.. should we close it? |
|
We won't be needing this in the near future. |
|
okie dokie. thanks for the feedback |
Intent to fix #399