Implement stop method for probes

Implement a `.stop` method on the `BaseProbes` class and on the
`Probes` interface, along with a read-only `.isRunning` property.

The new `BaseProbes` object handles only the stopping logic. The
actual behaviour of the probes is delegated to two private classes:
`BaseProbeRunner` (which used to be `BaseProbes`) and
`NoopProbeRunner` (which used to be `NoopProbes`). Both of these
implement the `ProbeRunner` private interface. Stopping the probes
permanently switches the implementation used.

`NoopProbes` no longer exists, but `BaseProbes({ run: false })`
can be used to initialise it in the stopped state, which is
functionally equivalent.

Call `metrics().probes().stop()` from the `BaseClient.stop()`
method, ensuring the probes system is stopped when AppSignal is
stopped.

Fixes #418 and closes #569.

packages/nodejs/.changesets/stop-minutely-probes-when-stopping-appsignal.md

@@ -0,0 +1,8 @@
+---
+bump: "patch"
+type: "fix"
+---
+
+The minutely probes are now stopped when `Appsignal.stop()` is called.
+In addition, the probes can be stopped independently of the rest of
+the integration by calling `probes.stop()`.

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

@@ -12,23 +12,35 @@ describe("BaseClient", () => {
 
   let client: BaseClient
 
-  // enableMinutelyProbes is set to false so we don't leak timers
-  const DEFAULT_OPTS = { name, pushApiKey, enableMinutelyProbes: false }
+  const DEFAULT_OPTS = { name, pushApiKey }
 
   beforeEach(() => {
     client = new BaseClient({ ...DEFAULT_OPTS })
   })
 
+  afterEach(() => {
+    client.stop()
+  })
+
   it("starts the client", () => {
     const startSpy = jest.spyOn(client.extension, "start")
     client.start()
     expect(startSpy).toHaveBeenCalled()
   })
 
   it("stops the client", () => {
-    const stopSpy = jest.spyOn(client.extension, "stop")
+    const extensionStopSpy = jest.spyOn(client.extension, "stop")
+    client.stop()
+    expect(extensionStopSpy).toHaveBeenCalled()
+  })
+
+  it("stops the probes when the client is active", () => {
+    client = new BaseClient({ ...DEFAULT_OPTS, active: true })
+    const probes = client.metrics().probes()
+    expect(probes.isRunning).toEqual(true)
+
     client.stop()
-    expect(stopSpy).toHaveBeenCalled()
+    expect(probes.isRunning).toEqual(false)
   })
 
   it("stores the client on global object", () => {

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

@@ -1,7 +1,5 @@
 import { BaseClient } from "../client"
 import { BaseMetrics as Metrics } from "../metrics"
-import { NoopProbes } from "../noops"
-import { BaseProbes } from "../probes"
 
 describe("Metrics", () => {
   let metrics: Metrics
@@ -11,14 +9,14 @@ describe("Metrics", () => {
     metrics = new Metrics()
   })
 
-  it("has `Probes` when minutely probes are on", () => {
-    expect(metrics.probes()).toBeInstanceOf(BaseProbes)
+  it("runs the probes when minutely probes are on", () => {
+    expect(metrics.probes().isRunning).toEqual(true)
   })
 
-  it("has `NoopProbes` when minutely probes are off", () => {
+  it("does not run the probes when minutely probes are off", () => {
     new BaseClient({ enableMinutelyProbes: false })
     metrics = new Metrics()
-    expect(metrics.probes()).toBeInstanceOf(NoopProbes)
+    expect(metrics.probes().isRunning).toEqual(false)
   })
 
   it("sets a gauge", () => {

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

@@ -23,6 +23,30 @@ describe("Probes", () => {
     return fn
   }
 
+  describe("when stopped", () => {
+    it("is not running", () => {
+      expect(probes.isRunning).toEqual(true)
+      probes.stop()
+      expect(probes.isRunning).toEqual(false)
+    })
+
+    it("does not register or call an already registered probe", () => {
+      const fn = registerMockProbe()
+      probes.stop()
+      jest.runOnlyPendingTimers()
+      expect(fn).not.toHaveBeenCalled()
+      expect(probes.count).toEqual(0)
+    })
+
+    it("does not register or call a newly registered probe", () => {
+      probes.stop()
+      const fn = registerMockProbe()
+      jest.runOnlyPendingTimers()
+      expect(fn).not.toHaveBeenCalled()
+      expect(probes.count).toEqual(0)
+    })
+  })
+
   it("registers a probe", () => {
     const fn = registerMockProbe()
     jest.runOnlyPendingTimers()

packages/nodejs/src/client.ts

@@ -105,6 +105,7 @@ export class BaseClient implements Client {
       console.log("Stopping AppSignal")
     }
 
+    this.metrics().probes().stop()
     this.extension.stop()
   }
 

packages/nodejs/src/instrumentation/http/lifecycle/__tests__/outgoing.test.ts

@@ -14,12 +14,10 @@ describe("HTTP outgoing requests", () => {
   let client: Client
   let tracer: Tracer
 
-  // enableMinutelyProbes is set to false so we don't leak timers
   const DEFAULT_OPTS = {
     active: true,
     name,
-    pushApiKey,
-    enableMinutelyProbes: false
+    pushApiKey
   }
 
   beforeEach(() => {
@@ -30,6 +28,10 @@ describe("HTTP outgoing requests", () => {
     instrument(http, tracer).install()
   })
 
+  afterEach(() => {
+    client.stop()
+  })
+
   async function performRequest() {
     return new Promise<HashMap<any>>((resolve, reject) => {
       const options = {

packages/nodejs/src/interfaces/probes.ts

@@ -2,6 +2,17 @@
  * The Minutely probes object.
  */
 export interface Probes {
+  /**
+   * Permanently stops the probes system, unregistering all probes
+   * and clearing the timers.
+   */
+  stop(): this
+
+  /**
+   * Whether the probes system is running.
+   */
+  readonly isRunning: boolean
+
   /**
    * Number of probes that are registered.
    */

packages/nodejs/src/metrics.ts

@@ -1,6 +1,5 @@
 import { Metrics, Probes } from "./interfaces"
 import { BaseProbes } from "./probes"
-import { NoopProbes } from "./noops"
 import { metrics } from "./extension_wrapper"
 import { Data } from "./internal/data"
 import { BaseClient } from "./client"
@@ -16,11 +15,7 @@ export class BaseMetrics implements Metrics {
   constructor() {
     let enableMinutelyProbes = BaseClient.config.data.enableMinutelyProbes
 
-    if (enableMinutelyProbes) {
-      this.#probes = new BaseProbes()
-    } else {
-      this.#probes = new NoopProbes()
-    }
+    this.#probes = new BaseProbes({ run: enableMinutelyProbes })
   }
 
   /**

packages/nodejs/src/noops/index.ts

@@ -1,4 +1,3 @@
 export * from "./span"
 export * from "./tracer"
 export * from "./metrics"
-export * from "./probes"

packages/nodejs/src/noops/metrics.ts

@@ -1,8 +1,8 @@
 import { Metrics, Probes } from "../interfaces"
-import { NoopProbes } from "../noops"
+import { BaseProbes } from "../probes"
 
 export class NoopMetrics implements Metrics {
-  #probes = new NoopProbes()
+  #probes = new BaseProbes({ run: false })
 
   public setGauge(
     key: string,

packages/nodejs/src/probes/index.ts

@@ -4,7 +4,54 @@ import { Probes } from "../interfaces"
 /**
  * The Minutely probes object.
  */
-export class BaseProbes extends EventEmitter implements Probes {
+export class BaseProbes implements Probes {
+  #probes: ProbeRunner
+  #running = true
+
+  constructor({ run = true } = {}) {
+    this.#probes = new BaseProbeRunner()
+    if (!run) this.stop()
+  }
+
+  public stop(): this {
+    this.#probes.clear()
+    this.#probes = new NoopProbeRunner()
+    this.#running = false
+    return this
+  }
+
+  get isRunning(): boolean {
+    return this.#running
+  }
+
+  get count(): number {
+    return this.#probes.count
+  }
+
+  public register(name: string, fn: () => void): this {
+    this.#probes.register(name, fn)
+    return this
+  }
+
+  public unregister(name: string): this {
+    this.#probes.unregister(name)
+    return this
+  }
+
+  public clear(): this {
+    this.#probes.clear()
+    return this
+  }
+}
+
+type ProbeRunner = {
+  readonly count: number
+  register(name: string, fn: () => void): void
+  unregister(name: string): void
+  clear(): void
+}
+
+class BaseProbeRunner extends EventEmitter implements ProbeRunner {
   #timers = new Map<string, NodeJS.Timeout>()
 
   constructor() {
@@ -22,35 +69,39 @@ export class BaseProbes extends EventEmitter implements Probes {
    * Registers a new minutely probe. Using a probe `name` that has already been set
    * will overwrite the current probe.
    */
-  public register(name: string, fn: () => void): this {
+  public register(name: string, fn: () => void): void {
     this.#timers.set(
       name,
       setInterval(() => this.emit(name), 60 * 1000)
     )
 
     this.removeAllListeners(name)
-    return this.on(name, fn)
+    this.on(name, fn)
   }
 
-  public unregister(name: string): this {
+  public unregister(name: string): void {
     const timer = this.#timers.get(name)
 
     if (typeof timer !== "undefined") {
       clearInterval(timer)
       this.#timers.delete(name)
       this.removeAllListeners(name)
     }
-
-    return this
   }
 
   /**
    * Unregisters all probes and clears the timers.
    */
-  public clear(): this {
+  public clear(): void {
     this.#timers.forEach(t => clearInterval(t))
     this.#timers = new Map()
     this.removeAllListeners()
-    return this
   }
 }
+
+class NoopProbeRunner implements ProbeRunner {
+  readonly count: number = 0
+  public register(_name: string, _fn: () => void): void {}
+  public unregister(_name: string): void {}
+  public clear(): void {}
+}