Adds ES_TIMEOUT to override the default 10second ES api request timeout

Adrian Cole · adriancole · commit 75cbb04a9030 · 2017-11-24T13:26:14.000+08:00
Implicitly, OkHttp defaults to 10seconds per request. This makes it possible to override that for lengthy searches. `ES_TIMEOUT` accepts milliseconds and is used for connect, read and write socket timeouts. Fixes openzipkin#1814
diff --git a/zipkin-autoconfigure/storage-elasticsearch-http/src/main/java/zipkin/autoconfigure/storage/elasticsearch/http/ZipkinElasticsearchHttpStorageProperties.java b/zipkin-autoconfigure/storage-elasticsearch-http/src/main/java/zipkin/autoconfigure/storage/elasticsearch/http/ZipkinElasticsearchHttpStorageProperties.java
@@ -52,6 +52,11 @@ public class ZipkinElasticsearchHttpStorageProperties implements Serializable {
   private HttpLoggingInterceptor.Level httpLogging;
   /** When true, Redundantly queries indexes made with pre v1.31 collectors. Defaults to true. */
   private boolean legacyReadsEnabled = true;
+  /**
+   * Controls the connect, read and write socket timeouts (in milliseconds) for Elasticsearch Api
+   * requests. Defaults to 10000 (10 seconds)
+   */
+  private int timeout = 10_000;
 
   public String getPipeline() {
     return pipeline;
@@ -161,6 +166,14 @@ public void setLegacyReadsEnabled(boolean legacyReadsEnabled) {
     this.legacyReadsEnabled = legacyReadsEnabled;
   }
 
+  public int getTimeout() {
+    return timeout;
+  }
+
+  public void setTimeout(int timeout) {
+    this.timeout = timeout;
+  }
+
   public ElasticsearchHttpStorage.Builder toBuilder(OkHttpClient client) {
     ElasticsearchHttpStorage.Builder builder = ElasticsearchHttpStorage.builder(client);
     if (hosts != null) builder.hosts(hosts);
diff --git a/zipkin-autoconfigure/storage-elasticsearch-http/src/main/java/zipkin/autoconfigure/storage/elasticsearch/http/ZipkinElasticsearchOkHttpAutoConfiguration.java b/zipkin-autoconfigure/storage-elasticsearch-http/src/main/java/zipkin/autoconfigure/storage/elasticsearch/http/ZipkinElasticsearchOkHttpAutoConfiguration.java
@@ -15,10 +15,12 @@
 
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.TimeUnit;
 import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
@@ -53,14 +55,19 @@ public class ZipkinElasticsearchOkHttpAutoConfiguration {
   @Bean
   @Qualifier("zipkinElasticsearchHttp")
   @ConditionalOnMissingBean
-  OkHttpClient elasticsearchOkHttpClient() {
+  OkHttpClient elasticsearchOkHttpClient(
+    @Value("${zipkin.storage.elasticsearch.timeout:10000}") int timeout
+  ) {
     OkHttpClient.Builder builder = elasticsearchOkHttpClientBuilder != null
         ? elasticsearchOkHttpClientBuilder
         : new OkHttpClient.Builder();
 
     for (Interceptor interceptor : networkInterceptors) {
       builder.addNetworkInterceptor(interceptor);
     }
+    builder.connectTimeout(timeout, TimeUnit.MILLISECONDS);
+    builder.readTimeout(timeout, TimeUnit.MILLISECONDS);
+    builder.writeTimeout(timeout, TimeUnit.MILLISECONDS);
     return builder.build();
   }
 }
diff --git a/zipkin-autoconfigure/storage-elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ZipkinElasticsearchHttpStorageAutoConfigurationTest.java b/zipkin-autoconfigure/storage-elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ZipkinElasticsearchHttpStorageAutoConfigurationTest.java
@@ -207,6 +207,50 @@ public void usesInterceptorsQualifiedWith_zipkinElasticsearchHttp() {
         .containsOnlyOnce(InterceptorConfiguration.one, InterceptorConfiguration.two);
   }
 
+  @Test
+  public void timeout_defaultsTo10Seconds() {
+    context = new AnnotationConfigApplicationContext();
+    addEnvironment(context,
+      "zipkin.storage.type:elasticsearch",
+      "zipkin.storage.elasticsearch.hosts:http://host1:9200"
+    );
+    context.register(PropertyPlaceholderAutoConfiguration.class,
+      ZipkinElasticsearchOkHttpAutoConfiguration.class,
+      InterceptorConfiguration.class);
+    context.refresh();
+
+    OkHttpClient client = context.getBean(OkHttpClient.class);
+    assertThat(client.connectTimeoutMillis())
+      .isEqualTo(10_000);
+    assertThat(client.readTimeoutMillis())
+      .isEqualTo(10_000);
+    assertThat(client.writeTimeoutMillis())
+      .isEqualTo(10_000);
+  }
+
+  @Test
+  public void timeout_override() {
+    context = new AnnotationConfigApplicationContext();
+    int timeout = 30_000;
+    addEnvironment(context,
+      "zipkin.storage.type:elasticsearch",
+      "zipkin.storage.elasticsearch.hosts:http://host1:9200",
+      "zipkin.storage.elasticsearch.timeout:" + timeout
+    );
+    context.register(PropertyPlaceholderAutoConfiguration.class,
+      ZipkinElasticsearchOkHttpAutoConfiguration.class,
+      InterceptorConfiguration.class);
+    context.refresh();
+
+    OkHttpClient client = context.getBean(OkHttpClient.class);
+    assertThat(client.connectTimeoutMillis())
+      .isEqualTo(timeout);
+    assertThat(client.readTimeoutMillis())
+      .isEqualTo(timeout);
+    assertThat(client.writeTimeoutMillis())
+      .isEqualTo(timeout);
+  }
+
   @Test
   public void strictTraceId_defaultsToTrue() {
     context = new AnnotationConfigApplicationContext();
diff --git a/zipkin-server/README.md b/zipkin-server/README.md
@@ -175,6 +175,8 @@ The following apply when `STORAGE_TYPE` is set to `elasticsearch`:
                   files, or ec2 profiles) to sign outbound requests to the cluster.
     * `ES_PIPELINE`: Only valid when the destination is Elasticsearch 5.x. Indicates the ingest
                      pipeline used before spans are indexed. No default.
+    * `ES_TIMEOUT`: Controls the connect, read and write socket timeouts (in milliseconds) for
+                    Elasticsearch Api. Defaults to 10000 (10 seconds)
     * `ES_MAX_REQUESTS`: Only valid when the transport is http. Sets maximum in-flight requests from
                          this process to any Elasticsearch host. Defaults to 64.
     * `ES_AWS_DOMAIN`: The name of the AWS-hosted elasticsearch domain to use. Supercedes any set
diff --git a/zipkin-server/src/main/resources/zipkin-server-shared.yml b/zipkin-server/src/main/resources/zipkin-server-shared.yml
@@ -102,6 +102,7 @@ zipkin:
       hosts: ${ES_HOSTS:}
       pipeline: ${ES_PIPELINE:}
       max-requests: ${ES_MAX_REQUESTS:64}
+      timeout: ${ES_TIMEOUT:10000}
       aws:
         domain: ${ES_AWS_DOMAIN:}
         region: ${ES_AWS_REGION:}
