Merge pull request #484 from appsignal/add-diagnose-report-sources

Add sources to the diagnose report

packages/nodejs/.changesets/output-configuration-sources-in-diagnose-report.md

@@ -0,0 +1,5 @@
+---
+bump: "patch"
+---
+
+Add information about the sources of each configuration value in the output of the diagnose report.

packages/nodejs/.changesets/send-configuration-sources-in-diagnose-report.md

@@ -0,0 +1,5 @@
+---
+bump: "patch"
+---
+
+Include the sources of each configuration value in the diagnose report that is sent to AppSignal.com.

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

@@ -6,44 +6,62 @@ describe("Configuration", () => {
   const apiKey = "TEST_API_KEY"
 
   let config: Configuration
+  let initialEnv: { [key: string]: any }
 
-  beforeEach(() => {
-    jest.resetModules()
-    config = new Configuration({ name, apiKey })
-  })
+  function resetEnv() {
+    Object.keys(process.env).forEach(key => {
+      if (!initialEnv.hasOwnProperty(key)) {
+        delete process.env[key]
+      }
+    })
+  }
 
-  it("writes key and name to the environment", () => {
-    expect(process.env["_APPSIGNAL_APP_NAME"]).toEqual(name)
-    expect(process.env["_APPSIGNAL_PUSH_API_KEY"]).toEqual(apiKey)
+  beforeAll(() => {
+    initialEnv = Object.assign({}, process.env)
   })
 
-  it("writes private constants to the environment", () => {
-    expect(process.env["_APPSIGNAL_AGENT_PATH"]).toBeDefined()
-    expect(process.env["_APPSIGNAL_ENVIRONMENT"]).toBeDefined()
-    expect(process.env["_APPSIGNAL_PROCESS_NAME"]).toBeDefined()
-    expect(process.env["_APPSIGNAL_LANGUAGE_INTEGRATION_VERSION"]).toBeDefined()
-    expect(process.env["_APPSIGNAL_APP_PATH"]).toBeDefined()
+  afterAll(() => {
+    resetEnv()
   })
 
-  it("recognises a valid configuration", () => {
-    expect(config.isValid).toBeTruthy()
+  beforeEach(() => {
+    jest.resetModules()
+    resetEnv()
+    config = new Configuration({ name, apiKey })
   })
 
-  it("loads configuration from the environment", () => {
-    process.env["APPSIGNAL_ACTIVE"] = "true"
+  describe("Private environment variables", () => {
+    it("writes initial values to the environment", () => {
+      expect(process.env["_APPSIGNAL_APP_NAME"]).toEqual(name)
+      expect(process.env["_APPSIGNAL_PUSH_API_KEY"]).toEqual(apiKey)
+    })
 
-    config = new Configuration({ name, apiKey })
-    expect(process.env["_APPSIGNAL_ACTIVE"]).toBeTruthy()
-  })
+    it("writes private constants to the environment", () => {
+      expect(process.env["_APPSIGNAL_AGENT_PATH"]).toBeDefined()
+      expect(process.env["_APPSIGNAL_ENVIRONMENT"]).toBeDefined()
+      expect(process.env["_APPSIGNAL_PROCESS_NAME"]).toBeDefined()
+      expect(
+        process.env["_APPSIGNAL_LANGUAGE_INTEGRATION_VERSION"]
+      ).toBeDefined()
+      expect(process.env["_APPSIGNAL_APP_PATH"]).toBeDefined()
+    })
 
-  it("uses a default log file path", () => {
-    expect(process.env["_APPSIGNAL_LOG_FILE_PATH"]).toEqual(
-      "/tmp/appsignal.log"
-    )
+    it("loads configuration from the environment", () => {
+      process.env["APPSIGNAL_ACTIVE"] = "true"
+
+      config = new Configuration({ name, apiKey })
+      expect(process.env["_APPSIGNAL_ACTIVE"]).toBeTruthy()
+    })
+
+    it("uses a default log file path", () => {
+      expect(process.env["_APPSIGNAL_LOG_FILE_PATH"]).toEqual(
+        "/tmp/appsignal.log"
+      )
+    })
   })
 
   describe("Default options", () => {
-    const expectedConfig = {
+    const expectedDefaultConfig = {
       caFilePath: path.join(__dirname, "../../cert/cacert.pem"),
       debug: false,
       dnsServers: [],
@@ -64,25 +82,59 @@ describe("Configuration", () => {
       transactionDebugMode: false
     }
 
+    const expectedInitialConfig = {
+      name,
+      apiKey
+    }
+
+    const expectedConfig = {
+      ...expectedDefaultConfig,
+      ...expectedInitialConfig
+    }
+
     it("loads all default options when no options are overwritten", () => {
-      expect(config.data).toMatchObject(expectedConfig)
+      expect(config.data).toEqual(expectedConfig)
+
+      expect(config.sources.default).toEqual(expectedDefaultConfig)
+      expect(config.sources.initial).toEqual(expectedInitialConfig)
+      expect(config.sources.env).toEqual({})
     })
 
-    it("loads default options and overwrites the specified ones", () => {
-      const expectedOverwrittenConfig = {
-        ...expectedConfig,
+    describe("overwrites default values", () => {
+      const overwrittenConfig = {
         debug: true,
         enableStatsd: true
       }
 
-      config = new Configuration({
-        name,
-        apiKey,
-        debug: true,
-        enableStatsd: true
+      const expectedConfig = {
+        ...expectedDefaultConfig,
+        ...overwrittenConfig
+      }
+
+      it("with initial values", () => {
+        config = new Configuration({
+          ...overwrittenConfig
+        })
+
+        expect(config.data).toEqual(expectedConfig)
+
+        expect(config.sources.default).toEqual(expectedDefaultConfig)
+        expect(config.sources.initial).toEqual(overwrittenConfig)
+        expect(config.sources.env).toEqual({})
       })
 
-      expect(config.data).toMatchObject(expectedOverwrittenConfig)
+      it("with environment values", () => {
+        process.env["APPSIGNAL_DEBUG"] = "true"
+        process.env["APPSIGNAL_ENABLE_STATSD"] = "true"
+
+        config = new Configuration({})
+
+        expect(config.data).toEqual(expectedConfig)
+
+        expect(config.sources.default).toEqual(expectedDefaultConfig)
+        expect(config.sources.initial).toEqual({})
+        expect(config.sources.env).toEqual(overwrittenConfig)
+      })
     })
   })
 

packages/nodejs/src/cli/diagnose.ts

@@ -5,6 +5,7 @@ const path = require("path")
 const util = require("util")
 const readline = require("readline")
 import { HashMap } from "@appsignal/types"
+import { AppsignalOptions } from ".."
 
 export class Diagnose {
   #diagnose: typeof DiagnoseTool
@@ -149,12 +150,7 @@ export class Diagnose {
 
     this.print_newline()
 
-    console.log(`Configuration`)
-    Object.keys(data["config"]["options"])
-      .sort()
-      .forEach(key => {
-        console.log(`  ${key}: ${format_value(data["config"]["options"][key])}`)
-      })
+    this.printConfiguration(data["config"])
 
     this.print_newline()
 
@@ -349,6 +345,60 @@ export class Diagnose {
     }
   }
 
+  printConfiguration({
+    options,
+    sources
+  }: {
+    options: { [key: string]: any }
+    sources: { [source: string]: { [key: string]: any } }
+  }) {
+    console.log(`Configuration`)
+    Object.keys(options)
+      .sort()
+      .forEach(key => {
+        let keySources = this.configurationKeySources(key, sources)
+
+        if (Object.keys(keySources).length == 1) {
+          const source = Object.keys(keySources)[0]
+
+          let extra = ""
+          if (source !== "default") {
+            extra = ` (Loaded from: ${source})`
+          }
+
+          console.log(`  ${key}: ${format_value(options[key])}${extra}`)
+        } else {
+          console.log(`  ${key}: ${format_value(options[key])}`)
+          console.log(`    Sources:`)
+          const maxSourceLength = Object.keys(keySources)
+            // Adding one to account for the `:` after the source name.
+            .map(source => source.length + 1)
+            .reduce((max, source) => Math.max(max, source), 0)
+
+          Object.entries(keySources).forEach(([source, value]) => {
+            source = `${source}:`.padEnd(maxSourceLength, " ")
+            console.log(`      ${source} ${format_value(value)}`)
+          })
+        }
+      })
+  }
+
+  configurationKeySources(
+    key: string,
+    sources: { [source: string]: { [key: string]: any } }
+  ): { [source: string]: any } {
+    return Object.entries(sources).reduce(
+      (keySources, [source, sourceOptions]) => {
+        if (sourceOptions.hasOwnProperty(key)) {
+          return { ...keySources, [source]: sourceOptions[key] }
+        } else {
+          return keySources
+        }
+      },
+      {}
+    )
+  }
+
   print_newline() {
     console.log(``)
   }

packages/nodejs/src/config.ts

@@ -3,6 +3,7 @@ import path from "path"
 import { VERSION } from "./version"
 import { AppsignalOptions } from "./interfaces/options"
 import { ENV_TO_KEY_MAPPING, PRIVATE_ENV_MAPPING } from "./config/configmap"
+import { HashMap } from "@appsignal/types"
 
 /**
  * The AppSignal configuration object.
@@ -14,17 +15,20 @@ import { ENV_TO_KEY_MAPPING, PRIVATE_ENV_MAPPING } from "./config/configmap"
  */
 export class Configuration {
   data: Partial<AppsignalOptions>
+  sources: HashMap<Partial<AppsignalOptions>>
 
   constructor(options: Partial<AppsignalOptions>) {
-    writePrivateConstants()
-
-    this.data = {
-      ...this._defaultValues(),
-      ...this._loadFromEnvironment(),
-      ...options
+    this.sources = {
+      default: this._defaultValues(),
+      env: this._loadFromEnvironment(),
+      initial: options
     }
 
-    this._write(this.data)
+    this.data = Object.values(this.sources).reduce((data, options) => {
+      return { ...data, ...options }
+    }, {})
+
+    this.writePrivateConfig(this.data)
   }
 
   /**
@@ -41,6 +45,16 @@ export class Configuration {
     return (this.data.apiKey || "").trim() !== ""
   }
 
+  public get logFilePath(): string {
+    let logPath = this.data["logPath"]!
+
+    if (!logPath.endsWith("appsignal.log")) {
+      logPath = path.join(logPath, "appsignal.log")
+    }
+
+    return logPath
+  }
+
   /**
    * Explicit default configuration values
    *
@@ -98,13 +112,10 @@ export class Configuration {
    *
    * @private
    */
-  private _write(config: { [key: string]: any }) {
-    // First write log file path based on log path
-    if (config["logPath"].endsWith("appsignal.log")) {
-      config["logFilePath"] = config["logPath"]
-    } else {
-      config["logFilePath"] = path.join(config["logPath"], "appsignal.log")
-    }
+  private writePrivateConfig(config: { [key: string]: any }) {
+    this.writePrivateConstants()
+    process.env["_APPSIGNAL_LOG_FILE_PATH"] = this.logFilePath
+
     // write to a "private" environment variable if it exists in the
     // config structure
     Object.entries(PRIVATE_ENV_MAPPING).forEach(([k, v]) => {
@@ -117,26 +128,24 @@ export class Configuration {
 
       if (current) process.env[k] = String(current)
     })
-
-    return
   }
-}
 
-/**
- * Writes private environment variables that are not user configured,
- * and static in the lifecycle of the agent.
- *
- * @function
- * @private
- */
-function writePrivateConstants() {
-  const priv = {
-    // @TODO: is this path always correct?
-    _APPSIGNAL_AGENT_PATH: path.join(__dirname, "/../../nodejs-ext/ext"),
-    _APPSIGNAL_PROCESS_NAME: process.title,
-    _APPSIGNAL_LANGUAGE_INTEGRATION_VERSION: `nodejs-${VERSION}`,
-    _APPSIGNAL_APP_PATH: process.cwd()
-  }
+  /**
+   * Writes private environment variables that are not user configured,
+   * and static in the lifecycle of the agent.
+   *
+   * @function
+   * @private
+   */
+  private writePrivateConstants() {
+    const priv = {
+      // @TODO: is this path always correct?
+      _APPSIGNAL_AGENT_PATH: path.join(__dirname, "/../../nodejs-ext/ext"),
+      _APPSIGNAL_PROCESS_NAME: process.title,
+      _APPSIGNAL_LANGUAGE_INTEGRATION_VERSION: `nodejs-${VERSION}`,
+      _APPSIGNAL_APP_PATH: process.cwd()
+    }
 
-  Object.entries(priv).forEach(([k, v]) => (process.env[k] = v))
+    Object.entries(priv).forEach(([k, v]) => (process.env[k] = v))
+  }
 }

packages/nodejs/src/config/configmap.ts

@@ -48,7 +48,6 @@ export const PRIVATE_ENV_MAPPING: { [key: string]: string } = {
   _APPSIGNAL_IGNORE_ERRORS: "ignoreErrors",
   _APPSIGNAL_IGNORE_NAMESPACES: "ignoreNamespaces",
   _APPSIGNAL_LOG: "log",
-  _APPSIGNAL_LOG_FILE_PATH: "logFilePath",
   _APPSIGNAL_PUSH_API_ENDPOINT: "endpoint",
   _APPSIGNAL_PUSH_API_KEY: "apiKey",
   _APPSIGNAL_RUNNING_IN_CONTAINER: "runningInContainer",
@@ -79,7 +78,6 @@ export const JS_TO_RUBY_MAPPING: { [key: string]: string } = {
   ignoreNamespaces: "ignore_namespaces",
   log: "log",
   logPath: "log_path",
-  logFilePath: "log_file_path",
   name: "name",
   revision: "revision",
   runningInContainer: "running_in_container",