Implement opentelemetryInstrumentations getter

Implement an `opentelemetryInstrumentations` getter on the client,
to allow users of the `initializeOpentelemetrySdk` config option to
configure OpenTelemetry in an AppSignal-compatible way.

This getter honors the settings in the `disableDefaultInstrumentations`
and `additionalInstrumentations` configuration options. Implementing
this was a bit of a refactor, so I took a moment to add tests for this
and the `initializeOpentelemetrySdk` config option.

Author: unflxw

.changesets/add-initialize-opentelemetry-sdk-config-option.md

@@ -7,8 +7,15 @@ Add `initializeOpentelemetrySdk` configuration option. This allows those who
 would rather take control of how OpenTelemetry is initialised in their
 application to skip AppSignal's initialization of the OpenTelemetry SDK.
 
+Additionally, add an `opentelemetryInstrumentations` method on the client,
+which returns AppSignal's default OpenTelemetry instrumentations, already
+configured to work correctly with AppSignal. The provided list of
+instrumentations will follow the `additionalInstrumentations` and
+`disableDefaultInstrumentations` config options, if those are set.
+
 This is not the recommended way to use AppSignal for Node.js. Only use this
-config option if you're really sure that you know what you're doing.
+config option and this method if you're really sure that you know what
+you're doing.
 
 When initialising OpenTelemetry, it is necessary to add the AppSignal span
 processor in order for data to be sent to AppSignal. For example, using the
