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 output separation in the common layer for tasks that already use
the shared execution path;
2. for tasks that do not use that path today(such as httptask,sqltask and
similar tasks), introduce a small common helper for writing logs, so they still
follow the same output logging model instead of each task implementing it
differently;
4. keep a consistent external API and log access model for all task types,
instead of introducing separate APIs or duplicated query methods.
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]
