[CIT-564] Improve error reporting on software update (#443)

* [CIT-564] Sketch plugin command logging

* [CIT-564] Move the logging code into the `logged_command` module

* [CIT-564] Test error cases

* [CIT-564] Log all the actions of a software operation

* [CIT-564] Clean log output

* [CIT-564] Log files are managed by the agent

* [CIT-564] Redirect the cloud user to the device log file on error

* [CIT-564] Remove dead code

* [CIT-564] Remove out dated logs

* [CIT-564] Address review comments

* [CIT-564] Cargo fmt

* [CIT-564] Make the log independent of the system

On my box, an exit status is displayed `exit code: 0`
while on some others, the same exit status is displayed as `exit status: 0`

* [CIT-564] Add comments

Co-authored-by: Wenzek <diw@softwareag.com>

12 files changed, 621 insertions, 128 deletions

diff --git a/sm/json_sm/src/lib.rs b/sm/json_sm/src/lib.rs
index a3db2f03..80ac6a32 100644
--- a/sm/json_sm/src/lib.rs
+++ b/sm/json_sm/src/lib.rs
@@ -655,6 +655,7 @@ mod tests {
         let request = SoftwareUpdateRequest::new_with_id("123");
         let mut response = SoftwareUpdateResponse::new(&request);
 
+        response.set_error("2 errors: fail to install [ collectd ] fail to remove [ mongodb ]");
         response.add_errors(
             "debian",
             vec![SoftwareError::Install {
diff --git a/sm/json_sm/src/messages.rs b/sm/json_sm/src/messages.rs
index e23303d0..cac82da0 100644
--- a/sm/json_sm/src/messages.rs
+++ b/sm/json_sm/src/messages.rs
@@ -233,9 +233,6 @@ impl SoftwareListResponse {
 pub struct SoftwareUpdateResponse {
     #[serde(flatten)]
     response: SoftwareRequestResponse,
-
-    #[serde(skip)]
-    errors: Vec<SoftwareError>,
 }
 
 impl<'a> Jsonify<'a> for SoftwareUpdateResponse {}
@@ -244,7 +241,6 @@ impl SoftwareUpdateResponse {
     pub fn new(req: &SoftwareUpdateRequest) -> SoftwareUpdateResponse {
         SoftwareUpdateResponse {
             response: SoftwareRequestResponse::new(&req.id, SoftwareOperationStatus::Executing),
-            errors: vec![],
         }
     }
 
@@ -263,8 +259,6 @@ impl SoftwareUpdateResponse {
     }
 
     pub fn add_errors(&mut self, plugin_type: &str, errors: Vec<SoftwareError>) {
-        self.errors.append(&mut errors.clone());
-        self.response.set_reason_errors(&self.errors);
         self.response.add_errors(
             plugin_type.to_string(),
             errors
@@ -274,6 +268,11 @@ impl SoftwareUpdateResponse {
         );
     }
 
+    pub fn set_error(&mut self, reason: &str) {
+        self.response.status = SoftwareOperationStatus::Failed;
+        self.response.reason = Some(reason.into());
+    }
+
     pub fn id(&self) -> &str {
         &self.response.id
     }
@@ -366,57 +365,6 @@ impl SoftwareRequestResponse {
         }
     }
 
-    pub fn set_reason_errors(&mut self, errors: &[SoftwareError]) {
-        let mut count = 0;
-        let mut failed_package = String::new();
-        let mut failed_install = String::new();
-        let mut failed_remove = String::new();
-
-        for error in errors.iter() {
-            count += 1;
-            match error {
-                SoftwareError::Install { module, .. } => {
-                    failed_install.push(' ');
-                    failed_install.push_str(&module.name);
-                }
-                SoftwareError::Remove { module, .. } => {
-                    failed_remove.push(' ');
-                    failed_remove.push_str(&module.name);
-                }
-                SoftwareError::Prepare { software_type, .. } => {
-                    failed_package.push(' ');
-                    failed_package.push_str(software_type);
-                }
-                SoftwareError::Finalize { software_type, .. } => {
-                    failed_package.push(' ');
-                    failed_package.push_str(software_type);
-                }
-                _ => {}
-            }
-        }
-
-        let mut reason = format!("{} errors:", count);
-
-        if !failed_package.is_empty() {
-            reason.push_str(" fail to update [");
-            reason.push_str(&failed_package);
-            reason.push_str(" ]");
-        }
-        if !failed_install.is_empty() {
-            reason.push_str(" fail to install [");
-            reason.push_str(&failed_install);
-            reason.push_str(" ]");
-        }
-        if !failed_remove.is_empty() {
-            reason.push_str(" fail to remove [");
-            reason.push_str(&failed_remove);
-            reason.push_str(" ]");
-        }
-
-        self.status = SoftwareOperationStatus::Failed;
-        self.reason = Some(reason);
-    }
-
     pub fn add_errors(&mut self, plugin_type: SoftwareType, modules: Vec<SoftwareModuleItem>) {
         self.status = SoftwareOperationStatus::Failed;
 
diff --git a/sm/plugin_sm/src/lib.rs b/sm/plugin_sm/src/lib.rs
index 787c9905..79800999 100644
--- a/sm/plugin_sm/src/lib.rs
+++ b/sm/plugin_sm/src/lib.rs
@@ -1,2 +1,4 @@
+pub mod log_file;
+pub mod logged_command;
 pub mod plugin;
 pub mod plugin_manager;
diff --git a/sm/plugin_sm/src/log_file.rs b/sm/plugin_sm/src/log_file.rs
new file mode 100644
index 00000000..86d50a7c
--- /dev/null
+++ b/sm/plugin_sm/src/log_file.rs
@@ -0,0 +1,25 @@
+use std::path::PathBuf;
+use tokio::fs::File;
+use tokio::io::BufWriter;
+
+pub struct LogFile {
+    path: PathBuf,
+    buffer: BufWriter<File>,
+}
+
+impl LogFile {
+    pub async fn try_new(path: PathBuf) -> Result<LogFile, std::io::Error> {
+        let file = File::create(path.clone()).await?;
+        let buffer = BufWriter::new(file);
+
+        Ok(LogFile { path, buffer })
+    }
+
+    pub fn path(&self) -> &str {
+        &self.path.to_str().unwrap_or("/var/log/tedge/agent")
+    }
+
+    pub fn buffer(&mut self) -> &mut BufWriter<File> {
+        &mut self.buffer
+    }
+}
diff --git a/sm/plugin_sm/src/logged_command.rs b/sm/plugin_sm/src/logged_command.rs
new file mode 100644
index 00000000..72cb2b01
--- /dev/null
+++ b/sm/plugin_sm/src/logged_command.rs
@@ -0,0 +1,204 @@
+use std::ffi::OsStr;
+use std::process::{Output, Stdio};
+use tokio::fs::File;
+use tokio::io::{AsyncWriteExt, BufWriter};
+use tokio::process::Command;
+
+/// A command which execution is logged.
+///
+/// This struct wraps the main command with a nice representation of that command.
+/// This `command_line` field is only required because the
+/// [`Command::get_program()`](https://doc.rust-lang.org/std/process/struct.Command.html#method.get_program)
+/// and
+/// [`Command::get_args()`](https://doc.rust-lang.org/std/process/struct.Command.html#method.get_args)
+/// are nightly-only experimental APIs.
+pub struct LoggedCommand {
+    command_line: String,
+    command: Command,
+}
+
+impl std::fmt::Display for LoggedCommand {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.command_line.fmt(f)
+    }
+}
+
+impl LoggedCommand {
+    pub fn new(program: impl AsRef<OsStr>) -> LoggedCommand {
+        let command_line = match program.as_ref().to_str() {
+            None => format!("{:?}", program.as_ref()),
+            Some(cmd) => cmd.to_string(),
+        };
+
+        let mut command = Command::new(program);
+        command
+            .current_dir("/tmp")
+            .stdin(Stdio::null())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped());
+
+        LoggedCommand {
+            command_line,
+            command,
+        }
+    }
+
+    pub fn arg(&mut self, arg: impl AsRef<OsStr>) -> &mut LoggedCommand {
+        // The arguments are displayed as debug, to be properly quoted and distinguished from each other.
+        self.command_line.push_str(&format!(" {:?}", arg.as_ref()));
+        self.command.arg(arg);
+        self
+    }
+
+    /// Execute the command and log its exit status, stdout and stderr
+    ///
+    /// If the command has been executed the outcome is returned (successful or not).
+    /// If the command fails to execute (say not found or not executable) an `std::io::Error` is returned.
+    ///
+    /// If the function fails to log the execution of the command,
+    /// this is logged with `log::error!` without changing the return value.
+    pub async fn execute(mut self, logger: &mut BufWriter<File>) -> Result<Output, std::io::Error> {
+        let outcome = self.command.output().await;
+
+        if let Err(err) = LoggedCommand::log_outcome(&self.command_line, &outcome, logger).await {
+            tracing::log::error!("Fail to log the command execution: {}", err);
+        }
+
+        outcome
+    }
+
+    async fn log_outcome(
+        command_line: &str,
+        result: &Result<Output, std::io::Error>,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), std::io::Error> {
+        logger
+            .write_all(format!("----- $ {}\n", command_line).as_bytes())
+            .await?;
+
+        match result.as_ref() {
+            Ok(output) => {
+                match &output.status.code() {
+                    None => logger.write_all(b"exit status: unknown\n\n").await?,
+                    Some(code) => {
+                        logger
+                            .write_all(format!("exit status: {}\n\n", code).as_bytes())
+                            .await?
+                    }
+                };
+                logger.write_all(b"stdout <<EOF\n").await?;
+                logger.write_all(&output.stdout).await?;
+                logger.write_all(b"EOF\n\n").await?;
+                logger.write_all(b"stderr <<EOF\n").await?;
+                logger.write_all(&output.stderr).await?;
+                logger.write_all(b"EOF\n").await?;
+            }
+            Err(err) => {
+                logger
+                    .write_all(format!("error: {}\n", &err).as_bytes())
+                    .await?;
+            }
+        }
+
+        logger.flush().await?;
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tempfile::*;
+    use tokio::fs::File;
+
+    #[tokio::test]
+    async fn on_execute_are_logged_command_line_exit_status_stdout_and_stderr(
+    ) -> Result<(), anyhow::Error> {
+        // Prepare a log file
+        let tmp_dir = TempDir::new()?;
+        let log_file_path = tmp_dir.path().join("operation.log");
+        let log_file = File::create(log_file_path.clone()).await?;
+        let mut logger = BufWriter::new(log_file);
+
+        // Prepare a command
+        let mut command = LoggedCommand::new("echo");
+        command.arg("Hello").arg("World!");
+
+        // Execute the command with logging
+        let _ = command.execute(&mut logger).await;
+
+        let log_content = String::from_utf8(std::fs::read(&log_file_path)?)?;
+        assert_eq!(
+            log_content,
+            r#"----- $ echo "Hello" "World!"
+exit status: 0
+
+stdout <<EOF
+Hello World!
+EOF
+
+stderr <<EOF
+EOF
+"#
+        );
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn on_execute_with_error_stderr_is_logged() -> Result<(), anyhow::Error> {
+        // Prepare a log file
+        let tmp_dir = TempDir::new()?;
+        let log_file_path = tmp_dir.path().join("operation.log");
+        let log_file = File::create(log_file_path.clone()).await?;
+        let mut logger = BufWriter::new(log_file);
+
+        // Prepare a command that triggers some content on stderr
+        let mut command = LoggedCommand::new("ls");
+        command.arg("dummy-file");
+
+        // Execute the command with logging
+        let _ = command.execute(&mut logger).await;
+
+        // On expect the errors to be logged
+        let log_content = String::from_utf8(std::fs::read(&log_file_path)?)?;
+        assert_eq!(
+            log_content,
+            r#"----- $ ls "dummy-file"
+exit status: 2
+
+stdout <<EOF
+EOF
+
+stderr <<EOF
+ls: cannot access 'dummy-file': No such file or directory
+EOF
+"#
+        );
+        Ok(())
+    }
+
+    #[tokio::test]
+    async fn on_execution_error_are_logged_command_line_and_error() -> Result<(), anyhow::Error> {
+        // Prepare a log file
+        let tmp_dir = TempDir::new()?;
+        let log_file_path = tmp_dir.path().join("operation.log");
+        let log_file = File::create(log_file_path.clone()).await?;
+        let mut logger = BufWriter::new(log_file);
+
+        // Prepare a command that cannot be executed
+        let command = LoggedCommand::new("dummy-command");
+
+        // Execute the command with logging
+        let _ = command.execute(&mut logger).await;
+
+        // The fact that the command cannot be executed must be logged
+        let log_content = String::from_utf8(std::fs::read(&log_file_path)?)?;
+        assert_eq!(
+            log_content,
+            r#"----- $ dummy-command
+error: No such file or directory (os error 2)
+"#
+        );
+        Ok(())
+    }
+}
diff --git a/sm/plugin_sm/src/plugin.rs b/sm/plugin_sm/src/plugin.rs
index 115bd121..0e3eb31d 100644
--- a/sm/plugin_sm/src/plugin.rs
+++ b/sm/plugin_sm/src/plugin.rs
@@ -1,43 +1,64 @@
+use crate::logged_command::LoggedCommand;
 use async_trait::async_trait;
 use json_sm::*;
-use std::{
-    iter::Iterator,
-    path::PathBuf,
-    process::{Output, Stdio},
-};
-use tokio::process::Command;
+use std::{iter::Iterator, path::PathBuf, process::Output};
+use tokio::fs::File;
+use tokio::io::BufWriter;
 
 #[async_trait]
 pub trait Plugin {
-    async fn prepare(&self) -> Result<(), SoftwareError>;
-    async fn install(&self, module: &SoftwareModule) -> Result<(), SoftwareError>;
-    async fn remove(&self, module: &SoftwareModule) -> Result<(), SoftwareError>;
-    async fn finalize(&self) -> Result<(), SoftwareError>;
-    async fn list(&self) -> Result<Vec<SoftwareModule>, SoftwareError>;
-    async fn version(&self, module: &SoftwareModule) -> Result<Option<String>, SoftwareError>;
-
-    async fn apply(&self, update: &SoftwareModuleUpdate) -> Result<(), SoftwareError> {
+    async fn prepare(&self, logger: &mut BufWriter<File>) -> Result<(), SoftwareError>;
+    async fn install(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), SoftwareError>;
+    async fn remove(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), SoftwareError>;
+    async fn finalize(&self, logger: &mut BufWriter<File>) -> Result<(), SoftwareError>;
+    async fn list(
+        &self,
+        logger: &mut BufWriter<File>,
+    ) -> Result<Vec<SoftwareModule>, SoftwareError>;
+    async fn version(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<Option<String>, SoftwareError>;
+
+    async fn apply(
+        &self,
+        update: &SoftwareModuleUpdate,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), SoftwareError> {
         match update {
-            SoftwareModuleUpdate::Install { module } => self.install(module).await,
-            SoftwareModuleUpdate::Remove { module } => self.remove(module).await,
+            SoftwareModuleUpdate::Install { module } => self.install(module, logger).await,
+            SoftwareModuleUpdate::Remove { module } => self.remove(module, logger).await,
         }
     }
 
-    async fn apply_all(&self, updates: Vec<SoftwareModuleUpdate>) -> Vec<SoftwareError> {
+    async fn apply_all(
+        &self,
+        updates: Vec<SoftwareModuleUpdate>,
+        logger: &mut BufWriter<File>,
+    ) -> Vec<SoftwareError> {
         let mut failed_updates = Vec::new();
 
-        if let Err(prepare_error) = self.prepare().await {
+        if let Err(prepare_error) = self.prepare(logger).await {
             failed_updates.push(prepare_error);
             return failed_updates;
         }
 
         for update in updates.iter() {
-            if let Err(error) = self.apply(update).await {
+            if let Err(error) = self.apply(update, logger).await {
                 failed_updates.push(error);
             };
         }
 
-        if let Err(finalize_error) = self.finalize().await {
+        if let Err(finalize_error) = self.finalize(logger).await {
             failed_updates.push(finalize_error);
         }
 
@@ -65,13 +86,13 @@ impl ExternalPluginCommand {
         &self,
         action: &str,
         maybe_module: Option<&SoftwareModule>,
-    ) -> Result<Command, SoftwareError> {
+    ) -> Result<LoggedCommand, SoftwareError> {
         let mut command = if let Some(sudo) = &self.sudo {
-            let mut command = Command::new(&sudo);
+            let mut command = LoggedCommand::new(sudo);
             command.arg(&self.path);
             command
         } else {
-            Command::new(&self.path)
+            LoggedCommand::new(&self.path)
         };
         command.arg(action);
 
@@ -84,18 +105,16 @@ impl ExternalPluginCommand {
             }
         }
 
-        command
-            .current_dir("/tmp")
-            .stdin(Stdio::null())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped());
-
         Ok(command)
     }
 
-    pub async fn execute(&self, mut command: Command) -> Result<Output, SoftwareError> {
+    pub async fn execute(
+        &self,
+        command: LoggedCommand,
+        logger: &mut BufWriter<File>,
+    ) -> Result<Output, SoftwareError> {
         let output = command
-            .output()
+            .execute(logger)
             .await
             .map_err(|err| self.plugin_error(err))?;
         Ok(output)
@@ -135,9 +154,9 @@ const VERSION: &str = "version";
 
 #[async_trait]
 impl Plugin for ExternalPluginCommand {
-    async fn prepare(&self) -> Result<(), SoftwareError> {
+    async fn prepare(&self, logger: &mut BufWriter<File>) -> Result<(), SoftwareError> {
         let command = self.command(PREPARE, None)?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             Ok(())
@@ -149,9 +168,13 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 
-    async fn install(&self, module: &SoftwareModule) -> Result<(), SoftwareError> {
+    async fn install(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), SoftwareError> {
         let command = self.command(INSTALL, Some(module))?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             Ok(())
@@ -163,9 +186,13 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 
-    async fn remove(&self, module: &SoftwareModule) -> Result<(), SoftwareError> {
+    async fn remove(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<(), SoftwareError> {
         let command = self.command(REMOVE, Some(module))?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             Ok(())
@@ -177,9 +204,9 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 
-    async fn finalize(&self) -> Result<(), SoftwareError> {
+    async fn finalize(&self, logger: &mut BufWriter<File>) -> Result<(), SoftwareError> {
         let command = self.command(FINALIZE, None)?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             Ok(())
@@ -191,9 +218,12 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 
-    async fn list(&self) -> Result<Vec<SoftwareModule>, SoftwareError> {
+    async fn list(
+        &self,
+        logger: &mut BufWriter<File>,
+    ) -> Result<Vec<SoftwareModule>, SoftwareError> {
         let command = self.command(LIST, None)?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             let mut software_list = Vec::new();
@@ -219,9 +249,13 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 
-    async fn version(&self, module: &SoftwareModule) -> Result<Option<String>, SoftwareError> {
+    async fn version(
+        &self,
+        module: &SoftwareModule,
+        logger: &mut BufWriter<File>,
+    ) -> Result<Option<String>, SoftwareError> {
         let command = self.command(VERSION, Some(module))?;
-        let output = self.execute(command).await?;
+        let output = self.execute(command, logger).await?;
 
         if output.status.success() {
             let version = String::from(self.content(output.stdout)?.trim());
diff --git a/sm/plugin_sm/src/plugin_manager.rs b/sm/plugin_sm/src/plugin_manager.rs
index 84cfdc55..f2ce49cb 100644
--- a/sm/plugin_sm/src/plugin_manager.rs
+++ b/sm/plugin_sm/src/plugin_manager.rs
@@ -1,3 +1,4 @@
+use crate::log_file::LogFile;
 use crate::plugin::*;
 use json_sm::*;
 use std::{
@@ -5,7 +6,7 @@ use std::{
     fs,
     io::{self, ErrorKind},
     path::PathBuf,
-    process::Command,
+    process::{Command, Stdio},
 };
 use tedge_utils::paths::pathbuf_to_string;
 use tracing::error;
@@ -102,7 +103,12 @@ impl ExternalPlugins {
             let entry = maybe_entry?;
             let path = entry.path();
             if path.is_file() {
-                match Command::new(&path).arg("list").status() {
+                match Command::new(&path)
+                    .arg("list")
+                    .stdout(Stdio::null())
+                    .stderr(Stdio::null())
+                    .status()
+                {
                     Ok(code) if code.success() => {}
 
                     // If the file is not executable or returned non 0 status code we assume it is not a valid and skip further processing.
@@ -150,30 +156,44 @@ impl ExternalPlugins {
         self.plugin_map.is_empty()
     }
 
-    pub async fn list(&self, request: &SoftwareListRequest) -> SoftwareListResponse {
+    pub async fn list(
+        &self,
+        request: &SoftwareListRequest,
+        mut log_file: LogFile,
+    ) -> SoftwareListResponse {
         let mut response = SoftwareListResponse::new(request);
+        let mut logger = log_file.buffer();
+        let mut error_count = 0;
 
         for (software_type, plugin) in self.plugin_map.iter() {
-            match plugin.list().await {
-                Ok(software_list) => response.add_modules(software_type, software_list),
-                Err(err) => {
-                    // TODO fix the response format to handle an error per module type
-                    let reason = format!("{}", err);
-                    response.set_error(&reason);
-                    return response;
+            match plugin.list(&mut logger).await {
+                Ok(software_list) => response.add_modules(&software_type, software_list),
+                Err(_) => {
+                    error_count += 1;
                 }
             }
         }
+
+        if let Some(reason) = ExternalPlugins::error_message(log_file.path(), error_count) {
+            response.set_error(&reason);
+        }
+
         response
     }
 
-    pub async fn process(&self, request: &SoftwareUpdateRequest) -> SoftwareUpdateResponse {
+    pub async fn process(
+        &self,
+        request: &SoftwareUpdateRequest,
+        mut log_file: LogFile,
+    ) -> SoftwareUpdateResponse {
         let mut response = SoftwareUpdateResponse::new(request);
+        let mut logger = log_file.buffer();
+        let mut error_count = 0;
 
         for software_type in request.modules_types() {
             let errors = if let Some(plugin) = self.by_software_type(&software_type) {
                 let updates = request.updates_for(&software_type);
-                plugin.apply_all(updates).await
+                plugin.apply_all(updates, &mut logger).await
             } else {
                 vec![SoftwareError::UnknownSoftwareType {
                     software_type: software_type.clone(),
@@ -181,17 +201,38 @@ impl ExternalPlugins {
             };
 
             if !errors.is_empty() {
+                error_count += 1;
                 response.add_errors(&software_type, errors);
             }
         }
 
         for (software_type, plugin) in self.plugin_map.iter() {
-            match plugin.list().await {
+            match plugin.list(&mut logger).await {
                 Ok(software_list) => response.add_modules(software_type, software_list),
-                Err(err) => response.add_errors(software_type, vec![err]),
+                Err(err) => {
+                    error_count += 1;
+                    response.add_errors(software_type, vec![err])
+                }
             }
         }
 
+        if let Some(reason) = ExternalPlugins::error_message(log_file.path(), error_count) {
+            response.set_error(&reason);
+        }
+
         response
     }
+
+    fn error_message(log_file: &str, error_count: i32) -> Option<String> {
+        if error_count > 0 {
+            let reason = if error_count == 1 {
+                format!("1 error, see device log file {}", log_file)
+            } else {
+                format!("{} errors, see device log file {}", error_count, log_file)
+            };
+            Some(reason)
+        } else {
+            None
+        }
+    }
 }
diff --git a/sm/plugin_sm/tests/plugin.rs b/sm/plugin_sm/tests/plugin.rs
index 440b1175..04c1e508 100644
--- a/sm/plugin_sm/tests/plugin.rs
+++ b/sm/plugin_sm/tests/plugin.rs
@@ -4,13 +4,17 @@ mod tests {
     use json_sm::{SoftwareError, SoftwareModule};
     use plugin_sm::plugin::{ExternalPluginCommand, Plugin};
     use std::{fs, io::Write, path::PathBuf, str::FromStr};
+    use tokio::fs::File;
+    use tokio::io::BufWriter;
+
     #[tokio::test]
     async fn plugin_get_command_prepare() {
         // Prepare dummy plugin.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
 
         // Call dummy plugin via plugin api.
-        let res = plugin.prepare().await;
+        let mut logger = dev_null().await;
+        let res = plugin.prepare(&mut logger).await;
 
         // Expect to get Ok as plugin should exit with code 0.
         assert_eq!(res, Ok(()));
@@ -22,7 +26,8 @@ mod tests {
         let (plugin, _plugin_path) = get_dummy_plugin("test");
 
         // Call dummy plugin via plugin api.
-        let res = plugin.finalize().await;
+        let mut logger = dev_null().await;
+        let res = plugin.finalize(&mut logger).await;
 
         // Expect Ok as plugin should exit with code 0. If Ok, there is no more checks to be done.
         assert_eq!(res, Ok(()));
@@ -56,7 +61,8 @@ mod tests {
         let expected_response = vec![module];
 
         // Call plugin via API.
-        let res = plugin.list().await;
+        let mut logger = dev_null().await;
+        let res = plugin.list(&mut logger).await;
 
         // Expect Ok as plugin should exit with code 0.
         assert!(res.is_ok());
@@ -87,7 +93,8 @@ mod tests {
         };
 
         // Call plugin install via API.
-        let res = plugin.install(&module).await;
+        let mut logger = dev_null().await;
+        let res = plugin.install(&module, &mut logger).await;
 
         // Expect Ok as plugin should exit with code 0. If Ok, there is no response to assert.
         assert!(res.is_ok());
@@ -117,7 +124,8 @@ mod tests {
         };
 
         // Call plugin remove API .
-        let res = plugin.remove(&module).await;
+        let mut logger = dev_null().await;
+        let res = plugin.remove(&module, &mut logger).await;
 
         // Expect Ok as plugin should exit with code 0. If Ok, no more output to be validated.
         assert!(res.is_ok());
@@ -229,4 +237,9 @@ mod tests {
         }
         path
     }
+
+    async fn dev_null() -> BufWriter<File> {
+        let log_file = File::create("/dev/null").await.unwrap();
+        BufWriter::new(log_file)
+    }
 }
diff --git a/sm/tedge_agent/src/agent.rs b/sm/tedge_agent/src/agent.rs
index fc268cc7..0c438e59 100644
--- a/sm/tedge_agent/src/agent.rs
+++ b/sm/tedge_agent/src/agent.rs
@@ -3,7 +3,7 @@ use crate::{
     state::{AgentStateRepository, State, StateRepository},
 };
 
-use flockfile::{check_another_instance_is_not_running, Flockfile, FlockfileError};
+use flockfile::{check_another_instance_is_not_running, Flockfile};
 
 use json_sm::{
     software_filter_topic, Jsonify, SoftwareError, SoftwareListRequest, SoftwareListResponse,
@@ -15,6 +15,7 @@ use plugin_sm::plugin_manager::ExternalPlugins;
 use std::{path::PathBuf, sync::Arc};
 use tracing::{debug, error, info, instrument};
 
+use crate::operation_logs::{LogKind, OperationLogs};
 use tedge_config::{
     ConfigRepository, ConfigSettingAccessor, ConfigSettingAccessorStringExt, MqttPortSetting,
     SoftwarePluginDefaultSetting, TEdgeConfigLocation,
@@ -31,6 +32,7 @@ pub struct SmAgentConfig {
     pub response_topic_list: Topic,
     pub response_topic_update: Topic,