Add the option to configure the zipkin exporter timeout. (open-telemetry#3203)

Author: jkwatson

exporters/zipkin/src/main/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterBuilder.java

@@ -5,6 +5,11 @@
 
 package io.opentelemetry.exporter.zipkin;
 
+import static io.opentelemetry.api.internal.Utils.checkArgument;
+import static java.util.Objects.requireNonNull;
+
+import java.time.Duration;
+import java.util.concurrent.TimeUnit;
 import zipkin2.Span;
 import zipkin2.codec.BytesEncoder;
 import zipkin2.codec.SpanBytesEncoder;
@@ -16,6 +21,7 @@ public final class ZipkinSpanExporterBuilder {
   private BytesEncoder<Span> encoder = SpanBytesEncoder.JSON_V2;
   private Sender sender;
   private String endpoint = ZipkinSpanExporter.DEFAULT_ENDPOINT;
+  private long readTimeoutMillis = TimeUnit.SECONDS.toMillis(10);
 
   /**
    * Sets the Zipkin sender. Implements the client side of the span transport. A {@link
@@ -27,6 +33,7 @@ public final class ZipkinSpanExporterBuilder {
    * @return this.
    */
   public ZipkinSpanExporterBuilder setSender(Sender sender) {
+    requireNonNull(sender, "sender");
     this.sender = sender;
     return this;
   }
@@ -40,6 +47,7 @@ public ZipkinSpanExporterBuilder setSender(Sender sender) {
    * @see SpanBytesEncoder
    */
   public ZipkinSpanExporterBuilder setEncoder(BytesEncoder<Span> encoder) {
+    requireNonNull(encoder, "encoder");
     this.encoder = encoder;
     return this;
   }
@@ -53,18 +61,43 @@ public ZipkinSpanExporterBuilder setEncoder(BytesEncoder<Span> encoder) {
    * @see OkHttpSender
    */
   public ZipkinSpanExporterBuilder setEndpoint(String endpoint) {
+    requireNonNull(endpoint, "endpoint");
     this.endpoint = endpoint;
     return this;
   }
 
+  /**
+   * Sets the maximum time to wait for the export of a batch of spans. If unset, defaults to 10s.
+   *
+   * @return this.
+   */
+  public ZipkinSpanExporterBuilder setReadTimeout(long timeout, TimeUnit unit) {
+    requireNonNull(unit, "unit");
+    checkArgument(timeout >= 0, "timeout must be non-negative");
+    this.readTimeoutMillis = unit.toMillis(timeout);
+    return this;
+  }
+
+  /**
+   * Sets the maximum time to wait for the export of a batch of spans. If unset, defaults to 10s.
+   *
+   * @return this.
+   */
+  public ZipkinSpanExporterBuilder setReadTimeout(Duration timeout) {
+    requireNonNull(timeout, "timeout");
+    setReadTimeout(timeout.toMillis(), TimeUnit.MILLISECONDS);
+    return this;
+  }
+
   /**
    * Builds a {@link ZipkinSpanExporter}.
    *
    * @return a {@code ZipkinSpanExporter}.
    */
   public ZipkinSpanExporter build() {
     if (sender == null) {
-      sender = OkHttpSender.create(endpoint);
+      sender =
+          OkHttpSender.newBuilder().endpoint(endpoint).readTimeout((int) readTimeoutMillis).build();
     }
     return new ZipkinSpanExporter(this.encoder, this.sender);
   }

exporters/zipkin/src/test/java/io/opentelemetry/exporter/zipkin/ZipkinSpanExporterTest.java

@@ -14,6 +14,7 @@
 import static io.opentelemetry.api.common.AttributeKey.stringArrayKey;
 import static io.opentelemetry.api.common.AttributeKey.stringKey;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.verify;
@@ -38,6 +39,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
@@ -423,4 +425,32 @@ private Span.Builder standardZipkinSpanBuilder(Span.Kind kind) {
         .addAnnotation(1505855799000000L + 433901068L / 1000, "RECEIVED")
         .addAnnotation(1505855799000000L + 459486280L / 1000, "SENT");
   }
+
+  @Test
+  @SuppressWarnings("PreferJavaTimeOverload")
+  void invalidConfig() {
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(-1, TimeUnit.MILLISECONDS))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("timeout must be non-negative");
+
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(1, null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("unit");
+
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setReadTimeout(null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("timeout");
+
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setEndpoint(null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("endpoint");
+
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setSender(null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("sender");
+
+    assertThatThrownBy(() -> ZipkinSpanExporter.builder().setEncoder(null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("encoder");
+  }
 }

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfiguration.java

@@ -135,6 +135,11 @@ private static SpanExporter configureZipkin(ConfigProperties config) {
       builder.setEndpoint(endpoint);
     }
 
+    Duration timeout = config.getDuration("otel.exporter.zipkin.timeout");
+    if (timeout != null) {
+      builder.setReadTimeout(timeout);
+    }
+
     return builder.build();
   }
 

sdk-extensions/autoconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/SpanExporterConfigurationTest.java

@@ -57,4 +57,19 @@ void configureJaegerTimeout() {
       exporter.shutdown();
     }
   }
+
+  // Timeout difficult to test using real exports so just check that things don't blow up.
+  @Test
+  void configureZipkinTimeout() {
+    SpanExporter exporter =
+        SpanExporterConfiguration.configureExporter(
+            "zipkin",
+            ConfigProperties.createForTest(
+                Collections.singletonMap("otel.exporter.zipkin.timeout", "5s")));
+    try {
+      assertThat(exporter).isNotNull();
+    } finally {
+      exporter.shutdown();
+    }
+  }
 }

sdk-extensions/autoconfigure/src/testZipkin/java/io/opentelemetry/sdk/autoconfigure/ZipkinConfigTest.java

@@ -52,6 +52,7 @@ void configures() {
     String endpoint = "localhost:" + server.httpPort();
 
     System.setProperty("otel.exporter.zipkin.endpoint", "http://" + endpoint + "/api/v2/spans");
+    System.setProperty("otel.exporter.zipkin.timeout", "5s");
 
     OpenTelemetrySdkAutoConfiguration.initialize();