yihua commented on code in PR #13836: URL: https://github.com/apache/hudi/pull/13836#discussion_r2323750274
########## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/audit/AuditOperationState.java: ########## @@ -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.hudi.client.transaction.lock.audit; + +/** + * Enumeration of audit operation states for different operations. + */ +public enum AuditOperationState { + /** + * Operation started. + */ + START, + + /** + * Operation renewal/heartbeat. + */ + RENEW, + + /** + * Operation end. + */ + END +} Review Comment: nit: add new line to all new class files? ########## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/audit/AuditService.java: ########## @@ -0,0 +1,35 @@ +/* + * 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.hudi.client.transaction.lock.audit; + +/** + * Generic audit service interface for tracking operation lifecycles. + * Provides a single method for recording all types of audit operations. + */ +public interface AuditService extends AutoCloseable { Review Comment: Is `AuditService` supposed to be user-facing that an user can provide their own implementation? If so, the interface should be marked with `@PublicAPIClass(maturity = ApiMaturityLevel.EVOLVING)`. ########## hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/audit/AuditServiceFactory.java: ########## @@ -0,0 +1,130 @@ +/* + * 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.hudi.client.transaction.lock.audit; + +import org.apache.hudi.client.transaction.lock.StorageLockClient; +import org.apache.hudi.common.config.TypedProperties; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.storage.StoragePath; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.apache.hudi.common.table.HoodieTableMetaClient.LOCKS_FOLDER_NAME; + +/** + * Generic factory for creating audit services. + * This factory determines whether auditing is enabled by checking configuration files. + */ +public class AuditServiceFactory { + + private static final Logger LOG = LoggerFactory.getLogger(AuditServiceFactory.class); + private static final String AUDIT_CONFIG_FILE_NAME = "audit_enabled.json"; + private static final String STORAGE_LOCK_AUDIT_SERVICE_ENABLED_FIELD = "STORAGE_LOCK_AUDIT_SERVICE_ENABLED"; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + /** + * Creates a lock provider audit service instance by checking the audit configuration file. + * + * @param properties Configuration properties + * @param ownerId The owner ID for the lock provider + * @param basePath The base path of the Hudi table + * @param storageLockClient The storage lock client to use for reading configuration + * @return An Option containing the audit service if enabled, Option.empty() otherwise + */ + public static Option<AuditService> createLockProviderAuditService( + TypedProperties properties, String ownerId, String basePath, StorageLockClient storageLockClient) { + return createLockProviderAuditService(properties, ownerId, basePath, storageLockClient, true); + } + + /** + * Creates a lock provider audit service instance with optional configuration checking. + * + * @param properties Configuration properties + * @param ownerId The owner ID for the lock provider + * @param basePath The base path of the Hudi table + * @param storageLockClient The storage lock client to use for reading configuration + * @param checkAuditEnabled Whether to check the audit configuration file + * @return An Option containing the audit service if enabled, Option.empty() otherwise + */ + public static Option<AuditService> createLockProviderAuditService( + TypedProperties properties, + String ownerId, + String basePath, + StorageLockClient storageLockClient, + boolean checkAuditEnabled) { + + if (checkAuditEnabled && !isAuditEnabled(basePath, storageLockClient)) { + return Option.empty(); + } + + // For now, return empty since we haven't implemented a concrete service yet + return Option.empty(); + } + + /** + * Checks if audit is enabled by reading the audit configuration file. + * + * @param basePath The base path of the Hudi table + * @param storageLockClient The storage lock client to use for reading configuration + * @return true if audit is enabled, false otherwise + */ + private static boolean isAuditEnabled(String basePath, StorageLockClient storageLockClient) { + try { + // Construct the audit config path the same way as lock file path + // basePath + / + .hoodie/locks + / + audit_enabled.json + String auditConfigPath = String.format("%s%s%s%s%s", + basePath, + StoragePath.SEPARATOR, + LOCKS_FOLDER_NAME, + StoragePath.SEPARATOR, Review Comment: nit: have a method to get lock folder from `storageLockClient` for reuse with the following, and use that for constructing audit config file path? ``` StorageBasedLockProvider( String ownerId, TypedProperties properties, Functions.Function3<String, Long, Supplier<Boolean>, HeartbeatManager> heartbeatManagerLoader, Functions.Function3<String, String, TypedProperties, StorageLockClient> storageLockClientLoader, Logger logger, HoodieLockMetrics hoodieLockMetrics) { ... this.lockFilePath = String.format( "%s%s%s%s%s", config.getHudiTableBasePath(), StoragePath.SEPARATOR, LOCKS_FOLDER_NAME, StoragePath.SEPARATOR, DEFAULT_TABLE_LOCK_FILE_NAME); ``` -- 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]
