Mrhs121 commented on code in PR #18098:
URL:
https://github.com/apache/dolphinscheduler/pull/18098#discussion_r3105472602
##########
dolphinscheduler-task-plugin/dolphinscheduler-task-http/src/main/java/org/apache/dolphinscheduler/plugin/task/http/HttpTask.java:
##########
@@ -125,10 +131,33 @@ private void validateResponse(String body, int
statusCode) {
}
// default success log
- log.info("http request success, url: {}, statusCode: {}, body: {}",
httpParameters.getUrl(), statusCode, body);
+ logHttpResponse("http request success, url: {}, statusCode: {}, body:
{}", statusCode, null, body);
exitStatusCode = TaskConstants.EXIT_CODE_SUCCESS;
}
+ 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();
+ }
+ }
Review Comment:
Thanks for the review. I agree with moving this to a common layer where
possible.
For tasks already using the shared command path, such as `ShellTask`,
`PythonTask`, `JavaTask`, `SeatunnelTask`, `DataxTask`, and similar tasks, we
can handle output separation in a unified way through
`AbstractCommandExecutor.run`. But tasks like `HttpTask`, `SqlTask`, and
`ProcedureTask` do not go through that shared execution path today, so moving
all of them into the same SPI layer would require a larger refactor.
So, a feasible plan might look like this:
1. unify the handling in the common layer for tasks that already have a
shared execution path;
2. keep the external API and log access model consistent for the remaining
tasks.
cc @ruanwenjun
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
