Return the full log path in the diagnose report (#402)

* Return the full log path in the diagnose report Currently, the diagnose command expects the logPath to include the filename for the `appsignal.log` file. To get the `log_dir_path`, the filename is stripped off. The `LogPath` is then used to return the full path to the file. In reality, it should be the other way around. The `LogPath` does not include the filename, so it can be used as the log_dir_path in the diagnose. Then, to get the full path to the `appsignal.log` file, this patch appends the filename to the log path. * Use internal value for logFilePath in diagnose Co-authored-by: Tom de Bruijn <[email protected]> * Explicitly cast logFilePath to <string> * Diagnose strips "/appsignal.log" from directory path, if present Co-authored-by: Tom de Bruijn <[email protected]>
appsignal · Jun 28, 2021 · a3bbb9f · a3bbb9f
1 parent 6dd70bb
commit a3bbb9f
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 4 deletions.
diff --git a/packages/nodejs/.changesets/fix-diagnose-log-paths.md b/packages/nodejs/.changesets/fix-diagnose-log-paths.md
@@ -0,0 +1,6 @@
+---
+bump: "patch"
+---
+
+Fix the log path reported when running the diagnose command to include the
+filename.
diff --git a/packages/nodejs/src/__tests__/diagnose.test.ts b/packages/nodejs/src/__tests__/diagnose.test.ts
@@ -26,4 +26,31 @@ describe("DiagnoseTool", () => {
 
     expect(output.process.uid).toEqual(process.getuid())
   })
+
+  it("returns the log_dir_path", () => {
+    expect(tool.generate().paths.log_dir_path.path).toEqual("/tmp")
+  })
+
+  it("returns the appsignal.log path", () => {
+    expect(tool.generate().paths["appsignal.log"].path).toEqual(
+      "/tmp/appsignal.log"
+    )
+  })
+
+  describe("when to log path is configured as a full path", () => {
+    beforeEach(() => {
+      process.env["APPSIGNAL_LOG_PATH"] = "/path/to/appsignal.log"
+      tool = new DiagnoseTool({})
+    })
+
+    it("returns the log_dir_path", () => {
+      expect(tool.generate().paths.log_dir_path.path).toEqual("/path/to")
+    })
+
+    it("returns the appsignal.log path", () => {
+      expect(tool.generate().paths["appsignal.log"].path).toEqual(
+        "/path/to/appsignal.log"
+      )
+    })
+  })
 })
diff --git a/packages/nodejs/src/diagnose.ts b/packages/nodejs/src/diagnose.ts
@@ -93,19 +93,19 @@ export class DiagnoseTool {
 
     // we want to fall over if this value isn't present
     // (it should be)
-    const logPath = this.#config.data.logPath!
+    const logFilePath = <string>this.#config.data.logFilePath!
 
     // add any paths we want to check to this object!
     const files = {
       working_dir: {
         path: process.cwd()
       },
       log_dir_path: {
-        path: logPath.replace("/appsignal.log", "")
+        path: this.#config.data.logPath!.replace("/appsignal.log", "")
       },
       "appsignal.log": {
-        path: logPath,
-        content: safeReadFromPath(logPath).split("\n")
+        path: logFilePath,
+        content: safeReadFromPath(logFilePath).split("\n")
       }
     }