[DSIP-107] Add separate workflow instance logs for UI viewing, similar to task logs.#18130
[DSIP-107] Add separate workflow instance logs for UI viewing, similar to task logs.#18130njnu-seafish wants to merge 21 commits into
Conversation
njnu-seafish
requested review from
EricGao888,
SbloodyS,
caishunfeng and
ruanwenjun
as code owners
SbloodyS
changed the title
# Conflicts: # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/WorkflowEventBusFireWorker.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/task/dispatcher/WorkerGroupDispatcher.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/engine/workflow/statemachine/AbstractWorkflowStateAction.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/failover/TaskFailover.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/rpc/TaskExecutorEventListenerImpl.java # dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/rpc/TaskInstanceControllerImpl.java
| workflowInstance.setLogPath(WorkflowLogUtils.getWorkflowInstanceLogFullPath( | ||
| workflowInstance.getStartTime(), | ||
| workflowInstance.getWorkflowDefinitionCode(), | ||
| workflowInstance.getWorkflowDefinitionVersion(), | ||
| workflowInstance.getId())); | ||
|
|
There was a problem hiding this comment.
It's best to inject the log path at runtime. You can write the code at CommandEngine, can add a new field at WorkflowStartLifecycleEvent.
There was a problem hiding this comment.
It's best to inject the log path at runtime. You can write the code at
CommandEngine, can add a new field atWorkflowStartLifecycleEvent.
That's a great idea! With your guidance, I found an even better way to implement it: we can uniformly set the logPath value in the abstract parent class AbstractCommandHandler.
protected void assembleWorkflowInstanceLogPath(final WorkflowExecuteContextBuilder workflowExecuteContextBuilder) {
final WorkflowInstance workflowInstance = workflowExecuteContextBuilder.getWorkflowInstance();
final Date logTime = workflowInstance.getRestartTime() != null
? workflowInstance.getRestartTime()
: workflowInstance.getStartTime();
final String logPath = WorkflowLogUtils.getWorkflowInstanceLogFullPath(
logTime,
workflowInstance.getWorkflowDefinitionCode(),
workflowInstance.getWorkflowDefinitionVersion(),
workflowInstance.getId());
workflowInstance.setLogPath(logPath);
workflowInstanceDao.updateById(workflowInstance);
workflowExecuteContextBuilder.setWorkflowInstance(workflowInstance);
}
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @Setter | ||
| public class WorkflowLogDiscriminator extends AbstractDiscriminator<ILoggingEvent> { | ||
|
|
||
| private String key; |
| */ | ||
|
|
||
| ALTER TABLE `t_ds_workflow_instance` | ||
| ADD COLUMN `log_path` longtext NULL COMMENT 'workflow instance log path'; No newline at end of file |
There was a problem hiding this comment.
| ADD COLUMN `log_path` longtext NULL COMMENT 'workflow instance log path'; | |
| ADD COLUMN `log_path` longtext NULL COMMENT 'workflow instance log path'; | |
There was a problem hiding this comment.
ADD COLUMN
log_pathlongtext NULL COMMENT 'workflow instance log path';
ok,thanks
| </sift> | ||
| </appender> | ||
|
|
||
| <appender name="WORKFLOWLOGFILE" class="ch.qos.logback.classic.sift.SiftingAppender"> |
There was a problem hiding this comment.
rollingPolicy should be added here.
There was a problem hiding this comment.
rollingPolicyshould be added here.
Since the logs for a single task don't use the rolling policy, does that mean the logs for a single workflow don't need it either?
Later, we'll need to add an entry point in the UI to view logs for a single workflow. If we use the rolling policy, would that be inappropriate?
|
@ruanwenjun @SbloodyS Could you please help review the solution and the code? I sincerely apologize for the delay on this PR. I've been caught up with some urgent company matters recently and haven't had the time to work on it. 
|
|
@ruanwenjun @SbloodyS Could you please help review the solution and the code? Thanks.
This has been tested and verified in a real-world environment. Please see the UI below: 
|
4 participants
Was this PR generated or assisted by AI?
NO
Purpose of the pull request
This is an implementation scheme for separating workflow logs. close #18129
Brief change log
Add separate workflow instance logs for UI viewing, similar to task logs.
Verify this pull request
I have already tested and verified this solution in my local environment.
The log_path field in the t_ds_workflow_instance table:
Dedicated log files for workflow instances on the Master service:
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md