BIGTOP-4166: Make task log messages more user friendly (#23)

bigtop-manager-agent/src/main/java/org/apache/bigtop/manager/agent/executor/AbstractCommandExecutor.java

@@ -35,6 +35,7 @@ public abstract class AbstractCommandExecutor implements CommandExecutor {
 
     @Override
     public CommandReply execute(CommandRequest request) {
+        log.info("Running task: {}", request.getTaskId());
         commandRequest = request;
         commandReplyBuilder = CommandReply.newBuilder();
 

bigtop-manager-agent/src/main/java/org/apache/bigtop/manager/agent/executor/CacheDistributeCommandExecutor.java

@@ -54,7 +54,6 @@ public CommandType getCommandType() {
     public void doExecute() {
         CacheMessagePayload cacheMessagePayload =
                 JsonUtils.readFromString(commandRequest.getPayload(), CacheMessagePayload.class);
-        log.info("[agent executeTask] taskEvent is: {}", commandRequest);
         String cacheDir = Constants.STACK_CACHE_DIR;
 
         LinuxFileUtils.createDirectories(cacheDir, "root", "root", "rwxr-xr-x", false);

bigtop-manager-agent/src/main/java/org/apache/bigtop/manager/agent/executor/CommandExecutors.java

@@ -58,10 +58,6 @@ private static synchronized void load() {
             }
 
             COMMAND_EXECUTORS.put(commandExecutor.getCommandType(), beanName);
-            log.info(
-                    "Load JobRunner: {} with identifier: {}",
-                    commandExecutor.getClass().getName(),
-                    commandExecutor.getCommandType());
         }
 
         LOADED.set(true);

bigtop-manager-agent/src/main/java/org/apache/bigtop/manager/agent/executor/ComponentCommandExecutor.java

@@ -47,7 +47,6 @@ protected void doExecuteOnDevMode() {
     @Override
     public void doExecute() {
         CommandPayload commandPayload = JsonUtils.readFromString(commandRequest.getPayload(), CommandPayload.class);
-        log.info("[agent executeTask] taskEvent is: {}", commandRequest);
         ShellResult shellResult = StackExecutor.execute(commandPayload);
 
         commandReplyBuilder.setCode(shellResult.getExitCode());

bigtop-manager-agent/src/main/java/org/apache/bigtop/manager/agent/executor/HostCheckCommandExecutor.java

@@ -44,7 +44,6 @@ public CommandType getCommandType() {
 
     @Override
     public void doExecute() {
-        log.info("[agent executeTask] taskEvent is: {}", commandRequest);
         ShellResult shellResult = runChecks(List.of(this::checkTimeSync));
         commandReplyBuilder.setCode(shellResult.getExitCode());
         commandReplyBuilder.setResult(shellResult.getResult());
@@ -63,9 +62,6 @@ private ShellResult runChecks(List<Supplier<ShellResult>> suppliers) {
     }
 
     private ShellResult checkTimeSync() {
-        ShellResult shellResult = TimeSyncDetection.checkTimeSync();
-        log.info("Time sync check result: {}", shellResult.getResult());
-
-        return shellResult;
+        return TimeSyncDetection.checkTimeSync();
     }
 }

bigtop-manager-common/src/main/java/org/apache/bigtop/manager/common/shell/ShellExecutor.java

@@ -20,6 +20,8 @@
 
 import org.apache.bigtop.manager.common.thread.TaskLogThreadDecorator;
 
+import org.apache.commons.lang3.StringUtils;
+
 import lombok.extern.slf4j.Slf4j;
 
 import java.io.BufferedReader;
@@ -36,7 +38,7 @@
 
 /**
  * shell command executor.
- *
+ * <br />
  * <code>ShellExecutor</code> should be used in cases where the output
  * of the command needs no explicit parsing and where the command, working
  * directory and the environment remains unchanged. The output of the command
@@ -59,7 +61,10 @@ public class ShellExecutor {
      */
     private final long timeoutInterval;
 
-    private final Consumer<String> consumer;
+    /**
+     * Whether we should append log to log file
+     */
+    private final Boolean appendLog;
 
     /**
      * Whether script timed out
@@ -91,15 +96,13 @@ public class ShellExecutor {
      * @param timeout    Specifies the time in milliseconds, after which the
      *                   command will be killed and the status marked as timedout.
      *                   If 0, the command will not be timed out.
-     * @param consumer   the consumer to consume the output of the executed command.
      */
-    private ShellExecutor(
-            String[] execString, File dir, Map<String, String> env, long timeout, Consumer<String> consumer) {
+    private ShellExecutor(String[] execString, File dir, Map<String, String> env, long timeout, Boolean appendLog) {
         this.command = execString.clone();
         this.dir = dir;
         this.environment = env;
         this.timeoutInterval = timeout;
-        this.consumer = consumer;
+        this.appendLog = appendLog;
     }
 
     /**
@@ -111,21 +114,20 @@ private ShellExecutor(
      * @throws IOException errors
      */
     public static ShellResult execCommand(List<String> builderParameters) throws IOException {
-        return execCommand(builderParameters, s -> {});
+        return execCommand(null, builderParameters, 0L, false);
     }
 
     /**
      * Static method to execute a shell command.
      * Covers most of the simple cases for user.
      *
      * @param builderParameters shell command to execute.
-     * @param consumer the consumer to consume the output of the executed command.
+     * @param appendLog append stream log to log file if true.
      * @return the output of the executed command.
      * @throws IOException errors
      */
-    public static ShellResult execCommand(List<String> builderParameters, Consumer<String> consumer)
-            throws IOException {
-        return execCommand(null, builderParameters, 0L, consumer);
+    public static ShellResult execCommand(List<String> builderParameters, Boolean appendLog) throws IOException {
+        return execCommand(null, builderParameters, 0L, appendLog);
     }
 
     /**
@@ -138,40 +140,9 @@ public static ShellResult execCommand(List<String> builderParameters, Consumer<S
      * @return the output of the executed command.
      * @throws IOException errors
      */
-    public static ShellResult execCommand(Map<String, String> env, List<String> builderParameters) throws IOException {
-        return execCommand(env, builderParameters, s -> {});
-    }
-
-    /**
-     * Static method to execute a shell command.
-     * Covers most of the simple cases without requiring the user to implement
-     * the <code>AbstractShell</code> interface.
-     *
-     * @param env the map of environment key=value
-     * @param builderParameters shell command to execute.
-     * @param consumer the consumer to consume the output of the executed command.
-     * @return the output of the executed command.
-     * @throws IOException errors
-     */
-    public static ShellResult execCommand(
-            Map<String, String> env, List<String> builderParameters, Consumer<String> consumer) throws IOException {
-        return execCommand(env, builderParameters, 0L, consumer);
-    }
-
-    /**
-     * Static method to execute a shell command.
-     * Covers most of the simple cases without requiring the user to implement
-     * the <code>AbstractShell</code> interface.
-     *
-     * @param env     the map of environment key=value
-     * @param builderParameters shell command to execute.
-     * @param timeout time in milliseconds after which script should be marked timeout
-     * @return the output of the executed command.
-     * @throws IOException errors
-     */
-    public static ShellResult execCommand(Map<String, String> env, List<String> builderParameters, long timeout)
+    public static ShellResult execCommand(Map<String, String> env, List<String> builderParameters, Boolean appendLog)
             throws IOException {
-        return execCommand(env, builderParameters, timeout, s -> {});
+        return execCommand(env, builderParameters, 0L, appendLog);
     }
 
     /**
@@ -182,17 +153,28 @@ public static ShellResult execCommand(Map<String, String> env, List<String> buil
      * @param env     the map of environment key=value
      * @param builderParameters shell command to execute.
      * @param timeout time in milliseconds after which script should be marked timeout
-     * @param consumer the consumer to consume the output of the executed command.
      * @return the output of the executed command.
      * @throws IOException errors
      */
     public static ShellResult execCommand(
-            Map<String, String> env, List<String> builderParameters, long timeout, Consumer<String> consumer)
+            Map<String, String> env, List<String> builderParameters, long timeout, Boolean appendLog)
             throws IOException {
         String[] cmd = builderParameters.toArray(new String[0]);
+        ShellExecutor shellExecutor = new ShellExecutor(cmd, null, env, timeout, appendLog);
+
+        if (appendLog) {
+            log.info(StringUtils.EMPTY);
+            log.info("********** Running: {} **********", String.join(" ", builderParameters));
+        }
+
+        ShellResult result = shellExecutor.execute();
 
-        ShellExecutor shellExecutor = new ShellExecutor(cmd, null, env, timeout, consumer);
-        return shellExecutor.execute();
+        if (appendLog) {
+            log.info("********** Finished: {} **********", String.join(" ", builderParameters));
+            log.info(StringUtils.EMPTY);
+        }
+
+        return result;
     }
 
     /**
@@ -224,11 +206,11 @@ private ShellResult execute() throws IOException {
         // free the error stream buffer
         BufferedReader errReader = createBufferedReader(process.getErrorStream());
         StringBuilder errMsg = new StringBuilder();
-        Thread errThread = createReaderThread(errReader, errMsg);
+        Thread errThread = createReaderThread(errReader, errMsg, log::error);
 
         BufferedReader inReader = createBufferedReader(process.getInputStream());
         StringBuilder inMsg = new StringBuilder();
-        Thread inThread = createReaderThread(inReader, inMsg);
+        Thread inThread = createReaderThread(inReader, inMsg, log::info);
 
         try {
             errThread.start();
@@ -306,13 +288,16 @@ private BufferedReader createBufferedReader(InputStream inputStream) {
         return new BufferedReader(new InputStreamReader(inputStream));
     }
 
-    private Thread createReaderThread(BufferedReader reader, StringBuilder msg) {
+    private Thread createReaderThread(BufferedReader reader, StringBuilder msg, Consumer<String> consumer) {
         TaskLogThreadDecorator decorator = new TaskLogThreadDecorator();
         return decorator.decorate(() -> {
             try {
                 String line = reader.readLine();
                 while ((line != null)) {
-                    consumer.accept(line);
+                    if (appendLog) {
+                        consumer.accept(line);
+                    }
+
                     msg.append(line);
                     msg.append(System.lineSeparator());
                     line = reader.readLine();

bigtop-manager-common/src/main/java/org/apache/bigtop/manager/common/utils/os/TimeSyncDetection.java

@@ -21,10 +21,13 @@
 import org.apache.bigtop.manager.common.shell.ShellExecutor;
 import org.apache.bigtop.manager.common.shell.ShellResult;
 
+import lombok.extern.slf4j.Slf4j;
+
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+@Slf4j
 public class TimeSyncDetection {
 
     public static ShellResult checkTimeSync() {
@@ -34,18 +37,27 @@ public static ShellResult checkTimeSync() {
         params.add("chronyd");
         ShellResult shellResult;
         try {
+            log.info("Checking service chronyd status");
             shellResult = ShellExecutor.execCommand(params);
+            if (shellResult.getExitCode() == 0) {
+                log.info("Service chronyd is enabled");
+                return shellResult;
+            }
 
-            if (shellResult.getExitCode() != 0) {
-                params.remove(params.size() - 1);
-                params.add("ntpd");
-                shellResult = ShellExecutor.execCommand(params);
+            log.info("Service chronyd is not enabled, checking ntpd status");
+            params.remove(params.size() - 1);
+            params.add("ntpd");
+            shellResult = ShellExecutor.execCommand(params);
+            if (shellResult.getExitCode() == 0) {
+                log.info("Service ntpd is enabled");
+                return shellResult;
             }
 
+            log.info("Service ntpd is not enabled");
         } catch (IOException e) {
             shellResult = new ShellResult();
             shellResult.setExitCode(-1);
-            shellResult.setErrMsg("Neither chronyd nor ntpd check failed.");
+            shellResult.setErrMsg("Neither chronyd nor ntpd check failed");
         }
 
         return shellResult;

bigtop-manager-stack/bigtop-manager-stack-bigtop/src/main/java/org/apache/bigtop/manager/stack/bigtop/v3_3_0/hdfs/HdfsSetup.java

@@ -46,7 +46,7 @@ public static ShellResult config(Params params) {
     }
 
     public static ShellResult config(Params params, String componentName) {
-        log.info("starting HDFS config");
+        log.info("Setting hdfs config");
         HdfsParams hdfsParams = (HdfsParams) params;
 
         String confDir = hdfsParams.confDir();

bigtop-manager-stack/bigtop-manager-stack-bigtop/src/main/java/org/apache/bigtop/manager/stack/bigtop/v3_3_0/yarn/YarnSetup.java

@@ -43,7 +43,7 @@ public static ShellResult config(Params params) {
     }
 
     public static ShellResult config(Params params, String componentName) {
-        log.info("starting YARN config");
+        log.info("Setting yarn config");
         YarnParams yarnParams = (YarnParams) params;
 
         String confDir = yarnParams.confDir();

bigtop-manager-stack/bigtop-manager-stack-bigtop/src/main/java/org/apache/bigtop/manager/stack/bigtop/v3_3_0/zookeeper/ZookeeperSetup.java

@@ -42,7 +42,7 @@
 public class ZookeeperSetup {
 
     public static ShellResult config(Params params) {
-        log.info("ZookeeperSetup config");
+        log.info("Setting zookeeper config");
         ZookeeperParams zookeeperParams = (ZookeeperParams) params;
 
         String confDir = zookeeperParams.confDir();