Skip to content

Commit 124bf56

Browse files
asadjan4611asadjan4611
committed
[Improvement][alert] Add configurable timeout for script alert plugin (#18034)
1 parent b12b682 commit 124bf56

7 files changed

Lines changed: 139 additions & 13 deletions

File tree

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtils.java‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package org.apache.dolphinscheduler.plugin.alert.script;
1919

2020
import java.io.IOException;
21+
import java.util.concurrent.TimeUnit;
2122

2223
import lombok.extern.slf4j.Slf4j;
2324

@@ -29,12 +30,13 @@ private ProcessUtils() {
2930
}
3031

3132
/**
32-
* executeScript
33+
* executeScript with timeout
3334
*
35+
* @param timeoutSeconds timeout in seconds, if <= 0 waits indefinitely
3436
* @param cmd cmd params
35-
* @return exit code
37+
* @return exit code, -1 if error, -2 if timeout
3638
*/
37-
static Integer executeScript(String... cmd) {
39+
static Integer executeScript(long timeoutSeconds, String... cmd) {
3840

3941
int exitCode = -1;
4042

@@ -46,12 +48,53 @@ static Integer executeScript(String... cmd) {
4648

4749
inputStreamGobbler.start();
4850
errorStreamGobbler.start();
49-
return process.waitFor();
50-
} catch (IOException | InterruptedException e) {
51-
log.error("execute alert script error {}", e.getMessage());
51+
52+
if (timeoutSeconds > 0) {
53+
boolean finished = process.waitFor(timeoutSeconds, TimeUnit.SECONDS);
54+
if (!finished) {
55+
log.error("script execution timed out after {} seconds, destroying process", timeoutSeconds);
56+
process.destroyForcibly();
57+
closeProcessStreams(process);
58+
joinGobbler(inputStreamGobbler);
59+
joinGobbler(errorStreamGobbler);
60+
return -2;
61+
}
62+
} else {
63+
process.waitFor();
64+
}
65+
int processExitCode = process.exitValue();
66+
joinGobbler(inputStreamGobbler);
67+
joinGobbler(errorStreamGobbler);
68+
return processExitCode;
69+
} catch (InterruptedException e) {
70+
log.error("execute alert script interrupted {}", e.getMessage());
5271
Thread.currentThread().interrupt();
72+
} catch (IOException e) {
73+
log.error("execute alert script error {}", e.getMessage());
5374
}
5475

5576
return exitCode;
5677
}
78+
79+
private static void closeProcessStreams(Process process) {
80+
try {
81+
process.getInputStream().close();
82+
} catch (IOException e) {
83+
log.warn("Failed to close process input stream after timeout", e);
84+
}
85+
try {
86+
process.getErrorStream().close();
87+
} catch (IOException e) {
88+
log.warn("Failed to close process error stream after timeout", e);
89+
}
90+
}
91+
92+
private static void joinGobbler(StreamGobbler gobbler) {
93+
try {
94+
gobbler.interrupt();
95+
gobbler.join(TimeUnit.SECONDS.toMillis(1));
96+
} catch (InterruptedException e) {
97+
Thread.currentThread().interrupt();
98+
}
99+
}
57100
}

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactory.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,12 @@
2121
import org.apache.dolphinscheduler.alert.api.AlertChannelFactory;
2222
import org.apache.dolphinscheduler.alert.api.AlertInputTips;
2323
import org.apache.dolphinscheduler.common.utils.JSONUtils;
24+
import org.apache.dolphinscheduler.spi.params.base.DataType;
2425
import org.apache.dolphinscheduler.spi.params.base.ParamsOptions;
2526
import org.apache.dolphinscheduler.spi.params.base.PluginParams;
2627
import org.apache.dolphinscheduler.spi.params.base.Validate;
2728
import org.apache.dolphinscheduler.spi.params.input.InputParam;
29+
import org.apache.dolphinscheduler.spi.params.input.number.InputNumberParam;
2830
import org.apache.dolphinscheduler.spi.params.radio.RadioParam;
2931

3032
import java.util.Arrays;
@@ -66,7 +68,18 @@ public List<PluginParams> params() {
6668
.addValidate(Validate.newBuilder().setRequired(true).build())
6769
.build();
6870

69-
return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams);
71+
InputNumberParam scriptTimeoutParam =
72+
InputNumberParam.newBuilder(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT,
73+
ScriptParamsConstants.SCRIPT_TIMEOUT)
74+
.setValue(ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT)
75+
.addValidate(Validate.newBuilder()
76+
.setType(DataType.NUMBER.getDataType())
77+
.setRequired(false)
78+
.setMin(0D)
79+
.build())
80+
.build();
81+
82+
return Arrays.asList(scriptUserParam, scriptPathParam, scriptTypeParams, scriptTimeoutParam);
7083
}
7184

