diff --git a/packages/nodejs/.changesets/stop-minutely-probes-when-stopping-appsignal.md b/packages/nodejs/.changesets/stop-minutely-probes-when-stopping-appsignal.md new file mode 100644 index 000000000..03b10f19f --- /dev/null +++ b/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()`. diff --git a/packages/nodejs/src/__tests__/client.test.ts b/packages/nodejs/src/__tests__/client.test.ts index b2745cc8e..f1040134a 100644 --- a/packages/nodejs/src/__tests__/client.test.ts +++ b/packages/nodejs/src/__tests__/client.test.ts @@ -12,13 +12,16 @@ 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() @@ -26,9 +29,18 @@ describe("BaseClient", () => { }) 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", () => { diff --git a/packages/nodejs/src/__tests__/metrics.test.ts b/packages/nodejs/src/__tests__/metrics.test.ts index 7306375cc..10f8d4087 100644 --- a/packages/nodejs/src/__tests__/metrics.test.ts +++ b/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", () => { diff --git a/packages/nodejs/src/__tests__/probes.test.ts b/packages/nodejs/src/__tests__/probes.test.ts index 3a77df7c1..91a0cb4d6 100644 --- a/packages/nodejs/src/__tests__/probes.test.ts +++ b/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() diff --git a/packages/nodejs/src/client.ts b/packages/nodejs/src/client.ts index 6ba55fd6e..91b8ea82e 100644 --- a/packages/nodejs/src/client.ts +++ b/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() } diff --git a/packages/nodejs/src/instrumentation/http/lifecycle/__tests__/outgoing.test.ts b/packages/nodejs/src/instrumentation/http/lifecycle/__tests__/outgoing.test.ts index e8a0186c5..43fe475fa 100644 --- a/packages/nodejs/src/instrumentation/http/lifecycle/__tests__/outgoing.test.ts +++ b/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>((resolve, reject) => { const options = { diff --git a/packages/nodejs/src/interfaces/probes.ts b/packages/nodejs/src/interfaces/probes.ts index 2eaecd600..880c8d6e1 100644 --- a/packages/nodejs/src/interfaces/probes.ts +++ b/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. */ diff --git a/packages/nodejs/src/metrics.ts b/packages/nodejs/src/metrics.ts index f9df6ad36..14b269b45 100644 --- a/packages/nodejs/src/metrics.ts +++ b/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 }) } /** diff --git a/packages/nodejs/src/noops/index.ts b/packages/nodejs/src/noops/index.ts index c1e744c1e..46c856fd7 100644 --- a/packages/nodejs/src/noops/index.ts +++ b/packages/nodejs/src/noops/index.ts @@ -1,4 +1,3 @@ export * from "./span" export * from "./tracer" export * from "./metrics" -export * from "./probes" diff --git a/packages/nodejs/src/noops/metrics.ts b/packages/nodejs/src/noops/metrics.ts index 70f9863ae..8db53fed1 100644 --- a/packages/nodejs/src/noops/metrics.ts +++ b/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, diff --git a/packages/nodejs/src/noops/probes.ts b/packages/nodejs/src/noops/probes.ts deleted file mode 100644 index f01ba7148..000000000 --- a/packages/nodejs/src/noops/probes.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { Probes } from "../interfaces" - -export class NoopProbes implements Probes { - public get count(): number { - return 0 - } - - public register(name: string, fn: () => void): this { - return this - } - - public unregister(name: string): this { - return this - } - - public clear(): this { - return this - } -} diff --git a/packages/nodejs/src/probes/index.ts b/packages/nodejs/src/probes/index.ts index 18d9adfe6..80abd5301 100644 --- a/packages/nodejs/src/probes/index.ts +++ b/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() constructor() { @@ -22,17 +69,17 @@ 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") { @@ -40,17 +87,21 @@ export class BaseProbes extends EventEmitter implements Probes { 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 {} +}