Skip to content

Commit

Permalink
fix: replace statsd-client with hot-shots (#4806)
Browse files Browse the repository at this point in the history
* fix: replace statsd-client with hot-shots

* chore: comment

* chore: fix test

* chore: refactor code

* chore: jsdoc

* chore: better framework check

* chore: variable name
  • Loading branch information
danez committed Jan 11, 2023
1 parent 724a5f7 commit 18cfdbb
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 238 deletions.
230 changes: 57 additions & 173 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions packages/build/package.json
Expand Up @@ -80,6 +80,7 @@
"figures": "^4.0.0",
"filter-obj": "^3.0.0",
"got": "^10.0.0",
"hot-shots": "9.3.0",
"indent-string": "^5.0.0",
"is-plain-obj": "^4.0.0",
"js-yaml": "^4.0.0",
Expand All @@ -105,7 +106,6 @@
"rfdc": "^1.3.0",
"safe-json-stringify": "^1.2.0",
"semver": "^7.0.0",
"statsd-client": "0.4.7",
"string-width": "^5.0.0",
"strip-ansi": "^7.0.0",
"supports-color": "^9.0.0",
Expand All @@ -120,7 +120,6 @@
"devDependencies": {
"@netlify/nock-udp": "^3.1.0",
"@types/node": "^14.18.31",
"@types/statsd-client": "^0.4.3",
"atob": "^2.1.2",
"ava": "^4.0.0",
"c8": "^7.12.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/build/src/core/main.ts
Expand Up @@ -107,7 +107,7 @@ export default async function buildSite(flags: Partial<BuildCLIFlags> = {}): Pro
testOpts,
errorParams,
})
await reportError({ error, framework, statsdOpts })
await reportError(error, statsdOpts, framework)

