diff --git a/api/src/main/java/io/grpc/GlobalInterceptors.java b/api/src/main/java/io/grpc/GlobalInterceptors.java index 44d1592b61a..e5fd86170f0 100644 --- a/api/src/main/java/io/grpc/GlobalInterceptors.java +++ b/api/src/main/java/io/grpc/GlobalInterceptors.java @@ -73,24 +73,19 @@ static synchronized void setInterceptorsTracers( isGlobalInterceptorsTracersSet = true; } - /** - * Returns the list of global {@link ClientInterceptor}. If not set, this returns am empty list. - */ + /** Returns the list of global {@link ClientInterceptor}. If not set, this returns null. */ static synchronized List getClientInterceptors() { isGlobalInterceptorsTracersGet = true; return clientInterceptors; } - /** Returns list of global {@link ServerInterceptor}. If not set, this returns an empty list. */ + /** Returns list of global {@link ServerInterceptor}. If not set, this returns null. */ static synchronized List getServerInterceptors() { isGlobalInterceptorsTracersGet = true; return serverInterceptors; } - /** - * Returns list of global {@link ServerStreamTracer.Factory}. If not set, this returns an empty - * list. - */ + /** Returns list of global {@link ServerStreamTracer.Factory}. If not set, this returns null. */ static synchronized List getServerStreamTracerFactories() { isGlobalInterceptorsTracersGet = true; return serverStreamTracerFactories; diff --git a/gcp-observability/build.gradle b/gcp-observability/build.gradle index a78640091da..c4f93afbd17 100644 --- a/gcp-observability/build.gradle +++ b/gcp-observability/build.gradle @@ -30,6 +30,7 @@ dependencies { project(':grpc-alts'), project(':grpc-census'), ("com.google.cloud:google-cloud-logging:${cloudLoggingVersion}"), + libraries.opencensus.contrib.grpc.metrics, libraries.opencensus.exporter.stats.stackdriver, libraries.opencensus.exporter.trace.stackdriver, libraries.animalsniffer.annotations, // Prefer our version diff --git a/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java b/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java index aa5641f6fcf..4bb9ffcb0c1 100644 --- a/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java +++ b/gcp-observability/src/main/java/io/grpc/gcp/observability/GcpObservability.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkNotNull; -import com.google.api.client.util.Strings; import com.google.common.annotations.VisibleForTesting; import io.grpc.ClientInterceptor; import io.grpc.ExperimentalApi; @@ -69,22 +68,16 @@ public static synchronized GcpObservability grpcInit() throws IOException { if (instance == null) { GlobalLoggingTags globalLoggingTags = new GlobalLoggingTags(); ObservabilityConfigImpl observabilityConfig = ObservabilityConfigImpl.getInstance(); - Sink sink = - new GcpLogSink( - observabilityConfig.getDestinationProjectId(), - globalLoggingTags.getLocationTags(), - globalLoggingTags.getCustomTags(), - observabilityConfig.getFlushMessageCount()); + Sink sink = new GcpLogSink(observabilityConfig.getDestinationProjectId(), + globalLoggingTags.getLocationTags(), globalLoggingTags.getCustomTags(), + observabilityConfig.getFlushMessageCount()); // TODO(dnvindhya): Cleanup code for LoggingChannelProvider and LoggingServerProvider // once ChannelBuilder and ServerBuilder are used LogHelper helper = new LogHelper(sink, TimeProvider.SYSTEM_TIME_PROVIDER); ConfigFilterHelper configFilterHelper = ConfigFilterHelper.factory(observabilityConfig); - instance = - grpcInit( - sink, - observabilityConfig, - new InternalLoggingChannelInterceptor.FactoryImpl(helper, configFilterHelper), - new InternalLoggingServerInterceptor.FactoryImpl(helper, configFilterHelper)); + instance = grpcInit(sink, observabilityConfig, + new InternalLoggingChannelInterceptor.FactoryImpl(helper, configFilterHelper), + new InternalLoggingServerInterceptor.FactoryImpl(helper, configFilterHelper)); instance.registerStackDriverExporter(observabilityConfig.getDestinationProjectId()); } return instance; @@ -142,12 +135,8 @@ private void setProducer( tracerFactories.add(InternalCensusTracingAccessor.getServerStreamTracerFactory()); } - if (!clientInterceptors.isEmpty() - || !serverInterceptors.isEmpty() - || !tracerFactories.isEmpty()) { - InternalGlobalInterceptors.setInterceptorsTracers( - clientInterceptors, serverInterceptors, tracerFactories); - } + InternalGlobalInterceptors.setInterceptorsTracers( + clientInterceptors, serverInterceptors, tracerFactories); } private void registerStackDriverExporter(String projectId) throws IOException { @@ -155,14 +144,10 @@ private void registerStackDriverExporter(String projectId) throws IOException { RpcViews.registerAllGrpcViews(); StackdriverStatsConfiguration.Builder statsConfigurationBuilder = StackdriverStatsConfiguration.builder(); - if (!Strings.isNullOrEmpty(projectId)) { + if (projectId != null) { statsConfigurationBuilder.setProjectId(projectId); } - try { - StackdriverStatsExporter.createAndRegister(statsConfigurationBuilder.build()); - } catch (IOException e) { - throw new IOException("Failed to register Stackdriver stats exporter, " + e.getMessage()); - } + StackdriverStatsExporter.createAndRegister(statsConfigurationBuilder.build()); metricsEnabled = true; } @@ -172,14 +157,10 @@ private void registerStackDriverExporter(String projectId) throws IOException { traceConfig.getActiveTraceParams().toBuilder().setSampler(config.getSampler()).build()); StackdriverTraceConfiguration.Builder traceConfigurationBuilder = StackdriverTraceConfiguration.builder(); - if (!Strings.isNullOrEmpty(projectId)) { + if (projectId != null) { traceConfigurationBuilder.setProjectId(projectId); } - try { - StackdriverTraceExporter.createAndRegister(traceConfigurationBuilder.build()); - } catch (IOException e) { - throw new IOException("Failed to register Stackdriver trace exporter, " + e.getMessage()); - } + StackdriverTraceExporter.createAndRegister(traceConfigurationBuilder.build()); tracesEnabled = true; } } diff --git a/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java b/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java index 84a965bb40f..08da2fede80 100644 --- a/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java +++ b/gcp-observability/src/main/java/io/grpc/gcp/observability/ObservabilityConfigImpl.java @@ -106,11 +106,15 @@ private void parseConfig(Map config) { if (enableCloudTracing && samplingRate == null) { this.sampler = Samplers.probabilitySampler(0.0); } + double epsilon = 1e-6; if (samplingRate != null) { checkArgument( samplingRate >= 0.0 && samplingRate <= 1.0, "'global_trace_sampling_rate' needs to be between [0.0, 1.0]"); - if (samplingRate == 1.0) { + // Using alwaysSample() instead of probabilitySampler() because according to + // {@link io.opencensus.trace.samplers.ProbabilitySampler#shouldSample} + // there is a (very) small chance of *not* sampling if probability = 1.00. + if (Math.abs(1 - samplingRate) < epsilon) { this.sampler = Samplers.alwaysSample(); } else { this.sampler = Samplers.probabilitySampler(samplingRate); diff --git a/gcp-observability/src/test/java/io/grpc/gcp/observability/GcpObservabilityTest.java b/gcp-observability/src/test/java/io/grpc/gcp/observability/GcpObservabilityTest.java index 4397729f88f..87601247a42 100644 --- a/gcp-observability/src/test/java/io/grpc/gcp/observability/GcpObservabilityTest.java +++ b/gcp-observability/src/test/java/io/grpc/gcp/observability/GcpObservabilityTest.java @@ -28,18 +28,21 @@ import io.grpc.ClientCall; import io.grpc.ClientInterceptor; import io.grpc.InternalGlobalInterceptors; +// import io.grpc.ManagedChannelProvider; import io.grpc.ManagedChannelProvider; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.ServerCall; import io.grpc.ServerCallHandler; import io.grpc.ServerInterceptor; +// import io.grpc.ServerProvider; import io.grpc.ServerProvider; import io.grpc.StaticTestingClassLoader; import io.grpc.gcp.observability.interceptors.InternalLoggingChannelInterceptor; import io.grpc.gcp.observability.interceptors.InternalLoggingServerInterceptor; import io.grpc.gcp.observability.logging.Sink; import io.opencensus.trace.samplers.Samplers; +import java.io.IOException; import java.util.regex.Pattern; import org.junit.Test; import org.junit.runner.RunWith; @@ -48,51 +51,77 @@ @RunWith(JUnit4.class) public class GcpObservabilityTest { - private static final String PROJECT_ID = "project"; + private final StaticTestingClassLoader classLoader = + new StaticTestingClassLoader( + getClass().getClassLoader(), + Pattern.compile( + "io\\.grpc\\.InternalGlobalInterceptors|io\\.grpc\\.GlobalInterceptors|" + + "io\\.grpc\\.gcp\\.observability\\.[^.]+|" + + "io\\.grpc\\.gcp\\.observability\\.interceptors\\.[^.]+|" + + "io\\.grpc\\.gcp\\.observability\\.GcpObservabilityTest\\$.*")); @Test public void initFinish() throws Exception { - ManagedChannelProvider prevChannelProvider = ManagedChannelProvider.provider(); - ServerProvider prevServerProvider = ServerProvider.provider(); - Sink sink = mock(Sink.class); - ObservabilityConfig config = mock(ObservabilityConfig.class); - InternalLoggingChannelInterceptor.Factory channelInterceptorFactory = - mock(InternalLoggingChannelInterceptor.Factory.class); - InternalLoggingServerInterceptor.Factory serverInterceptorFactory = - mock(InternalLoggingServerInterceptor.Factory.class); - GcpObservability observability1; - try (GcpObservability observability = - GcpObservability.grpcInit( - sink, config, channelInterceptorFactory, serverInterceptorFactory)) { - assertThat(ManagedChannelProvider.provider()).isInstanceOf(LoggingChannelProvider.class); - assertThat(ServerProvider.provider()).isInstanceOf(ServerProvider.class); - observability1 = - GcpObservability.grpcInit( - sink, config, channelInterceptorFactory, serverInterceptorFactory); - assertThat(observability1).isSameInstanceAs(observability); - } - verify(sink).close(); - assertThat(ManagedChannelProvider.provider()).isSameInstanceAs(prevChannelProvider); - assertThat(ServerProvider.provider()).isSameInstanceAs(prevServerProvider); - try { - observability1.close(); - fail("should have failed for calling close() second time"); - } catch (IllegalStateException e) { - assertThat(e).hasMessageThat().contains("GcpObservability already closed!"); - } + Class runnable = + classLoader.loadClass(StaticTestingClassInitFinish.class.getName()); + ((Runnable) runnable.getDeclaredConstructor().newInstance()).run(); } @Test public void enableObservability() throws Exception { - StaticTestingClassLoader classLoader = - new StaticTestingClassLoader( - getClass().getClassLoader(), Pattern.compile("io\\.grpc\\.[^.]+")); - Class runnable = classLoader.loadClass(StaticTestingClassLoaderSet.class.getName()); + Class runnable = + classLoader.loadClass(StaticTestingClassEnableObservability.class.getName()); + ((Runnable) runnable.getDeclaredConstructor().newInstance()).run(); + } + + @Test + public void disableObservability() throws Exception { + Class runnable = + classLoader.loadClass(StaticTestingClassDisableObservability.class.getName()); ((Runnable) runnable.getDeclaredConstructor().newInstance()).run(); } // UsedReflectively - public static final class StaticTestingClassLoaderSet implements Runnable { + public static final class StaticTestingClassInitFinish implements Runnable { + + @Override + public void run() { + ManagedChannelProvider prevChannelProvider = ManagedChannelProvider.provider(); + ServerProvider prevServerProvider = ServerProvider.provider(); + Sink sink = mock(Sink.class); + ObservabilityConfig config = mock(ObservabilityConfig.class); + InternalLoggingChannelInterceptor.Factory channelInterceptorFactory = + mock(InternalLoggingChannelInterceptor.Factory.class); + InternalLoggingServerInterceptor.Factory serverInterceptorFactory = + mock(InternalLoggingServerInterceptor.Factory.class); + GcpObservability observability1; + try { + GcpObservability observability = + GcpObservability.grpcInit( + sink, config, channelInterceptorFactory, serverInterceptorFactory); + assertThat(ManagedChannelProvider.provider()).isInstanceOf(LoggingChannelProvider.class); + assertThat(ServerProvider.provider()).isInstanceOf(ServerProvider.class); + observability1 = + GcpObservability.grpcInit( + sink, config, channelInterceptorFactory, serverInterceptorFactory); + assertThat(observability1).isSameInstanceAs(observability); + observability.close(); + verify(sink).close(); + assertThat(ManagedChannelProvider.provider()).isSameInstanceAs(prevChannelProvider); + assertThat(ServerProvider.provider()).isSameInstanceAs(prevServerProvider); + try { + observability1.close(); + fail("should have failed for calling close() second time"); + } catch (IllegalStateException e) { + assertThat(e).hasMessageThat().contains("GcpObservability already closed!"); + } + } catch (IOException e) { + fail("Encountered exception: " + e); + } + } + } + + public static final class StaticTestingClassEnableObservability implements Runnable { @Override public void run() { @@ -102,7 +131,6 @@ public void run() { when(config.isEnableCloudMonitoring()).thenReturn(true); when(config.isEnableCloudTracing()).thenReturn(true); when(config.getSampler()).thenReturn(Samplers.neverSample()); - when(config.getDestinationProjectId()).thenReturn(PROJECT_ID); ClientInterceptor clientInterceptor = mock(ClientInterceptor.class, delegatesTo(new NoopClientInterceptor())); @@ -128,6 +156,35 @@ public void run() { } } + public static final class StaticTestingClassDisableObservability implements Runnable { + + @Override + public void run() { + Sink sink = mock(Sink.class); + ObservabilityConfig config = mock(ObservabilityConfig.class); + when(config.isEnableCloudLogging()).thenReturn(false); + when(config.isEnableCloudMonitoring()).thenReturn(false); + when(config.isEnableCloudTracing()).thenReturn(false); + when(config.getSampler()).thenReturn(Samplers.neverSample()); + + InternalLoggingChannelInterceptor.Factory channelInterceptorFactory = + mock(InternalLoggingChannelInterceptor.Factory.class); + InternalLoggingServerInterceptor.Factory serverInterceptorFactory = + mock(InternalLoggingServerInterceptor.Factory.class);; + + try (GcpObservability observability = + GcpObservability.grpcInit( + sink, config, channelInterceptorFactory, serverInterceptorFactory)) { + assertThat(InternalGlobalInterceptors.getClientInterceptors()).isEmpty(); + assertThat(InternalGlobalInterceptors.getServerInterceptors()).isEmpty(); + assertThat(InternalGlobalInterceptors.getServerStreamTracerFactories()).isEmpty(); + } catch (Exception e) { + fail("Encountered exception: " + e); + } + verify(sink).close(); + } + } + private static class NoopClientInterceptor implements ClientInterceptor { @Override public ClientCall interceptCall(