Exporter/OcAgent: Make required fields non-null. (#1895)

* Exporter/OcAgent: Make required fields non-null.

* Add Javadoc for default values.

Author: songy23

exporters/metrics/ocagent/src/main/java/io/opencensus/exporter/metrics/ocagent/OcAgentMetricsExporter.java

@@ -20,7 +20,6 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
-import com.google.common.annotations.VisibleForTesting;
 import io.netty.handler.ssl.SslContext;
 import io.opencensus.common.Duration;
 import io.opencensus.metrics.Metrics;
@@ -47,12 +46,6 @@
 @ThreadSafe
 public final class OcAgentMetricsExporter {
 
-  @VisibleForTesting static final String DEFAULT_END_POINT = "localhost:55678";
-  private static final String DEFAULT_SERVICE_NAME = "OpenCensus";
-  private static final Duration DEFAULT_RETRY_INTERVAL = Duration.create(300, 0); // 5 minutes
-  private static final Duration DEFAULT_EXPORT_INTERVAL = Duration.create(60, 0); // 1 minute
-  private static final Duration ZERO = Duration.create(0, 0);
-
   private static final Object monitor = new Object();
 
   @GuardedBy("monitor")
@@ -80,29 +73,14 @@ public static void createAndRegister(OcAgentMetricsExporterConfiguration configu
   }
 
   private static void createInternal(
-      @Nullable String endPoint,
-      @Nullable Boolean useInsecure,
+      String endPoint,
+      boolean useInsecure,
       @Nullable SslContext sslContext,
-      @Nullable String serviceName,
-      @Nullable Duration exportInterval,
-      @Nullable Duration retryInterval) {
-    if (endPoint == null || endPoint.isEmpty()) {
-      endPoint = DEFAULT_END_POINT;
-    }
-    if (useInsecure == null) {
-      useInsecure = false;
-    }
+      String serviceName,
+      Duration exportInterval,
+      Duration retryInterval) {
     checkArgument(
         useInsecure == (sslContext == null), "Either use insecure or provide a valid SslContext.");
-    if (serviceName == null || serviceName.isEmpty()) {
-      serviceName = DEFAULT_SERVICE_NAME;
-    }
-    if (exportInterval == null) {
-      exportInterval = DEFAULT_EXPORT_INTERVAL;
-    }
-    if (retryInterval == null) {
-      retryInterval = DEFAULT_RETRY_INTERVAL;
-    }
     synchronized (monitor) {
       checkState(exporter == null, "OcAgent Metrics exporter is already created.");
       exporter =
@@ -126,8 +104,6 @@ private OcAgentMetricsExporter(
       Duration exportInterval,
       Duration retryInterval,
       MetricProducerManager metricProducerManager) {
-    checkArgument(exportInterval.compareTo(ZERO) > 0, "Duration must be positive");
-    checkArgument(retryInterval.compareTo(ZERO) > 0, "Duration must be positive");
     OcAgentMetricsExporterWorker worker =
         new OcAgentMetricsExporterWorker(
             endPoint,

exporters/metrics/ocagent/src/main/java/io/opencensus/exporter/metrics/ocagent/OcAgentMetricsExporterConfiguration.java

@@ -17,6 +17,8 @@
 package io.opencensus.exporter.metrics.ocagent;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import io.netty.handler.ssl.SslContext;
 import io.opencensus.common.Duration;
 import javax.annotation.Nullable;
@@ -31,29 +33,39 @@
 @Immutable
 public abstract class OcAgentMetricsExporterConfiguration {
 
+  @VisibleForTesting static final String DEFAULT_END_POINT = "localhost:55678";
+  @VisibleForTesting static final String DEFAULT_SERVICE_NAME = "OpenCensus";
+  @VisibleForTesting static final Duration DEFAULT_RETRY_INTERVAL = Duration.create(300, 0);
+  @VisibleForTesting static final Duration DEFAULT_EXPORT_INTERVAL = Duration.create(60, 0);
+  @VisibleForTesting static final Duration ZERO = Duration.create(0, 0);
+
   OcAgentMetricsExporterConfiguration() {}
 
   /**
    * Returns the end point of OC-Agent. The end point can be dns, ip:port, etc.
    *
+   * <p>Default value is "localhost:55678" if not set.
+   *
    * @return the end point of OC-Agent.
    * @since 0.20
    */
-  @Nullable
   public abstract String getEndPoint();
 
   /**
    * Returns whether to disable client transport security for the exporter's gRPC connection or not.
    *
+   * <p>Default value is true if not set.
+   *
    * @return whether to disable client transport security for the exporter's gRPC connection or not.
    * @since 0.20
    */
-  @Nullable
   public abstract Boolean getUseInsecure();
 
   /**
    * Returns the {@link SslContext} for secure TLS gRPC connection.
    *
+   * <p>If not set OcAgent exporter will use insecure connection by default.
+   *
    * @return the {@code SslContext}.
    * @since 0.20
    */
@@ -63,28 +75,31 @@ public abstract class OcAgentMetricsExporterConfiguration {
   /**
    * Returns the service name to be used for the {@code OcAgentMetricsExporter}.
    *
+   * <p>Default value is "OpenCensus" if not set.
+   *
    * @return the service name.
    * @since 0.20
    */
-  @Nullable
   public abstract String getServiceName();
 
   /**
    * Returns the retry time interval when trying to connect to Agent.
    *
+   * <p>Default value is 5 minutes.
+   *
    * @return the retry time interval.
    * @since 0.20
    */
-  @Nullable
   public abstract Duration getRetryInterval();
 
   /**
    * Returns the export interval between pushes to Agent.
    *
+   * <p>Default value is 1 minute.
+   *
    * @return the export interval.
    * @since 0.20
    */
-  @Nullable
   public abstract Duration getExportInterval();
 
   /**
@@ -94,7 +109,12 @@ public abstract class OcAgentMetricsExporterConfiguration {
    * @since 0.20
    */
   public static Builder builder() {
-    return new AutoValue_OcAgentMetricsExporterConfiguration.Builder().setUseInsecure(true);
+    return new AutoValue_OcAgentMetricsExporterConfiguration.Builder()
+        .setEndPoint(DEFAULT_END_POINT)
+        .setServiceName(DEFAULT_SERVICE_NAME)
+        .setRetryInterval(DEFAULT_RETRY_INTERVAL)
+        .setExportInterval(DEFAULT_EXPORT_INTERVAL)
+        .setUseInsecure(true);
   }
 
   /**
@@ -164,12 +184,24 @@ public abstract static class Builder {
 
     // TODO(songya): add an option that controls whether to always keep the RPC connection alive.
 
+    abstract Duration getRetryInterval();
+
+    abstract Duration getExportInterval();
+
+    abstract OcAgentMetricsExporterConfiguration autoBuild();
+
     /**
      * Builds a {@link OcAgentMetricsExporterConfiguration}.
      *
      * @return a {@code OcAgentMetricsExporterConfiguration}.
      * @since 0.20
      */
-    public abstract OcAgentMetricsExporterConfiguration build();
+    public OcAgentMetricsExporterConfiguration build() {
+      Preconditions.checkArgument(
+          getRetryInterval().compareTo(ZERO) > 0, "Retry interval must be positive.");
+      Preconditions.checkArgument(
+          getExportInterval().compareTo(ZERO) > 0, "Export interval must be positive.");
+      return autoBuild();
+    }
   }
 }

exporters/metrics/ocagent/src/test/java/io/opencensus/exporter/metrics/ocagent/OcAgentMetricsExporterConfigurationTest.java

@@ -34,12 +34,16 @@ public class OcAgentMetricsExporterConfigurationTest {
   public void defaultConfiguration() {
     OcAgentMetricsExporterConfiguration configuration =
         OcAgentMetricsExporterConfiguration.builder().build();
-    assertThat(configuration.getEndPoint()).isNull();
-    assertThat(configuration.getServiceName()).isNull();
+    assertThat(configuration.getEndPoint())
+        .isEqualTo(OcAgentMetricsExporterConfiguration.DEFAULT_END_POINT);
+    assertThat(configuration.getServiceName())
+        .isEqualTo(OcAgentMetricsExporterConfiguration.DEFAULT_SERVICE_NAME);
     assertThat(configuration.getUseInsecure()).isTrue();
     assertThat(configuration.getSslContext()).isNull();
-    assertThat(configuration.getRetryInterval()).isNull();
-    assertThat(configuration.getExportInterval()).isNull();
+    assertThat(configuration.getRetryInterval())
+        .isEqualTo(OcAgentMetricsExporterConfiguration.DEFAULT_RETRY_INTERVAL);
+    assertThat(configuration.getExportInterval())
+        .isEqualTo(OcAgentMetricsExporterConfiguration.DEFAULT_EXPORT_INTERVAL);
   }
 
   @Test

exporters/metrics/ocagent/src/test/java/io/opencensus/exporter/metrics/ocagent/OcAgentMetricsExporterIntegrationTest.java

@@ -181,7 +181,9 @@ public class OcAgentMetricsExporterIntegrationTest {
   @Before
   public void setUp() {
     fakeOcAgentMetricsServiceGrpc = new FakeOcAgentMetricsServiceGrpcImpl();
-    agent = getServer(OcAgentMetricsExporter.DEFAULT_END_POINT, fakeOcAgentMetricsServiceGrpc);
+    agent =
+        getServer(
+            OcAgentMetricsExporterConfiguration.DEFAULT_END_POINT, fakeOcAgentMetricsServiceGrpc);
   }
 
   @After

exporters/trace/ocagent/build.gradle

@@ -9,7 +9,8 @@ dependencies {
     compileOnly libraries.auto_value
 
     compile project(':opencensus-api'),
-            project(':opencensus-contrib-resource-util')
+            project(':opencensus-contrib-resource-util'),
+            project(':opencensus-exporter-trace-util')
 
     compile (libraries.grpc_core) {
         // We will always be more up to date.

exporters/trace/ocagent/src/main/java/io/opencensus/exporter/trace/ocagent/OcAgentTraceExporter.java

@@ -59,11 +59,7 @@ private OcAgentTraceExporter() {}
    * @since 0.20
    */
   public static void createAndRegister() {
-    synchronized (monitor) {
-      checkState(handler == null, "OC-Agent exporter is already registered.");
-      OcAgentTraceExporterHandler newHandler = new OcAgentTraceExporterHandler();
-      registerInternal(newHandler);
-    }
+    createAndRegister(OcAgentTraceExporterConfiguration.builder().build());
   }
 
   /**

exporters/trace/ocagent/src/main/java/io/opencensus/exporter/trace/ocagent/OcAgentTraceExporterConfiguration.java

@@ -17,6 +17,8 @@
 package io.opencensus.exporter.trace.ocagent;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import io.netty.handler.ssl.SslContext;
 import io.opencensus.common.Duration;
 import javax.annotation.Nullable;
@@ -31,29 +33,38 @@
 @Immutable
 public abstract class OcAgentTraceExporterConfiguration {
 
+  @VisibleForTesting static final String DEFAULT_END_POINT = "localhost:55678";
+  @VisibleForTesting static final String DEFAULT_SERVICE_NAME = "OpenCensus";
+  @VisibleForTesting static final Duration DEFAULT_RETRY_INTERVAL = Duration.create(300, 0);
+  @VisibleForTesting static final Duration ZERO = Duration.create(0, 0);
+
   OcAgentTraceExporterConfiguration() {}
 
   /**
    * Returns the end point of OC-Agent. The end point can be dns, ip:port, etc.
    *
+   * <p>Default value is "localhost:55678" if not set.
+   *
    * @return the end point of OC-Agent.
    * @since 0.20
    */
-  @Nullable
   public abstract String getEndPoint();
 
   /**
    * Returns whether to disable client transport security for the exporter's gRPC connection or not.
    *
+   * <p>Default value is true if not set.
+   *
    * @return whether to disable client transport security for the exporter's gRPC connection or not.
    * @since 0.20
    */
-  @Nullable
   public abstract Boolean getUseInsecure();
 
   /**
    * Returns the {@link SslContext} for secure TLS gRPC connection.
    *
+   * <p>If not set OcAgent exporter will use insecure connection by default.
+   *
    * @return the {@code SslContext}.
    * @since 0.20
    */
@@ -63,24 +74,28 @@ public abstract class OcAgentTraceExporterConfiguration {
   /**
    * Returns the service name to be used for this {@link OcAgentTraceExporter}.
    *
+   * <p>Default value is "OpenCensus" if not set.
+   *
    * @return the service name.
    * @since 0.20
    */
-  @Nullable
   public abstract String getServiceName();
 
   /**
    * Returns the retry time interval when trying to connect to Agent.
    *
+   * <p>Default value is 5 minutes.
+   *
    * @return the retry time interval.
    * @since 0.20
    */
-  @Nullable
   public abstract Duration getRetryInterval();
 
   /**
    * Returns whether the {@link OcAgentTraceExporter} should handle the config streams.
    *
+   * <p>Config service is enabled by default.
+   *
    * @return whether the {@code OcAgentTraceExporter} should handle the config streams.
    * @since 0.20
    */
@@ -94,8 +109,11 @@ public abstract class OcAgentTraceExporterConfiguration {
    */
   public static Builder builder() {
     return new AutoValue_OcAgentTraceExporterConfiguration.Builder()
+        .setEndPoint(DEFAULT_END_POINT)
+        .setServiceName(DEFAULT_SERVICE_NAME)
         .setEnableConfig(true)
-        .setUseInsecure(true);
+        .setUseInsecure(true)
+        .setRetryInterval(DEFAULT_RETRY_INTERVAL);
   }
 
   /**
@@ -165,12 +183,20 @@ public abstract static class Builder {
 
     // TODO(songya): add an option that controls whether to always keep the RPC connection alive.
 
+    abstract Duration getRetryInterval();
+
+    abstract OcAgentTraceExporterConfiguration autoBuild();
+
     /**
      * Builds a {@link OcAgentTraceExporterConfiguration}.
      *
      * @return a {@code OcAgentTraceExporterConfiguration}.
      * @since 0.20
      */
-    public abstract OcAgentTraceExporterConfiguration build();
+    public OcAgentTraceExporterConfiguration build() {
+      Preconditions.checkArgument(
+          getRetryInterval().compareTo(ZERO) > 0, "Retry interval must be positive.");
+      return autoBuild();
+    }
   }
 }