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]