@@ -20,12 +27,17 @@ import { SpanProcessor, Appsignal } from "@appsignal/nodejs";
 
 const sdk = new NodeSDK({
   spanProcessor: new SpanProcessor(Appsignal.client)
+  instrumentations: Appsignal.client.opentelemetryInstrumentations()
 });
 
 sdk.start()
 ```
 
 The above snippet assumes that the AppSignal client has been initialised
-beforehand. For an optimal experience with AppSignal, those making use of this
-config option should configure their OpenTelemetry instrumentation in a
-similar way as it is done in the AppSignal integration's source code.
+beforehand.
+
+When making use of this config option, the OpenTelemetry instrumentations
+must be configured in the same way as it is done in the AppSignal integration.
+In the above snippet, the `instrumentations` property in the OpenTelemetry SDK
+is set to the AppSignal client's list of OpenTelemetry instrumentations, which
+are configured to work correctly with AppSignal.

src/__tests__/client.test.ts

@@ -1,4 +1,7 @@
-import { InstrumentationTestRegistry } from "../../test/instrumentation_registry"
+import {
+  InstrumentationTestRegistry,
+  instrumentationNames
+} from "../../test/instrumentation_registry"
 import { Extension } from "../extension"
 import { Client } from "../client"
 import { Metrics } from "../metrics"
@@ -16,6 +19,7 @@ describe("Client", () => {
   const DEFAULT_OPTS = { name, pushApiKey }
 
   beforeEach(() => {
+    InstrumentationTestRegistry.clear()
     client = new Client({ ...DEFAULT_OPTS })
   })
 
@@ -142,16 +146,37 @@ describe("Client", () => {
     expect(meter).toBeInstanceOf(Metrics)
   })
 
+  it("initializes the OpenTelemetry SDK when active", () => {
+    client = new Client({ ...DEFAULT_OPTS, active: true })
+    expect(InstrumentationTestRegistry.didInitializeSDK).toEqual(true)
+  })
+
+  it("does not initialize the OpenTelemetry SDK when its config option is false", () => {
+    client = new Client({
+      ...DEFAULT_OPTS,
+      active: true,
+      initializeOpentelemetrySdk: false
+    })
+    expect(InstrumentationTestRegistry.didInitializeSDK).toEqual(false)
+  })
+
   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 =
+      const registryInstrumentationNames =
         InstrumentationTestRegistry.instrumentationNames()
-      expect(instrumentationNames.length).toBeGreaterThan(10)
-      expect(instrumentationNames).toContain(
+      expect(registryInstrumentationNames.length).toBeGreaterThan(10)
+      expect(registryInstrumentationNames).toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+
+      const opentelemetryInstrumentations =
+        client.opentelemetryInstrumentations()
+      expect(opentelemetryInstrumentations.length).toBeGreaterThan(10)
+      expect(instrumentationNames(opentelemetryInstrumentations)).toContain(
         "@opentelemetry/instrumentation-http"
       )
     })
@@ -165,6 +190,7 @@ describe("Client", () => {
       const instrumentationNames =
         InstrumentationTestRegistry.instrumentationNames()
       expect(instrumentationNames).toEqual([])
+      expect(client.opentelemetryInstrumentations()).toEqual([])
     })
 
     it("can disable some default instrumentations", () => {
@@ -173,12 +199,19 @@ describe("Client", () => {
         active: true,
         disableDefaultInstrumentations: ["@opentelemetry/instrumentation-http"]
       })
-      const instrumentationNames =
+      const registryInstrumentationNames =
         InstrumentationTestRegistry.instrumentationNames()
-      expect(instrumentationNames).not.toContain(
+      expect(registryInstrumentationNames).not.toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+      expect(registryInstrumentationNames.length).toBeGreaterThan(10)
+
+      const opentelemetryInstrumentations =
+        client.opentelemetryInstrumentations()
+      expect(instrumentationNames(opentelemetryInstrumentations)).not.toContain(
         "@opentelemetry/instrumentation-http"
       )
-      expect(instrumentationNames.length).toBeGreaterThan(0)
+      expect(opentelemetryInstrumentations.length).toBeGreaterThan(10)
     })
 
     it("can add additional instrumentations", () => {
@@ -211,13 +244,22 @@ describe("Client", () => {
         additionalInstrumentations: [new TestInstrumentation()]
       })
 
-      const instrumentationNames =
+      const registryInstrumentationNames =
         InstrumentationTestRegistry.instrumentationNames()
-      expect(instrumentationNames).toContain(
+      expect(registryInstrumentationNames).toContain(
+        "@opentelemetry/instrumentation-http"
+      )
+      expect(registryInstrumentationNames.length).toBeGreaterThan(10)
+      expect(registryInstrumentationNames).toContain("test/instrumentation")
+      const opentelemetryInstrumentations =
+        client.opentelemetryInstrumentations()
+      expect(instrumentationNames(opentelemetryInstrumentations)).toContain(
         "@opentelemetry/instrumentation-http"
       )
-      expect(instrumentationNames.length).toBeGreaterThan(10)
-      expect(instrumentationNames).toContain("test/instrumentation")
+      expect(opentelemetryInstrumentations.length).toBeGreaterThan(10)
+      expect(instrumentationNames(opentelemetryInstrumentations)).toContain(
+        "test/instrumentation"
+      )
     })
   })
 })

src/client.ts

@@ -70,10 +70,10 @@ type DefaultInstrumentationsConfigMap = {
   >
 }
 
-type AdditionalInstrumentationsOption = NodeSDKConfiguration["instrumentations"]
+type NodeSDKInstrumentationsOption = NodeSDKConfiguration["instrumentations"]
 
 export type Options = AppsignalOptions & {
-  additionalInstrumentations: AdditionalInstrumentationsOption
+  additionalInstrumentations: NodeSDKInstrumentationsOption
 }
 
 /**
@@ -93,6 +93,7 @@ export class Client {
 
   #metrics: Metrics
   #sdk?: NodeSDK
+  #additionalInstrumentations: NodeSDKInstrumentationsOption
 
   /**
    * Global accessors for the AppSignal client
@@ -138,6 +139,8 @@ export class Client {
    * Creates a new instance of the `Appsignal` object
    */
   constructor(options: Partial<Options> = {}) {
+    this.#additionalInstrumentations = options.additionalInstrumentations || []
+
     this.config = new Configuration(options)
     this.extension = new Extension()
     this.integrationLogger = this.setUpIntegrationLogger()
@@ -153,9 +156,7 @@ export class Client {
         this.start()
         this.#metrics = new Metrics()
         if (this.config.data.initializeOpentelemetrySdk) {
-          this.#sdk = this.initOpenTelemetry(
-            options.additionalInstrumentations || []
-          )
+          this.#sdk = this.initOpenTelemetry()
         }
       }
     } else {
@@ -364,12 +365,16 @@ export class Client {
       )
   }
 
+  public opentelemetryInstrumentations(): NodeSDKInstrumentationsOption {
+    return this.#additionalInstrumentations.concat(
+      this.defaultInstrumentations()
+    )
+  }
+
   /**
    * Initialises OpenTelemetry instrumentation
    */
-  private initOpenTelemetry(
-    additionalInstrumentations: AdditionalInstrumentationsOption
-  ) {
+  private initOpenTelemetry() {
     const testMode = process.env["_APPSIGNAL_TEST_MODE"]
     const testModeFilePath = process.env["_APPSIGNAL_TEST_MODE_FILE_PATH"]
     let spanProcessor
@@ -380,9 +385,7 @@ export class Client {
       spanProcessor = new SpanProcessor(this)
     }
 
-    const instrumentations = additionalInstrumentations.concat(
-      this.defaultInstrumentations()
-    )
+    const instrumentations = this.opentelemetryInstrumentations()
 
     const sdk = new NodeSDK({
       instrumentations,

test/instrumentation_registry.ts

@@ -3,22 +3,28 @@ import { Instrumentation } from "@opentelemetry/instrumentation"
 
 type InstrumentationsOption = NodeSDKConfiguration["instrumentations"]
 
+export function instrumentationNames(instrumentations: InstrumentationsOption) {
+  return instrumentations.map(
+    instrumentation => (instrumentation as Instrumentation).instrumentationName
+  )
+}
+
 export class InstrumentationTestRegistry {
   static instrumentations?: InstrumentationsOption
+  static didInitializeSDK = false
 
   static setInstrumentations(instrumentations: InstrumentationsOption) {
     this.instrumentations = instrumentations
+    this.didInitializeSDK = true
   }
 
   static instrumentationNames() {
-    return (this.instrumentations || []).map(
-      instrumentation =>
-        (instrumentation as Instrumentation).instrumentationName
-    )
+    return instrumentationNames(this.instrumentations || [])
   }
 
   static clear() {
     this.instrumentations = undefined
+    this.didInitializeSDK = false
   }
 }