Remove default logPath option value

Remove the default value of the logPath option and only fall back on
the system tmp directory if no value is set. This way it matches its
behavior with the other integrations.

This will also make it easier to distinguish between user configured
paths and fallback paths in the future when we check if the fallback
directory is writable or not.

Fixes #510

packages/nodejs/src/__tests__/config.test.ts

@@ -28,7 +28,6 @@ describe("Configuration", () => {
     ignoreErrors: [],
     ignoreNamespaces: [],
     log: "file",
-    logPath: "/tmp",
     requestHeaders: [
       "accept",
       "accept-charset",
@@ -170,37 +169,34 @@ describe("Configuration", () => {
     })
 
     describe("with logPath option", () => {
-      beforeEach(() => {
+      it("uses the configured path", () => {
         config = new Configuration({ logPath: "/other_path" })
-      })
-
-      it("uses the overwritten path", () => {
         const fsAccessSpy = jest
           .spyOn(fs, "accessSync")
           .mockImplementation(() => {})
 
         jest.spyOn(fs, "accessSync").mockImplementation(() => {})
         expect(config.logFilePath).toEqual("/other_path/appsignal.log")
       })
-    })
 
-    describe("when logPath is not writtable", () => {
-      it("switches it to default tmp dir", () => {
-        const fsAccessSpy = jest
-          .spyOn(fs, "accessSync")
-          .mockImplementation(() => {
-            throw "Error"
-          })
-        const warnMock = jest
-          .spyOn(console, "warn")
-          .mockImplementation(() => {})
-
-        config = new Configuration({ logPath: "/foo_dir" })
-
-        expect(warnMock).toBeCalledWith(
-          `Unable to log to '/foo_dir'. Logging to '/tmp' instead. Please check the permissions for the configured 'logPath' directory`
-        )
-        expect(config.logFilePath).toEqual("/tmp/appsignal.log")
+      describe("when the logPath directory can't be written to", () => {
+        it("uses the system tmp dir", () => {
+          const fsAccessSpy = jest
+            .spyOn(fs, "accessSync")
+            .mockImplementation(path => {
+              throw "Error"
+            })
+          const warnMock = jest
+            .spyOn(console, "warn")
+            .mockImplementation(() => {})
+
+          config = new Configuration({ logPath: "/foo_dir" })
+
+          expect(warnMock).toHaveBeenLastCalledWith(
+            `Unable to log to '/foo_dir'. Logging to '/tmp' instead. Please check the permissions of the 'logPath' directory.`
+          )
+          expect(config.logFilePath).toEqual("/tmp/appsignal.log")
+        })
       })
     })
 
@@ -215,7 +211,7 @@ describe("Configuration", () => {
           .mockImplementation(() => {})
         config = new Configuration({ logPath: "/other_path/foo.log" })
 
-        expect(warnMock).toBeCalledWith(
+        expect(warnMock).toHaveBeenLastCalledWith(
           "DEPRECATED: File names are no longer supported in the 'logPath' config option. Changing the filename to 'appsignal.log'"
         )
         // Test backwards compatibility with previous behaviour

packages/nodejs/src/__tests__/diagnose.test.ts

@@ -23,7 +23,6 @@ describe("DiagnoseTool", () => {
 
     expect(output.config.options).toHaveProperty("debug")
     expect(output.config.options).toHaveProperty("log")
-    expect(output.config.options).toHaveProperty("logPath")
     expect(output.config.options).toHaveProperty("caFilePath")
     expect(output.config.options).toHaveProperty("endpoint")
     expect(output.config.options).toHaveProperty("environment")

packages/nodejs/src/config.ts

@@ -57,27 +57,27 @@ export class Configuration {
   }
 
   public get logFilePath(): string {
-    let logPath = this.data["logPath"]!
-
-    if (path.extname(logPath) != "") {
+    const filename = "appsignal.log"
+    let logPath = this.data["logPath"]
+    if (logPath && path.extname(logPath) != "") {
       console.warn(
         "DEPRECATED: File names are no longer supported in the 'logPath' config option. Changing the filename to 'appsignal.log'"
       )
 
       logPath = path.dirname(logPath)
     }
 
-    if (!isWritable(logPath)) {
-      const newLogPath = this._tmpdir()
-
-      console.warn(
-        `Unable to log to '${logPath}'. Logging to '${newLogPath}' instead. Please check the permissions for the configured 'logPath' directory`
-      )
-
-      logPath = newLogPath
+    if (logPath && isWritable(logPath)) {
+      return path.join(logPath, filename)
+    } else {
+      const tmpDir = this._tmpdir()
+      if (logPath) {
+        console.warn(
+          `Unable to log to '${logPath}'. Logging to '${tmpDir}' instead. Please check the permissions of the 'logPath' directory.`
+        )
+      }
+      return path.join(tmpDir, filename)
     }
-
-    return path.join(logPath, "appsignal.log")
   }
 
   /**
@@ -118,7 +118,6 @@ export class Configuration {
       ignoreErrors: [],
       ignoreNamespaces: [],
       log: "file",
-      logPath: this._tmpdir(),
       requestHeaders: [
         "accept",
         "accept-charset",