fix: use keepalive for TCP & use unit in env variable name (#945)

Signed-off-by: Alexandra Oberaigner <[email protected]>
open-feature · Sep 10, 2024 · d615499 · d615499
1 parent 7b0cf34
commit d615499
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 12 deletions.
diff --git a/providers/flagd/README.md b/providers/flagd/README.md
@@ -105,7 +105,7 @@ Given below are the supported configurations:
 | socketPath            | FLAGD_SOCKET_PATH              | String                   | null      | rpc & in-process    |
 | certPath              | FLAGD_SERVER_CERT_PATH         | String                   | null      | rpc & in-process    |
 | deadline              | FLAGD_DEADLINE_MS              | int                      | 500       | rpc & in-process    |
-| keepAliveTime         | FLAGD_KEEP_ALIVE_TIME          | long                     | 0         | rpc & in-process    |
+| keepAliveTime         | FLAGD_KEEP_ALIVE_TIME_MS       | long                     | 0         | rpc & in-process    |
 | selector              | FLAGD_SOURCE_SELECTOR          | String                   | null      | in-process          |
 | cache                 | FLAGD_CACHE                    | String - lru, disabled   | lru       | rpc                 |
 | maxCacheSize          | FLAGD_MAX_CACHE_SIZE           | int                      | 1000      | rpc                 |

diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java
@@ -33,7 +33,8 @@ public final class Config {
     static final String DEADLINE_MS_ENV_VAR_NAME = "FLAGD_DEADLINE_MS";
     static final String SOURCE_SELECTOR_ENV_VAR_NAME = "FLAGD_SOURCE_SELECTOR";
     static final String OFFLINE_SOURCE_PATH = "FLAGD_OFFLINE_FLAG_SOURCE_PATH";
-    static final String KEEP_ALIVE_ENV_VAR_NAME = "FLAGD_KEEP_ALIVE_TIME";
+    static final String KEEP_ALIVE_MS_ENV_VAR_NAME_OLD = "FLAGD_KEEP_ALIVE_TIME";
+    static final String KEEP_ALIVE_MS_ENV_VAR_NAME = "FLAGD_KEEP_ALIVE_TIME_MS";
 
     static final String RESOLVER_RPC = "rpc";
     static final String RESOLVER_IN_PROCESS = "in-process";

diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java
@@ -99,7 +99,8 @@ public class FlagdOptions {
      * 
      **/
     @Builder.Default
-    private long keepAlive = fallBackToEnvOrDefault(Config.KEEP_ALIVE_ENV_VAR_NAME, Config.DEFAULT_KEEP_ALIVE);
+    private long keepAlive = fallBackToEnvOrDefault(Config.KEEP_ALIVE_MS_ENV_VAR_NAME,
+            fallBackToEnvOrDefault(Config.KEEP_ALIVE_MS_ENV_VAR_NAME_OLD, Config.DEFAULT_KEEP_ALIVE));
 
     /**
      * File source of flags to be used by offline mode.

diff --git a/...src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelBuilder.java b/...src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelBuilder.java
@@ -28,6 +28,10 @@ private ChannelBuilder() {
      */
     @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "certificate path is a user input")
     public static ManagedChannel nettyChannel(final FlagdOptions options) {
+
+        // keepAliveTime: Long.MAX_VALUE disables keepAlive; very small values are increased automatically
+        long keepAliveMs = options.getKeepAlive() == 0 ? Long.MAX_VALUE : options.getKeepAlive();
+
         // we have a socket path specified, build a channel with a unix socket
         if (options.getSocketPath() != null) {
             // check epoll availability
@@ -37,9 +41,7 @@ public static ManagedChannel nettyChannel(final FlagdOptions options) {
 
             return NettyChannelBuilder
                     .forAddress(new DomainSocketAddress(options.getSocketPath()))
-                    // keepAliveTime: Long.MAX_VALUE disables keepAlive; very small values are increased automatically
-                    .keepAliveTime(options.getKeepAlive() == 0 ? Long.MAX_VALUE : options.getKeepAlive(),
-                            TimeUnit.MILLISECONDS)
+                    .keepAliveTime(keepAliveMs, TimeUnit.MILLISECONDS)
                     .eventLoopGroup(new EpollEventLoopGroup())
                     .channelType(EpollDomainSocketChannel.class)
                     .usePlaintext()
@@ -48,7 +50,9 @@ public static ManagedChannel nettyChannel(final FlagdOptions options) {
 
         // build a TCP socket
         try {
-            final NettyChannelBuilder builder = NettyChannelBuilder.forAddress(options.getHost(), options.getPort());
+            final NettyChannelBuilder builder = NettyChannelBuilder
+                    .forAddress(options.getHost(), options.getPort())
+                    .keepAliveTime(keepAliveMs, TimeUnit.MILLISECONDS);
             if (options.isTls()) {
                 SslContextBuilder sslContext = GrpcSslContexts.forClient();
 

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java
@@ -3,6 +3,7 @@
 import dev.openfeature.contrib.providers.flagd.resolver.process.storage.MockConnector;
 import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.Connector;
 import io.opentelemetry.api.OpenTelemetry;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junitpioneer.jupiter.SetEnvironmentVariable;
 import org.mockito.Mockito;
@@ -105,14 +106,36 @@ void testInProcessProviderFromEnv_noPortConfigured_defaultsToCorrectPort() {
         assertThat(flagdOptions.getPort()).isEqualTo(Integer.parseInt(DEFAULT_IN_PROCESS_PORT));
     }
 
-    @Test
-    @SetEnvironmentVariable(key = KEEP_ALIVE_ENV_VAR_NAME, value = "1337")
-    void testInProcessProviderFromEnv_keepAliveEnvSet_usesSet() {
-        FlagdOptions flagdOptions = FlagdOptions.builder().build();
+    @Nested
+    class TestInProcessProviderFromEnv_keepAliveEnvSet {
+        @Test
+        @SetEnvironmentVariable(key = KEEP_ALIVE_MS_ENV_VAR_NAME, value = "1336")
+        void usesSet() {
+            FlagdOptions flagdOptions = FlagdOptions.builder().build();
+
+            assertThat(flagdOptions.getKeepAlive()).isEqualTo(1336);
+        }
+
+        @Test
+        @SetEnvironmentVariable(key = KEEP_ALIVE_MS_ENV_VAR_NAME_OLD, value = "1337")
+        void usesSetOldName() {
+            FlagdOptions flagdOptions = FlagdOptions.builder().build();
 
-        assertThat(flagdOptions.getKeepAlive()).isEqualTo(1337);
+            assertThat(flagdOptions.getKeepAlive()).isEqualTo(1337);
+        }
+
+        @Test
+        @SetEnvironmentVariable(key = KEEP_ALIVE_MS_ENV_VAR_NAME_OLD, value = "2222")
+        @SetEnvironmentVariable(key = KEEP_ALIVE_MS_ENV_VAR_NAME, value = "1338")
+        void usesSetOldAndNewName() {
+            FlagdOptions flagdOptions = FlagdOptions.builder().build();
+
+            assertThat(flagdOptions.getKeepAlive()).isEqualTo(1338);
+        }
     }
 
+
+
     @Test
     void testInProcessProvider_noPortConfigured_defaultsToCorrectPort() {
         FlagdOptions flagdOptions = FlagdOptions.builder()