Merge branch 'refs/heads/master' into amithkb/parallel-exception

walmartlabs · Dec 20, 2024 · 38d53a1 · 38d53a1
2 parents f4c0f2e + 901a542
commit 38d53a1
Show file tree

Hide file tree

Showing 12 changed files with 59 additions and 14 deletions.
diff --git a/agent/src/main/java/com/walmartlabs/concord/agent/cfg/AgentConfiguration.java b/agent/src/main/java/com/walmartlabs/concord/agent/cfg/AgentConfiguration.java
@@ -53,6 +53,7 @@ public class AgentConfiguration {
 
     private final Path logDir;
     private final long logMaxDelay;
+    private final boolean workDirMasking;
 
     private final int workersCount;
     private final long pollInterval;
@@ -81,6 +82,7 @@ public AgentConfiguration(Config cfg) {
 
         this.logDir = getOrCreatePath(cfg, "logDir");
         this.logMaxDelay = cfg.getDuration("logMaxDelay", TimeUnit.MILLISECONDS);
+        this.workDirMasking = cfg.getBoolean("workDirMasking");
 
         this.workersCount = cfg.getInt("workersCount");
         this.maintenanceModeListenerHost = cfg.getString("maintenanceModeListenerHost");
@@ -136,6 +138,10 @@ public long getLogMaxDelay() {
         return logMaxDelay;
     }
 
+    public boolean isWorkDirMaskings() {
+        return workDirMasking;
+    }
+
     public int getWorkersCount() {
         return workersCount;
     }

diff --git a/agent/src/main/java/com/walmartlabs/concord/agent/executors/JobExecutorFactory.java b/agent/src/main/java/com/walmartlabs/concord/agent/executors/JobExecutorFactory.java
@@ -124,6 +124,7 @@ public JobExecutor create(JobRequest.Type jobType) {
                     .exposeDockerDaemon(dockerCfg.exposeDockerDaemon())
                     .maxHeartbeatInterval(serverCfg.getMaxNoHeartbeatInterval())
                     .segmentedLogs(segmentedLogs)
+                    .workDirMasking(agentCfg.isWorkDirMaskings())
                     .persistentWorkDir(runnerCfg.getPersistentWorkDir())
                     .preforkEnabled(preForkCfg.isEnabled())
                     .cleanRunnerDescendants(runnerCfg.getCleanRunnerDescendants())

diff --git a/agent/src/main/java/com/walmartlabs/concord/agent/executors/runner/RunnerJob.java b/agent/src/main/java/com/walmartlabs/concord/agent/executors/runner/RunnerJob.java
@@ -184,6 +184,7 @@ private static RunnerConfiguration createRunnerConfiguration(RunnerJobExecutorCo
                 .logging(LoggingConfiguration.builder()
                         .sendSystemOutAndErrToSLF4J(true)
                         .segmentedLogs(execCfg.segmentedLogs())
+                        .workDirMasking(execCfg.workDirMasking())
                         .build())
                 .build();
     }

diff --git a/agent/src/main/java/com/walmartlabs/concord/agent/executors/runner/RunnerJobExecutor.java b/agent/src/main/java/com/walmartlabs/concord/agent/executors/runner/RunnerJobExecutor.java
@@ -759,6 +759,8 @@ public interface RunnerJobExecutorConfiguration {
 
         boolean segmentedLogs();
 
+        boolean workDirMasking();
+
         @Value.Default
         default List<String> extraDockerVolumes() {
             return Collections.emptyList();

diff --git a/agent/src/main/resources/concord-agent.conf b/agent/src/main/resources/concord-agent.conf
@@ -61,6 +61,9 @@ concord-agent {
     # determines how ofter the logs are send back to the server
     logMaxDelay = "2 seconds"
 
+    # replace the current process' workDir in logs with literal "$WORK_DIR"
+    workDirMasking = true
+
     # maximum number of concurrent processes
     workersCount = 3
     workersCount = ${?WORKERS_COUNT}

diff --git a/...est/resources/com/walmartlabs/concord/it/runtime/v2/processMetadataSend/debug_logback.xml b/...est/resources/com/walmartlabs/concord/it/runtime/v2/processMetadataSend/debug_logback.xml
@@ -3,7 +3,7 @@
         <filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />
 
         <encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
-            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
+            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
                 <!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
                 <pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} [%-5level] %msg%n%rEx{full, com.sun, sun}</pattern>
             </layout>

diff --git a/...common/src/main/java/com/walmartlabs/concord/runtime/common/cfg/LoggingConfiguration.java b/...common/src/main/java/com/walmartlabs/concord/runtime/common/cfg/LoggingConfiguration.java
@@ -53,6 +53,15 @@ default boolean sendSystemOutAndErrToSLF4J() {
         return true;
     }
 
+    /**
+     * If {@code true}, any ${workDir} value will be replaced with literal
+     * "$WORK_DIR" string. Reduces noise in the logs.
+     */
+    @Value.Default
+    default boolean workDirMasking() {
+        return true;
+    }
+
     static ImmutableLoggingConfiguration.Builder builder() {
         return ImmutableLoggingConfiguration.builder();
     }

diff --git a/...v2/runner-test/src/main/java/com/walmartlabs/concord/runtime/v2/runner/TestRuntimeV2.java b/...v2/runner-test/src/main/java/com/walmartlabs/concord/runtime/v2/runner/TestRuntimeV2.java
@@ -50,8 +50,8 @@
 import com.walmartlabs.concord.runtime.v2.runner.vm.ParallelCommand;
 import com.walmartlabs.concord.runtime.v2.sdk.*;
 import com.walmartlabs.concord.sdk.Constants;
-import com.walmartlabs.concord.svm.*;
 import com.walmartlabs.concord.svm.Runtime;
+import com.walmartlabs.concord.svm.*;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.BeforeEachCallback;
 import org.junit.jupiter.api.extension.ExtensionContext;
@@ -70,7 +70,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -196,7 +196,10 @@ public byte[] run(RunnerConfiguration baseCfg) throws Exception {
         }
 
         ImmutableRunnerConfiguration.Builder runnerCfg = RunnerConfiguration.builder()
-                .logging(LoggingConfiguration.builder().segmentedLogs(false).build());
+                .logging(LoggingConfiguration.builder()
+                        .segmentedLogs(false)
+                        .workDirMasking(false)
+                        .build());
 
         if (baseCfg != null) {
             runnerCfg.from(baseCfg);
@@ -377,7 +380,7 @@ protected void configure() {
                 taskCallListeners.addBinding().to(TaskResultListener.class);
 
                 Multibinder<ExecutionListener> executionListeners = Multibinder.newSetBinder(binder(), ExecutionListener.class);
-                executionListeners.addBinding().toInstance(new ExecutionListener(){
+                executionListeners.addBinding().toInstance(new ExecutionListener() {
                     @Override
                     public void beforeProcessStart(Runtime runtime, State state) {
                         SensitiveDataHolder.getInstance().get().clear();
@@ -390,7 +393,7 @@ public void beforeProcessStart(Runtime runtime, State state) {
                         @Override
                         public Result afterCommand(Runtime runtime, VM vm, State state, ThreadId threadId, Command cmd) {
                             if (cmd instanceof BlockCommand
-                                    || cmd instanceof ParallelCommand) {
+                                || cmd instanceof ParallelCommand) {
                                 return ExecutionListener.super.afterCommand(runtime, vm, state, threadId, cmd);
                             }
 

diff --git a/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java b/runtime/v2/runner-test/src/test/java/com/walmartlabs/concord/runtime/v2/runner/MainTest.java
@@ -625,6 +625,7 @@ public void testSegmentedLogging() throws Exception {
         RunnerConfiguration runnerCfg = RunnerConfiguration.builder()
                 .logging(LoggingConfiguration.builder()
                         .sendSystemOutAndErrToSLF4J(false)
+                        .workDirMasking(false)
                         .build())
                 .build();
 

diff --git a/...me/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/InjectorFactory.java b/...me/v2/runner/src/main/java/com/walmartlabs/concord/runtime/v2/runner/InjectorFactory.java
@@ -32,6 +32,7 @@
 import com.walmartlabs.concord.runtime.v2.runner.guice.CurrentClasspathModule;
 import com.walmartlabs.concord.runtime.v2.runner.guice.DefaultRunnerModule;
 import com.walmartlabs.concord.runtime.v2.runner.guice.ProcessDependenciesModule;
+import com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout;
 import com.walmartlabs.concord.runtime.v2.runner.tasks.V2;
 import com.walmartlabs.concord.runtime.v2.sdk.ProcessConfiguration;
 import com.walmartlabs.concord.runtime.v2.sdk.Task;
@@ -123,6 +124,10 @@ private ConfigurationModule(WorkingDirectory workDir,
         @Override
         protected void configure() {
             bind(WorkingDirectory.class).toInstance(workDir);
+            if (runnerCfg.logging().workDirMasking()) {
+                CustomLayout.enableWorkingDirectoryMasking(workDir);
+            }
+
             bind(RunnerConfiguration.class).toInstance(runnerCfg);
             bind(ProcessConfiguration.class).toProvider(processCfgProvider);
             bind(InstanceId.class).toProvider(InstanceIdProvider.class);

diff --git a/...r/logging/MaskingSensitiveDataLayout.java → ...ntime/v2/runner/logging/CustomLayout.java b/...r/logging/MaskingSensitiveDataLayout.java → ...ntime/v2/runner/logging/CustomLayout.java
@@ -23,17 +23,31 @@
 import ch.qos.logback.classic.PatternLayout;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import com.walmartlabs.concord.runtime.v2.runner.SensitiveDataHolder;
+import com.walmartlabs.concord.runtime.v2.sdk.WorkingDirectory;
 
-import java.util.Collection;
+import static java.util.Objects.requireNonNull;
 
-public class MaskingSensitiveDataLayout extends PatternLayout {
+public class CustomLayout extends PatternLayout {
+
+    private static volatile String workDirToReplace;
+
+    /**
+     * Enables masking of ${workDir} values in logs. Such values often add noise to logs.
+     */
+    public static void enableWorkingDirectoryMasking(WorkingDirectory workDir) {
+        requireNonNull(workDir);
+        CustomLayout.workDirToReplace = workDir.getValue().toString();
+    }
 
     @Override
     public String doLayout(ILoggingEvent event) {
-        Collection<String> sensitiveData = SensitiveDataHolder.getInstance().get();
-        String msg = super.doLayout(event);
-        for (String d : sensitiveData) {
-            msg = msg.replace(d, "******");
+        var sensitiveData = SensitiveDataHolder.getInstance().get();
+        var msg = super.doLayout(event);
+        for (var sensitiveString : sensitiveData) {
+            msg = msg.replace(sensitiveString, "******");
+        }
+        if (CustomLayout.workDirToReplace != null) {
+            msg = msg.replace(workDirToReplace, "$WORK_DIR");
         }
         return msg;
     }

diff --git a/runtime/v2/runner/src/main/resources/logback.xml b/runtime/v2/runner/src/main/resources/logback.xml
@@ -3,7 +3,7 @@
         <filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />
 
         <encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
-            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
+            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
                 <!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
                 <pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} [%-5level] %msg%n%rEx{full, com.sun, sun}</pattern>
             </layout>
@@ -14,7 +14,7 @@
         <filter class="com.walmartlabs.concord.runtime.v2.runner.logging.LogLevelFilter" />
 
         <encoder class="com.walmartlabs.concord.runtime.v2.runner.logging.ConcordLogEncoder">
-            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.MaskingSensitiveDataLayout">
+            <layout class="com.walmartlabs.concord.runtime.v2.runner.logging.CustomLayout">
                 <!-- the UI expects log timestamps in a specific format to be able to convert it to the local time -->
                 <pattern>%date{yyyy-MM-dd'T'HH:mm:ss.SSSZ, UTC} %msg%n</pattern>
             </layout>