7285
@Override

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptParamsConstants.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ public final class ScriptParamsConstants {
3131

3232
static final String NAME_SCRIPT_USER_PARAMS = "userParams";
3333

34+
static final String SCRIPT_TIMEOUT = "$t('timeout')";
35+
36+
static final String NAME_SCRIPT_TIMEOUT = "timeout";
37+
38+
static final int DEFAULT_SCRIPT_TIMEOUT = 60;
39+
3440
private ScriptParamsConstants() {
3541
throw new UnsupportedOperationException("This is a utility class and cannot be instantiated");
3642
}

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public final class ScriptSender {
3636
private final String scriptPath;
3737
private final String scriptType;
3838
private final String userParams;
39+
private final long timeout;
3940

4041
ScriptSender(Map<String, String> config) {
4142
scriptPath = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_PATH))
@@ -47,6 +48,19 @@ public final class ScriptSender {
4748
userParams = StringUtils.isNotBlank(config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS))
4849
? config.get(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS)
4950
: "";
51+
String timeoutConfig = config.get(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT);
52+
if (StringUtils.isNotBlank(timeoutConfig)) {
53+
long parsedTimeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT;
54+
try {
55+
parsedTimeout = Long.parseLong(timeoutConfig);
56+
} catch (NumberFormatException ex) {
57+
log.warn("Invalid script timeout config value: '{}', using default: {}",
58+
timeoutConfig, ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT, ex);
59+
}
60+
timeout = parsedTimeout;
61+
} else {
62+
timeout = ScriptParamsConstants.DEFAULT_SCRIPT_TIMEOUT;
63+
}
5064
}
5165

