Skip to content

Commit

Permalink
Implement stop method for probes
Browse files Browse the repository at this point in the history
Implement a `.stop` method on the `BaseProbes` class and on the
`Probes` interface, along with a read-only `.isStopped` property.

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

`NoopProbes` no longer exists, but `BaseProbes({ stopped: true })`
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.
  • Loading branch information
unflxw committed Feb 11, 2022
1 parent 47a851b commit 1a92d5f
Show file tree
Hide file tree
Showing 11 changed files with 122 additions and 38 deletions.
Original file line number Diff line number Diff line change
@@ -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()`.
6 changes: 4 additions & 2 deletions packages/nodejs/src/__tests__/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,11 @@ describe("BaseClient", () => {
})

it("stops the client", () => {
const stopSpy = jest.spyOn(client.extension, "stop")
const extensionStopSpy = jest.spyOn(client.extension, "stop")
const probesStopSpy = jest.spyOn(client.metrics().probes(), "stop")
client.stop()
expect(stopSpy).toHaveBeenCalled()
expect(extensionStopSpy).toHaveBeenCalled()
expect(probesStopSpy).toHaveBeenCalled()
})

it("stores the client on global object", () => {
Expand Down
10 changes: 4 additions & 6 deletions packages/nodejs/src/__tests__/metrics.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -11,14 +9,14 @@ describe("Metrics", () => {
metrics = new Metrics()
})

it("has `Probes` when minutely probes are on", () => {
expect(metrics.probes()).toBeInstanceOf(BaseProbes)
it("is not stopped when minutely probes are on", () => {
expect(metrics.probes().isStopped).toEqual(false)
})

it("has `NoopProbes` when minutely probes are off", () => {
it("is stopped when minutely probes are off", () => {
new BaseClient({ enableMinutelyProbes: false })
metrics = new Metrics()
expect(metrics.probes()).toBeInstanceOf(NoopProbes)
expect(metrics.probes().isStopped).toEqual(true)
})

it("sets a gauge", () => {
Expand Down
24 changes: 24 additions & 0 deletions packages/nodejs/src/__tests__/probes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,30 @@ describe("Probes", () => {
return fn
}

describe("when stopped", () => {
it("is stopped", () => {
expect(probes.isStopped).toEqual(false)
probes.stop()
expect(probes.isStopped).toEqual(true)
})

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()
Expand Down
1 change: 1 addition & 0 deletions packages/nodejs/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class BaseClient implements Client {
console.log("Stopping AppSignal")
}

this.metrics().probes().stop()
this.extension.stop()
}

Expand Down
11 changes: 11 additions & 0 deletions packages/nodejs/src/interfaces/probes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 has been stopped.
*/
readonly isStopped: boolean

/**
* Number of probes that are registered.
*/
Expand Down
7 changes: 1 addition & 6 deletions packages/nodejs/src/metrics.ts
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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({ stopped: !enableMinutelyProbes })
}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/nodejs/src/noops/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export * from "./span"
export * from "./tracer"
export * from "./metrics"
export * from "./probes"
4 changes: 2 additions & 2 deletions packages/nodejs/src/noops/metrics.ts
Original file line number Diff line number Diff line change
@@ -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({ stopped: true })

public setGauge(
key: string,
Expand Down
19 changes: 0 additions & 19 deletions packages/nodejs/src/noops/probes.ts

This file was deleted.

69 changes: 67 additions & 2 deletions packages/nodejs/src/probes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,72 @@ import { Probes } from "../interfaces"
/**
* The Minutely probes object.
*/
export class BaseProbes extends EventEmitter implements Probes {
export class BaseProbes implements Probes {
#probes: InternalProbes
#stopped = false

constructor({ stopped = false } = {}) {
this.#probes = new StartedProbes()
if (stopped) this.stop()
}

public stop(): this {
this.#probes.clear()
this.#probes = new StoppedProbes()
this.#stopped = true
return this
}

get isStopped(): boolean {
return this.#stopped
}

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 InternalProbes = {
readonly count: number
register(name: string, fn: () => void): void
unregister(name: string): void
clear(): void
}

class StoppedProbes implements InternalProbes {
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
}
}

class StartedProbes extends EventEmitter implements InternalProbes {
#timers = new Map<string, NodeJS.Timeout>()

constructor() {
Expand Down Expand Up @@ -48,7 +113,7 @@ export class BaseProbes extends EventEmitter implements Probes {
* Unregisters all probes and clears the timers.
*/
public clear(): this {
this.#timers.forEach(t => clearInterval(t))
this.#timers.forEach(clearInterval)
this.#timers = new Map()
this.removeAllListeners()
return this
Expand Down

0 comments on commit 1a92d5f

Please sign in to comment.