Merge pull request #746 from appsignal/add-instrumentation-config-opt…

…ions

Add instrumentation config options

src/__tests__/client.test.ts

@@ -1,7 +1,9 @@
+import { InstrumentationTestRegistry } from "../../test/instrumentation_registry"
 import { Extension } from "../extension"
 import { Client } from "../client"
 import { Metrics } from "../metrics"
 import { NoopMetrics } from "../noops"
+import { Instrumentation } from "@opentelemetry/instrumentation"
 
 describe("Client", () => {
   const name = "TEST APP"
@@ -102,4 +104,83 @@ describe("Client", () => {
     const meter = client.metrics()
     expect(meter).toBeInstanceOf(Metrics)
   })
+
+  describe("Instrumentations", () => {
+    it("registers the default instrumentations", () => {
+      client = new Client({ ...DEFAULT_OPTS, active: true })
+      // Not testing against all of them or a fixed number so
+      // that we don't have to change these tests every time we
+      // add a new instrumentation.
+      const instrumentationNames =
+        InstrumentationTestRegistry.instrumentationNames()
+      expect(instrumentationNames.length).toBeGreaterThan(10)
+      expect(instrumentationNames).toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+    })
+
+    it("can disable all default instrumentations", () => {
+      client = new Client({
+        ...DEFAULT_OPTS,
+        active: true,
+        disableDefaultInstrumentations: true
+      })
+      const instrumentationNames =
+        InstrumentationTestRegistry.instrumentationNames()
+      expect(instrumentationNames).toEqual([])
+    })
+
+    it("can disable some default instrumentations", () => {
+      client = new Client({
+        ...DEFAULT_OPTS,
+        active: true,
+        disableDefaultInstrumentations: ["@opentelemetry/instrumentation-http"]
+      })
+      const instrumentationNames =
+        InstrumentationTestRegistry.instrumentationNames()
+      expect(instrumentationNames).not.toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+      expect(instrumentationNames.length).toBeGreaterThan(0)
+    })
+
+    it("can add additional instrumentations", () => {
+      class TestInstrumentation implements Instrumentation {
+        instrumentationName = "test/instrumentation"
+        instrumentationVersion = "0.0.0"
+        enable() {
+          // Does nothing
+        }
+        disable() {
+          // Does nothing
+        }
+        setTracerProvider(_tracerProvider: any) {
+          // Does nothing
+        }
+        setMeterProvider(_meterProvider: any) {
+          // Does nothing
+        }
+        setConfig(_config: any) {
+          // Does nothing
+        }
+        getConfig() {
+          return {}
+        }
+      }
+
+      client = new Client({
+        ...DEFAULT_OPTS,
+        active: true,
+        additionalInstrumentations: [new TestInstrumentation()]
+      })
+
+      const instrumentationNames =
+        InstrumentationTestRegistry.instrumentationNames()
+      expect(instrumentationNames).toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+      expect(instrumentationNames.length).toBeGreaterThan(10)
+      expect(instrumentationNames).toContain("test/instrumentation")
+    })
+  })
 })

src/__tests__/config.test.ts

