Add options for disabling default instrumentation (#664)

* Add options for disabling default instrumentation

We previously had the undocumented configuration option
`ignoreInstrumentation` that didn't follow our previously set out naming
and behavior conventions for configuration options.

Split up this configuration option in separate configuration options for
the different instrumentations.

Closes #519

* Merge the instrumentHttp and https options

These two options basically do the same thing, instrument the http and
https modules, but there will most likely not be a scenario where one
would instrument http requests but not https requests or the other way
around. Merge both options into the `instrumentHttp` option so one
option disables both instrumentations.

* Fix config reading issue for instrument options

The client loaded the instrument options too early, before the config
was all read properly. This mean the `APPSIGNAL_INSTRUMENT_*` env vars
were ignored.

* Fix instrument options always returning true

The `|| true` fallback for the default value would always set the
options to true for the `initCorePlugins` function.

Allow the value to be `undefined`, which is necessary because of the
`Partial<AppsignalOptions>` argument to the client constructor.

We only check if the `instrumentHttp` option value is `false` and not if
it's true (the default). If a user explicitly sets it to `false` the
instrumentation will not be enabled.

packages/nodejs/.changesets/add-config-options-for-disabling-default-instrumentation.md

@@ -0,0 +1,23 @@
+---
+bump: "patch"
+type: "add"
+---
+
+Add config options for disabling default instrumentation like HTTP, HTTPS, PostgreSQL (pg package) and Redis (node-redis package).
+
+The following configuration options have been added:
+
+- `instrumentHttp`
+- `instrumentPg`
+- `instrumentRedis`
+
+By default these configuration options are set to `true`, which means the instrumentation is active by default. If you want to disable one of these instrumentations, configure it by setting the configuration option to `false`.
+
+```js
+// appsignal.js
+// Brief example, see our docs for a full example
+
+const appsignal = new Appsignal({
+  instrumentRedis: false // Disables the node-redis package instrumentation
+});
+```

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

@@ -9,12 +9,26 @@ describe("Bootstrap", () => {
     } as unknown) as Instrumentation
 
     it("bootstraps the core instumentation plugins", () => {
-      initCorePlugins(mock, { ignoreInstrumentation: undefined })
+      initCorePlugins(mock, {
+        instrumentationConfig: {
+          http: true,
+          https: true,
+          pg: true,
+          redis: true
+        }
+      })
       expect(mock.load).toHaveBeenCalledTimes(4)
     })
 
     it("can ignore a core instumentation plugin", () => {
-      initCorePlugins(mock, { ignoreInstrumentation: ["http"] })
+      initCorePlugins(mock, {
+        instrumentationConfig: {
+          http: false,
+          https: true,
+          pg: undefined,
+          redis: true
+        }
+      })
 
       expect(mock.load).toHaveBeenCalledTimes(3)
       expect((mock.load as any).mock.calls.map((m: any) => m[0])).not.toContain(

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

@@ -27,6 +27,9 @@ describe("Configuration", () => {
     ignoreActions: [],
     ignoreErrors: [],
     ignoreNamespaces: [],
+    instrumentHttp: true,
+    instrumentPg: true,
+    instrumentRedis: true,
     log: "file",
     requestHeaders: [
       "accept",

packages/nodejs/src/bootstrap.ts

@@ -15,14 +15,16 @@ import * as gcProbe from "./probes/v8"
  */
 export function initCorePlugins(
   instrumentation: Instrumentation,
-  { ignoreInstrumentation }: { ignoreInstrumentation?: string[] }
+  {
+    instrumentationConfig
+  }: { instrumentationConfig?: { [key: string]: boolean | undefined } }
 ) {
   let plugins: any[] = [httpPlugin, httpsPlugin, redisPlugin, pgPlugin]
 
-  // cull ignored plugins
-  if (ignoreInstrumentation && Array.isArray(ignoreInstrumentation)) {
+  // Do not load disabled instrumentation plugins
+  if (instrumentationConfig) {
     plugins = plugins.filter(
-      p => !ignoreInstrumentation.includes(p.PLUGIN_NAME)
+      plugin => instrumentationConfig[plugin.PLUGIN_NAME] !== false
     )
   }
 

packages/nodejs/src/client.ts

@@ -55,8 +55,6 @@ export class BaseClient implements Client {
    * Creates a new instance of the `Appsignal` object
    */
   constructor(options: Partial<AppsignalOptions> = {}) {
-    const { ignoreInstrumentation } = options
-
     this.config = new Configuration(options)
     this.extension = new Extension()
     this.logger = this.setUpLogger()
@@ -73,7 +71,15 @@ export class BaseClient implements Client {
 
     this.instrumentation = new Instrumentation(this.tracer(), this.metrics())
 
-    initCorePlugins(this.instrumentation, { ignoreInstrumentation })
+    const { instrumentRedis, instrumentHttp, instrumentPg } = this.config.data
+    initCorePlugins(this.instrumentation, {
+      instrumentationConfig: {
+        http: instrumentHttp,
+        https: instrumentHttp,
+        pg: instrumentPg,
+        redis: instrumentRedis
+      }
+    })
     initCoreProbes(this.metrics())
   }
 

packages/nodejs/src/config.ts

@@ -127,6 +127,9 @@ export class Configuration {
       ignoreActions: [],
       ignoreErrors: [],
       ignoreNamespaces: [],
+      instrumentHttp: true,
+      instrumentPg: true,
+      instrumentRedis: true,
       log: "file",
       requestHeaders: [
         "accept",

packages/nodejs/src/config/configmap.ts

@@ -17,6 +17,9 @@ export const ENV_TO_KEY_MAPPING: { [key: string]: string } = {
   APPSIGNAL_IGNORE_ACTIONS: "ignoreActions",
   APPSIGNAL_IGNORE_ERRORS: "ignoreErrors",
   APPSIGNAL_IGNORE_NAMESPACES: "ignoreNamespaces",
+  APPSIGNAL_INSTRUMENT_HTTP: "instrumentHttp",
+  APPSIGNAL_INSTRUMENT_PG: "instrumentPg",
+  APPSIGNAL_INSTRUMENT_REDIS: "instrumentRedis",
   APPSIGNAL_LOG: "log",
   APPSIGNAL_LOG_LEVEL: "logLevel",
   APPSIGNAL_LOG_PATH: "logPath",
@@ -80,6 +83,9 @@ export const JS_TO_RUBY_MAPPING: { [key: string]: string } = {
   ignoreActions: "ignore_actions",
   ignoreErrors: "ignore_errors",
   ignoreNamespaces: "ignore_namespaces",
+  instrumentHttp: "instrument_http",
+  instrumentPg: "instrument_pg",
+  instrumentRedis: "instrument_redis",
   log: "log",
   logLevel: "log_level",
   logPath: "log_path",

packages/nodejs/src/interfaces/options.ts

@@ -18,6 +18,9 @@ export interface AppsignalOptions {
   ignoreErrors: string[]
   ignoreInstrumentation: string[]
   ignoreNamespaces: string[]
+  instrumentHttp: boolean
+  instrumentPg: boolean
+  instrumentRedis: boolean
   log: string
   logPath: string
   name: string