From 124bf56ddbf604c2092383bf8f1e3ec59db831f4 Mon Sep 17 00:00:00 2001 From: asadjan4611 Date: Thu, 12 Mar 2026 18:52:30 +0500 Subject: [PATCH] [Improvement][alert] Add configurable timeout for script alert plugin (#18034) --- .../plugin/alert/script/ProcessUtils.java | 55 +++++++++++++++++-- .../script/ScriptAlertChannelFactory.java | 15 ++++- .../alert/script/ScriptParamsConstants.java | 6 ++ .../plugin/alert/script/ScriptSender.java | 22 +++++++- .../plugin/alert/script/ProcessUtilsTest.java | 16 +++++- .../script/ScriptAlertChannelFactoryTest.java | 2 +- .../plugin/alert/script/ScriptSenderTest.java | 36 +++++++++++- 7 files changed, 139 insertions(+), 13 deletions(-) diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java index b629ff5030ba..286245bdc390 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java @@ -18,6 +18,7 @@ package org.apache.dolphinscheduler.plugin.alert.script; import java.io.IOException; +import java.util.concurrent.TimeUnit; import lombok.extern.slf4j.Slf4j; @@ -29,12 +30,13 @@ private ProcessUtils() { } /** - * executeScript + * executeScript with timeout * + * @param timeoutSeconds timeout in seconds, if <= 0 waits indefinitely * @param cmd cmd params - * @return exit code + * @return exit code, -1 if error, -2 if timeout */ - static Integer executeScript(String... cmd) { + static Integer executeScript(long timeoutSeconds, String... cmd) { int exitCode = -1; @@ -46,12 +48,53 @@ static Integer executeScript(String... cmd) { inputStreamGobbler.start(); errorStreamGobbler.start(); - return process.waitFor(); - } catch (IOException | InterruptedException e) { - log.error("execute alert script error {}", e.getMessage()); + + if (timeoutSeconds > 0) { + boolean finished = process.waitFor(timeoutSeconds, TimeUnit.SECONDS); + if (!finished) { + log.error("script execution timed out after {} seconds, destroying process", timeoutSeconds); + process.destroyForcibly(); + closeProcessStreams(process); + joinGobbler(inputStreamGobbler); + joinGobbler(errorStreamGobbler); + return -2; + } + } else { + process.waitFor(); + } + int processExitCode = process.exitValue(); + joinGobbler(inputStreamGobbler); + joinGobbler(errorStreamGobbler); + return processExitCode; + } catch (InterruptedException e) { + log.error("execute alert script interrupted {}", e.getMessage()); Thread.currentThread().interrupt(); + } catch (IOException e) { + log.error("execute alert script error {}", e.getMessage()); } return exitCode; } + + private static void closeProcessStreams(Process process) { + try { + process.getInputStream().close(); + } catch (IOException e) { + log.warn("Failed to close process input stream after timeout", e); + } + try { + process.getErrorStream().close(); + } catch (IOException e) { + log.warn("Failed to close process error stream after timeout", e); + } + } + + private static void joinGobbler(StreamGobbler gobbler) { + try { + gobbler.interrupt(); + gobbler.join(TimeUnit.SECONDS.toMillis(1)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } } diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java index a8364c2f332a..cbd08bc15e1d 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java @@ -21,10 +21,12 @@ import org.apache.dolphinscheduler.alert.api.AlertChannelFactory; import org.apache.dolphinscheduler.alert.api.AlertInputTips; import org.apache.dolphinscheduler.common.utils.JSONUtils; +import org.apache.dolphinscheduler.spi.params.base.DataType; import org.apache.dolphinscheduler.spi.params.base.ParamsOptions; import org.apache.dolphinscheduler.spi.params.base.PluginParams; import org.apache.dolphinscheduler.spi.params.base.Validate; import org.apache.dolphinscheduler.spi.params.input.InputParam; +import org.apache.dolphinscheduler.spi.params.input.number.InputNumberParam; import org.apache.dolphinscheduler.spi.params.radio.RadioParam; import java.util.Arrays; @@ -66,7 +68,18 @@ public List params() { .addValidate(Validate.newBuilder().setRequired(true).build()) .build(); - return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams); + InputNumberParam scriptTimeoutParam = + InputNumberParam.newBuilder(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, + ScriptParamsConstants.SCRIPT_TIMEOUT) + .setValue(ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT) + .addValidate(Validate.newBuilder() + .setType(DataType.NUMBER.getDataType()) + .setRequired(false) + .setMin(0D) + .build()) + .build(); + + return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams, scriptTimeoutParam); } @Override diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java index 1c759525b399..c99ece0cfe89 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java @@ -31,6 +31,12 @@ public final class ScriptParamsConstants { static final String NAME_SCRIPT_USER_PARAMS = "userParams"; + static final String SCRIPT_TIMEOUT = "$t('timeout')"; + + static final String NAME_SCRIPT_TIMEOUT = "timeout"; + + static final int DEFAULT_SCRIPT_TIMEOUT = 60; + private ScriptParamsConstants() { throw new UnsupportedOperationException("This is a utility class and cannot be instantiated"); } diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java index 19b7149e74ee..4eee3c45df51 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java @@ -36,6 +36,7 @@ public final class ScriptSender { private final String scriptPath; private final String scriptType; private final String userParams; + private final long timeout; ScriptSender(Map config) { scriptPath = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_PATH)) @@ -47,6 +48,19 @@ public final class ScriptSender { userParams = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS)) ? config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS) : ""; + String timeoutConfig = config.get(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT); + if (StringUtils.isNotBlank(timeoutConfig)) { + long parsedTimeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT; + try { + parsedTimeout = Long.parseLong(timeoutConfig); + } catch (NumberFormatException ex) { + log.warn("Invalid script timeout config value: '{}', using default: {}", + timeoutConfig, ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT, ex); + } + timeout = parsedTimeout; + } else { + timeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT; + } } AlertResult sendScriptAlert(String title, String content) { @@ -108,13 +122,19 @@ private AlertResult executeShellScript(String title, String content) { String[] cmd = {"/bin/sh", "-c", scriptPath + ALERT_TITLE_OPTION + "'" + title + "'" + ALERT_CONTENT_OPTION + "'" + content + "'" + ALERT_USER_PARAMS_OPTION + "'" + userParams + "'"}; - int exitCode = ProcessUtils.executeScript(cmd); + int exitCode = ProcessUtils.executeScript(timeout, cmd); if (exitCode == 0) { alertResult.setSuccess(true); alertResult.setMessage("send script alert msg success"); return alertResult; } + if (exitCode == -2) { + alertResult.setMessage("send script alert msg error, script execution timed out after " + timeout + + " seconds"); + log.error("send script alert msg error, script execution timed out after {} seconds", timeout); + return alertResult; + } alertResult.setMessage("send script alert msg error,exitCode is " + exitCode); log.info("send script alert msg error,exitCode is {}", exitCode); return alertResult; diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java index 3d85d5d638a3..24d939811b11 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java @@ -17,7 +17,10 @@ package org.apache.dolphinscheduler.plugin.alert.script; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; /** * ProcessUtilsTest @@ -32,8 +35,17 @@ public class ProcessUtilsTest { private String[] cmd = {"/bin/sh", "-c", shellFilPath + " -t 1"}; @Test + @DisabledOnOs(OS.WINDOWS) public void testExecuteScript() { - int code = ProcessUtils.executeScript(cmd); - assert code != -1; + int code = ProcessUtils.executeScript(60, cmd); + Assertions.assertNotEquals(-1, code); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + public void testExecuteScriptTimeout() { + String[] sleepCmd = {"/bin/sh", "-c", "sleep 30"}; + int code = ProcessUtils.executeScript(2, sleepCmd); + Assertions.assertEquals(-2, code); } } diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java index f9b7e338f65e..2447dba10d3f 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java @@ -34,7 +34,7 @@ public class ScriptAlertChannelFactoryTest { public void testGetParams() { ScriptAlertChannelFactory scriptAlertChannelFactory = new ScriptAlertChannelFactory(); List params = scriptAlertChannelFactory.params(); - Assertions.assertEquals(3, params.size()); + Assertions.assertEquals(4, params.size()); } @Test diff --git a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java index c392b6f7587f..677b51aa7abb 100644 --- a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java +++ b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java @@ -27,6 +27,8 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; /** * ScriptSenderTest @@ -35,17 +37,18 @@ public class ScriptSenderTest { private static final String rootPath = System.getProperty("user.dir"); private static final String shellFilPath = rootPath + "/src/test/script/shell/scriptExample.sh"; - private static Map scriptConfig = new HashMap<>(); + private Map scriptConfig; @BeforeEach public void initScriptConfig() { - + scriptConfig = new HashMap<>(); scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TYPE, String.valueOf(ScriptType.SHELL.getDescp())); scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "userParams"); scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, shellFilPath); } @Test + @DisabledOnOs(OS.WINDOWS) public void testScriptSenderTest() { ScriptSender scriptSender = new ScriptSender(scriptConfig); AlertResult alertResult; @@ -56,6 +59,7 @@ public void testScriptSenderTest() { } @Test + @DisabledOnOs(OS.WINDOWS) public void testScriptSenderInjectionTest() { scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "' ; calc.exe ; '"); ScriptSender scriptSender = new ScriptSender(scriptConfig); @@ -64,6 +68,7 @@ public void testScriptSenderInjectionTest() { } @Test + @DisabledOnOs(OS.WINDOWS) public void testUserParamsNPE() { scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, null); ScriptSender scriptSender = new ScriptSender(scriptConfig); @@ -82,6 +87,7 @@ public void testPathNPE() { } @Test + @DisabledOnOs(OS.WINDOWS) public void testPathError() { scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, "/usr/sbin/abc"); ScriptSender scriptSender = new ScriptSender(scriptConfig); @@ -100,4 +106,30 @@ public void testTypeIsError() { assertFalse(alertResult.isSuccess()); } + @Test + @DisabledOnOs(OS.WINDOWS) + public void testDefaultTimeout() { + ScriptSender scriptSender = new ScriptSender(scriptConfig); + AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content"); + Assertions.assertTrue(alertResult.isSuccess()); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + public void testCustomTimeout() { + scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "30"); + ScriptSender scriptSender = new ScriptSender(scriptConfig); + AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content"); + Assertions.assertTrue(alertResult.isSuccess()); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + public void testInvalidTimeoutFallsBackToDefault() { + scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "notANumber"); + ScriptSender scriptSender = new ScriptSender(scriptConfig); + AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content"); + Assertions.assertTrue(alertResult.isSuccess()); + } + }