From c77f880c380e0f5bafdfff7ffb88cecf37f1d266 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Sat, 15 Aug 2026 14:50:57 -0700 Subject: [PATCH] perf(feature-flagging): bound provider initialization time --- .../trace/api/openfeature/DDEvaluator.java | 12 ++++- .../api/openfeature/DDEvaluatorTest.java | 11 +++++ .../AgentlessConfigurationSource.java | 12 ++--- .../AgentlessConfigurationSourceTest.java | 47 ++++++++++++++----- 4 files changed, 60 insertions(+), 22 deletions(-) diff --git a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java index e74074a640a..9958b7eb65e 100644 --- a/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java +++ b/products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/DDEvaluator.java @@ -121,9 +121,19 @@ public DDEvaluator(final Runnable configCallback) { @Override public boolean initialize( final long timeout, final TimeUnit unit, final EvaluationContext context) throws Exception { + final long initializationStarted = System.nanoTime(); FeatureFlaggingGateway.activate(); FeatureFlaggingGateway.addConfigListener(this); - return initializationLatch.await(timeout, unit) || hasConfiguration(); + final long elapsedNanos = System.nanoTime() - initializationStarted; + return initializationLatch.await( + remainingTimeoutNanos(timeout, unit, elapsedNanos), TimeUnit.NANOSECONDS) + || hasConfiguration(); + } + + static long remainingTimeoutNanos( + final long timeout, final TimeUnit unit, final long elapsedNanos) { + final long timeoutNanos = Math.max(0, unit.toNanos(timeout)); + return Math.max(0, timeoutNanos - elapsedNanos); } @Override diff --git a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java index aeddda0bfd2..a7570bb9bba 100644 --- a/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java +++ b/products/feature-flagging/feature-flagging-api/src/test/java/datadog/trace/api/openfeature/DDEvaluatorTest.java @@ -95,6 +95,17 @@ public void testInitializeSignalsApplicationProviderActivation() throws Exceptio } } + @Test + public void testInitializationTimeoutIncludesActivationTime() { + assertThat( + DDEvaluator.remainingTimeoutNanos(100, MILLISECONDS, MILLISECONDS.toNanos(40)), + equalTo(MILLISECONDS.toNanos(60))); + assertThat( + DDEvaluator.remainingTimeoutNanos(100, MILLISECONDS, MILLISECONDS.toNanos(150)), + equalTo(0L)); + assertThat(DDEvaluator.remainingTimeoutNanos(-1, MILLISECONDS, 0), equalTo(0L)); + } + private static Arguments[] valueMappingTestCases() { return new Arguments[] { // String mappings diff --git a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/AgentlessConfigurationSource.java b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/AgentlessConfigurationSource.java index f4fc35cce30..dad48b67056 100644 --- a/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/AgentlessConfigurationSource.java +++ b/products/feature-flagging/feature-flagging-lib/src/main/java/com/datadog/featureflag/AgentlessConfigurationSource.java @@ -123,19 +123,13 @@ public void init() { started = true; } - // Complete the first poll cycle on the activation thread. This lets OpenFeature provider - // initialization observe a successful retry before it checks whether configuration is ready. - // No request occurs before application code activates the provider. - pollOnceSafely(); - synchronized (lifecycleLock) { if (!closed) { + // Start the first poll immediately on the configuration executor. Network time and retry + // delays must not run before the OpenFeature provider starts its initialization timeout. scheduledPoll = executor.scheduleWithFixedDelay( - this::pollOnceSafely, - pollIntervalMillis, - pollIntervalMillis, - TimeUnit.MILLISECONDS); + this::pollOnceSafely, 0, pollIntervalMillis, TimeUnit.MILLISECONDS); } } } diff --git a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/AgentlessConfigurationSourceTest.java b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/AgentlessConfigurationSourceTest.java index b02bd05642b..2845292e71b 100644 --- a/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/AgentlessConfigurationSourceTest.java +++ b/products/feature-flagging/feature-flagging-lib/src/test/java/com/datadog/featureflag/AgentlessConfigurationSourceTest.java @@ -10,6 +10,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -30,6 +31,7 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.BlockingQueue; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -892,7 +894,7 @@ void rejectsOverlappingPolls() throws Exception { } @Test - void initCompletesFirstPollAndCloseCancelsScheduledFuture() throws Exception { + void initSchedulesFirstPollAndCloseCancelsScheduledFuture() throws Exception { final FakeClient client = new FakeClient(response(200, "etag-a", emptyConfig())); final AgentlessConfigurationSource service = new AgentlessConfigurationSource( @@ -903,16 +905,19 @@ void initCompletesFirstPollAndCloseCancelsScheduledFuture() throws Exception { Executors.newSingleThreadScheduledExecutor()); FeatureFlaggingGateway.addConfigListener(listener); - service.init(); - assertEquals(1, client.calls.get()); - service.close(); + try { + service.init(); - verify(listener).accept(any(ServerConfiguration.class)); + verify(listener, timeout(1_000)).accept(any(ServerConfiguration.class)); + assertEquals(1, client.calls.get()); + } finally { + service.close(); + } } @Test - void initCompletesInitialRetryCycleBeforeReturning() throws Exception { - final List requests = new ArrayList<>(); + void initSchedulesInitialRetryCycle() throws Exception { + final List requests = new CopyOnWriteArrayList<>(); final AgentlessConfigurationSource.OkHttpUfcHttpClient client = scriptedClient( requests, @@ -932,13 +937,33 @@ void initCompletesInitialRetryCycleBeforeReturning() throws Exception { try { service.init(); + verify(listener, timeout(1_000)).accept(any(ServerConfiguration.class)); assertEquals(2, requests.size()); - verify(listener).accept(any(ServerConfiguration.class)); } finally { service.close(); } } + @Test + void initReturnsWhileInitialRequestIsInFlight() throws Exception { + final CountDownLatch requestStarted = new CountDownLatch(1); + final CountDownLatch releaseRequest = new CountDownLatch(1); + final FakeClient client = new FakeClient(response(200, "etag-a", emptyConfig())); + client.block(requestStarted, releaseRequest); + final AgentlessConfigurationSource service = service(client); + final ExecutorService runner = Executors.newSingleThreadExecutor(); + + try { + final Future initialization = runner.submit(service::init); + initialization.get(250, TimeUnit.MILLISECONDS); + assertTrue(requestStarted.await(1, TimeUnit.SECONDS)); + } finally { + releaseRequest.countDown(); + service.close(); + runner.shutdownNow(); + } + } + @Test void repeatedInitStartsOnlyOnePoller() throws Exception { final FakeClient client = new FakeClient(response(200, "etag-a", emptyConfig())); @@ -1058,19 +1083,17 @@ void closeInterruptsRetryBackoff() throws Exception { final AgentlessConfigurationSource service = new AgentlessConfigurationSource( HttpUrl.get("http://localhost" + CONFIG_PATH), config(), 30_000, client, executor); - final ExecutorService runner = Executors.newSingleThreadExecutor(); try { - final Future initialization = runner.submit(service::init); + service.init(); assertTrue(backoffStarted.await(1, TimeUnit.SECONDS)); service.close(); - initialization.get(1, TimeUnit.SECONDS); assertTrue(executor.awaitTermination(1, TimeUnit.SECONDS)); assertEquals(1, requests.size()); } finally { - runner.shutdownNow(); + service.close(); } }