You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/01 18:07:29 UTC
[GitHub] [hadoop-ozone] dineshchitlangia opened a new pull request #1001: HDDS-3694. Refactor dn-audit log
dineshchitlangia opened a new pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001
## What changes were proposed in this pull request?
Refactored dn audit log:
1. Reduce audit to Container related operations for all success + failure events
2. Audit for all other operations (blocks, chunks etc) only on failures/errors.
3. Updated username & IP when performing audit log
4. Addressed few sonar code smells.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3694
## How was this patch tested?
Build and deployed 3 node cluster, created a few keys. Confirmed that DN audit log no longer logs for CHUNK/BLOCK related operations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia commented on a change in pull request #1001: HDDS-3694. Reduce dn-audit log
Posted by GitBox <gi...@apache.org>.
dineshchitlangia commented on a change in pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#discussion_r433966975
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -613,28 +617,26 @@ private void audit(AuditAction action, EventType eventType,
}
}
- //TODO: use GRPC to fetch user and ip details
@Override
public AuditMessage buildAuditMessageForSuccess(AuditAction op,
Map<String, String> auditMap) {
return new AuditMessage.Builder()
- .setUser(null)
- .atIp(null)
+ .setUser(getRemoteUserName())
+ .atIp(Server.getRemoteAddress())
Review comment:
@adoroszlai Filed HDDS-3706 to address this issue separately.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia edited a comment on pull request #1001: HDDS-3694. Refactor dn-audit log
Posted by GitBox <gi...@apache.org>.
dineshchitlangia edited a comment on pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#issuecomment-637097661
Failure is unrelated to patch, failed due to timeout.
Verified test passes in local and the fork.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1001: HDDS-3694. Reduce dn-audit log
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#issuecomment-638255454
Thanks @dineshchitlangia for updating the patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai merged pull request #1001: HDDS-3694. Reduce dn-audit log
Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1001: HDDS-3694. Refactor dn-audit log
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#discussion_r433660441
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -583,14 +585,16 @@ private void audit(AuditAction action, EventType eventType,
AuditMessage amsg;
switch (result) {
case SUCCESS:
- if(eventType == EventType.READ &&
- AUDIT.getLogger().isInfoEnabled(AuditMarker.READ.getMarker())) {
- amsg = buildAuditMessageForSuccess(action, params);
- AUDIT.logReadSuccess(amsg);
- } else if(eventType == EventType.WRITE &&
- AUDIT.getLogger().isInfoEnabled(AuditMarker.WRITE.getMarker())) {
- amsg = buildAuditMessageForSuccess(action, params);
- AUDIT.logWriteSuccess(amsg);
+ if(action.getAction().contains("CONTAINER")) {
Review comment:
Instead of the substring check, can we explicitly list `DNAction` types that should be logged even in case of success? I think that would be better for both performance and documentation. It would also allow finer grained control. I would consider keeping log for `DELETE_BLOCK` and `DELETE_CHUNK`.
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -613,28 +617,26 @@ private void audit(AuditAction action, EventType eventType,
}
}
- //TODO: use GRPC to fetch user and ip details
@Override
public AuditMessage buildAuditMessageForSuccess(AuditAction op,
Map<String, String> auditMap) {
return new AuditMessage.Builder()
- .setUser(null)
- .atIp(null)
+ .setUser(getRemoteUserName())
+ .atIp(Server.getRemoteAddress())
Review comment:
Judging by the following message, I think there is more to be done here:
```
2020-06-02 06:48:10,478 | INFO | DNAudit | user=null | ip=null | op=CLOSE_CONTAINER {containerID=1} | ret=SUCCESS |
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia commented on pull request #1001: HDDS-3694. Reduce dn-audit log
Posted by GitBox <gi...@apache.org>.
dineshchitlangia commented on pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#issuecomment-638284710
Thanks @adoroszlai for review/commit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia commented on pull request #1001: HDDS-3694. Refactor dn-audit log
Posted by GitBox <gi...@apache.org>.
dineshchitlangia commented on pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#issuecomment-637097661
Failure is unrelated to patch, failed due to timeout.
Verified test passes in local.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] dineshchitlangia commented on a change in pull request #1001: HDDS-3694. Reduce dn-audit log
Posted by GitBox <gi...@apache.org>.
dineshchitlangia commented on a change in pull request #1001:
URL: https://github.com/apache/hadoop-ozone/pull/1001#discussion_r433978299
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -583,14 +585,16 @@ private void audit(AuditAction action, EventType eventType,
AuditMessage amsg;
switch (result) {
case SUCCESS:
- if(eventType == EventType.READ &&
- AUDIT.getLogger().isInfoEnabled(AuditMarker.READ.getMarker())) {
- amsg = buildAuditMessageForSuccess(action, params);
- AUDIT.logReadSuccess(amsg);
- } else if(eventType == EventType.WRITE &&
- AUDIT.getLogger().isInfoEnabled(AuditMarker.WRITE.getMarker())) {
- amsg = buildAuditMessageForSuccess(action, params);
- AUDIT.logWriteSuccess(amsg);
+ if(action.getAction().contains("CONTAINER")) {
Review comment:
Addressed in next commit. I have added DELETE_BLOCK to the list.
Left out DELETE_CHUNK for now as discussed in community call yesterday.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org