ruanwenjun commented on a change in pull request #6322:
URL: https://github.com/apache/dolphinscheduler/pull/6322#discussion_r714904158
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/AuditPublishService.java
##########
@@ -0,0 +1,79 @@
+package org.apache.dolphinscheduler.api.audit;
+
+import org.apache.dolphinscheduler.api.configuration.AuditConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+@Component
+public class AuditPublishService {
+
+ /**
+ * audit message queue
+ */
+ private BlockingQueue<AuditMessage> queue = new LinkedBlockingQueue<>();
Review comment:
It's better to rename the queue to auditMessageQueue, rather than add a
comment.
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/AuditPublishService.java
##########
@@ -0,0 +1,79 @@
+package org.apache.dolphinscheduler.api.audit;
+
+import org.apache.dolphinscheduler.api.configuration.AuditConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+@Component
+public class AuditPublishService {
+
+ /**
+ * audit message queue
+ */
+ private BlockingQueue<AuditMessage> queue = new LinkedBlockingQueue<>();
+
+ /**
+ * subscribers list
+ */
+ @Autowired
+ private List<AuditSubscriber> subscribers;
+
+ @Autowired
+ private AuditConfiguration auditConfiguration;
+
+ private static final Logger logger =
LoggerFactory.getLogger(AuditPublishService.class);
+
+ /**
+ * create a daemon thread to process the message queue
+ */
+ @PostConstruct
+ private void init() {
+ if (auditConfiguration.isAuditGlobalControlSwitch()) {
+ Thread thread = new Thread(() -> doPublish());
+ thread.setDaemon(true);
+ thread.setName("Audit-Log-Consume-Thread");
+ thread.start();
+ }
+ }
+
+ /**
+ * publish a new audit message
+ *
+ * @param message audit message
+ */
+ public void publish(AuditMessage message) {
+ if (auditConfiguration.isAuditGlobalControlSwitch()) {
+ queue.offer(message);
+ }
+ }
+
+ /**
+ * subscribers execute the message processor method
+ */
+ private void doPublish() {
+ AuditMessage message;
+ while (true) {
+ try {
+ message = queue.take();
+ for (AuditSubscriber subscriber : subscribers) {
+ try {
+ subscriber.execute(message);
+ } catch (Exception e) {
+ logger.error("consume audit message failed {}",
message.toString());
+ e.printStackTrace();
Review comment:
Use logger.error("consume audit message failed {}", message.toString(),
e);
Remove e.printStackTrace();
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/AuditSubscriberImpl.java
##########
@@ -0,0 +1,26 @@
+package org.apache.dolphinscheduler.api.audit;
+
+
+import org.apache.dolphinscheduler.dao.entity.AuditLog;
+import org.apache.dolphinscheduler.dao.mapper.AuditLogMapper;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+@Component
+public class AuditSubscriberImpl implements AuditSubscriber {
+
+ @Autowired
+ AuditLogMapper logMapper;
Review comment:
Add private
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AuditService.java
##########
@@ -0,0 +1,46 @@
+package org.apache.dolphinscheduler.api.service;
+
+import org.apache.dolphinscheduler.api.utils.Result;
+import org.apache.dolphinscheduler.common.enums.AuditModuleType;
+import org.apache.dolphinscheduler.common.enums.AuditOperationType;
+import org.apache.dolphinscheduler.dao.entity.User;
+
+import java.util.Map;
+
+/**
+ * audit information service
+ */
+public interface AuditService {
+
+ /**
+ *
+ * @param user login user
+ * @param module module type
+ * @param operation operation type
+ * @param projectName project name
+ * @param processName process name
+ */
+ void addAudit(User user, AuditModuleType module, AuditOperationType
operation,
+ String projectName, String processName);
+
+ /**
+ * query audit log list
+ *
+ * @param loginUser login user
Review comment:
Why the two formats are not match
##########
File path:
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/vo/AlertGroupVo.java
##########
@@ -30,6 +32,18 @@
* group_name
*/
private String groupName;
+ /**
Review comment:
Remove this file changes
##########
File path:
dolphinscheduler-service/src/test/java/org/apache/dolphinscheduler/service/process/ProcessServiceTest.java
##########
@@ -100,6 +103,8 @@
@Mock
private TaskDefinitionLogMapper taskDefinitionLogMapper;
@Mock
+ private TaskDefinitionMapper taskDefinitionMapper;
Review comment:
Remove this file changes
##########
File path: dolphinscheduler-ui/src/js/module/i18n/locale/en_US.js
##########
@@ -708,5 +708,11 @@ export default {
condition: 'condition',
'The condition content cannot be empty': 'The condition content cannot be
empty',
'Reference from': 'Reference from',
- 'No more...': 'No more...'
+ 'No more...': 'No more...',
+ 'Audit Log': 'Audit Log',
+ 'AuditType': 'audit type',
+ 'AllModules': 'all modules',
+ 'AllOperations': 'all operations',
+ 'UserAudit': 'user management audit',
+ 'Project Module': 'project management audit',
Review comment:
```suggestion
AuditLog: 'Audit Log',
AuditType: 'audit type',
AllModules: 'all modules',
AllOperations: 'all operations',
UserAudit: 'user management audit',
ProjectModule: 'project management audit',
```
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AuditLogController.java
##########
@@ -0,0 +1,84 @@
+package org.apache.dolphinscheduler.api.controller;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiImplicitParam;
+import io.swagger.annotations.ApiImplicitParams;
+import io.swagger.annotations.ApiOperation;
+import org.apache.dolphinscheduler.api.aspect.AccessLogAnnotation;
+import org.apache.dolphinscheduler.common.enums.AuditModuleType;
+import org.apache.dolphinscheduler.common.enums.AuditOperationType;
+import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.exceptions.ApiException;
+import org.apache.dolphinscheduler.api.service.AuditService;
+import org.apache.dolphinscheduler.api.utils.Result;
+import org.apache.dolphinscheduler.common.Constants;
+import org.apache.dolphinscheduler.dao.entity.User;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
+import org.springframework.web.bind.annotation.*;
+import springfox.documentation.annotations.ApiIgnore;
+
+import java.util.Map;
+
+import static
org.apache.dolphinscheduler.api.enums.Status.QUERY_AUDIT_LOG_LIST_PAGING;
+
+@Api(tags = "AUDIT_LOG_TAG")
+@RestController
+@RequestMapping("projects/audit")
+public class AuditLogController extends BaseController {
+
+ private static final Logger logger =
LoggerFactory.getLogger(AuditLogController.class);
+
+ @Autowired
+ AuditService auditService;
Review comment:
Add private
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/AuditPublishService.java
##########
@@ -0,0 +1,79 @@
+package org.apache.dolphinscheduler.api.audit;
+
+import org.apache.dolphinscheduler.api.configuration.AuditConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+@Component
+public class AuditPublishService {
+
+ /**
+ * audit message queue
+ */
+ private BlockingQueue<AuditMessage> queue = new LinkedBlockingQueue<>();
+
+ /**
+ * subscribers list
+ */
+ @Autowired
+ private List<AuditSubscriber> subscribers;
Review comment:
Remove this comment.
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/AuditPublishService.java
##########
@@ -0,0 +1,79 @@
+package org.apache.dolphinscheduler.api.audit;
+
+import org.apache.dolphinscheduler.api.configuration.AuditConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import javax.annotation.PostConstruct;
+import java.util.List;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+
+@Component
+public class AuditPublishService {
+
+ /**
+ * audit message queue
+ */
+ private BlockingQueue<AuditMessage> queue = new LinkedBlockingQueue<>();
+
+ /**
+ * subscribers list
+ */
+ @Autowired
+ private List<AuditSubscriber> subscribers;
+
+ @Autowired
+ private AuditConfiguration auditConfiguration;
+
+ private static final Logger logger =
LoggerFactory.getLogger(AuditPublishService.class);
+
+ /**
+ * create a daemon thread to process the message queue
+ */
+ @PostConstruct
+ private void init() {
+ if (auditConfiguration.isAuditGlobalControlSwitch()) {
+ Thread thread = new Thread(() -> doPublish());
+ thread.setDaemon(true);
+ thread.setName("Audit-Log-Consume-Thread");
+ thread.start();
+ }
+ }
+
+ /**
+ * publish a new audit message
+ *
+ * @param message audit message
+ */
+ public void publish(AuditMessage message) {
+ if (auditConfiguration.isAuditGlobalControlSwitch()) {
+ queue.offer(message);
+ }
+ }
+
+ /**
+ * subscribers execute the message processor method
+ */
+ private void doPublish() {
+ AuditMessage message;
+ while (true) {
+ try {
+ message = queue.take();
+ for (AuditSubscriber subscriber : subscribers) {
+ try {
+ subscriber.execute(message);
+ } catch (Exception e) {
+ logger.error("consume audit message failed {}",
message.toString());
+ e.printStackTrace();
+ }
+ }
+ } catch (InterruptedException e) {
+ logger.error("consume audit message failed");
Review comment:
logger.error("consume audit message failed", e);
##########
File path:
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuditModuleType.java
##########
@@ -0,0 +1,44 @@
+package org.apache.dolphinscheduler.common.enums;
+
+import java.util.HashMap;
+
+/**
+ * Audit Module type
+ */
+public enum AuditModuleType {
+ // TODO: add other audit module enums
+ DEFAULT(0, "default"),
+ USER_MODULE(1, "user module"),
+ PROJECT_MODULE(2, "project module");
+
+ private final int code;
+ private final String enMsg;
+
+ private static HashMap<Integer, AuditModuleType> AUDIT_MODULE_MAP = new
HashMap<>();
+
+ static {
+ for (AuditModuleType auditModuleType : AuditModuleType.values()) {
+ AUDIT_MODULE_MAP.put(auditModuleType.code, auditModuleType);
+ }
+ }
+
+ AuditModuleType(int code, String enMsg) {
+ this.code = code;
+ this.enMsg = enMsg;
+ }
+
+ public int getCode() {
+ return this.code;
+ }
+
+ public String getMsg() {
+ return this.enMsg;
+ }
+
+ public static AuditModuleType of(int status) {
+ if (AUDIT_MODULE_MAP.containsKey(status)) {
+ return AUDIT_MODULE_MAP.get(status);
+ }
+ throw new IllegalArgumentException("invalid audit module type " +
status);
Review comment:
```suggestion
throw new IllegalArgumentException("invalid audit module type code " +
status);
```
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/enums/Status.java
##########
@@ -331,8 +331,10 @@
VERIFY_ENVIRONMENT_ERROR(1200011, "verify environment error", "验证环境信息错误"),
ENVIRONMENT_WORKER_GROUPS_IS_INVALID(1200012, "environment worker groups
is invalid format", "环境关联的工作组参数解析错误"),
UPDATE_ENVIRONMENT_WORKER_GROUP_RELATION_ERROR(1200013,"You can't modify
the worker group, because the worker group [{0}] and this environment [{1}]
already be used in the task [{2}]",
- "您不能修改工作组选项，因为该工作组 [{0}] 和 该环境 [{1}] 已经被用在任务 [{2}] 中");
+ "您不能修改工作组选项，因为该工作组 [{0}] 和 该环境 [{1}] 已经被用在任务 [{2}] 中"),
+ // audit log
+ QUERY_AUDIT_LOG_LIST_PAGING(10057, "query resources list paging",
"分页查询资源列表错误");
Review comment:
```suggestion
QUERY_AUDIT_LOG_LIST_PAGING(10057, "query resources list paging",
"分页查询资源列表错误"),
;
```
If someone add a enum in the future, this format may help review
##########
File path:
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/AlertGroupMapper.xml
##########
@@ -33,7 +33,7 @@
order by update_time desc
</select>
<select id="queryAlertGroupVo"
resultType="org.apache.dolphinscheduler.dao.vo.AlertGroupVo">
- select id, group_name
+ select id, group_name, description, create_time, update_time
Review comment:
Remove this file changes
##########
File path: dolphinscheduler-api/src/main/resources/application-api.properties
##########
@@ -56,6 +56,9 @@ security.authentication.type=PASSWORD
#traffic.control.default.tenant.qps.rate=10
#traffic.control.customize.tenant.qps.rate={'tenant1':11,'tenant2':20}
+# Audit log control
+#audit.control.global.switch=false
Review comment:
The switch can set true, otherwise the front page will display white. Or
you have a better way to let the front end don't show audit tab
##########
File path:
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuditOperationType.java
##########
@@ -0,0 +1,45 @@
+package org.apache.dolphinscheduler.common.enums;
+
+import java.util.HashMap;
+
+/**
+ * Audit Operation type
+ */
+public enum AuditOperationType {
+
+ // TODO: add other audit operation enums
+ DEFAULT(0, "default" ),
+ CREATE_USER(1, "create user"),
+ CREATE_PROJECT(2, "create project");
+
+ private final int code;
+ private final String enMsg;
+
+ private static HashMap<Integer, AuditOperationType> AUDIT_OPERATION_MAP =
new HashMap<>();
+
+ static {
+ for (AuditOperationType operationType : AuditOperationType.values()) {
+ AUDIT_OPERATION_MAP.put(operationType.code, operationType);
+ }
+ }
+
+ AuditOperationType(int code, String enMsg) {
+ this.code = code;
+ this.enMsg = enMsg;
+ }
+
+ public static AuditOperationType of(int status) {
+ if (AUDIT_OPERATION_MAP.containsKey(status)) {
+ return AUDIT_OPERATION_MAP.get(status);
+ }
+ throw new IllegalArgumentException("invalid audit operation type " +
status);
Review comment:
```suggestion
throw new IllegalArgumentException("invalid audit operation type code " +
status);
```
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/dto/AuditDto.java
##########
@@ -0,0 +1,87 @@
+package org.apache.dolphinscheduler.api.dto;
+
+import com.fasterxml.jackson.annotation.JsonFormat;
+
+import java.util.Date;
+
+public class AuditDto {
+
+ /**
+ * operator
+ */
+ private String userName;
Review comment:
Use swagger annotion instead of comment.
##########
File path:
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/configuration/AuditConfigurationTest.java
##########
@@ -0,0 +1,19 @@
+package org.apache.dolphinscheduler.api.configuration;
+
+import org.apache.dolphinscheduler.api.controller.AbstractControllerTest;
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+
+import static org.junit.Assert.*;
Review comment:
Don't use import *
##########
File path:
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AuditServiceImpl.java
##########
@@ -0,0 +1,126 @@
+package org.apache.dolphinscheduler.api.service.impl;
+
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
+import org.apache.dolphinscheduler.api.audit.AuditMessage;
+import org.apache.dolphinscheduler.api.audit.AuditPublishService;
+import org.apache.dolphinscheduler.api.dto.AuditDto;
+import org.apache.dolphinscheduler.api.utils.Result;
+import org.apache.dolphinscheduler.common.enums.AuditModuleType;
+import org.apache.dolphinscheduler.common.enums.AuditOperationType;
+import org.apache.dolphinscheduler.api.enums.Status;
+import org.apache.dolphinscheduler.api.service.AuditService;
+import org.apache.dolphinscheduler.api.utils.PageInfo;
+import org.apache.dolphinscheduler.common.Constants;
+import org.apache.dolphinscheduler.common.utils.CollectionUtils;
+import org.apache.dolphinscheduler.dao.entity.AuditLog;
+import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.AuditLogMapper;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Service;
+
+import java.util.*;
Review comment:
Don't use import *
##########
File path:
dolphinscheduler-spi/src/main/java/org/apache/dolphinscheduler/spi/task/TaskConstants.java
##########
@@ -320,5 +320,8 @@ private TaskConstants() {
*/
public static final String HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE =
"hadoop.security.authentication.startup.state";
-
+ /**
Review comment:
Remove this file changes
##########
File path:
dolphinscheduler-service/src/main/java/org/apache/dolphinscheduler/service/process/ProcessService.java
##########
@@ -2188,23 +2188,23 @@ public int saveTaskDefine(User operator, long
projectCode, List<TaskDefinitionLo
}
newTaskDefinitionLogs.add(taskDefinitionLog);
}
+ int insertResult = 0;
Review comment:
Remove this file changes
##########
File path:
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java
##########
@@ -308,7 +307,7 @@ private void clear() {
* @param process process
*/
private void parseProcessOutput(Process process) {
- String threadLoggerInfoName =
String.format(LoggerUtils.TASK_LOGGER_THREAD_NAME + "-%s",
taskRequest.getTaskAppId());
+ String threadLoggerInfoName =
String.format(TaskConstants.TASK_LOGGER_THREAD_NAME + "-%s",
taskRequest.getTaskAppId());
Review comment:
Remove this file changes
##########
File path:
dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractTaskExecutor.java
##########
@@ -45,10 +41,7 @@
*/
protected AbstractTaskExecutor(TaskRequest taskRequest) {
super(taskRequest);
- logger =
LoggerFactory.getLogger(LoggerUtils.buildTaskId(LoggerUtils.TASK_LOGGER_INFO_PREFIX,
- taskRequest.getProcessDefineId(),
- taskRequest.getProcessInstanceId(),
- taskRequest.getTaskInstanceId()));
+ logger = LoggerFactory.getLogger(taskRequest.getLogPath());
Review comment:
It seems the upstream dev is
```java
logger = LoggerFactory.getLogger(taskRequest.getLogPath());
```
you didn't do this change, this is strange.
##########
File path: dolphinscheduler-ui/src/js/module/i18n/locale/zh_CN.js
##########
@@ -707,5 +707,11 @@ export default {
condition: '条件',
'The condition content cannot be empty': '条件内容不能为空',
'Reference from': '使用已有任务',
- 'No more...': '没有更多了...'
+ 'No more...': '没有更多了...',
+ 'Audit Log': '审计日志',
+ 'AuditType': '审计类型',
+ 'AllTypes': '所有类型',
+ 'AllOperations': '所有操作',
+ 'UserAudit': '用户管理审计',
+ 'ProjectAudit': '项目管理审计'
Review comment:
```suggestion
Audit Log: '审计日志',
AuditType: '审计类型',
AllTypes: '所有类型',
AllOperations: '所有操作',
UserAudit: '用户管理审计',
ProjectAudit: '项目管理审计'
```
--
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]
