From d615499b7f213983da10c2fb9269cf47340f7110 Mon Sep 17 00:00:00 2001 From: alexandraoberaigner <82218944+alexandraoberaigner@users.noreply.github.com> Date: Tue, 10 Sep 2024 18:11:18 +0200 Subject: [PATCH] fix: use keepalive for TCP & use unit in env variable name (#945) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alexandra Oberaigner --- providers/flagd/README.md | 2 +- .../contrib/providers/flagd/Config.java | 3 +- .../contrib/providers/flagd/FlagdOptions.java | 3 +- .../flagd/resolver/common/ChannelBuilder.java | 12 ++++--- .../providers/flagd/FlagdOptionsTest.java | 33 ++++++++++++++++--- 5 files changed, 41 insertions(+), 12 deletions(-) diff --git a/providers/flagd/README.md b/providers/flagd/README.md index 5813e4d6d..55a97587b 100644 --- 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 index 97fa343d5..415b509b6 100644 --- 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 index 871a2889a..c88709a32 100644 --- 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/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelBuilder.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelBuilder.java index ec18c52a4..e5c94e21d 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelBuilder.java +++ b/providers/flagd/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 index d703c0a12..081193c22 100644 --- 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()