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/02 07:06:45 UTC

[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1001: HDDS-3694. Refactor dn-audit log

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