-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DSIP-26][Audit log] Audit log improvement design #15554
Changes from 73 commits
771e934
844585a
27277f8
c95faa4
e79ff87
a68357c
6fd8671
1eb5d03
1de52c1
f46860a
bbb785b
a7017f1
91432e5
51ee4d3
20b0085
adf4651
5aff032
29ed211
7ff0a50
4a3558a
6aeec45
133a6cc
bc34a49
9a3ef75
1214761
b6d08ed
9c8eab8
c223d28
9fdcafe
625e7bc
3f933c6
5e39e4e
1fd6e65
c885883
b75f927
81be755
0d9dd8d
53a2079
a7ab53f
83bda8e
ce0e1ba
bae273e
10f380d
82222a5
94f9fd3
d499513
b56e74c
7a4f833
44874bd
8f1c06e
4eebc90
beb529f
1411b3e
ccd0699
0f17a77
066c0ec
963825b
3ee0fcd
23680a9
0eee722
7e6650f
ec07e92
90c36a3
72b2fae
c324855
ad89230
ff24270
f79d6fe
3430054
ddad807
836c978
d8c1241
36332ea
a584530
3b7f63c
84df67e
549b985
847f193
ed57b8b
91ae786
61f5f50
c7e8ecd
a7f137a
ce108d8
cc5de74
5e881fc
8e0613b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package org.apache.dolphinscheduler.api.audit; | ||
|
||
import org.apache.dolphinscheduler.api.audit.enums.AuditType; | ||
|
||
import java.lang.annotation.Documented; | ||
import java.lang.annotation.ElementType; | ||
import java.lang.annotation.Retention; | ||
import java.lang.annotation.RetentionPolicy; | ||
import java.lang.annotation.Target; | ||
|
||
/** | ||
* Custom annotation for logging and auditing operator actions in the system. | ||
* This annotation can be applied to methods to indicate the type of operation, object type, | ||
* and specific parameters to be recorded in the logs. | ||
*/ | ||
@Target(ElementType.METHOD) | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@Documented | ||
public @interface OperatorLog { | ||
|
||
AuditType auditType(); | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,65 @@ | ||||||||||||
/* | ||||||||||||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||||||||||||
* contributor license agreements. See the NOTICE file distributed with | ||||||||||||
* this work for additional information regarding copyright ownership. | ||||||||||||
* The ASF licenses this file to You under the Apache License, Version 2.0 | ||||||||||||
* (the "License"); you may not use this file except in compliance with | ||||||||||||
* the License. You may obtain a copy of the License at | ||||||||||||
* | ||||||||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||||
* | ||||||||||||
* Unless required by applicable law or agreed to in writing, software | ||||||||||||
* distributed under the License is distributed on an "AS IS" BASIS, | ||||||||||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||||||||
* See the License for the specific language governing permissions and | ||||||||||||
* limitations under the License. | ||||||||||||
*/ | ||||||||||||
|
||||||||||||
package org.apache.dolphinscheduler.api.audit; | ||||||||||||
|
||||||||||||
import org.apache.dolphinscheduler.api.audit.operator.Operator; | ||||||||||||
import org.apache.dolphinscheduler.service.bean.SpringApplicationContext; | ||||||||||||
|
||||||||||||
import java.lang.reflect.Method; | ||||||||||||
|
||||||||||||
import lombok.extern.slf4j.Slf4j; | ||||||||||||
|
||||||||||||
import org.aspectj.lang.ProceedingJoinPoint; | ||||||||||||
import org.aspectj.lang.annotation.Around; | ||||||||||||
import org.aspectj.lang.annotation.Aspect; | ||||||||||||
import org.aspectj.lang.annotation.Pointcut; | ||||||||||||
import org.aspectj.lang.reflect.MethodSignature; | ||||||||||||
import org.springframework.stereotype.Component; | ||||||||||||
|
||||||||||||
import io.swagger.v3.oas.annotations.Operation; | ||||||||||||
|
||||||||||||
@Aspect | ||||||||||||
@Slf4j | ||||||||||||
@Component | ||||||||||||
public class OperatorLogAspect { | ||||||||||||
|
||||||||||||
@Pointcut("@annotation(org.apache.dolphinscheduler.api.audit.OperatorLog)") | ||||||||||||
public void logPointCut() { | ||||||||||||
} | ||||||||||||
|
||||||||||||
@Around("logPointCut()") | ||||||||||||
public Object around(ProceedingJoinPoint point) throws Throwable { | ||||||||||||
MethodSignature signature = (MethodSignature) point.getSignature(); | ||||||||||||
Method method = signature.getMethod(); | ||||||||||||
|
||||||||||||
OperatorLog operatorLog = method.getAnnotation(OperatorLog.class); | ||||||||||||
// Api don't need record log | ||||||||||||
if (operatorLog == null) { | ||||||||||||
return point.proceed(); | ||||||||||||
} | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case shouldn't happen?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we don't record search method There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||||||||||||
|
||||||||||||
Operation operation = method.getAnnotation(Operation.class); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't find the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||
if (operation == null) { | ||||||||||||
log.warn("Operation is null of method: {}", method.getName()); | ||||||||||||
return point.proceed(); | ||||||||||||
} | ||||||||||||
|
||||||||||||
Operator operator = SpringApplicationContext.getBean(operatorLog.auditType().getOperatorClass()); | ||||||||||||
return operator.recordAudit(point, operation.description(), operatorLog.auditType()); | ||||||||||||
} | ||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we choose to do auditing with AOP, one important thing is exception handling. If there are bugs / defects in aspect code, it could cause trouble across all the related methods with the audit annotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, so please help point the place needs more logic to avoid issue