5266
AlertResult sendScriptAlert(String title, String content) {
@@ -108,13 +122,19 @@ private AlertResult executeShellScript(String title, String content) {
108122

109123
String[] cmd = {"/bin/sh", "-c", scriptPath + ALERT_TITLE_OPTION + "'" + title + "'" + ALERT_CONTENT_OPTION
110124
+ "'" + content + "'" + ALERT_USER_PARAMS_OPTION + "'" + userParams + "'"};
111-
int exitCode = ProcessUtils.executeScript(cmd);
125+
int exitCode = ProcessUtils.executeScript(timeout, cmd);
112126

113127
if (exitCode == 0) {
114128
alertResult.setSuccess(true);
115129
alertResult.setMessage("send script alert msg success");
116130
return alertResult;
117131
}
132+
if (exitCode == -2) {
133+
alertResult.setMessage("send script alert msg error, script execution timed out after " + timeout
134+
+ " seconds");
135+
log.error("send script alert msg error, script execution timed out after {} seconds", timeout);
136+
return alertResult;
137+
}
118138
alertResult.setMessage("send script alert msg error,exitCode is " + exitCode);
119139
log.info("send script alert msg error,exitCode is {}", exitCode);
120140
return alertResult;

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ProcessUtilsTest.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@
1717

1818
package org.apache.dolphinscheduler.plugin.alert.script;
1919

20+
import org.junit.jupiter.api.Assertions;
2021
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.condition.DisabledOnOs;
23+
import org.junit.jupiter.api.condition.OS;
2124

2225
/**
2326
* ProcessUtilsTest
@@ -32,8 +35,17 @@ public class ProcessUtilsTest {
3235
private String[] cmd = {"/bin/sh", "-c", shellFilPath + " -t 1"};
3336

3437
@Test
38+
@DisabledOnOs(OS.WINDOWS)
3539
public void testExecuteScript() {
36-
int code = ProcessUtils.executeScript(cmd);
37-
assert code != -1;
40+
int code = ProcessUtils.executeScript(60, cmd);
41+
Assertions.assertNotEquals(-1, code);
42+
}
43+
44+
@Test
45+
@DisabledOnOs(OS.WINDOWS)
46+
public void testExecuteScriptTimeout() {
47+
String[] sleepCmd = {"/bin/sh", "-c", "sleep 30"};
48+
int code = ProcessUtils.executeScript(2, sleepCmd);
49+
Assertions.assertEquals(-2, code);
3850
}
3951
}

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptAlertChannelFactoryTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class ScriptAlertChannelFactoryTest {
3434
public void testGetParams() {
3535
ScriptAlertChannelFactory scriptAlertChannelFactory = new ScriptAlertChannelFactory();
3636
List<PluginParams> params = scriptAlertChannelFactory.params();
37-
Assertions.assertEquals(3, params.size());
37+
Assertions.assertEquals(4, params.size());
3838
}
3939

4040
@Test

‎dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import org.junit.jupiter.api.Assertions;
2828
import org.junit.jupiter.api.BeforeEach;
2929
import org.junit.jupiter.api.Test;
30+
import org.junit.jupiter.api.condition.DisabledOnOs;
31+
import org.junit.jupiter.api.condition.OS;
3032

3133
/**
3234
* ScriptSenderTest
@@ -35,17 +37,18 @@ public class ScriptSenderTest {
3537

3638
private static final String rootPath = System.getProperty("user.dir");
3739
private static final String shellFilPath = rootPath + "/src/test/script/shell/scriptExample.sh";
38-
private static Map<String, String> scriptConfig = new HashMap<>();
40+
private Map<String, String> scriptConfig;
3941

4042
@BeforeEach
4143
public void initScriptConfig() {
42-
44+
scriptConfig = new HashMap<>();
4345
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TYPE, String.valueOf(ScriptType.SHELL.getDescp()));
4446
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "userParams");
4547
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, shellFilPath);
4648
}
4749

4850
@Test
51+
@DisabledOnOs(OS.WINDOWS)
4952
public void testScriptSenderTest() {
5053
ScriptSender scriptSender = new ScriptSender(scriptConfig);
5154
AlertResult alertResult;
@@ -56,6 +59,7 @@ public void testScriptSenderTest() {
5659
}
5760

5861
@Test
62+
@DisabledOnOs(OS.WINDOWS)
5963
public void testScriptSenderInjectionTest() {
6064
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "' ; calc.exe ; '");
6165
ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -64,6 +68,7 @@ public void testScriptSenderInjectionTest() {
6468
}
6569

6670
@Test
71+
@DisabledOnOs(OS.WINDOWS)
6772
public void testUserParamsNPE() {
6873
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, null);
6974
ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -82,6 +87,7 @@ public void testPathNPE() {
8287
}
8388

8489
@Test
90+
@DisabledOnOs(OS.WINDOWS)
8591
public void testPathError() {
8692
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_PATH, "/usr/sbin/abc");
8793
ScriptSender scriptSender = new ScriptSender(scriptConfig);
@@ -100,4 +106,30 @@ public void testTypeIsError() {
100106
assertFalse(alertResult.isSuccess());
101107
}
102108

109+
@Test
110+
@DisabledOnOs(OS.WINDOWS)
111+
public void testDefaultTimeout() {
112+
ScriptSender scriptSender = new ScriptSender(scriptConfig);
113+
AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
114+
Assertions.assertTrue(alertResult.isSuccess());
115+
}
116+
117+
@Test
118+
@DisabledOnOs(OS.WINDOWS)
119+
public void testCustomTimeout() {
120+
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "30");
121+
ScriptSender scriptSender = new ScriptSender(scriptConfig);
122+
AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
123+
Assertions.assertTrue(alertResult.isSuccess());
124+
}
125+
126+
@Test
127+
@DisabledOnOs(OS.WINDOWS)
128+
public void testInvalidTimeoutFallsBackToDefault() {
129+
scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_TIMEOUT, "notANumber");
130+
ScriptSender scriptSender = new ScriptSender(scriptConfig);
131+
AlertResult alertResult = scriptSender.sendScriptAlert("test title Kris", "test content");
132+
Assertions.assertTrue(alertResult.isSuccess());
133+
}
134+
103135
}

0 commit comments

Comments
 (0)