[DSIP-99][TaskPlugin] Save task output to a separate file by Mrhs121 · Pull Request #18098 · apache/dolphinscheduler

Mrhs121 · 2026-03-26T12:06:16Z

Was this PR generated or assisted by AI?

Purpose of the pull request

close #17791

Brief change log

Verify this pull request

This pull request is code cleanup without any test coverage.

(or)

This pull request is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(or)

Pull Request Notice

If your pull request contains incompatible change, you should also add it to docs/docs/en/guide/upgrade/incompatible.md

Mrhs121 · 2026-03-26T12:11:34Z

initial commit

Mrhs121 · 2026-03-26T14:32:06Z

This commit was done a long time ago. When I just rebase the code, there were many conflicts. After using AI to automatically solve them, I found many error points. I need to recheck them

Mrhs121 · 2026-03-26T15:38:40Z

...inscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/LoggerServiceTest.java


    @Test
-    public void testQueryLogInSpecifiedProject() {
-        long projectCode = 1L;


dc38d34 this commit removed the related service, So I casually removed the useless code from the test as well

SbloodyS

Is this PR ready to review? @Mrhs121

Mrhs121 · 2026-04-07T07:07:37Z

Is this PR ready to review? @Mrhs121

I'm not ready yet. I haven't tested myself yet

SbloodyS · 2026-04-07T07:09:54Z

Feel free to ping me when you are ready to review. @Mrhs121

Mrhs121 · 2026-04-09T10:50:46Z

Self-test results of common tasks

stored procedure task

http task

sql task
shell task
spark task

sonarqubecloud · 2026-04-12T03:27:41Z

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 60%)

See analysis details on SonarQube Cloud

ruanwenjun · 2026-04-12T03:15:01Z

...cheduler-api/src/main/java/org/apache/dolphinscheduler/api/executor/logging/TaskLogType.java

+public enum TaskLogType {
+
+    LOG {
+
+        @Override
+        public String getLogPath(TaskInstance taskInstance) {
+            return taskInstance.getLogPath();
+        }
+    },
+    OUTPUT {
+
+        @Override
+        public String getLogPath(TaskInstance taskInstance) {
+            return taskInstance.getTaskOutputLogPath();
+        }
+    };
+
+    public abstract String getLogPath(TaskInstance taskInstance);
+}


It's better to split this kind of logic from enum to a seperate class.

ruanwenjun · 2026-04-12T03:21:03Z

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql

    host                    varchar(135) DEFAULT NULL,
    execute_path            varchar(200) DEFAULT NULL,
    log_path                longtext DEFAULT NULL,
+    task_output_log_path    longtext DEFAULT NULL,


Suggested change

task_output_log_path longtext DEFAULT NULL,

task_output_log_path varchar(255) DEFAULT NULL,

Don't use longtext

ruanwenjun · 2026-04-12T03:23:03Z

dolphinscheduler-master/src/main/resources/logback-spring.xml

+    <logger name="TaskOutput" level="INFO" additivity="false">
+        <appender-ref ref="TASKOUTPUTLOGFILE"/>
+    </logger>


Move under root?

ruanwenjun · 2026-04-12T03:28:10Z

...k-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java

