From d840545dede32bc4fac85656b8179ae56901a70a Mon Sep 17 00:00:00 2001 From: nickhill Date: Fri, 29 Mar 2019 12:48:46 -0700 Subject: [PATCH] core: disable stats and tracing when OpenCensus impls aren't present 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. --- .../AbstractManagedChannelImplBuilder.java | 4 +-- .../internal/AbstractServerImplBuilder.java | 4 +-- .../io/grpc/internal/CensusStatsModule.java | 9 ++++++ .../io/grpc/internal/CensusTracingModule.java | 7 ++++ .../main/java/io/grpc/internal/GrpcUtil.java | 13 ++++++++ ...AbstractManagedChannelImplBuilderTest.java | 32 +++++++++---------- .../AbstractServerImplBuilderTest.java | 29 ++++++++--------- 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index 8cf301a275b..27122a71d22 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -172,11 +172,11 @@ protected final int maxInboundMessageSize() { return maxInboundMessageSize; } - private boolean statsEnabled = true; + private boolean statsEnabled = CensusStatsModule.OPENCENSUS_IMPL_PRESENT; private boolean recordStartedRpcs = true; private boolean recordFinishedRpcs = true; private boolean recordRealTimeMetrics = false; - private boolean tracingEnabled = true; + private boolean tracingEnabled = CensusTracingModule.OPENCENSUS_IMPL_PRESENT; @Nullable private CensusStatsModule censusStatsOverride; diff --git a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java index 04e3fbcc54b..4ad3b84cd6f 100644 --- a/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractServerImplBuilder.java @@ -79,11 +79,11 @@ public static ServerBuilder forPort(int port) { CompressorRegistry compressorRegistry = DEFAULT_COMPRESSOR_REGISTRY; long handshakeTimeoutMillis = DEFAULT_HANDSHAKE_TIMEOUT_MILLIS; @Nullable private CensusStatsModule censusStatsOverride; - private boolean statsEnabled = true; + private boolean statsEnabled = CensusStatsModule.OPENCENSUS_IMPL_PRESENT; private boolean recordStartedRpcs = true; private boolean recordFinishedRpcs = true; private boolean recordRealTimeMetrics = false; - private boolean tracingEnabled = true; + private boolean tracingEnabled = CensusTracingModule.OPENCENSUS_IMPL_PRESENT; @Nullable BinaryLog binlog; TransportTracer.Factory transportTracerFactory = TransportTracer.getDefaultFactory(); InternalChannelz channelz = InternalChannelz.instance(); diff --git a/core/src/main/java/io/grpc/internal/CensusStatsModule.java b/core/src/main/java/io/grpc/internal/CensusStatsModule.java index 385cfdc97ed..af63462bbe8 100644 --- a/core/src/main/java/io/grpc/internal/CensusStatsModule.java +++ b/core/src/main/java/io/grpc/internal/CensusStatsModule.java @@ -71,6 +71,15 @@ public final class CensusStatsModule { private static final Logger logger = Logger.getLogger(CensusStatsModule.class.getName()); private static final double NANOS_PER_MILLI = TimeUnit.MILLISECONDS.toNanos(1); + static final boolean OPENCENSUS_IMPL_PRESENT; + + static { + ClassLoader loader = io.opencensus.stats.StatsComponent.class.getClassLoader(); + OPENCENSUS_IMPL_PRESENT = + GrpcUtil.classIsFound("io.opencensus.impl.stats.StatsComponentImpl", loader) + || GrpcUtil.classIsFound("io.opencensus.impllite.stats.StatsComponentImplLite", loader); + } + private final Tagger tagger; private final StatsRecorder statsRecorder; private final Supplier stopwatchSupplier; diff --git a/core/src/main/java/io/grpc/internal/CensusTracingModule.java b/core/src/main/java/io/grpc/internal/CensusTracingModule.java index 08f7947fdd7..ddd587ce49e 100644 --- a/core/src/main/java/io/grpc/internal/CensusTracingModule.java +++ b/core/src/main/java/io/grpc/internal/CensusTracingModule.java @@ -60,6 +60,8 @@ final class CensusTracingModule { private static final Logger logger = Logger.getLogger(CensusTracingModule.class.getName()); + static final boolean OPENCENSUS_IMPL_PRESENT; + @Nullable private static final AtomicIntegerFieldUpdater callEndedUpdater; @Nullable private static final AtomicIntegerFieldUpdater streamClosedUpdater; @@ -84,6 +86,11 @@ final class CensusTracingModule { } callEndedUpdater = tmpCallEndedUpdater; streamClosedUpdater = tmpStreamClosedUpdater; + + ClassLoader loader = io.opencensus.trace.TraceComponent.class.getClassLoader(); + OPENCENSUS_IMPL_PRESENT = + GrpcUtil.classIsFound("io.opencensus.impl.trace.TraceComponentImpl", loader) + || GrpcUtil.classIsFound("io.opencensus.impllite.trace.TraceComponentImplLite", loader); } private final Tracer censusTracer; diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index ec0418e9496..6933152dc91 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -781,5 +781,18 @@ static boolean iterableContains(Iterable iterable, T item) { return false; } + /** + * Returns true if the specified class is found on the classpath, false otherwise. + */ + public static boolean classIsFound(String className, ClassLoader classLoader) { + try { + Class.forName(className, /* initialize = */ false, classLoader); + return true; + } catch (ClassNotFoundException e) { + // no problem + } + return false; + } + private GrpcUtil() {} } diff --git a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java index 3310e919e8e..22ac45b12c4 100644 --- a/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/AbstractManagedChannelImplBuilderTest.java @@ -296,43 +296,43 @@ public void makeTargetStringForDirectAddress_scopedIpv6() throws Exception { public void getEffectiveInterceptors_default() { builder.intercept(DUMMY_USER_INTERCEPTOR); List effectiveInterceptors = builder.getEffectiveInterceptors(); - assertEquals(3, effectiveInterceptors.size()); - assertThat(effectiveInterceptors.get(0)) - .isInstanceOf(CensusTracingModule.TracingClientInterceptor.class); - assertThat(effectiveInterceptors.get(1)) - .isInstanceOf(CensusStatsModule.StatsClientInterceptor.class); - assertThat(effectiveInterceptors.get(2)).isSameAs(DUMMY_USER_INTERCEPTOR); + assertThat(effectiveInterceptors).containsExactly(DUMMY_USER_INTERCEPTOR); } @Test - public void getEffectiveInterceptors_disableStats() { + public void getEffectiveInterceptors_enableStats() { builder.intercept(DUMMY_USER_INTERCEPTOR); - builder.setStatsEnabled(false); + builder.setStatsEnabled(true); List effectiveInterceptors = builder.getEffectiveInterceptors(); assertEquals(2, effectiveInterceptors.size()); assertThat(effectiveInterceptors.get(0)) - .isInstanceOf(CensusTracingModule.TracingClientInterceptor.class); + .isInstanceOf(CensusStatsModule.StatsClientInterceptor.class); assertThat(effectiveInterceptors.get(1)).isSameAs(DUMMY_USER_INTERCEPTOR); } @Test - public void getEffectiveInterceptors_disableTracing() { + public void getEffectiveInterceptors_enableTracing() { builder.intercept(DUMMY_USER_INTERCEPTOR); - builder.setTracingEnabled(false); + builder.setTracingEnabled(true); List effectiveInterceptors = builder.getEffectiveInterceptors(); assertEquals(2, effectiveInterceptors.size()); assertThat(effectiveInterceptors.get(0)) - .isInstanceOf(CensusStatsModule.StatsClientInterceptor.class); + .isInstanceOf(CensusTracingModule.TracingClientInterceptor.class); assertThat(effectiveInterceptors.get(1)).isSameAs(DUMMY_USER_INTERCEPTOR); } @Test - public void getEffectiveInterceptors_disableBoth() { + public void getEffectiveInterceptors_enableBoth() { builder.intercept(DUMMY_USER_INTERCEPTOR); - builder.setStatsEnabled(false); - builder.setTracingEnabled(false); + builder.setStatsEnabled(true); + builder.setTracingEnabled(true); List effectiveInterceptors = builder.getEffectiveInterceptors(); - assertThat(effectiveInterceptors).containsExactly(DUMMY_USER_INTERCEPTOR); + assertEquals(3, effectiveInterceptors.size()); + assertThat(effectiveInterceptors.get(0)) + .isInstanceOf(CensusTracingModule.TracingClientInterceptor.class); + assertThat(effectiveInterceptors.get(1)) + .isInstanceOf(CensusStatsModule.StatsClientInterceptor.class); + assertThat(effectiveInterceptors.get(2)).isSameAs(DUMMY_USER_INTERCEPTOR); } @Test diff --git a/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java b/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java index 7879931c345..4e854bcdc91 100644 --- a/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java +++ b/core/src/test/java/io/grpc/internal/AbstractServerImplBuilderTest.java @@ -49,44 +49,43 @@ public void getTracerFactories_default() { builder.addStreamTracerFactory(DUMMY_USER_TRACER); List factories = builder.getTracerFactories(); - - assertEquals(3, factories.size()); - assertThat(factories.get(0)).isInstanceOf(CensusStatsModule.ServerTracerFactory.class); - assertThat(factories.get(1)).isInstanceOf(CensusTracingModule.ServerTracerFactory.class); - assertThat(factories.get(2)).isSameAs(DUMMY_USER_TRACER); + assertThat(factories).containsExactly(DUMMY_USER_TRACER); } @Test - public void getTracerFactories_disableStats() { + public void getTracerFactories_enableStats() { builder.addStreamTracerFactory(DUMMY_USER_TRACER); - builder.setStatsEnabled(false); + builder.setStatsEnabled(true); List factories = builder.getTracerFactories(); assertEquals(2, factories.size()); - assertThat(factories.get(0)).isInstanceOf(CensusTracingModule.ServerTracerFactory.class); + assertThat(factories.get(0)).isInstanceOf(CensusStatsModule.ServerTracerFactory.class); assertThat(factories.get(1)).isSameAs(DUMMY_USER_TRACER); } @Test - public void getTracerFactories_disableTracing() { + public void getTracerFactories_enableTracing() { builder.addStreamTracerFactory(DUMMY_USER_TRACER); - builder.setTracingEnabled(false); + builder.setTracingEnabled(true); List factories = builder.getTracerFactories(); assertEquals(2, factories.size()); - assertThat(factories.get(0)).isInstanceOf(CensusStatsModule.ServerTracerFactory.class); + assertThat(factories.get(0)).isInstanceOf(CensusTracingModule.ServerTracerFactory.class); assertThat(factories.get(1)).isSameAs(DUMMY_USER_TRACER); } @Test - public void getTracerFactories_disableBoth() { + public void getTracerFactories_enableBoth() { builder.addStreamTracerFactory(DUMMY_USER_TRACER); - builder.setTracingEnabled(false); - builder.setStatsEnabled(false); + builder.setTracingEnabled(true); + builder.setStatsEnabled(true); List factories = builder.getTracerFactories(); - assertThat(factories).containsExactly(DUMMY_USER_TRACER); + assertEquals(3, factories.size()); + assertThat(factories.get(0)).isInstanceOf(CensusStatsModule.ServerTracerFactory.class); + assertThat(factories.get(1)).isInstanceOf(CensusTracingModule.ServerTracerFactory.class); + assertThat(factories.get(2)).isSameAs(DUMMY_USER_TRACER); } static class Builder extends AbstractServerImplBuilder {