From 1a94009f1b9e2cc399b351cc40d906c28a895b08 Mon Sep 17 00:00:00 2001 From: om7057 Date: Sun, 9 Aug 2026 14:18:57 +0530 Subject: [PATCH] fix: bound preOnline timeout so RTT-sensitive agents are not kicked offline preOnline() called Future.get() with no timeout while waiting for a build agent to acknowledge its OpenTelemetry SDK configuration. This blocked SlaveComputer.setChannel() for however long the remote call took, giving unrelated NAT idle timeouts or network disturbances a much bigger window to close the channel before the online handshake finished, especially for agents connecting over higher RTT links. Bound the wait to a configurable timeout (default 10 seconds, matching the existing timeout in afterConfiguration), and log a warning instead of blocking indefinitely when it elapses. The agent is still allowed online even if its OpenTelemetry configuration RPC has not completed. Fixes #1285 --- ...enTelemetryConfigurerComputerListener.java | 29 +++- .../semconv/ConfigurationKey.java | 6 + ...lemetryConfigurerComputerListenerTest.java | 161 ++++++++++++++++++ 3 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 src/test/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListenerTest.java diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListener.java b/src/main/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListener.java index 317cbc259..868e09f98 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListener.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListener.java @@ -26,6 +26,7 @@ import io.opentelemetry.semconv.incubating.CicdIncubatingAttributes; import io.opentelemetry.semconv.incubating.ServiceIncubatingAttributes; import java.io.IOException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -59,6 +60,15 @@ public class OpenTelemetryConfigurerComputerListener extends ComputerListener final AtomicBoolean buildAgentsInstrumentationEnabled = new AtomicBoolean(false); + /** + * Default timeout applied while waiting, in {@link #preOnline(Computer, Channel, FilePath, TaskListener)}, + * for a build agent to acknowledge its OpenTelemetry SDK configuration. Matches the timeout historically + * used in {@link #afterConfiguration(ConfigProperties)}. + */ + static final Duration DEFAULT_PRE_ONLINE_TIMEOUT = Duration.ofSeconds(10); + + private volatile Duration preOnlineTimeout = DEFAULT_PRE_ONLINE_TIMEOUT; + JenkinsOpenTelemetryPluginConfiguration jenkinsOpenTelemetryPluginConfiguration; private SemConvStability semConvStability; @@ -72,10 +82,10 @@ public void preOnline(Computer computer, Channel channel, FilePath root, TaskLis Map otelSdkProperties = openTelemetryConfiguration.toOpenTelemetryProperties(); Map otelSdkResourceProperties = openTelemetryConfiguration.toOpenTelemetryResourceAsMap(); + Future future = + configureOpenTelemetrySdkOnComputer(computer, channel, otelSdkProperties, otelSdkResourceProperties); try { - Object result = configureOpenTelemetrySdkOnComputer( - computer, channel, otelSdkProperties, otelSdkResourceProperties) - .get(); + Object result = future.get(preOnlineTimeout.toMillis(), TimeUnit.MILLISECONDS); logger.log( Level.FINE, () -> "Updated OpenTelemetry configuration for computer/build-agent '" + computer.getName() @@ -93,6 +103,16 @@ public void preOnline(Computer computer, Channel channel, FilePath root, TaskLis e, () -> "Failure to update OpenTelemetry configuration for computer/build-agent '" + computer.getName() + "'"); + } catch (TimeoutException e) { + // Don't let a slow or unresponsive agent connection block the agent from coming online. + // Blocking here indefinitely gave RTT-sensitive channels time to be closed by unrelated + // idle-timeout or network disturbances, which then kicked the agent off entirely. + future.cancel(true); + logger.log( + Level.WARNING, + () -> "Timed out after " + preOnlineTimeout + + " waiting for OpenTelemetry configuration of computer/build-agent '" + + computer.getName() + "', the agent will still be allowed online"); } } @@ -125,6 +145,9 @@ public void afterConfiguration(@NonNull ConfigProperties configProperties) { .equalsIgnoreCase(configProperties.getString( ConfigurationKey.OTEL_INSTRUMENTATION_JENKINS_AGENTS_ENABLED.asProperty())); this.buildAgentsInstrumentationEnabled.set(otlpLogsEnabled || !jenkinsAgentInstrumentationDisabled); + this.preOnlineTimeout = configProperties.getDuration( + ConfigurationKey.OTEL_INSTRUMENTATION_JENKINS_AGENT_PRE_ONLINE_TIMEOUT.asProperty(), + DEFAULT_PRE_ONLINE_TIMEOUT); if (!buildAgentsInstrumentationEnabled.get()) { return; } diff --git a/src/main/java/io/jenkins/plugins/opentelemetry/semconv/ConfigurationKey.java b/src/main/java/io/jenkins/plugins/opentelemetry/semconv/ConfigurationKey.java index 15b4cb8c6..510d4c15a 100644 --- a/src/main/java/io/jenkins/plugins/opentelemetry/semconv/ConfigurationKey.java +++ b/src/main/java/io/jenkins/plugins/opentelemetry/semconv/ConfigurationKey.java @@ -62,6 +62,12 @@ public final class ConfigurationKey { */ public static final ConfigurationKey OTEL_INSTRUMENTATION_JENKINS_AGENTS_ENABLED = new ConfigurationKey("otel.instrumentation.jenkins.agent.enabled"); + /** + * Timeout applied while waiting for a build agent to acknowledge its OpenTelemetry SDK configuration + * before it is allowed to come online. + */ + public static final ConfigurationKey OTEL_INSTRUMENTATION_JENKINS_AGENT_PRE_ONLINE_TIMEOUT = + new ConfigurationKey("otel.instrumentation.jenkins.agent.pre_online.timeout"); public static final ConfigurationKey OTEL_INSTRUMENTATION_JENKINS_EXPORT_OTEL_CONFIG_AS_ENV_VARS = new ConfigurationKey("otel.instrumentation.jenkins.export_otel_config_as_env_vars"); diff --git a/src/test/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListenerTest.java b/src/test/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListenerTest.java new file mode 100644 index 000000000..e00cc3666 --- /dev/null +++ b/src/test/java/io/jenkins/plugins/opentelemetry/jenkins/OpenTelemetryConfigurerComputerListenerTest.java @@ -0,0 +1,161 @@ +/* + * Copyright The Original Author or Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package io.jenkins.plugins.opentelemetry.jenkins; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import hudson.model.Computer; +import hudson.remoting.Channel; +import io.jenkins.plugins.opentelemetry.JenkinsOpenTelemetryPluginConfiguration; +import io.jenkins.plugins.opentelemetry.OpenTelemetryConfiguration; +import io.jenkins.plugins.opentelemetry.semconv.SemConvStability; +import java.lang.reflect.Field; +import java.time.Duration; +import java.util.Collections; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.junit.jupiter.api.Test; + +/** + * Regression tests for issue #1285: + * {@link OpenTelemetryConfigurerComputerListener#preOnline(Computer, Channel, hudson.FilePath, hudson.model.TaskListener)} + * used to block indefinitely on the agent configuration RPC, which left RTT-sensitive agents exposed to being + * kicked offline if the underlying channel closed before the RPC completed. + */ +class OpenTelemetryConfigurerComputerListenerTest { + + @Test + void preOnlineReturnsPromptlyWhenAgentConfigurationRpcNeverCompletes() throws Exception { + OpenTelemetryConfigurerComputerListener listener = newListener(Duration.ofMillis(100)); + Computer computer = mockComputer("agent-high-rtt"); + Channel channel = mock(Channel.class); + when(channel.callAsync(any())).thenReturn(new NeverCompletingFuture()); + + long startNanos = System.nanoTime(); + assertDoesNotThrow(() -> listener.preOnline(computer, channel, null, null)); + long elapsedMillis = (System.nanoTime() - startNanos) / 1_000_000; + + assertTrue( + elapsedMillis < 5_000, + "preOnline() must return once the configured timeout elapses instead of blocking indefinitely, took " + + elapsedMillis + "ms"); + } + + @Test + void preOnlineDoesNotPropagateExecutionExceptionFromAgent() throws Exception { + OpenTelemetryConfigurerComputerListener listener = newListener(Duration.ofSeconds(10)); + Computer computer = mockComputer("agent-failing-rpc"); + Channel channel = mock(Channel.class); + CompletableFuture failed = new CompletableFuture<>(); + failed.completeExceptionally(new RuntimeException("simulated remoting failure")); + when(channel.callAsync(any())).thenReturn(new DelegatingFuture(failed)); + + assertDoesNotThrow(() -> listener.preOnline(computer, channel, null, null)); + } + + private static Computer mockComputer(String name) { + Computer computer = mock(Computer.class); + when(computer.getName()).thenReturn(name); + return computer; + } + + private static OpenTelemetryConfigurerComputerListener newListener(Duration preOnlineTimeout) throws Exception { + OpenTelemetryConfigurerComputerListener listener = new OpenTelemetryConfigurerComputerListener(); + listener.buildAgentsInstrumentationEnabled.set(true); + + OpenTelemetryConfiguration openTelemetryConfiguration = mock(OpenTelemetryConfiguration.class); + when(openTelemetryConfiguration.toOpenTelemetryProperties()).thenReturn(Collections.emptyMap()); + when(openTelemetryConfiguration.toOpenTelemetryResourceAsMap()).thenReturn(Collections.emptyMap()); + + JenkinsOpenTelemetryPluginConfiguration pluginConfiguration = + mock(JenkinsOpenTelemetryPluginConfiguration.class); + when(pluginConfiguration.getSemConvStability()).thenReturn(SemConvStability.OTEL); + when(pluginConfiguration.toOpenTelemetryConfiguration()).thenReturn(openTelemetryConfiguration); + listener.setJenkinsOpenTelemetryPluginConfiguration(pluginConfiguration); + + Field timeoutField = OpenTelemetryConfigurerComputerListener.class.getDeclaredField("preOnlineTimeout"); + timeoutField.setAccessible(true); + timeoutField.set(listener, preOnlineTimeout); + + return listener; + } + + /** + * A {@link hudson.remoting.Future} that never completes, simulating a build agent configuration RPC sent over + * a slow or RTT-bound remoting channel. + */ + private static final class NeverCompletingFuture implements hudson.remoting.Future { + private final CompletableFuture delegate = new CompletableFuture<>(); + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return delegate.cancel(mayInterruptIfRunning); + } + + @Override + public boolean isCancelled() { + return delegate.isCancelled(); + } + + @Override + public boolean isDone() { + return delegate.isDone(); + } + + @Override + public Object get() throws InterruptedException, ExecutionException { + return delegate.get(); + } + + @Override + public Object get(long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, TimeoutException { + return delegate.get(timeout, unit); + } + } + + /** + * Adapts a {@link CompletableFuture} to {@link hudson.remoting.Future}. + */ + private static final class DelegatingFuture implements hudson.remoting.Future { + private final CompletableFuture delegate; + + DelegatingFuture(CompletableFuture delegate) { + this.delegate = delegate; + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return delegate.cancel(mayInterruptIfRunning); + } + + @Override + public boolean isCancelled() { + return delegate.isCancelled(); + } + + @Override + public boolean isDone() { + return delegate.isDone(); + } + + @Override + public Object get() throws InterruptedException, ExecutionException { + return delegate.get(); + } + + @Override + public Object get(long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, TimeoutException { + return delegate.get(timeout, unit); + } + } +}