@@ -205,13 +214,19 @@ private Optional<CompletableFuture<?>> collectPodLogIfNeeded() {
                    String line;
                    try (BufferedReader reader = new BufferedReader(new InputStreamReader(watcher.getOutput()))) {
                        while ((line = reader.readLine()) != null) {
-                            log.info("[K8S-pod-log-{}]: {}", taskRequest.getTaskName(), line);
+                            if (StringUtils.isBlank(taskRequest.getTaskOutputLogPath())) {
+                                log.info("[K8S-pod-log-{}]: {}", taskRequest.getTaskName(), line);
+                            } else {
+                                TASK_OUTPUT_LOGGER.info(line);
+                            }
                        }
                    }
                }
            } catch (Exception e) {
                log.error("Collect pod log error", e);
                throw new RuntimeException(e);
+            } finally {
+                LogUtils.removeTaskInstanceLogFullPathMDC();


We shouldn't only one logger is enough.
Don't need to do below judge.

if (StringUtils.isBlank(taskRequest.getTaskOutputLogPath())) { log.info("[K8S-pod-log-{}]: {}", taskRequest.getTaskName(), line); } else { TASK_OUTPUT_LOGGER.info(line); }

ruanwenjun · 2026-04-12T03:28:46Z

...k-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java

+                try (
+                        LogUtils.MDCAutoClosableContext ignored =
+                                LogUtils.withTaskOutputLogPathMDC(taskRequest.getTaskOutputLogPath())) {
+                    for (String line : (Iterable<String>) inReader.lines()::iterator) {
+                        if (StringUtils.isBlank(taskRequest.getTaskOutputLogPath())) {
+                            log.info(" -> {}", line);
+                        } else {
+                            TASK_OUTPUT_LOGGER.info(line);
+                        }
+                        taskOutputParameterParser.appendParseLog(line);
+                    }
+                } finally {
+                    LogUtils.removeTaskInstanceLogFullPathMDC();


In which case taskRequest.getTaskOutputLogPath() can be empty?

ruanwenjun · 2026-04-12T03:30:18Z

...scheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/LoggerController.java

+    public Result<ResponseTaskLog> queryTaskLog(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,
+                                                @RequestParam(value = "taskInstanceId") int taskInstanceId,
+                                                @RequestParam(value = "skipLineNum") int skipNum,
+                                                @RequestParam(value = "limit") int limit) {
+        return loggerService.queryTaskLog(loginUser, taskInstanceId, skipNum, limit);
+    }


Add a request param logType is enough, don't add a new api.

ruanwenjun · 2026-04-12T03:30:59Z

...duler-api/src/main/java/org/apache/dolphinscheduler/api/executor/logging/LocalLogClient.java

+    public TaskInstanceLogFileDownloadResponse getTaskLog(TaskInstance taskInstance) {
+        return getLocalWholeLog(taskInstance, TaskLogType.LOG);
+    }
+
+    public TaskInstanceLogFileDownloadResponse getTaskOutput(TaskInstance taskInstance) {
+        return getLocalWholeLog(taskInstance, TaskLogType.OUTPUT);
    }


Suggested change

public TaskInstanceLogFileDownloadResponse getTaskLog(TaskInstance taskInstance) {

return getLocalWholeLog(taskInstance, TaskLogType.LOG);

}

public TaskInstanceLogFileDownloadResponse getTaskOutput(TaskInstance taskInstance) {

return getLocalWholeLog(taskInstance, TaskLogType.OUTPUT);

}

public TaskInstanceLogFileDownloadResponse getTaskLog(TaskInstance taskInstance, TaskLogType logType) {

return getLocalWholeLog(taskInstance, TaskLogType.LOG);

}

ruanwenjun · 2026-04-12T03:31:28Z

...duler-api/src/main/java/org/apache/dolphinscheduler/api/executor/logging/LocalLogClient.java

+    public TaskInstanceLogPageQueryResponse getTaskLog(TaskInstance taskInstance, int skipLineNum, int limit) {
+        return getLocalPartLog(taskInstance, skipLineNum, limit, TaskLogType.LOG);
+    }
+
+    public TaskInstanceLogPageQueryResponse getTaskOutput(TaskInstance taskInstance, int skipLineNum, int limit) {
+        return getLocalPartLog(taskInstance, skipLineNum, limit, TaskLogType.OUTPUT);
    }


Suggested change

public TaskInstanceLogPageQueryResponse getTaskLog(TaskInstance taskInstance, int skipLineNum, int limit) {

return getLocalPartLog(taskInstance, skipLineNum, limit, TaskLogType.LOG);

}

public TaskInstanceLogPageQueryResponse getTaskOutput(TaskInstance taskInstance, int skipLineNum, int limit) {

return getLocalPartLog(taskInstance, skipLineNum, limit, TaskLogType.OUTPUT);

}

public TaskInstanceLogPageQueryResponse getTaskLog(TaskInstance taskInstance, int skipLineNum, int limit, TaskLogType logType) {

return getLocalPartLog(taskInstance, skipLineNum, limit, TaskLogType.LOG);

}

Mrhs121 · 2026-04-13T03:24:31Z

@ruanwenjun Thanks for the review. I summarized the feedback and these points make sense, I'll update.

Unify the API by adding a logType parameter instead of introducing separate task output endpoints.
Merge LocalLogClient into a single getTaskLog(..., logType) design for both full and paged queries.
Refactor task output logging to remove the empty-path check and dual-logger branching in AbstractCommandExecutor.
Keep TaskLogType lightweight and move task-instance-to-path resolution into a separate resolver/util class.
Change task_output_log_path from longtext to a bounded string type such as varchar(255) across all related SQL definitions.

Regarding this idea :#17791 (comment).

My understanding is that the concern is not only about adding another file path field for task output, but also about the long-term storage model.

Instead of storing separate file paths like log_path and task_output_log_path, a more extensible approach would be to store a single directory path such as task_out_path, and place all task-instance-generated files under it, for example log, output, and possibly other generated files in the future.

maybe such as:

${task_out_path}/task.log
${task_out_path}/task.out
${task_out_path}/stderr.log
${task_out_path}/result.json

SbloodyS · 2026-04-13T06:15:39Z

...scheduler-task-http/src/main/java/org/apache/dolphinscheduler/plugin/task/http/HttpTask.java

+    private void logHttpResponse(String message, int statusCode, String checkCondition, String body) {
+        if (StringUtils.isBlank(taskExecutionContext.getTaskOutputLogPath())) {
+            if (checkCondition == null) {
+                log.info(message, httpParameters.getUrl(), statusCode, body);
+            } else {
+                log.error(message, httpParameters.getUrl(), statusCode, checkCondition, body);
+            }
+            return;
+        }
+        LogUtils.setTaskInstanceLogFullPathMDC(taskExecutionContext.getLogPath());
+        try (
+                LogUtils.MDCAutoClosableContext ignored =
+                        LogUtils.withTaskOutputLogPathMDC(taskExecutionContext.getTaskOutputLogPath())) {
+            if (checkCondition == null) {
+                TASK_OUTPUT_LOGGER.info(message, httpParameters.getUrl(), statusCode, body);
+            } else {
+                TASK_OUTPUT_LOGGER.info(message, httpParameters.getUrl(), statusCode, checkCondition, body);
+            }
+        } finally {
+            LogUtils.removeTaskInstanceLogFullPathMDC();
+        }
+    }


Logic like this should not be customized by users, but should be handled at spi level.

SbloodyS · 2026-04-13T06:17:57Z

dolphinscheduler-master/src/main/resources/logback-spring.xml

            </appender>
        </sift>
    </appender>
+    <appender name="TASKOUTPUTLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender">


We should add an automatic cleaning policy like other logs.

Mrhs121 requested review from Gallardot, SbloodyS, caishunfeng, ruanwenjun and songjianet as code owners March 26, 2026 12:06

github-actions bot added UI ui and front end related backend test labels Mar 26, 2026

github-actions bot assigned Mrhs121 Mar 26, 2026

Mrhs121 changed the title ~~[DSIP-99] initial commit~~ [DSIP-99][TaskPlugin] Save task output to a separate file Mar 26, 2026

SbloodyS added the DSIP label Mar 26, 2026

SbloodyS added this to the 3.4.2 milestone Mar 26, 2026

Mrhs121 commented Mar 26, 2026

View reviewed changes

SbloodyS reviewed Apr 6, 2026

View reviewed changes

Mrhs121 force-pushed the DSIP-99 branch 2 times, most recently from 0100189 to 111aeca Compare April 9, 2026 10:45

ruanwenjun reviewed Apr 12, 2026

View reviewed changes

SbloodyS reviewed Apr 13, 2026

View reviewed changes

Mrhs121 added 4 commits April 13, 2026 14:19

[DSIP-99] initial commit

a0778da

[DSIP-99] Save task output to a separate file

e32bcc9

fix conflict

1062a61

add more tasks

80f2b46

Mrhs121 force-pushed the DSIP-99 branch from 111aeca to 80f2b46 Compare April 13, 2026 08:49

	task_output_log_path longtext DEFAULT NULL,
	task_output_log_path varchar(255) DEFAULT NULL,

Conversation

Mrhs121 commented Mar 26, 2026

Was this PR generated or assisted by AI?

Purpose of the pull request

Brief change log

Verify this pull request

Pull Request Notice

Uh oh!

Mrhs121 commented Mar 26, 2026

Uh oh!

Mrhs121 commented Mar 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SbloodyS left a comment

Choose a reason for hiding this comment

Uh oh!

Mrhs121 commented Apr 7, 2026

Uh oh!

SbloodyS commented Apr 7, 2026

Uh oh!

Mrhs121 commented Apr 9, 2026

Self-test results of common tasks

Uh oh!

sonarqubecloud bot commented Apr 12, 2026

Quality Gate failed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mrhs121 commented Apr 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants