[DSIP-26][Audit log] Audit log improvement design by qingwli · Pull Request #15554 · apache/dolphinscheduler

qingwli · 2024-02-02T07:39:23Z

close [DSIP-26][Audit log] Audit log improvement design #15423

# Conflicts: # dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ClusterController.java # dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/AuditLogMapperTest.java

update audit log ui

update object name

# Conflicts: # dolphinscheduler-dao/src/main/resources/sql/upgrade/3.3.0_schema/mysql/dolphinscheduler_ddl.sql # dolphinscheduler-dao/src/main/resources/sql/upgrade/3.3.0_schema/postgresql/dolphinscheduler_ddl.sql

ruanwenjun · 2024-04-01T02:04:22Z

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

+  `object_id` bigint(20) DEFAULT NULL COMMENT 'object id',
+  `object_name` varchar(100) DEFAULT NULL COMMENT 'object id',
+  `object_type` varchar(100) NOT NULL COMMENT 'object type',


Suggested change

`object_id` bigint(20) DEFAULT NULL COMMENT 'object id',

`object_name` varchar(100) DEFAULT NULL COMMENT 'object id',

`object_type` varchar(100) NOT NULL COMMENT 'object type',

`object_id` bigint(20) DEFAULT NULL COMMENT 'object id',

`object_name` varchar(100) DEFAULT NULL COMMENT 'object id',

`object_type` varchar(100) NOT NULL COMMENT 'object type',

Use domain_id or module_id is better than object_id?

For this part, I defined object id means the object I modified, task instance it's not a module. File is not a domain, I think object is more suit for this, can cover all the stuff we record, WDYT

ruanwenjun · 2024-04-01T02:05:41Z

...scheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuditObjectType.java

+
+    PROJECT("Project", null), // 1
+    PROCESS("Process", PROJECT),
+    PROCESS_INSTANCE("Process instance", PROCESS),


Suggested change

PROCESS_INSTANCE("Process instance", PROCESS),

PROCESS_INSTANCE("ProcessInstance", PROCESS),

ruanwenjun · 2024-04-01T02:12:19Z

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/OperatorLogAspect.java

+        // Api don't need record log
+        if (operatorLog == null) {
+            return point.proceed();
+        }


The logPointCut should make sure the method exist @OperatorLog.

ruanwenjun · 2024-04-01T02:14:02Z

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/OperatorLogAspect.java

+            return point.proceed();
+        }
+
+        Operation operation = method.getAnnotation(Operation.class);


I didn't find the Operation class, is this exist?

@Operation(summary = "queryAuditObjectTypeList", description = "QUERY_AUDIT_OBJECT_TYPE_LIST") @GetMapping(value = "/audit-log-object-type") @ResponseStatus(HttpStatus.OK) @ApiException(QUERY_AUDIT_LOG_LIST_PAGING) public Result<List<AuditObjectTypeDto>> queryAuditObjectTypeList() { return Result.success(AuditObjectTypeDto.getObjectTypeDtoList()); }

ruanwenjun · 2024-04-01T02:15:23Z

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/operator/Operator.java