return { success, severityCode, logs }
}
Expand All @@ -122,7 +122,7 @@ const handleBuildSuccess = async function ({ framework, dry, logs, timers, durat
logBuildSuccess(logs)

logTimer(logs, durationNs, 'Netlify Build', systemLog)
await reportTimers({ timers, statsdOpts, framework })
await reportTimers(timers, statsdOpts, framework)
}

// Handles the calls and errors of telemetry reports
Expand Down
51 changes: 30 additions & 21 deletions packages/build/src/error/report.ts
@@ -1,22 +1,29 @@
import type { Tags } from 'hot-shots'

import { isNetlifyMaintainedPlugin } from '../plugins/internal.js'
import { closeClient, normalizeTagName, startClient } from '../report/statsd.js'
import {
closeClient,
formatTags,
InputStatsDOptions,
normalizeTagName,
startClient,
validateStatsDOptions,
} from '../report/statsd.js'

import { getErrorInfo } from './info.js'

const TOP_PARENT_TAG = 'run_netlify_build'

// Record error rates of the build phase for monitoring.
// Sends to statsd daemon.
export const reportError = async function ({
error,
statsdOpts: { host, port },
framework,
}: {
error: Error
statsdOpts: { host?: string; port: number }
framework?: string
}) {
if (host === undefined) {
/**
* Record error rates of the build phase for monitoring.
* Sends to statsd daemon.
*/
export const reportError = async function (
error: Error,
statsdOpts: InputStatsDOptions,
framework?: string,
): Promise<void> {
if (!validateStatsDOptions(statsdOpts)) {
return
}

Expand All @@ -30,14 +37,16 @@ export const reportError = async function ({

const parent = pluginName ? pluginName : TOP_PARENT_TAG
const stage = pluginName ? errorInfo.location?.event : errorInfo.stage
const client = await startClient(host, port)

const frameworkTag: { framework?: string } = framework === undefined ? {} : { framework }
client.increment('buildbot.build.stage.error', 1, {
stage: stage ?? 'system',
parent,
...frameworkTag,
})
const statsDTags: Tags = { stage: stage ?? 'system', parent }

// Do not add a framework tag if empty string or null/undefined
if (framework) {
statsDTags.framework = framework
}

const client = await startClient(statsdOpts)

client.increment('buildbot.build.stage.error', 1, formatTags(statsDTags))

await closeClient(client)
}
79 changes: 54 additions & 25 deletions packages/build/src/report/statsd.ts
@@ -1,40 +1,69 @@
import { promisify } from 'util'

import slugify from '@sindresorhus/slugify'
import StatsdClient from 'statsd-client'
import { StatsD } from 'hot-shots'

// TODO: replace with `timers/promises` after dropping Node < 15.0.0
const pSetTimeout = promisify(setTimeout)
export interface InputStatsDOptions {
host?: string
port?: number
}

// See https://github.com/msiebuhr/node-statsd-client/blob/45a93ee4c94ca72f244a40b06cb542d4bd7c3766/lib/EphemeralSocket.js#L81
const CLOSE_TIMEOUT = 11
export type StatsDOptions = Required<InputStatsDOptions>

// The socket creation is delayed until the first packet is sent. In our
// case, this happens just before `client.close()` is called, which is too
// late and make it not send anything. We need to manually create it using
// the internal API.
export const startClient = async function (host: string, port: number): Promise<StatsdClient> {
const client = new StatsdClient({ host, port, socketTimeout: 0 })
export const validateStatsDOptions = function (statsdOpts: InputStatsDOptions): statsdOpts is StatsDOptions {
return !!(statsdOpts && statsdOpts.host && statsdOpts.port)
}

// @ts-expect-error using internals :D
await promisify(client._socket._createSocket.bind(client._socket))()
/**
* Start a new StatsD Client and a new UDP socket
*/
export const startClient = async function (statsdOpts: StatsDOptions): Promise<StatsD> {
const { host, port } = statsdOpts

return client
return new StatsD({
host,
port,
// This caches the dns resolution for subsequent sends of metrics for this instance
// Because we only try to send the metrics on close, this comes only into effect if `bufferFlushInterval` time is exceeded
cacheDns: true,
// set the maxBufferSize to infinite and the bufferFlushInterval very high, so that we only
// send the metrics on close or if more than 10 seconds past by
maxBufferSize: Infinity,
bufferFlushInterval: 10_000,
})
}

// UDP packets are buffered and flushed at regular intervals by statsd-client.
// Closing force flushing all of them.
export const closeClient = async function (client: StatsdClient): Promise<void> {
client.close()

// statsd-client does not provide a way of knowing when the socket is done
// closing, so we need to use the following hack.
await pSetTimeout(CLOSE_TIMEOUT)
await pSetTimeout(CLOSE_TIMEOUT)
/**
* Close the StatsD Client
*
* UDP packets are buffered and flushed every 10 seconds and
* closing the client forces all buffered metrics to be flushed.
*/
export const closeClient = async function (client: StatsD): Promise<void> {
await promisify(client.close.bind(client))()
}

// Make sure the timer name does not include special characters.
// For example, the `packageName` of local plugins includes dots.
/**
* Make sure the timer name does not include special characters.
* For example, the `packageName` of local plugins includes dots.
*/
export const normalizeTagName = function (name: string): string {
return slugify(name, { separator: '_' })
}

/**
* Formats and flattens the tags as array
* We do this because we might send the same tag with different values `{ tag: ['a', 'b'] }`
* which cannot be represented in an flattened object and `hot-shots` also supports tags as array in the format `['tag:a', 'tag:b']`
*/
export const formatTags = function (tags: Record<string, string | string[]>): string[] {
return Object.entries(tags)
.map(([key, value]) => {
if (Array.isArray(value)) {
return value.map((subValue) => `${key}:${subValue}`)
} else {
return `${key}:${value}`
}
})
.flat()
}
55 changes: 42 additions & 13 deletions packages/build/src/time/report.ts
@@ -1,29 +1,58 @@
import { closeClient, startClient } from '../report/statsd.js'
import type { Tags, StatsD } from 'hot-shots'

import {
closeClient,
formatTags,
InputStatsDOptions,
startClient,
StatsDOptions,
validateStatsDOptions,
} from '../report/statsd.js'

import { addAggregatedTimers } from './aggregate.js'
import { roundTimerToMillisecs } from './measure.js'

// Record the duration of a build phase, for monitoring.
// Sends to statsd daemon.
export const reportTimers = async function ({ timers, statsdOpts: { host, port }, framework }) {
if (host === undefined) {
interface Timer {
metricName: string
stageTag: string
parentTag: string
durationNs: number
tags: Record<string, string | string[]>
}

/**
* Record the duration of a build phase, for monitoring.
* Sends to statsd daemon.
*/
export const reportTimers = async function (
timers: Timer[],
statsdOpts: InputStatsDOptions,
framework?: string,
): Promise<void> {
if (!validateStatsDOptions(statsdOpts)) {
return
}

const timersA = addAggregatedTimers(timers)
await sendTimers({ timers: timersA, host, port, framework })
await sendTimers(addAggregatedTimers(timers), statsdOpts, framework)
}

const sendTimers = async function ({ timers, host, port, framework }) {
const client = await startClient(host, port)
const sendTimers = async function (timers: Timer[], statsdOpts: StatsDOptions, framework?: string): Promise<void> {
const client = await startClient(statsdOpts)
timers.forEach((timer) => {
sendTimer({ timer, client, framework })
sendTimer(timer, client, framework)
})
await closeClient(client)
}

const sendTimer = function ({ timer: { metricName, stageTag, parentTag, durationNs, tags }, client, framework }) {
const sendTimer = function (timer: Timer, client: StatsD, framework?: string): void {
const { metricName, stageTag, parentTag, durationNs, tags } = timer
const durationMs = roundTimerToMillisecs(durationNs)
const frameworkTag = framework === undefined ? {} : { framework }
client.distribution(metricName, durationMs, { stage: stageTag, parent: parentTag, ...tags, ...frameworkTag })
const statsDTags: Tags = { stage: stageTag, parent: parentTag, ...tags }

// Do not add a framework tag if empty string or null/undefined
if (framework) {
statsDTags.framework = framework
}

client.distribution(metricName, durationMs, formatTags(statsDTags))
}
21 changes: 20 additions & 1 deletion packages/build/tests/error_reporting/tests.js
@@ -1,8 +1,27 @@
import dns from 'dns'

import { intercept, cleanAll } from '@netlify/nock-udp'
import { Fixture } from '@netlify/testing'
import test from 'ava'
import sinon from 'sinon'

test.before(() => {
const origLookup = dns.lookup
// we have to stub dns lookup as hot-shots is caching dns and therefore calling dns.lookup directly
sinon.stub(dns, 'lookup').callsFake((host, options, cb = options) => {
if (options === cb) {
options = {}
}
if (host.startsWith(`errorreportingtest.`)) {
cb(undefined, host, 4)
} else {
origLookup(host, options, cb)
}
})
})

test.after(() => {
dns.lookup.restore()
cleanAll()
})

Expand Down Expand Up @@ -31,7 +50,7 @@ const getTrackingRequestsString = async function (t, fixtureName, used = true) {

const getAllTrackingRequests = async function (t, fixtureName, used) {
// Ensure there's no conflict between each test scope
const host = encodeURI(t.title)
const host = `errorreportingtest.${encodeURI(t.title)}`
const port = '1234'
const scope = intercept(`${host}:${port}`, { persist: true, allowUnknown: true })

Expand Down
21 changes: 20 additions & 1 deletion packages/build/tests/time/tests.js
@@ -1,8 +1,27 @@
import dns from 'dns'

import { intercept, cleanAll } from '@netlify/nock-udp'
import { Fixture } from '@netlify/testing'
import test from 'ava'
import sinon from 'sinon'

test.before(() => {
const origLookup = dns.lookup
// we have to stub dns lookup as hot-shots is caching dns and therefore calling dns.lookup directly
sinon.stub(dns, 'lookup').callsFake((host, options, cb = options) => {
if (options === cb) {
options = {}
}
if (host.startsWith(`timetest.`)) {
cb(undefined, host, 4)
} else {
origLookup(host, options, cb)
}
})
})

test.after(() => {
dns.lookup.restore()
cleanAll()
})

Expand Down Expand Up @@ -70,7 +89,7 @@ const getTimerRequestsString = async function (t, fixtureName, flags) {

const getAllTimerRequests = async function (t, fixtureName, flags = {}) {
// Ensure there's no conflict between each test scope
const host = encodeURI(t.title)
const host = `timetest.${encodeURI(t.title)}`
const port = '1234'
const scope = intercept(`${host}:${port}`, { persist: true, allowUnknown: true })

Expand Down

0 comments on commit 18cfdbb

Please sign in to comment.