EngineBuilder API: addDnsQueryTimeoutSeconds (#1583)

Description: API for addDnsQueryTimeoutSeconds to configure behavior introduced in #1580
Risk Level: low - optional builder API
Testing: updated suite.
Docs Changes: added

Signed-off-by: Jose Nino <[email protected]>
Signed-off-by: JP Simard <[email protected]>

mobile/docs/root/api/starting_envoy.rst

@@ -84,19 +84,31 @@ Specify the interval at which Envoy should forcefully refresh DNS.
   builder.addDNSRefreshSeconds(60)
 
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-``addDNSPreresolveHostnames``
+``addDNSQueryTimeoutSeconds``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Add a list of hostnames to preresolve on Engine startup.
-The configuration is expected as a JSON list.
+Specify the interval at which Envoy should timeout a DNS query.
+
+**Example**::
+
+  // Kotlin
+  builder.addDNSQueryTimeoutSeconds(60L)
+
+  // Swift
+  builder.addDNSQueryTimeoutSeconds(60)
+
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+``addDNSPreresolveHostnames``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 .. attention::
 
-    This API is non-ideal as it exposes lower-level internals of Envoy than desired by this
-    project.
-    :issue:`#1581 <1581>` tracks enhancing this API.
+  This API is non-ideal as it exposes lower-level internals of Envoy than desired by this
+  project.
+  :issue:`#1581 <1581>` tracks enhancing this API.
 
-**Example**::
+Add a list of hostnames to preresolve on Engine startup.
+The configuration is expected as a JSON list.
 
   // Kotlin
   builder.addDNSPreresolveHostnames("[{\"address\": \"foo.com", \"port_value\": 443}]")

mobile/library/cc/engine_builder.cc

@@ -43,6 +43,11 @@ EngineBuilder& EngineBuilder::addDnsFailureRefreshSeconds(int base, int max) {
   return *this;
 }
 
+EngineBuilder& EngineBuilder::addDnsQueryTimeoutSeconds(int dns_query_timeout_seconds) {
+  this->dns_query_timeout_seconds_ = dns_query_timeout_seconds;
+  return *this;
+}
+
 EngineBuilder&
 EngineBuilder::addDnsPreresolveHostnames(const std::string& dns_preresolve_hostnames) {
   this->dns_preresolve_hostnames_ = dns_preresolve_hostnames;
@@ -81,6 +86,7 @@ std::string EngineBuilder::generateConfigStr() {
       {"dns_fail_max_interval", fmt::format("{}s", this->dns_failure_refresh_seconds_max_)},
       {"dns_preresolve_hostnames", this->dns_preresolve_hostnames_},
       {"dns_refresh_rate", fmt::format("{}s", this->dns_refresh_seconds_)},
+      {"dns_query_timeout", fmt::format("{}s", this->dns_query_timeout_seconds_)},
       {
           "metadata",
           fmt::format("{{ device_os: {}, app_version: {}, app_id: {} }}", this->device_os_,

mobile/library/cc/engine_builder.h

@@ -22,6 +22,7 @@ class EngineBuilder {
   EngineBuilder& addConnectTimeoutSeconds(int connect_timeout_seconds);
   EngineBuilder& addDnsRefreshSeconds(int dns_refresh_seconds);
   EngineBuilder& addDnsFailureRefreshSeconds(int base, int max);
+  EngineBuilder& addDnsQueryTimeoutSeconds(int dns_query_timeout_seconds);
   EngineBuilder& addDnsPreresolveHostnames(const std::string& dns_preresolve_hostnames);
   EngineBuilder& addStatsFlushSeconds(int stats_flush_seconds);
   EngineBuilder& addVirtualClusters(const std::string& virtual_clusters);
@@ -50,6 +51,7 @@ class EngineBuilder {
   int dns_refresh_seconds_ = 60;
   int dns_failure_refresh_seconds_base_ = 2;
   int dns_failure_refresh_seconds_max_ = 10;
+  int dns_query_timeout_seconds_ = 25;
   std::string dns_preresolve_hostnames_ = "[]";
   int stats_flush_seconds_ = 60;
   std::string app_version_ = "unspecified";

mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java

@@ -17,6 +17,7 @@ public class EnvoyConfiguration {
   public final Integer dnsRefreshSeconds;
   public final Integer dnsFailureRefreshSecondsBase;
   public final Integer dnsFailureRefreshSecondsMax;
+  public final Integer dnsQueryTimeoutSeconds;
   public final String dnsPreresolveHostnames;
   public final List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories;
   public final Integer statsFlushSeconds;
@@ -38,6 +39,7 @@ public class EnvoyConfiguration {
    * @param dnsRefreshSeconds            rate in seconds to refresh DNS.
    * @param dnsFailureRefreshSecondsBase base rate in seconds to refresh DNS on failure.
    * @param dnsFailureRefreshSecondsMax  max rate in seconds to refresh DNS on failure.
+   * @param dnsQueryTimeoutSeconds       rate in seconds to timeout DNS queries.
    * @param dnsPreresolveHostnames       hostnames to preresolve on Envoy Client construction.
    * @param statsFlushSeconds            interval at which to flush Envoy stats.
    * @param streamIdleTimeoutSeconds     idle timeout for HTTP streams.
@@ -51,9 +53,10 @@ public class EnvoyConfiguration {
   public EnvoyConfiguration(String grpcStatsDomain, @Nullable Integer statsdPort,
                             int connectTimeoutSeconds, int dnsRefreshSeconds,
                             int dnsFailureRefreshSecondsBase, int dnsFailureRefreshSecondsMax,
-                            String dnsPreresolveHostnames, int statsFlushSeconds,
-                            int streamIdleTimeoutSeconds, String appVersion, String appId,
-                            String virtualClusters, List<EnvoyNativeFilterConfig> nativeFilterChain,
+                            int dnsQueryTimeoutSeconds, String dnsPreresolveHostnames,
+                            int statsFlushSeconds, int streamIdleTimeoutSeconds, String appVersion,
+                            String appId, String virtualClusters,
+                            List<EnvoyNativeFilterConfig> nativeFilterChain,
                             List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories,
                             Map<String, EnvoyStringAccessor> stringAccessors) {
     this.grpcStatsDomain = grpcStatsDomain;
@@ -62,6 +65,7 @@ public EnvoyConfiguration(String grpcStatsDomain, @Nullable Integer statsdPort,
     this.dnsRefreshSeconds = dnsRefreshSeconds;
     this.dnsFailureRefreshSecondsBase = dnsFailureRefreshSecondsBase;
     this.dnsFailureRefreshSecondsMax = dnsFailureRefreshSecondsMax;
+    this.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds;
     this.dnsPreresolveHostnames = dnsPreresolveHostnames;
     this.statsFlushSeconds = statsFlushSeconds;
     this.streamIdleTimeoutSeconds = streamIdleTimeoutSeconds;
@@ -109,6 +113,7 @@ String resolveTemplate(final String templateYAML, final String platformFilterTem
         .append(String.format("- &dns_refresh_rate %ss\n", dnsRefreshSeconds))
         .append(String.format("- &dns_fail_base_interval %ss\n", dnsFailureRefreshSecondsBase))
         .append(String.format("- &dns_fail_max_interval %ss\n", dnsFailureRefreshSecondsMax))
+        .append(String.format("- &dns_query_timeout %ss\n", dnsQueryTimeoutSeconds))
         .append(String.format("- &dns_preresolve_hostnames %s\n", dnsPreresolveHostnames))
         .append(String.format("- &stream_idle_timeout %ss\n", streamIdleTimeoutSeconds))
         .append(String.format("- &metadata { device_os: %s, app_version: %s, app_id: %s }\n",

mobile/library/kotlin/io/envoyproxy/envoymobile/EngineBuilder.kt

@@ -29,6 +29,7 @@ open class EngineBuilder(
   private var dnsRefreshSeconds = 60
   private var dnsFailureRefreshSecondsBase = 2
   private var dnsFailureRefreshSecondsMax = 10
+  private var dnsQueryTimeoutSeconds = 25
   private var dnsPreresolveHostnames = "[]"
   private var statsFlushSeconds = 60
   private var streamIdleTimeoutSeconds = 15
@@ -106,28 +107,40 @@ open class EngineBuilder(
   }
 
   /**
-   * Add a list of hostnames to preresolve on Engine startup.
+   * Add a rate at which to refresh DNS in case of DNS failure.
    *
-   * @param dnsPreresolveHostnames hostnames to preresolve.
+   * @param base rate in seconds.
+   * @param max rate in seconds.
    *
    * @return this builder.
    */
-  fun addDNSPreresolveHostnames(dnsPreresolveHostnames: String): EngineBuilder {
-    this.dnsPreresolveHostnames = dnsPreresolveHostnames
+  fun addDNSFailureRefreshSeconds(base: Int, max: Int): EngineBuilder {
+    this.dnsFailureRefreshSecondsBase = base
+    this.dnsFailureRefreshSecondsMax = max
     return this
   }
 
   /**
-   * Add a rate at which to refresh DNS in case of DNS failure.
+   * Add a rate at which to timeout DNS queries.
    *
-   * @param base rate in seconds.
-   * @param max rate in seconds.
+   * @param dnsQueryTimeoutSeconds rate in seconds to timeout DNS queries.
    *
    * @return this builder.
    */
-  fun addDNSFailureRefreshSeconds(base: Int, max: Int): EngineBuilder {
-    this.dnsFailureRefreshSecondsBase = base
-    this.dnsFailureRefreshSecondsMax = max
+  fun addDNSQueryTimeoutSeconds(dnsQueryTimeoutSeconds: Int): EngineBuilder {
+    this.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds
+    return this
+  }
+
+  /**
+   * Add a list of hostnames to preresolve on Engine startup.
+   *
+   * @param dnsPreresolveHostnames hostnames to preresolve.
+   *
+   * @return this builder.
+   */
+  fun addDNSPreresolveHostnames(dnsPreresolveHostnames: String): EngineBuilder {
+    this.dnsPreresolveHostnames = dnsPreresolveHostnames
     return this
   }
 
@@ -285,6 +298,7 @@ open class EngineBuilder(
           EnvoyConfiguration(
             grpcStatsDomain, statsDPort, connectTimeoutSeconds,
             dnsRefreshSeconds, dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax,
+            dnsQueryTimeoutSeconds,
             dnsPreresolveHostnames, statsFlushSeconds, streamIdleTimeoutSeconds, appVersion, appId,
             virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
           ),
@@ -298,6 +312,7 @@ open class EngineBuilder(
           EnvoyConfiguration(
             grpcStatsDomain, statsDPort, connectTimeoutSeconds,
             dnsRefreshSeconds, dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax,
+            dnsQueryTimeoutSeconds,
             dnsPreresolveHostnames, statsFlushSeconds, streamIdleTimeoutSeconds, appVersion, appId,
             virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
           ),

mobile/library/objective-c/EnvoyConfiguration.m

@@ -9,6 +9,7 @@ - (instancetype)initWithGrpcStatsDomain:(nullable NSString *)grpcStatsDomain
                       dnsRefreshSeconds:(UInt32)dnsRefreshSeconds
            dnsFailureRefreshSecondsBase:(UInt32)dnsFailureRefreshSecondsBase
             dnsFailureRefreshSecondsMax:(UInt32)dnsFailureRefreshSecondsMax
+                 dnsQueryTimeoutSeconds:(UInt32)dnsQueryTimeoutSeconds
                  dnsPreresolveHostnames:(NSString *)dnsPreresolveHostnames
                       statsFlushSeconds:(UInt32)statsFlushSeconds
                streamIdleTimeoutSeconds:(UInt32)streamIdleTimeoutSeconds
@@ -32,6 +33,7 @@ - (instancetype)initWithGrpcStatsDomain:(nullable NSString *)grpcStatsDomain
   self.dnsRefreshSeconds = dnsRefreshSeconds;
   self.dnsFailureRefreshSecondsBase = dnsFailureRefreshSecondsBase;
   self.dnsFailureRefreshSecondsMax = dnsFailureRefreshSecondsMax;
+  self.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds;
   self.dnsPreresolveHostnames = dnsPreresolveHostnames;
   self.statsFlushSeconds = statsFlushSeconds;
   self.streamIdleTimeoutSeconds = streamIdleTimeoutSeconds;
@@ -101,6 +103,8 @@ - (nullable NSString *)resolveTemplate:(NSString *)templateYAML {
                             (unsigned long)self.dnsFailureRefreshSecondsBase];
   [definitions appendFormat:@"- &dns_fail_max_interval %lus\n",
                             (unsigned long)self.dnsFailureRefreshSecondsMax];
+  [definitions
+      appendFormat:@"- &dns_query_timeout %lus\n", (unsigned long)self.dnsQueryTimeoutSeconds];
   [definitions appendFormat:@"- &dns_preresolve_hostnames %@\n", self.dnsPreresolveHostnames];
   [definitions
       appendFormat:@"- &stream_idle_timeout %lus\n", (unsigned long)self.streamIdleTimeoutSeconds];

mobile/library/objective-c/EnvoyEngine.h

@@ -266,6 +266,7 @@ extern const int kEnvoyFilterResumeStatusResumeIteration;
 @property (nonatomic, assign) UInt32 dnsRefreshSeconds;
 @property (nonatomic, assign) UInt32 dnsFailureRefreshSecondsBase;
 @property (nonatomic, assign) UInt32 dnsFailureRefreshSecondsMax;
+@property (nonatomic, assign) UInt32 dnsQueryTimeoutSeconds;
 @property (nonatomic, strong) NSString *dnsPreresolveHostnames;
 @property (nonatomic, assign) UInt32 statsFlushSeconds;
 @property (nonatomic, assign) UInt32 streamIdleTimeoutSeconds;
@@ -287,6 +288,7 @@ extern const int kEnvoyFilterResumeStatusResumeIteration;
                dnsRefreshSeconds:(UInt32)dnsRefreshSeconds
     dnsFailureRefreshSecondsBase:(UInt32)dnsFailureRefreshSecondsBase
      dnsFailureRefreshSecondsMax:(UInt32)dnsFailureRefreshSecondsMax
+          dnsQueryTimeoutSeconds:(UInt32)dnsQueryTimeoutSeconds
           dnsPreresolveHostnames:(NSString *)dnsPreresolveHostnames
                statsFlushSeconds:(UInt32)statsFlushSeconds
         streamIdleTimeoutSeconds:(UInt32)streamIdleTimeoutSeconds

mobile/library/python/envoy_engine.pyi

@@ -16,6 +16,7 @@ class EngineBuilder:
     def add_connect_timeout_seconds(self, connect_timeout_seconds: int) -> "EngineBuilder": ...
     def add_dns_refresh_seconds(self, dns_refresh_seconds: int) -> "EngineBuilder": ...
     def add_dns_failure_refresh_seconds(self, base: int, max: int) -> "EngineBuilder": ...
+    def add_dns_query_timeout_seconds(self, dns_query_timeout_seconds: int) -> "EngineBuilder": ...
     def add_dns_preresolve_hostnames(self, dns_preresolve_hostnames: str) -> "EngineBuilder": ...
     def add_stats_flush_seconds(self, stats_flush_seconds: int) -> "EngineBuilder": ...
     def set_app_version(self, app_version: str) -> "EngineBuilder": ...

mobile/library/python/module_definition.cc

@@ -57,6 +57,7 @@ PYBIND11_MODULE(envoy_engine, m) {
       .def("add_connect_timeout_seconds", &EngineBuilder::addConnectTimeoutSeconds)
       .def("add_dns_refresh_seconds", &EngineBuilder::addDnsRefreshSeconds)
       .def("add_dns_failure_refresh_seconds", &EngineBuilder::addDnsFailureRefreshSeconds)
+      .def("add_dns_query_timeout_seconds", &EngineBuilder::addDnsQueryTimeoutSeconds)
       .def("add_dns_preresolve_hostnames", &EngineBuilder::addDnsPreresolveHostnames)
       .def("add_stats_flush_seconds", &EngineBuilder::addStatsFlushSeconds)
       .def("set_app_version", &EngineBuilder::setAppVersion)

mobile/library/swift/EngineBuilder.swift

@@ -18,6 +18,7 @@ public class EngineBuilder: NSObject {
   private var dnsRefreshSeconds: UInt32 = 60
   private var dnsFailureRefreshSecondsBase: UInt32 = 2
   private var dnsFailureRefreshSecondsMax: UInt32 = 10
+  private var dnsQueryTimeoutSeconds: UInt32 = 25
   private var dnsPreresolveHostnames: String = "[]"
   private var statsFlushSeconds: UInt32 = 60
   private var streamIdleTimeoutSeconds: UInt32 = 15
@@ -106,6 +107,17 @@ public class EngineBuilder: NSObject {
     return self
   }
 
+  /// Add a rate at which to timeout DNS queries.
+  ///
+  /// - parameter dnsQueryTimeoutSeconds: Rate in seconds to timeout DNS queries.
+  ///
+  /// - returns: This builder.
+  @discardableResult
+  public func addDNSQueryTimeoutSeconds(_ dnsQueryTimeoutSeconds: UInt32) -> Self {
+    self.dnsQueryTimeoutSeconds = dnsQueryTimeoutSeconds
+    return self
+  }
+
   /// Add a list of hostnames to preresolve on Engine startup.
   ///
   /// - parameter dnsPreresolveHostnames: the hostnames to resolve.
@@ -272,6 +284,7 @@ public class EngineBuilder: NSObject {
       dnsRefreshSeconds: self.dnsRefreshSeconds,
       dnsFailureRefreshSecondsBase: self.dnsFailureRefreshSecondsBase,
       dnsFailureRefreshSecondsMax: self.dnsFailureRefreshSecondsMax,
+      dnsQueryTimeoutSeconds: self.dnsQueryTimeoutSeconds,
       dnsPreresolveHostnames: self.dnsPreresolveHostnames,
       statsFlushSeconds: self.statsFlushSeconds,
       streamIdleTimeoutSeconds: self.streamIdleTimeoutSeconds,

mobile/test/cc/unit/envoy_config_test.cc

@@ -17,6 +17,7 @@ TEST(TestConfig, ConfigIsApplied) {
       .addConnectTimeoutSeconds(123)
       .addDnsRefreshSeconds(456)
       .addDnsFailureRefreshSeconds(789, 987)
+      .addDnsQueryTimeoutSeconds(321)
       .addDnsPreresolveHostnames("[hostname]")
       .addStatsFlushSeconds(654)
       .addVirtualClusters("[virtual-clusters]")
@@ -31,6 +32,7 @@ TEST(TestConfig, ConfigIsApplied) {
       "- &dns_refresh_rate 456s",
       "- &dns_fail_base_interval 789s",
       "- &dns_fail_max_interval 987s",
+      "- &dns_query_timeout 321s",
       "- &dns_preresolve_hostnames [hostname]",
       "- &stats_flush_interval 654s",
       "- &virtual_clusters [virtual-clusters]",

mobile/test/java/io/envoyproxy/envoymobile/engine/EnvoyConfigurationTest.kt

@@ -28,7 +28,7 @@ class EnvoyConfigurationTest {
   @Test
   fun `resolving with default configuration resolves with values`() {
     val envoyConfiguration = EnvoyConfiguration(
-      "stats.foo.com", null, 123, 234, 345, 456, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
+      "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
       listOf<EnvoyNativeFilterConfig>(EnvoyNativeFilterConfig("filter_name", "test_config")),
       emptyList(), emptyMap()
     )
@@ -37,9 +37,12 @@ class EnvoyConfigurationTest {
       TEST_CONFIG, PLATFORM_FILTER_CONFIG, NATIVE_FILTER_CONFIG
     )
     assertThat(resolvedTemplate).contains("&connect_timeout 123s")
+
+    // DNS
     assertThat(resolvedTemplate).contains("&dns_refresh_rate 234s")
     assertThat(resolvedTemplate).contains("&dns_fail_base_interval 345s")
     assertThat(resolvedTemplate).contains("&dns_fail_max_interval 456s")
+    assertThat(resolvedTemplate).contains("&dns_query_timeout 321s")
     assertThat(resolvedTemplate).contains("&dns_preresolve_hostnames [hostname]")
 
     // Metadata
@@ -61,7 +64,7 @@ class EnvoyConfigurationTest {
   @Test
   fun `resolve templates with invalid templates will throw on build`() {
     val envoyConfiguration = EnvoyConfiguration(
-      "stats.foo.com", null, 123, 234, 345, 456, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
+      "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
       emptyList(), emptyList(), emptyMap()
     )
 
@@ -76,7 +79,7 @@ class EnvoyConfigurationTest {
   @Test
   fun `cannot configure both statsD and gRPC stat sink`() {
     val envoyConfiguration = EnvoyConfiguration(
-      "stats.foo.com", 5050, 123, 234, 345, 456, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
+      "stats.foo.com", 5050, 123, 234, 345, 456, 321, "[hostname]", 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
       emptyList(), emptyList(), emptyMap()
     )
 

mobile/test/kotlin/io/envoyproxy/envoymobile/EngineBuilderTest.kt

@@ -60,6 +60,16 @@ class EngineBuilderTest {
     assertThat(engine.envoyConfiguration!!.dnsFailureRefreshSecondsMax).isEqualTo(5678)
   }
 
+  @Test
+  fun `specifying DNS query timeout overrides default`() {
+    engineBuilder = EngineBuilder(Standard())
+    engineBuilder.addEngineType { envoyEngine }
+    engineBuilder.addDNSQueryTimeoutSeconds(1234)
+
+    val engine = engineBuilder.build() as EngineImpl
+    assertThat(engine.envoyConfiguration!!.dnsQueryTimeoutSeconds).isEqualTo(1234)
+  }
+
   @Test
   fun `specifying stats flush overrides default`() {
     engineBuilder = EngineBuilder(Standard())