@@ -15,6 +15,7 @@ describe("Configuration", () => {
     active: false,
     caFilePath: path.join(__dirname, "../../cert/cacert.pem"),
     debug: false,
+    disableDefaultInstrumentations: false,
     dnsServers: [],
     enableHostMetrics: true,
     enableMinutelyProbes: true,
@@ -27,9 +28,6 @@ describe("Configuration", () => {
     ignoreActions: [],
     ignoreErrors: [],
     ignoreNamespaces: [],
-    instrumentHttp: true,
-    instrumentPg: true,
-    instrumentRedis: true,
     log: "file",
     requestHeaders: [
       "accept",

src/__tests__/span_processor.test.ts

@@ -1,4 +1,4 @@
-import { SpanTestRegistry } from "../../test/registry"
+import { SpanTestRegistry } from "../../test/span_registry"
 import { Client } from "../client"
 import { SpanProcessor } from "../span_processor"
 import { Tracer, BasicTracerProvider } from "@opentelemetry/sdk-trace-base"

src/client.ts

@@ -9,6 +9,7 @@ import { demo } from "./demo"
 import { VERSION } from "./version"
 import { setParams, setSessionData } from "./helpers"
 
+import { Instrumentation } from "@opentelemetry/instrumentation"
 import {
   ExpressInstrumentation,
   ExpressLayerType
@@ -19,15 +20,42 @@ import { IORedisInstrumentation } from "@opentelemetry/instrumentation-ioredis"
 import { KoaInstrumentation } from "@opentelemetry/instrumentation-koa"
 import { MySQL2Instrumentation } from "@opentelemetry/instrumentation-mysql2"
 import { MySQLInstrumentation } from "@opentelemetry/instrumentation-mysql"
-import { NodeSDK } from "@opentelemetry/sdk-node"
+import { NodeSDK, NodeSDKConfiguration } from "@opentelemetry/sdk-node"
 import { PgInstrumentation } from "@opentelemetry/instrumentation-pg"
 import { PrismaInstrumentation } from "@prisma/instrumentation"
 import { RedisDbStatementSerializer } from "./instrumentation/redis/serializer"
 import { RedisInstrumentation as Redis4Instrumentation } from "@opentelemetry/instrumentation-redis-4"
 import { RedisInstrumentation } from "@opentelemetry/instrumentation-redis"
-import { SpanProcessor } from "./span_processor"
+import { SpanProcessor, TestModeSpanProcessor } from "./span_processor"
+
+const DefaultInstrumentations = {
+  "@opentelemetry/instrumentation-express": ExpressInstrumentation,
+  "@opentelemetry/instrumentation-graphql": GraphQLInstrumentation,
+  "@opentelemetry/instrumentation-http": HttpInstrumentation,
+  "@opentelemetry/instrumentation-ioredis": IORedisInstrumentation,
+  "@opentelemetry/instrumentation-koa": KoaInstrumentation,
+  "@opentelemetry/instrumentation-mysql2": MySQL2Instrumentation,
+  "@opentelemetry/instrumentation-mysql": MySQLInstrumentation,
+  "@opentelemetry/instrumentation-pg": PgInstrumentation,
+  "@opentelemetry/instrumentation-redis": RedisInstrumentation,
+  "@opentelemetry/instrumentation-redis-4": Redis4Instrumentation,
+  "@prisma/instrumentation": PrismaInstrumentation
+}
+
+export type DefaultInstrumentationName = keyof typeof DefaultInstrumentations
+
+type ConfigArg<T> = T extends new (...args: infer U) => unknown ? U[0] : never
+type DefaultInstrumentationsConfigMap = {
+  [Name in DefaultInstrumentationName]?: ConfigArg<
+    typeof DefaultInstrumentations[Name]
+  >
+}
+
+type AdditionalInstrumentationsOption = NodeSDKConfiguration["instrumentations"]
 
-import * as fs from "fs"
+export type Options = AppsignalOptions & {
+  additionalInstrumentations: AdditionalInstrumentationsOption
+}
 
 /**
  * AppSignal for Node.js's main class.
@@ -71,7 +99,7 @@ export class Client {
   /**
    * Creates a new instance of the `Appsignal` object
    */
-  constructor(options: Partial<AppsignalOptions> = {}) {
+  constructor(options: Partial<Options> = {}) {
     this.config = new Configuration(options)
     this.extension = new Extension()
     this.logger = this.setUpLogger()
@@ -80,7 +108,9 @@ export class Client {
     if (this.isActive) {
       this.extension.start()
       this.#metrics = new Metrics()
-      this.#sdk = this.initOpenTelemetry()
+      this.#sdk = this.initOpenTelemetry(
+        options.additionalInstrumentations || []
+      )
     } else {
       this.#metrics = new NoopMetrics()
       console.error("AppSignal not starting, no valid configuration found")
@@ -176,64 +206,85 @@ export class Client {
     )
   }
 
-  /**
-   * Initialises OpenTelemetry instrumentation
-   */
-  private initOpenTelemetry() {
+  private defaultInstrumentationsConfig(): DefaultInstrumentationsConfigMap {
     const sendParams = this.config.data.sendParams
     const sendSessionData = this.config.data.sendSessionData
+    const requestHeaders = this.config.data.requestHeaders
 
-    const instrumentations = [
-      new HttpInstrumentation({
-        headersToSpanAttributes: {
-          server: { requestHeaders: this.config.data["requestHeaders"] }
-        }
-      }),
-      new ExpressInstrumentation({
+    return {
+      "@opentelemetry/instrumentation-express": {
         requestHook: function (_span, info) {
           if (info.layerType === ExpressLayerType.REQUEST_HANDLER) {
             if (sendParams) {
-              // Request parameters to magic attributes
               const queryParams = info.request.query
               const requestBody = info.request.body
               const params = { ...queryParams, ...requestBody }
               setParams(params)
             }
 
             if (sendSessionData) {
-              // Session data to magic attributes
               setSessionData(info.request.cookies)
             }
           }
         }
-      }),
-      new GraphQLInstrumentation(),
-      new KoaInstrumentation({
+      },
+      "@opentelemetry/instrumentation-http": {
+        headersToSpanAttributes: {
+          server: { requestHeaders }
+        }
+      },
+      "@opentelemetry/instrumentation-ioredis": {
+        dbStatementSerializer: RedisDbStatementSerializer
+      },
+      "@opentelemetry/instrumentation-koa": {
         requestHook: function (_span, info) {
           if (sendParams) {
-            // Request parameters to magic attributes
             const queryParams = info.context.request.query
-
             setParams(queryParams)
           }
         }
-      }),
-      new MySQLInstrumentation(),
-      new MySQL2Instrumentation(),
-      new PgInstrumentation(),
-      new RedisInstrumentation({
-        dbStatementSerializer: RedisDbStatementSerializer
-      }),
-      new Redis4Instrumentation({
+      },
+      "@opentelemetry/instrumentation-redis": {
         dbStatementSerializer: RedisDbStatementSerializer
-      }),
-      new IORedisInstrumentation({
+      },
+      "@opentelemetry/instrumentation-redis-4": {
         dbStatementSerializer: RedisDbStatementSerializer
-      }),
-      new PrismaInstrumentation({
+      },
+      "@prisma/instrumentation": {
         middleware: true
-      })
-    ]
+      }
+    }
+  }
+
+  private defaultInstrumentations(): Instrumentation[] {
+    const disabledInstrumentations =
+      this.config.data.disableDefaultInstrumentations
+    if (disabledInstrumentations === true) {
+      return []
+    }
+
+    const instrumentationConfigs =
+      this.defaultInstrumentationsConfig() as Record<string, any>
+    return Object.entries(DefaultInstrumentations)
+      .filter(
+        ([name, _constructor]) =>
+          !(disabledInstrumentations || ([] as string[])).includes(name)
+      )
+      .map(
+        ([name, constructor]) =>
+          new constructor(instrumentationConfigs[name] || {})
+      )
+  }
+
+  /**
+   * Initialises OpenTelemetry instrumentation
+   */
+  private initOpenTelemetry(
+    additionalInstrumentations: AdditionalInstrumentationsOption
+  ) {
+    const instrumentations = additionalInstrumentations.concat(
+      this.defaultInstrumentations()
+    )
 
     const testMode = process.env["_APPSIGNAL_TEST_MODE"]
     const testModeFilePath = process.env["_APPSIGNAL_TEST_MODE_FILE_PATH"]
@@ -281,47 +332,3 @@ export class Client {
     global.__APPSIGNAL__ = this
   }
 }
-
-class TestModeSpanProcessor {
-  #filePath: string
-
-  constructor(testModeFilePath: string) {
-    this.#filePath = testModeFilePath
-  }
-
-  forceFlush() {
-    return Promise.resolve()
-  }
-
-  onStart(_span: any, _parentContext: any) {
-    // Does nothing
-  }
-
-  onEnd(span: any) {
-    // must grab specific attributes only because
-    // the span is a circular object
-    const serializableSpan = {
-      attributes: span.attributes,
-      events: span.events,
-      status: span.status,
-      name: span.name,
-      spanId: span._spanContext.spanId,
-      traceId: span._spanContext.traceId,
-      parentSpanId: span.parentSpanId,
-      instrumentationLibrary: span.instrumentationLibrary,
-      startTime: span.startTime,
-      endTime: span.endTime
-    }
-
-    // Re-open the file for every write, as the test process might have
-    // truncated it in between writes.
-    const file = fs.openSync(this.#filePath, "a")
-    fs.appendFileSync(file, `${JSON.stringify(serializableSpan)}\n`)
-    fs.closeSync(file)
-  }
-
-  shutdown() {
-    // Does nothing
-    return Promise.resolve()
-  }
-}

src/config.ts

@@ -114,6 +114,7 @@ export class Configuration {
       active: false,
       caFilePath: path.join(__dirname, "../cert/cacert.pem"),
       debug: false,
+      disableDefaultInstrumentations: false,
       dnsServers: [],
       enableHostMetrics: true,
       enableMinutelyProbes: true,
@@ -126,9 +127,6 @@ export class Configuration {
       ignoreActions: [],
       ignoreErrors: [],
       ignoreNamespaces: [],
-      instrumentHttp: true,
-      instrumentPg: true,
-      instrumentRedis: true,
       log: "file",
       requestHeaders: [
         "accept",