engine: per try idle timeout (#1805)

Description: expose the new per try idle timeout via the engine builder.
Risk Level: low
Testing: unit test
Docs Changes: added

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

Co-authored-by: Alan Chiu <[email protected]>
Signed-off-by: JP Simard <[email protected]>

mobile/docs/root/api/starting_envoy.rst

@@ -174,7 +174,7 @@ Specify the rate at which Envoy Mobile should flush its queued stats.
 
 Specifies the length of time a stream should wait without a headers or data event before timing out.
 Defaults to 15 seconds.
-See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-stream-idle-timeout>`_
+See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-stream-idle-timeout>`__
 for further information.
 
 **Example**::
@@ -185,6 +185,23 @@ for further information.
   // Swift
   builder.addStreamIdleTimeoutSeconds(5)
 
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+``addPerTryIdleTimeoutSeconds``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Specifies the length of time a retry (including the initial attempt) should wait without a headers
+or data event before timing out. Defaults to 15 seconds.
+See `the Envoy docs <https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto.html#config-route-v3-retrypolicy>`__
+for further information.
+
+**Example**::
+
+  // Kotlin
+  builder.addPerTryIdleTimeoutSeconds(5L)
+
+  // Swift
+  builder.addPerTryIdleTimeoutSeconds(5)
+
 ~~~~~~~~~~~~~~~~~
 ``addAppVersion``
 ~~~~~~~~~~~~~~~~~

mobile/library/cc/engine_builder.cc

@@ -112,6 +112,7 @@ std::string EngineBuilder::generateConfigStr() {
       {"stats_domain", this->stats_domain_},
       {"stats_flush_interval", fmt::format("{}s", this->stats_flush_seconds_)},
       {"stream_idle_timeout", fmt::format("{}s", this->stream_idle_timeout_seconds_)},
+      {"per_try_idle_timeout", fmt::format("{}s", this->per_try_idle_timeout_seconds_)},
       {"virtual_clusters", this->virtual_clusters_},
   };
 

mobile/library/cc/engine_builder.h

@@ -65,6 +65,7 @@ class EngineBuilder {
   std::string device_os_ = "unspecified";
   std::string virtual_clusters_ = "[]";
   int stream_idle_timeout_seconds_ = 15;
+  int per_try_idle_timeout_seconds_ = 15;
 
   // TODO(crockeo): add after filter integration
   // private var platformFilterChain = mutableListOf<EnvoyHTTPFilterFactory>()

mobile/library/common/config/config.cc

@@ -48,6 +48,7 @@ const std::string config_header = R"(
 - &statsd_host 127.0.0.1
 - &statsd_port 8125
 - &stream_idle_timeout 15s
+- &per_try_idle_timeout 15s
 - &virtual_clusters []
 
 !ignore stats_defs:
@@ -228,6 +229,7 @@ const char* config_template = R"(
                   cluster_header: x-envoy-mobile-cluster
                   timeout: 0s
                   retry_policy:
+                    per_try_idle_timeout: *per_try_idle_timeout
                     retry_back_off:
                       base_interval: 0.25s
                       max_interval: 60s

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

@@ -25,6 +25,7 @@ public class EnvoyConfiguration {
   public final List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories;
   public final Integer statsFlushSeconds;
   public final Integer streamIdleTimeoutSeconds;
+  public final Integer perTryIdleTimeoutSeconds;
   public final String appVersion;
   public final String appId;
   public final String virtualClusters;
@@ -50,6 +51,7 @@ public class EnvoyConfiguration {
    * @param h2ConnectionKeepaliveTimeoutSeconds rate in seconds to timeout h2 pings.
    * @param statsFlushSeconds            interval at which to flush Envoy stats.
    * @param streamIdleTimeoutSeconds     idle timeout for HTTP streams.
+   * @param perTryIdleTimeoutSeconds     per try idle timeout for HTTP streams.
    * @param appVersion                   the App Version of the App using this Envoy Client.
    * @param appId                        the App ID of the App using this Envoy Client.
    * @param virtualClusters              the JSON list of virtual cluster configs.
@@ -64,8 +66,9 @@ public EnvoyConfiguration(Boolean adminInterfaceEnabled, String grpcStatsDomain,
                             String dnsPreresolveHostnames,
                             int h2ConnectionKeepaliveIdleIntervalMilliseconds,
                             int h2ConnectionKeepaliveTimeoutSeconds, int statsFlushSeconds,
-                            int streamIdleTimeoutSeconds, String appVersion, String appId,
-                            String virtualClusters, List<EnvoyNativeFilterConfig> nativeFilterChain,
+                            int streamIdleTimeoutSeconds, int perTryIdleTimeoutSeconds,
+                            String appVersion, String appId, String virtualClusters,
+                            List<EnvoyNativeFilterConfig> nativeFilterChain,
                             List<EnvoyHTTPFilterFactory> httpPlatformFilterFactories,
                             Map<String, EnvoyStringAccessor> stringAccessors) {
     this.adminInterfaceEnabled = adminInterfaceEnabled;
@@ -82,6 +85,7 @@ public EnvoyConfiguration(Boolean adminInterfaceEnabled, String grpcStatsDomain,
     this.h2ConnectionKeepaliveTimeoutSeconds = h2ConnectionKeepaliveTimeoutSeconds;
     this.statsFlushSeconds = statsFlushSeconds;
     this.streamIdleTimeoutSeconds = streamIdleTimeoutSeconds;
+    this.perTryIdleTimeoutSeconds = perTryIdleTimeoutSeconds;
     this.appVersion = appVersion;
     this.appId = appId;
     this.virtualClusters = virtualClusters;
@@ -133,6 +137,7 @@ String resolveTemplate(final String templateYAML, final String platformFilterTem
         .append(String.format("- &h2_connection_keepalive_timeout %ss\n",
                               h2ConnectionKeepaliveTimeoutSeconds))
         .append(String.format("- &stream_idle_timeout %ss\n", streamIdleTimeoutSeconds))
+        .append(String.format("- &per_try_idle_timeout %ss\n", perTryIdleTimeoutSeconds))
         .append(String.format("- &metadata { device_os: %s, app_version: %s, app_id: %s }\n",
                               "Android", appVersion, appId))
         .append("- &virtual_clusters ")

mobile/library/java/org/chromium/net/impl/NativeCronetEngineBuilderImpl.java

@@ -38,6 +38,7 @@ public class NativeCronetEngineBuilderImpl extends CronetEngineBuilderImpl {
   private int mH2ConnectionKeepaliveTimeoutSeconds = 10;
   private int mStatsFlushSeconds = 60;
   private int mStreamIdleTimeoutSeconds = 15;
+  private int mPerTryIdleTimeoutSeconds = 15;
   private String mAppVersion = "unspecified";
   private String mAppId = "unspecified";
   private String mVirtualClusters = "[]";
@@ -75,7 +76,7 @@ private EnvoyConfiguration createEnvoyConfiguration() {
         mDnsRefreshSeconds, mDnsFailureRefreshSecondsBase, mDnsFailureRefreshSecondsMax,
         mDnsQueryTimeoutSeconds, mDnsPreresolveHostnames,
         mH2ConnectionKeepaliveIdleIntervalMilliseconds, mH2ConnectionKeepaliveTimeoutSeconds,
-        mStatsFlushSeconds, mStreamIdleTimeoutSeconds, mAppVersion, mAppId, mVirtualClusters,
-        mNativeFilterChain, mPlatformFilterChain, mStringAccessors);
+        mStatsFlushSeconds, mStreamIdleTimeoutSeconds, mPerTryIdleTimeoutSeconds, mAppVersion,
+        mAppId, mVirtualClusters, mNativeFilterChain, mPlatformFilterChain, mStringAccessors);
   }
 }

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

@@ -39,6 +39,7 @@ open class EngineBuilder(
   private var h2ConnectionKeepaliveTimeoutSeconds = 10
   private var statsFlushSeconds = 60
   private var streamIdleTimeoutSeconds = 15
+  private var perTryIdleTimeoutSeconds = 15
   private var appVersion = "unspecified"
   private var appId = "unspecified"
   private var virtualClusters = "[]"
@@ -199,6 +200,18 @@ open class EngineBuilder(
     return this
   }
 
+  /**
+   * Add a custom per try idle timeout for HTTP streams. Defaults to 15 seconds.
+   *
+   * @param perTryIdleTimeoutSeconds per try idle timeout for HTTP streams.
+   *
+   * @return this builder.
+   */
+  fun addPerTryIdleTimeoutSeconds(perTryIdleTimeoutSeconds: Int): EngineBuilder {
+    this.perTryIdleTimeoutSeconds = perTryIdleTimeoutSeconds
+    return this
+  }
+
   /**
    * Add an HTTP filter factory used to create platform filters for streams sent by this client.
    *
@@ -350,8 +363,8 @@ open class EngineBuilder(
             dnsRefreshSeconds, dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax,
             dnsQueryTimeoutSeconds, dnsPreresolveHostnames,
             h2ConnectionKeepaliveIdleIntervalMilliseconds, h2ConnectionKeepaliveTimeoutSeconds,
-            statsFlushSeconds, streamIdleTimeoutSeconds, appVersion, appId,
-            virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
+            statsFlushSeconds, streamIdleTimeoutSeconds, perTryIdleTimeoutSeconds, appVersion,
+            appId, virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
           ),
           configuration.yaml,
           logLevel
@@ -365,8 +378,8 @@ open class EngineBuilder(
             dnsRefreshSeconds, dnsFailureRefreshSecondsBase, dnsFailureRefreshSecondsMax,
             dnsQueryTimeoutSeconds, dnsPreresolveHostnames,
             h2ConnectionKeepaliveIdleIntervalMilliseconds, h2ConnectionKeepaliveTimeoutSeconds,
-            statsFlushSeconds, streamIdleTimeoutSeconds, appVersion, appId,
-            virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
+            statsFlushSeconds, streamIdleTimeoutSeconds, perTryIdleTimeoutSeconds, appVersion,
+            appId, virtualClusters, nativeFilterChain, platformFilterChain, stringAccessors
           ),
           logLevel
         )

mobile/library/objective-c/EnvoyConfiguration.m

@@ -17,6 +17,7 @@ - (instancetype)initWithAdminInterfaceEnabled:(BOOL)adminInterfaceEnabled
               h2ConnectionKeepaliveTimeoutSeconds:(UInt32)h2ConnectionKeepaliveTimeoutSeconds
                                 statsFlushSeconds:(UInt32)statsFlushSeconds
                          streamIdleTimeoutSeconds:(UInt32)streamIdleTimeoutSeconds
+                         perTryIdleTimeoutSeconds:(UInt32)perTryIdleTimeoutSeconds
                                        appVersion:(NSString *)appVersion
                                             appId:(NSString *)appId
                                   virtualClusters:(NSString *)virtualClusters
@@ -47,6 +48,7 @@ - (instancetype)initWithAdminInterfaceEnabled:(BOOL)adminInterfaceEnabled
   self.h2ConnectionKeepaliveTimeoutSeconds = h2ConnectionKeepaliveTimeoutSeconds;
   self.statsFlushSeconds = statsFlushSeconds;
   self.streamIdleTimeoutSeconds = streamIdleTimeoutSeconds;
+  self.perTryIdleTimeoutSeconds = perTryIdleTimeoutSeconds;
   self.appVersion = appVersion;
   self.appId = appId;
   self.virtualClusters = virtualClusters;
@@ -122,6 +124,8 @@ - (nullable NSString *)resolveTemplate:(NSString *)templateYAML {
                             (unsigned long)self.h2ConnectionKeepaliveTimeoutSeconds];
   [definitions
       appendFormat:@"- &stream_idle_timeout %lus\n", (unsigned long)self.streamIdleTimeoutSeconds];
+  [definitions
+      appendFormat:@"- &per_try_idle_timeout %lus\n", (unsigned long)self.perTryIdleTimeoutSeconds];
   [definitions appendFormat:@"- &metadata { device_os: %@, app_version: %@, app_id: %@ }\n", @"iOS",
                             self.appVersion, self.appId];
   [definitions appendFormat:@"- &virtual_clusters %@\n", self.virtualClusters];

mobile/library/objective-c/EnvoyEngine.h

@@ -305,6 +305,7 @@ extern const int kEnvoyFilterResumeStatusResumeIteration;
 @property (nonatomic, assign) UInt32 h2ConnectionKeepaliveTimeoutSeconds;
 @property (nonatomic, assign) UInt32 statsFlushSeconds;
 @property (nonatomic, assign) UInt32 streamIdleTimeoutSeconds;
+@property (nonatomic, assign) UInt32 perTryIdleTimeoutSeconds;
 @property (nonatomic, strong) NSString *appVersion;
 @property (nonatomic, strong) NSString *appId;
 @property (nonatomic, strong) NSString *virtualClusters;
@@ -330,6 +331,7 @@ extern const int kEnvoyFilterResumeStatusResumeIteration;
               h2ConnectionKeepaliveTimeoutSeconds:(UInt32)h2ConnectionKeepaliveTimeoutSeconds
                                 statsFlushSeconds:(UInt32)statsFlushSeconds
                          streamIdleTimeoutSeconds:(UInt32)streamIdleTimeoutSeconds
+                         perTryIdleTimeoutSeconds:(UInt32)perTryIdleTimeoutSeconds
                                        appVersion:(NSString *)appVersion
                                             appId:(NSString *)appId
                                   virtualClusters:(NSString *)virtualClusters

mobile/library/swift/EngineBuilder.swift

@@ -25,6 +25,7 @@ open class EngineBuilder: NSObject {
   private var h2ConnectionKeepaliveTimeoutSeconds: UInt32 = 10
   private var statsFlushSeconds: UInt32 = 60
   private var streamIdleTimeoutSeconds: UInt32 = 15
+  private var perTryIdleTimeoutSeconds: UInt32 = 15
   private var appVersion: String = "unspecified"
   private var appId: String = "unspecified"
   private var virtualClusters: String = "[]"
@@ -180,6 +181,17 @@ open class EngineBuilder: NSObject {
     return self
   }
 
+  /// Add a custom per try idle timeout for HTTP streams. Defaults to 15 seconds.
+  ///
+  /// - parameter perTryIdleSeconds: Idle timeout for HTTP streams.
+  ///
+  /// - returns: This builder.
+  @discardableResult
+  public func addPerTryIdleTimeoutSeconds(_ perTryIdleTimeoutSeconds: UInt32) -> Self {
+    self.perTryIdleTimeoutSeconds = perTryIdleTimeoutSeconds
+    return self
+  }
+
   /// Add an HTTP platform filter factory used to construct filters for streams sent by this client.
   ///
   /// - parameter name:    Custom name to use for this filter factory. Useful for having
@@ -332,6 +344,7 @@ open class EngineBuilder: NSObject {
       h2ConnectionKeepaliveTimeoutSeconds: self.h2ConnectionKeepaliveTimeoutSeconds,
       statsFlushSeconds: self.statsFlushSeconds,
       streamIdleTimeoutSeconds: self.streamIdleTimeoutSeconds,
+      perTryIdleTimeoutSeconds: self.perTryIdleTimeoutSeconds,
       appVersion: self.appVersion,
       appId: self.appId,
       virtualClusters: self.virtualClusters,

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

@@ -28,8 +28,8 @@ class EnvoyConfigurationTest {
   @Test
   fun `resolving with default configuration resolves with values`() {
     val envoyConfiguration = EnvoyConfiguration(
-      false, "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 222, 333, 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
-      listOf<EnvoyNativeFilterConfig>(EnvoyNativeFilterConfig("filter_name", "test_config")),
+      false, "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 222, 333, 567, 678, 910, "v1.2.3", "com.mydomain.myapp", "[test]",
+      listOf(EnvoyNativeFilterConfig("filter_name", "test_config")),
       emptyList(), emptyMap()
     )
 
@@ -62,6 +62,10 @@ class EnvoyConfigurationTest {
     assertThat(resolvedTemplate).contains("&stats_domain stats.foo.com")
     assertThat(resolvedTemplate).contains("&stats_flush_interval 567s")
 
+    // Idle timeouts
+    assertThat(resolvedTemplate).contains("&stream_idle_timeout 678s")
+    assertThat(resolvedTemplate).contains("&per_try_idle_timeout 910s")
+
     // Filters
     assertThat(resolvedTemplate).contains("filter_name")
     assertThat(resolvedTemplate).contains("test_config")
@@ -70,7 +74,7 @@ class EnvoyConfigurationTest {
   @Test
   fun `resolve templates with invalid templates will throw on build`() {
     val envoyConfiguration = EnvoyConfiguration(
-      false, "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 123, 123, 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
+      false, "stats.foo.com", null, 123, 234, 345, 456, 321, "[hostname]", 123, 123, 567, 678, 910, "v1.2.3", "com.mydomain.myapp", "[test]",
       emptyList(), emptyList(), emptyMap()
     )
 
@@ -85,7 +89,7 @@ class EnvoyConfigurationTest {
   @Test
   fun `cannot configure both statsD and gRPC stat sink`() {
     val envoyConfiguration = EnvoyConfiguration(
-      false, "stats.foo.com", 5050, 123, 234, 345, 456, 321, "[hostname]", 123, 123, 567, 678, "v1.2.3", "com.mydomain.myapp", "[test]",
+      false, "stats.foo.com", 5050, 123, 234, 345, 456, 321, "[hostname]", 123, 123, 567, 678, 910, "v1.2.3", "com.mydomain.myapp", "[test]",
       emptyList(), emptyList(), emptyMap()
     )
 

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

@@ -120,6 +120,16 @@ class EngineBuilderTest {
     assertThat(engine.envoyConfiguration!!.streamIdleTimeoutSeconds).isEqualTo(1234)
   }
 
+  @Test
+  fun `specifying per try idle timeout overrides default`() {
+    engineBuilder = EngineBuilder(Standard())
+    engineBuilder.addEngineType { envoyEngine }
+    engineBuilder.addPerTryIdleTimeoutSeconds(5678)
+
+    val engine = engineBuilder.build() as EngineImpl
+    assertThat(engine.envoyConfiguration!!.perTryIdleTimeoutSeconds).isEqualTo(5678)
+  }
+
   @Test
   fun `specifying app version overrides default`() {
     engineBuilder = EngineBuilder(Standard())

mobile/test/swift/EngineBuilderTests.swift

@@ -222,6 +222,20 @@ final class EngineBuilderTests: XCTestCase {
     self.waitForExpectations(timeout: 0.01)
   }
 
+  func testAddingPerTryIdleTimeoutSecondsAddsToConfigurationWhenRunningEnvoy() {
+    let expectation = self.expectation(description: "Run called with expected data")
+    MockEnvoyEngine.onRunWithConfig = { config, _ in
+      XCTAssertEqual(21, config.perTryIdleTimeoutSeconds)
+      expectation.fulfill()
+    }
+
+    _ = EngineBuilder()
+      .addEngineType(MockEnvoyEngine.self)
+      .addPerTryIdleTimeoutSeconds(21)
+      .build()
+    self.waitForExpectations(timeout: 0.01)
+  }
+
   func testAddingAppVersionAddsToConfigurationWhenRunningEnvoy() {
     let expectation = self.expectation(description: "Run called with expected data")
     MockEnvoyEngine.onRunWithConfig = { config, _ in
@@ -306,6 +320,7 @@ final class EngineBuilderTests: XCTestCase {
       h2ConnectionKeepaliveTimeoutSeconds: 333,
       statsFlushSeconds: 600,
       streamIdleTimeoutSeconds: 700,
+      perTryIdleTimeoutSeconds: 777,
       appVersion: "v1.2.3",
       appId: "com.envoymobile.ios",
       virtualClusters: "[test]",
@@ -331,6 +346,7 @@ final class EngineBuilderTests: XCTestCase {
     XCTAssertTrue(resolvedYAML.contains("&h2_connection_keepalive_timeout 333s"))
 
     XCTAssertTrue(resolvedYAML.contains("&stream_idle_timeout 700s"))
+    XCTAssertTrue(resolvedYAML.contains("&per_try_idle_timeout 777s"))
 
     XCTAssertFalse(resolvedYAML.contains("admin: *admin_interface"))
 
@@ -365,6 +381,7 @@ final class EngineBuilderTests: XCTestCase {
       h2ConnectionKeepaliveTimeoutSeconds: 333,
       statsFlushSeconds: 600,
       streamIdleTimeoutSeconds: 700,
+      perTryIdleTimeoutSeconds: 700,
       appVersion: "v1.2.3",
       appId: "com.envoymobile.ios",
       virtualClusters: "[test]",