sumitagrawl commented on code in PR #8858:
URL: https://github.com/apache/ozone/pull/8858#discussion_r2330857574


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/audit/AuditLoggerType.java:
##########
@@ -25,7 +25,8 @@ public enum AuditLoggerType {
   OMLOGGER("OMAudit"),
   SCMLOGGER("SCMAudit"),
   S3GLOGGER("S3GAudit"),
-  OMSYSTEMLOGGER("OMSystemAudit");
+  OMSYSTEMLOGGER("OMSystemAudit"),
+  OMDELETIONLOGGER("OMDeletionAudit");

Review Comment:
   IMO, we should not introduce another Audit log for deletion, already System 
audit is created for all internal request and major event to be audited.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java:
##########
@@ -172,22 +206,34 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager, Execut
           }
         }
         if (path.hasDeletedDir()) {
+          deletedDirNames.add(path.getDeletedDir());
           numDirsDeleted++;
         }
       }
       deletingServiceMetrics.incrNumSubDirectoriesMoved(numSubDirMoved);
       deletingServiceMetrics.incrNumSubFilesMoved(numSubFilesMoved);
       deletingServiceMetrics.incrNumDirPurged(numDirsDeleted);
 
+      Map<String, String> auditParams = new LinkedHashMap<>();
       if (fromSnapshotInfo != null) {
         
fromSnapshotInfo.setLastTransactionInfo(TransactionInfo.valueOf(context.getTermIndex()).toByteString());
         omMetadataManager.getSnapshotInfoTable().addCacheEntry(new 
CacheKey<>(fromSnapshotInfo.getTableKey()),
             CacheValue.get(context.getIndex(), fromSnapshotInfo));
+        auditParams.put(AUDIT_PARAM_SNAPSHOT_ID, 
fromSnapshotInfo.getSnapshotId().toString());
       }
+
+      auditParams.put(AUDIT_PARAM_DIRS_DELETED, 
String.valueOf(numDirsDeleted));
+      auditParams.put(AUDIT_PARAM_SUBDIRS_MOVED, 
String.valueOf(numSubDirMoved));
+      auditParams.put(AUDIT_PARAM_SUBFILES_MOVED, 
String.valueOf(numSubFilesMoved));
+      auditParams.put(AUDIT_PARAM_DIRS_DELETED_LIST, String.join(",", 
deletedDirNames));

Review Comment:
   directory deletion is preparing request simulating iteration from top-down. 
So, same request, it will show move and then delete of same directory.
   So, its better we remove entry of deletedDirNames from numSubDirMoved.



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java:
##########
@@ -228,10 +239,28 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager, Execut
       omMetrics.incNumSnapshotDeleted();
       LOG.info("Deleted snapshot '{}' under path '{}'",
           snapshotName, snapshotPath);
+      
+      Map<String, String> auditParams = new LinkedHashMap<>();
+      auditParams.put(AUDIT_PARAM_VOLUME_NAME, volumeName);
+      auditParams.put(AUDIT_PARAM_BUCKET_NAME, bucketName);
+      auditParams.put(AUDIT_PARAM_SNAPSHOT_NAME, snapshotName);
+      auditParams.put(AUDIT_PARAM_SNAPSHOT_TABLE_KEY, 
snapshotInfo.getTableKey());
+      auditParams.put(AUDIT_PARAM_SNAPSHOT_ID, 
snapshotInfo.getSnapshotId().toString());
+      
AUDIT.logWriteSuccess(ozoneManager.buildAuditMessageForSuccess(OMDeletionAction.SNAPSHOT_DELETION,
 auditParams));

Review Comment:
   already audit log is there DELETE_SNAPSHOT above, this is redundant and to 
be removed



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java:
##########
@@ -125,11 +134,18 @@ public OMClientResponse 
validateAndUpdateCache(OzoneManager ozoneManager, Execut
       omSnapshotIntMetrics.incNumSnapshotPurges();
       LOG.info("Successfully executed snapshotPurgeRequest: {{}} along with 
updating snapshots:{}.",
           snapshotPurgeRequest, updatedSnapshotInfos);
+      
+      Map<String, String> auditParams = new LinkedHashMap<>();
+      auditParams.put(AUDIT_PARAM_SNAPSHOTS_PURGED, 
String.valueOf(snapshotDbKeys.size()));
+      auditParams.put(AUDIT_PARAM_SNAPSHOT_DB_KEYS, snapshotDbKeys.toString());
+      auditParams.put(AUDIT_PARAM_SNAPSHOTS_UPDATED, 
updatedSnapshotInfos.toString());
+      
AUDIT.logWriteSuccess(ozoneManager.buildAuditMessageForSuccess(OMDeletionAction.SNAPSHOT_PURGE,
 auditParams));
     } catch (IOException ex) {
       omClientResponse = new OMSnapshotPurgeResponse(
           createErrorOMResponse(omResponse, ex));
       omSnapshotIntMetrics.incNumSnapshotPurgeFails();
       LOG.error("Failed to execute snapshotPurgeRequest:{{}}.", 
snapshotPurgeRequest, ex);
+      
AUDIT.logWriteFailure(ozoneManager.buildAuditMessageForFailure(OMDeletionAction.SNAPSHOT_PURGE,
 null, ex));

Review Comment:
   failure log need have snapshotDbKeys also



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to