core: disable stats and tracing when OpenCensus impls aren't present #5520
Conversation
| static final boolean OPENCENSUS_IMPL_PRESENT; | ||
|
|
||
| static { | ||
| ClassLoader loader = io.opencensus.stats.StatsComponent.class.getClassLoader(); |
There was a problem hiding this comment.
I'm wondering if this can be CensusStatsModule.class.getClassLoader(), and people who don't want census can remove runtime dependency of census completely, namely making census api as a compile-time-only dependency for grpc library.
There was a problem hiding this comment.
@dapengzhang0 you mean also remove the existing opencensus-api dependency? I don't think that would be possible without the options discussed in #5510 which both require some kind of breakage, at a minimum would need existing census users to amend their builds.
There was a problem hiding this comment.
I mean not removing opencensus-api transparenttransitive dependency from grpc library, but about the ability for people who don't want census to explicitly remove that dependency in their own project's classpath.
There was a problem hiding this comment.
oh I see what you mean, yes I think that should be possible, it would require a couple more changes than this though, for example io.opencensus.trace.Tracing is also referenced directly in the channel and server builder classes.
|
@zhangkun83, how do you feel about this? I think I'm in favor. Although I think it also shows there is a problem that we lose 4µs per-RPC just by using the Census API. It might be worth finding someone to spend the time to figure out why the API is so expensive. |
|
@bogdandrutu, do you have any concern about hard-coding the Census implementation class names in here? It may be better that OpenCensus had an API to indicate the availability of its implementation. |
|
@bogdandrutu, ping. I think it would be better to have an API exposed for this. |
|
Alternatively, we could kick census out of grpc-core and then have a grpc-census that has a hard dependency on an implementation. The "multi-implementation" part of census doesn't seem to be serving the OSS community; others aren't making their own implementations of the API, right? |
|
ping @bogdandrutu |
|
We moved census out of core with #6577. It does not currently depend on the census implementation, but we can still consider doing so. |
The interceptors do nothing when the OpenCensus implementation classes aren't loadable, yet still have significant overhead. So this change should not break any existing users of opencensus. See #5510 for example of the performance benefit to other users.
Contributes to #5510.