Add separate flag for enabling operation level traces (#1105)

* Add separate flag for enabling operation level traces * Fix a bug
GoogleCloudDataproc · Jun 18, 2024 · ac86720 · ac86720
1 parent 9bbff28
commit ac86720
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 4 deletions.
diff --git a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystem.java b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystem.java
@@ -22,8 +22,8 @@
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_FILE_CHECKSUM_TYPE;
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_GLOB_ALGORITHM;
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_LAZY_INITIALIZATION_ENABLE;
+import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_OPERATION_TRACE_LOG_ENABLE;
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_OUTPUT_STREAM_SYNC_MIN_INTERVAL;
-import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_TRACE_LOG_ENABLE;
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.GCS_WORKING_DIRECTORY;
 import static com.google.cloud.hadoop.fs.gcs.GoogleHadoopFileSystemConfiguration.PERMISSIONS_TO_REPORT;
 import static com.google.cloud.hadoop.util.HadoopCredentialsConfiguration.CLOUD_PLATFORM_SCOPE;
@@ -303,7 +303,8 @@ public void initialize(URI path, Configuration config) throws IOException {
 
     initializeGcsFs(config);
 
-    this.traceFactory = TraceFactory.get(GCS_TRACE_LOG_ENABLE.get(config, config::getBoolean));
+    this.traceFactory =
+        TraceFactory.get(GCS_OPERATION_TRACE_LOG_ENABLE.get(config, config::getBoolean));
   }
 
   private void initializeFsRoot() {

diff --git a/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java b/gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfiguration.java
@@ -478,6 +478,11 @@ public class GoogleHadoopFileSystemConfiguration {
       new HadoopConfigurationProperty<>(
           "fs.gs.tracelog.enable", GoogleCloudStorageOptions.DEFAULT.isTraceLogEnabled());
 
+  /** Configuration key to enable operational level traces */
+  public static final HadoopConfigurationProperty<Boolean> GCS_OPERATION_TRACE_LOG_ENABLE =
+      new HadoopConfigurationProperty<>(
+          "fs.gs.operation.tracelog.enable", GoogleCloudStorageOptions.DEFAULT.isTraceLogEnabled());
+
   /** Configuration key to configure client to use for GCS access. */
   public static final HadoopConfigurationProperty<ClientType> GCS_CLIENT_TYPE =
       new HadoopConfigurationProperty<>(
@@ -601,6 +606,7 @@ static GoogleCloudStorageOptions.Builder getGcsOptionsBuilder(Configuration conf
         .setStorageRootUrl(GCS_ROOT_URL.get(config, config::get))
         .setStorageServicePath(GCS_SERVICE_PATH.get(config, config::get))
         .setTraceLogEnabled(GCS_TRACE_LOG_ENABLE.get(config, config::getBoolean))
+        .setOperationTraceLogEnabled(GCS_OPERATION_TRACE_LOG_ENABLE.get(config, config::getBoolean))
         .setTrafficDirectorEnabled(GCS_GRPC_TRAFFICDIRECTOR_ENABLE.get(config, config::getBoolean))
         .setWriteChannelOptions(getWriteChannelOptions(config));
   }

diff --git a/...src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfigurationTest.java b/...src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemConfigurationTest.java
@@ -116,6 +116,7 @@ public class GoogleHadoopFileSystemConfigurationTest {
           put("fs.gs.storage.root.url", "https://storage.googleapis.com/");
           put("fs.gs.storage.service.path", "storage/v1/");
           put("fs.gs.tracelog.enable", false);
+          put("fs.gs.operation.tracelog.enable", false);
           put("fs.gs.working.dir", "/");
           put("fs.gs.client.upload.type", UploadType.CHUNK_UPLOAD);
           put("fs.gs.write.temporary.dirs", ImmutableSet.of());

diff --git a/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java b/gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleHadoopFileSystemIntegrationTest.java
@@ -2269,7 +2269,7 @@ public void testInitializeCompatibleWithHadoopCredentialProvider() throws Except
   @Test
   public void testThreadTraceEnabledRename() throws Exception {
     Configuration config = ghfs.getConf();
-    config.set("fs.gs.tracelog.enable", "true");
+    config.set("fs.gs.application.tracelog.enable", "true");
     ghfs.initialize(ghfs.getUri(), config);
 
     Path testRoot = new Path(sharedBucketName1, "/directory1/");

diff --git a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageOptions.java b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageOptions.java
@@ -70,6 +70,7 @@ public static Builder builder() {
         .setStorageRootUrl(Storage.DEFAULT_ROOT_URL)
         .setStorageServicePath(Storage.DEFAULT_SERVICE_PATH)
         .setTraceLogEnabled(false)
+        .setOperationTraceLogEnabled(false)
         .setTrafficDirectorEnabled(true)
         .setWriteChannelOptions(AsyncWriteChannelOptions.DEFAULT);
   }
@@ -146,6 +147,8 @@ public static Builder builder() {
 
   public abstract boolean isTraceLogEnabled();
 
+  public abstract boolean isOperationTraceLogEnabled();
+
   public RetryHttpInitializerOptions toRetryHttpInitializerOptions() {
     return RetryHttpInitializerOptions.builder()
         .setDefaultUserAgent(getAppName())
@@ -229,6 +232,8 @@ public abstract Builder setGrpcMessageTimeoutCheckInterval(
 
     public abstract Builder setTraceLogEnabled(Boolean enable);
 
+    public abstract Builder setOperationTraceLogEnabled(Boolean enable);
+
     abstract GoogleCloudStorageOptions autoBuild();
 
     public GoogleCloudStorageOptions build() {

diff --git a/util/src/main/java/com/google/cloud/hadoop/util/ThreadTrace.java b/util/src/main/java/com/google/cloud/hadoop/util/ThreadTrace.java
@@ -94,7 +94,7 @@ public String getTrackingId() {
     return this.trackingId;
   }
 
-  ThreadTrace getSubTrace() {
+  synchronized ThreadTrace getSubTrace() {
     long threadId = Thread.currentThread().getId();
 
     subEvents.putIfAbsent(threadId, new ArrayList<>());