-    void execute(AuditMessage message);
+import org.aspectj.lang.ProceedingJoinPoint;
+
+public interface Operator {


Suggested change

public interface Operator {

public interface AuditOperator {

ruanwenjun · 2024-04-01T02:20:58Z

...scheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/operator/BaseOperator.java

+        long beginTime = System.currentTimeMillis();
+
+        MethodSignature signature = (MethodSignature) point.getSignature();
+        Map<String, Object> paramsMap = OperatorUtils.getParamsMap(point, signature);
+
+        User user = OperatorUtils.getUser(paramsMap);
+
+        if (user == null) {
+            log.error("user is null");
+            return point.proceed();
+        }
+
+        List<AuditLog> auditLogList = OperatorUtils.buildAuditLogList(describe, auditType, user);
+        setRequestParam(auditType, auditLogList, paramsMap);
+
+        Result result = (Result) point.proceed();
+        if (OperatorUtils.resultFail(result)) {
+            log.error("request fail, code {}", result.getCode());
+            return result;
+        }
+
+        setObjectIdentityFromReturnObject(auditType, result, auditLogList);
+
+        modifyAuditOperationType(auditType, paramsMap, auditLogList);
+        modifyAuditObjectType(auditType, paramsMap, auditLogList);
+
+        long latency = System.currentTimeMillis() - beginTime;
+        auditService.addAudit(auditLogList, latency);
+
+        return result;


We may need to audit the case when proceed throw exception.

todo #15788

ruanwenjun · 2024-04-01T02:23:48Z

...scheduler-api/src/main/java/org/apache/dolphinscheduler/api/audit/operator/BaseOperator.java

+            return;
+        }
+
+        Long objId = checkNum(returnObjectMap.get(params[0]).toString());


Suggested change

Long objId = checkNum(returnObjectMap.get(params[0]).toString());

Long objId = NumberUtils.toLong(returnObjectMap.get(params[0]).toString(), -1);

ruanwenjun · 2024-04-01T02:27:03Z

...uler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ExecutorServiceImpl.java

        // no master
        if (masterServers.isEmpty()) {
-            throw new ServiceException(Status.MASTER_NOT_EXISTS);
+            // throw new ServiceException(Status.MASTER_NOT_EXISTS);


Please rollback this kind of code.

ruanwenjun · 2024-04-01T02:28:28Z

...heduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AuditServiceImpl.java

+        auditDto.setOperation(AuditOperationType.of(auditLog.getOperationType()).getName());
        auditDto.setUserName(auditLog.getUserName());
-        auditDto.setResourceName(auditLogMapper.queryResourceNameByType(resourceType, auditLog.getResourceId()));
+        auditDto.setLatency(String.format("%.2f", (double) auditLog.getLatency() / 1000));


Suggested change

auditDto.setLatency(String.format("%.2f", (double) auditLog.getLatency() / 1000));

auditDto.setLatency(String.format("%.2f", (double) auditLog.getLatency() / 1000));

It's better to use ms.

ruanwenjun · 2024-04-01T02:49:31Z

Quality Gate passed

Issues 42 New issues 0 Accepted issues

Measures 0 Security Hotspots 62.8% Coverage on New Code 0.0% Duplication on New Code

See analysis details on SonarCloud

Please take care of the new issues.

...uler-api/src/main/java/org/apache/dolphinscheduler/api/audit/operator/BaseAuditOperator.java

ruanwenjun

LGTM, please move the ddl to 3.2.2

qingwli · 2024-04-15T03:09:13Z

PTAL @ruanwenjun

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

sonarqubecloud · 2024-04-15T04:50:14Z

Quality Gate passed

Issues
28 New issues
0 Accepted issues

Measures
0 Security Hotspots
63.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

qingwli and others added 15 commits January 15, 2024 15:34

init

771e934

update

844585a

Merge branch 'dev' into augit-log

27277f8

# Conflicts: # dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ClusterController.java # dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/AuditLogMapperTest.java

support multi codes

c95faa4

update

e79ff87

update

a68357c

update

6fd8671

update audit log ui

1eb5d03

fix

1de52c1

Merge pull request #3 from yeahhhz/audit-log-2

f46860a

update audit log ui

update object name

bbb785b

Merge pull request #4 from yeahhhz/audit-log-3

a7017f1

update object name

update

91432e5

Merge remote-tracking branch 'upstream/dev' into augit-log

51ee4d3

# Conflicts: # dolphinscheduler-dao/src/main/resources/sql/upgrade/3.3.0_schema/mysql/dolphinscheduler_ddl.sql # dolphinscheduler-dao/src/main/resources/sql/upgrade/3.3.0_schema/postgresql/dolphinscheduler_ddl.sql

update

20b0085

qingwli requested review from Amy0104, SbloodyS, caishunfeng, songjianet and zhongjiajie as code owners February 2, 2024 07:39

qingwli self-assigned this Feb 2, 2024

github-actions bot added UI ui and front end related backend labels Feb 2, 2024

qingwli added 3 commits February 2, 2024 15:41

update

adf4651

update

5aff032

update

29ed211

qingwli added this to the 3.3.0 milestone Feb 2, 2024

qingwli added improvement make more easy to user or prompt friendly 3.3.0 labels Feb 2, 2024

update

7ff0a50

ruanwenjun changed the title ~~[DSIP-26] Audit log Improvement~~ [DSIP-26][Audit log] Audit log improvement design Apr 1, 2024

ruanwenjun requested changes Apr 1, 2024

View reviewed changes

qE0a44OX6s and others added 2 commits April 2, 2024 10:13

Merge branch 'dev' into audit-log

a584530

update

3b7f63c

github-advanced-security bot found potential problems Apr 2, 2024

View reviewed changes

...uler-api/src/main/java/org/apache/dolphinscheduler/api/audit/operator/BaseAuditOperator.java Fixed Show fixed Hide fixed

qingwli and others added 9 commits April 2, 2024 13:50

update

84df67e

update

549b985

change object -> model

847f193

Merge branch 'dev' into audit-log

ed57b8b

Merge branch 'dev' into audit-log

91ae786

Merge branch 'dev' into audit-log

61f5f50

Merge branch 'dev' into audit-log

c7e8ecd

Merge branch 'dev' into audit-log

a7f137a

Merge branch 'dev' into audit-log

ce108d8

ruanwenjun previously approved these changes Apr 14, 2024

View reviewed changes

qE0a44OX6s and others added 2 commits April 15, 2024 10:30

Merge branch 'dev' into audit-log

cc5de74

update

5e881fc

qingwli dismissed ruanwenjun’s stale review via 5e881fc April 15, 2024 02:34

ruanwenjun approved these changes Apr 15, 2024

View reviewed changes

ruanwenjun added the ready-to-merge label Apr 15, 2024

Merge branch 'dev' into audit-log

8e0613b

ruanwenjun added the DSIP label Apr 15, 2024

ruanwenjun requested changes Apr 15, 2024

View reviewed changes

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_postgresql.sql Show resolved Hide resolved

ruanwenjun approved these changes Apr 15, 2024

View reviewed changes

ruanwenjun merged commit 2263455 into apache:dev Apr 15, 2024

qingwli deleted the audit-log branch April 15, 2024 05:41

SbloodyS mentioned this pull request Jul 25, 2024

[DSIP-57][Audit] Refactor PR 14981 code #16367

Closed

2 tasks

	PROCESS_INSTANCE("Process instance", PROCESS),
	PROCESS_INSTANCE("ProcessInstance", PROCESS),

	Long objId = checkNum(returnObjectMap.get(params[0]).toString());
	Long objId = NumberUtils.toLong(returnObjectMap.get(params[0]).toString(), -1);

	auditDto.setLatency(String.format("%.2f", (double) auditLog.getLatency() / 1000));
	auditDto.setLatency(String.format("%.2f", (double) auditLog.getLatency() / 1000));

Conversation

qingwli commented Feb 2, 2024

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!

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!

Choose a reason for hiding this comment

Uh oh!

ruanwenjun commented Apr 1, 2024

Quality Gate passed

Uh oh!

Uh oh!

ruanwenjun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qingwli commented Apr 15, 2024

Uh oh!

Uh oh!

sonarqubecloud bot commented Apr 15, 2024

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ruanwenjun left a comment •

edited

Loading