diff --git a/CHANGELOG.md b/CHANGELOG.md index 82e79c2f..f0456a3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ project adheres to [Semantic Versioning](http://semver.org/). - Dropped support for end-of-life Node.js versions 6.x and 8.x - Dropped the previously deprecated support for positional parameters in constructors, only the config object forms remain. +- Default metrics are collected on scrape of metrics endpoint, not on an + interval. The `timeout` option to `collectDefaultMetrics(conf)` is no longer + supported or needed, and the function no longer returns a `Timeout` object. ### Changed diff --git a/README.md b/README.md index b017d1e4..3f13921c 100644 --- a/README.md +++ b/README.md @@ -51,24 +51,13 @@ In addition, some Node-specific metrics are included, such as event loop lag, active handles, GC and Node.js version. See what metrics there are in [lib/metrics](lib/metrics). -`collectDefaultMetrics` takes 1 options object with following entries: +`collectDefaultMetrics` optionally accepts a config object with following entries: -- `timeout` for how often the probe should be fired. Default: 10 seconds. -- `prefix` an optional prefix for metric names. -- `registry` to which metrics should be registered. +- `prefix` an optional prefix for metric names. Default: no prefix. +- `registry` to which metrics should be registered. Default: the global default registry. - `gcDurationBuckets` with custom buckets for GC duration histogram. Default buckets of GC duration histogram are `[0.001, 0.01, 0.1, 1, 2, 5]` (in seconds). - `eventLoopMonitoringPrecision` with sampling rate in milliseconds. Must be greater than zero. Default: 10. -By default probes are launched every 10 seconds, but this can be modified like this: - -```js -const client = require('prom-client'); - -const collectDefaultMetrics = client.collectDefaultMetrics; - -// Probe every 5th second. -collectDefaultMetrics({ timeout: 5000 }); -``` To register metrics to another registry, pass it in as `register`: @@ -78,7 +67,6 @@ const client = require('prom-client'); const collectDefaultMetrics = client.collectDefaultMetrics; const Registry = client.Registry; const register = new Registry(); - collectDefaultMetrics({ register }); ``` @@ -96,11 +84,9 @@ To prefix metric names with your own arbitrary string, pass in a `prefix`: ```js const client = require('prom-client'); - const collectDefaultMetrics = client.collectDefaultMetrics; - -// Probe every 5th second. -collectDefaultMetrics({ prefix: 'my_application_' }); +const prefix = 'my_application_'; +collectDefaultMetrics({ prefix }); ``` To disable metric timestamps set `timestamps` to `false` (You can find the list of metrics that support this feature in `test/defaultMetricsTest.js`): diff --git a/example/server.js b/example/server.js index b6de8201..c371993f 100644 --- a/example/server.js +++ b/example/server.js @@ -79,11 +79,14 @@ server.get('/metrics/counter', (req, res) => { res.end(register.getSingleMetricAsString('test_counter')); }); -//Enable collection of default metrics +// Enable collection of default metrics require('../').collectDefaultMetrics({ timeout: 10000, gcDurationBuckets: [0.001, 0.01, 0.1, 1, 2, 5] // These are the default buckets. }); -console.log('Server listening to 3000, metrics exposed on /metrics endpoint'); -server.listen(3000); +const port = process.env.PORT || 3000; +console.log( + `Server listening to ${port}, metrics exposed on /metrics endpoint` +); +server.listen(port); diff --git a/index.d.ts b/index.d.ts index cc576df8..7d85e028 100644 --- a/index.d.ts +++ b/index.d.ts @@ -590,7 +590,6 @@ export function exponentialBuckets( ): number[]; export interface DefaultMetricsCollectorConfiguration { - timeout?: number; timestamps?: boolean; register?: Registry; prefix?: string; @@ -601,11 +600,10 @@ export interface DefaultMetricsCollectorConfiguration { /** * Configure default metrics * @param config Configuration object for default metrics collector - * @return The setInterval number */ export function collectDefaultMetrics( config?: DefaultMetricsCollectorConfiguration -): ReturnType; +): void; export interface defaultMetrics { /** diff --git a/lib/defaultMetrics.js b/lib/defaultMetrics.js index 4f86a04e..5005d8d9 100644 --- a/lib/defaultMetrics.js +++ b/lib/defaultMetrics.js @@ -33,12 +33,7 @@ const metrics = { }; const metricsList = Object.keys(metrics); -let existingInterval = null; -// This is used to ensure the program throws on duplicate metrics during first run -// We might want to consider not supporting running the default metrics function more than once -let init = true; - -module.exports = function startDefaultMetrics(config) { +module.exports = function collectDefaultMetrics(config) { if (config !== null && config !== undefined && !isObject(config)) { throw new Error('config must be null, undefined, or an object'); } @@ -52,33 +47,39 @@ module.exports = function startDefaultMetrics(config) { config ); - if (existingInterval !== null) { - clearInterval(existingInterval); - } + const registry = config.register || globalRegistry; + const last = registry + .collectors() + .find(collector => collector._source === metrics); - const initialisedMetrics = metricsList.map(metric => { - const defaultMetric = metrics[metric]; - if (!init) { - defaultMetric.metricNames.map( - globalRegistry.removeSingleMetric, - globalRegistry - ); - } + if (last) { + throw new Error( + 'Cannot add the default metrics twice to the same registry' + ); + } - return defaultMetric(config.register, config); + const scrapers = metricsList.map(key => { + const metric = metrics[key]; + return metric(config.register, config); }); - function updateAllMetrics() { - initialisedMetrics.forEach(metric => metric.call()); + // Ideally the library would be based around a concept of collectors and + // async callbacks, but in the short-term, trigger scraping of the + // current metric value synchronously. + // - // https://prometheus.io/docs/instrumenting/writing_clientlibs/#overall-structure + function defaultMetricCollector() { + scrapers.forEach(scraper => scraper()); } - updateAllMetrics(); - - existingInterval = setInterval(updateAllMetrics, config.timeout).unref(); - - init = false; + // defaultMetricCollector has to be dynamic, because the scrapers are in + // its closure, but we still want to identify a default collector, so + // tag it with a value known only to this module (the const metric array + // value) so we can find it later. + defaultMetricCollector._source = metrics; + registry.registerCollector(defaultMetricCollector); - return existingInterval; + // Because the tests expect an immediate collection. + defaultMetricCollector(); }; module.exports.metricsList = metricsList; diff --git a/lib/metrics/eventLoopLag.js b/lib/metrics/eventLoopLag.js index 07f73e0f..64394be2 100644 --- a/lib/metrics/eventLoopLag.js +++ b/lib/metrics/eventLoopLag.js @@ -1,4 +1,5 @@ 'use strict'; + const Gauge = require('../gauge'); // Check if perf_hooks module is available @@ -10,7 +11,11 @@ try { // node version is too old } +// Reported always, but because legacy lag_seconds is collected async, the value +// will always be stale by one scrape interval. const NODEJS_EVENTLOOP_LAG = 'nodejs_eventloop_lag_seconds'; + +// Reported only when perf_hooks is available. const NODEJS_EVENTLOOP_LAG_MIN = 'nodejs_eventloop_lag_min_seconds'; const NODEJS_EVENTLOOP_LAG_MAX = 'nodejs_eventloop_lag_max_seconds'; const NODEJS_EVENTLOOP_LAG_MEAN = 'nodejs_eventloop_lag_mean_seconds'; diff --git a/lib/metrics/heapSizeAndUsed.js b/lib/metrics/heapSizeAndUsed.js index 7f6eee15..17d01793 100644 --- a/lib/metrics/heapSizeAndUsed.js +++ b/lib/metrics/heapSizeAndUsed.js @@ -17,27 +17,22 @@ module.exports = (registry, config = {}) => { const heapSizeTotal = new Gauge({ name: namePrefix + NODEJS_HEAP_SIZE_TOTAL, - help: 'Process heap size from node.js in bytes.', + help: 'Process heap size from Node.js in bytes.', registers }); const heapSizeUsed = new Gauge({ name: namePrefix + NODEJS_HEAP_SIZE_USED, - help: 'Process heap size used from node.js in bytes.', + help: 'Process heap size used from Node.js in bytes.', + registers + }); + const externalMemUsed = new Gauge({ + name: namePrefix + NODEJS_EXTERNAL_MEMORY, + help: 'Node.js external memory size in bytes.', registers }); - let externalMemUsed; - - const usage = safeMemoryUsage(); - if (usage && usage.external) { - externalMemUsed = new Gauge({ - name: namePrefix + NODEJS_EXTERNAL_MEMORY, - help: 'Nodejs external memory size in bytes.', - registers - }); - } return () => { - // process.memoryUsage() can throw EMFILE errors, see #67 + // process.memoryUsage() can throw on some platforms, see #67 const memUsage = safeMemoryUsage(); if (memUsage) { if (config.timestamps) { diff --git a/lib/metrics/heapSpacesSizeAndUsed.js b/lib/metrics/heapSpacesSizeAndUsed.js index f3ae7f1a..0f0e8236 100644 --- a/lib/metrics/heapSpacesSizeAndUsed.js +++ b/lib/metrics/heapSpacesSizeAndUsed.js @@ -1,17 +1,9 @@ 'use strict'; const Gauge = require('../gauge'); -let v8; - -try { - v8 = require('v8'); -} catch (e) { - // node version is too old - // probably we can use v8-heap-space-statistics for >=node-4.0.0 and { @@ -19,13 +11,6 @@ METRICS.forEach(metricType => { }); module.exports = (registry, config = {}) => { - if ( - typeof v8 === 'undefined' || - typeof v8.getHeapSpaceStatistics !== 'function' - ) { - return () => {}; - } - const registers = registry ? [registry] : undefined; const namePrefix = config.prefix ? config.prefix : ''; @@ -34,7 +19,7 @@ module.exports = (registry, config = {}) => { METRICS.forEach(metricType => { gauges[metricType] = new Gauge({ name: namePrefix + NODEJS_HEAP_SIZE[metricType], - help: `Process heap space size ${metricType} from node.js in bytes.`, + help: `Process heap space size ${metricType} from Node.js in bytes.`, labelNames: ['space'], registers }); diff --git a/lib/metrics/helpers/processMetricsHelpers.js b/lib/metrics/helpers/processMetricsHelpers.js index b2bb26e1..207eead7 100644 --- a/lib/metrics/helpers/processMetricsHelpers.js +++ b/lib/metrics/helpers/processMetricsHelpers.js @@ -19,14 +19,10 @@ function aggregateByObjectName(list) { return data; } -function updateMetrics(gauge, data, includeTimestamp) { +function updateMetrics(gauge, data) { gauge.reset(); for (const key in data) { - if (includeTimestamp) { - gauge.set({ type: key }, data[key], Date.now()); - } else { - gauge.set({ type: key }, data[key]); - } + gauge.set({ type: key }, data[key]); } } diff --git a/lib/metrics/helpers/safeMemoryUsage.js b/lib/metrics/helpers/safeMemoryUsage.js index fc9991e7..b0b44cb5 100644 --- a/lib/metrics/helpers/safeMemoryUsage.js +++ b/lib/metrics/helpers/safeMemoryUsage.js @@ -1,14 +1,11 @@ 'use strict'; function safeMemoryUsage() { - let memoryUsage; try { - memoryUsage = process.memoryUsage(); + return process.memoryUsage(); } catch (ex) { - // empty + return; } - - return memoryUsage; } module.exports = safeMemoryUsage; diff --git a/lib/metrics/osMemoryHeapLinux.js b/lib/metrics/osMemoryHeapLinux.js index 07c0c6d0..7393b485 100644 --- a/lib/metrics/osMemoryHeapLinux.js +++ b/lib/metrics/osMemoryHeapLinux.js @@ -51,18 +51,23 @@ module.exports = (registry, config = {}) => { registers }); + // Sync I/O is often problematic, but /proc isn't really I/O, it a + // virtual filesystem that maps directly to in-kernel data structures + // and never blocks. + // + // Node.js/libuv do this already for process.memoryUsage(), see: + // - https://github.com/libuv/libuv/blob/a629688008694ed8022269e66826d4d6ec688b83/src/unix/linux-core.c#L506-L523 return () => { - fs.readFile('/proc/self/status', 'utf8', (err, status) => { - if (err) { - return; - } - const now = Date.now(); - const structuredOutput = structureOutput(status); + try { + const stat = fs.readFileSync('/proc/self/status', 'utf8'); + const structuredOutput = structureOutput(stat); - residentMemGauge.set(structuredOutput.VmRSS, now); - virtualMemGauge.set(structuredOutput.VmSize, now); - heapSizeMemGauge.set(structuredOutput.VmData, now); - }); + residentMemGauge.set(structuredOutput.VmRSS); + virtualMemGauge.set(structuredOutput.VmSize); + heapSizeMemGauge.set(structuredOutput.VmData); + } catch (er) { + return; + } }; }; diff --git a/lib/metrics/processCpuTotal.js b/lib/metrics/processCpuTotal.js index 277600ee..e00e4bbb 100644 --- a/lib/metrics/processCpuTotal.js +++ b/lib/metrics/processCpuTotal.js @@ -6,11 +6,6 @@ const PROCESS_CPU_SYSTEM_SECONDS = 'process_cpu_system_seconds_total'; const PROCESS_CPU_SECONDS = 'process_cpu_seconds_total'; module.exports = (registry, config = {}) => { - // Don't do anything if the function doesn't exist (introduced in node@6.1.0) - if (typeof process.cpuUsage !== 'function') { - return () => {}; - } - const registers = registry ? [registry] : undefined; const namePrefix = config.prefix ? config.prefix : ''; diff --git a/lib/metrics/processHandles.js b/lib/metrics/processHandles.js index cf876eea..48c268f8 100644 --- a/lib/metrics/processHandles.js +++ b/lib/metrics/processHandles.js @@ -28,19 +28,11 @@ module.exports = (registry, config = {}) => { registers: registry ? [registry] : undefined }); - const updater = config.timestamps - ? () => { - const handles = process._getActiveHandles(); - updateMetrics(gauge, aggregateByObjectName(handles), true); - totalGauge.set(handles.length, Date.now()); - } - : () => { - const handles = process._getActiveHandles(); - updateMetrics(gauge, aggregateByObjectName(handles), false); - totalGauge.set(handles.length); - }; - - return updater; + return () => { + const handles = process._getActiveHandles(); + updateMetrics(gauge, aggregateByObjectName(handles)); + totalGauge.set(handles.length); + }; }; module.exports.metricNames = [ diff --git a/lib/metrics/processMaxFileDescriptors.js b/lib/metrics/processMaxFileDescriptors.js index 032d0582..1b4d7ba8 100644 --- a/lib/metrics/processMaxFileDescriptors.js +++ b/lib/metrics/processMaxFileDescriptors.js @@ -5,55 +5,38 @@ const fs = require('fs'); const PROCESS_MAX_FDS = 'process_max_fds'; -module.exports = (registry, config = {}) => { - let isSet = false; - - if (process.platform !== 'linux') { - return () => {}; - } - - const namePrefix = config.prefix ? config.prefix : ''; - - const fileDescriptorsGauge = new Gauge({ - name: namePrefix + PROCESS_MAX_FDS, - help: 'Maximum number of open file descriptors.', - registers: registry ? [registry] : undefined - }); - - return () => { - if (isSet) { - return; - } - - fs.readFile('/proc/self/limits', 'utf8', (err, limits) => { - if (err) { - return; - } +let maxFds; +module.exports = (registry, config = {}) => { + if (maxFds === undefined) { + // This will fail if a linux-like procfs is not available. + try { + const limits = fs.readFileSync('/proc/self/limits', 'utf8'); const lines = limits.split('\n'); - - let maxFds; lines.find(line => { if (line.startsWith('Max open files')) { const parts = line.split(/ +/); - maxFds = parts[1]; + maxFds = Number(parts[1]); return true; } }); + } catch (er) { + return () => {}; + } + } - if (maxFds === undefined) return; + if (maxFds === undefined) return () => {}; - isSet = true; + const namePrefix = config.prefix ? config.prefix : ''; + const fileDescriptorsGauge = new Gauge({ + name: namePrefix + PROCESS_MAX_FDS, + help: 'Maximum number of open file descriptors.', + registers: registry ? [registry] : undefined + }); - fileDescriptorsGauge.set(Number(maxFds)); + fileDescriptorsGauge.set(Number(maxFds)); - // Only for interal use by tests, so they know when the - // value has been read. - if (config.ready) { - config.ready(); - } - }); - }; + return () => {}; }; module.exports.metricNames = [PROCESS_MAX_FDS]; diff --git a/lib/metrics/processOpenFileDescriptors.js b/lib/metrics/processOpenFileDescriptors.js index 1ebba36d..cc1bd8c8 100644 --- a/lib/metrics/processOpenFileDescriptors.js +++ b/lib/metrics/processOpenFileDescriptors.js @@ -20,14 +20,14 @@ module.exports = (registry, config = {}) => { }); return () => { - fs.readdir('/proc/self/fd', (err, list) => { - if (err) { - return; - } - - // Minus 1, as this invocation created one - fileDescriptorsGauge.set(list.length - 1, Date.now()); - }); + try { + const fds = fs.readdirSync('/proc/self/fd'); + // Minus 1 to not count the fd that was used by readdirSync(), its now + // closed. + fileDescriptorsGauge.set(fds.length - 1); + } catch (er) { + return; + } }; }; diff --git a/lib/metrics/processStartTime.js b/lib/metrics/processStartTime.js index 43158883..742ee28b 100644 --- a/lib/metrics/processStartTime.js +++ b/lib/metrics/processStartTime.js @@ -14,15 +14,9 @@ module.exports = (registry, config = {}) => { registers: registry ? [registry] : undefined, aggregator: 'omit' }); - let isSet = false; + cpuUserGauge.set(startInSeconds); - return () => { - if (isSet) { - return; - } - cpuUserGauge.set(startInSeconds); - isSet = true; - }; + return () => {}; }; module.exports.metricNames = [PROCESS_START_TIME]; diff --git a/lib/metrics/version.js b/lib/metrics/version.js index 40367e38..bb65c1a7 100644 --- a/lib/metrics/version.js +++ b/lib/metrics/version.js @@ -19,22 +19,11 @@ module.exports = (registry, config = {}) => { registers: registry ? [registry] : undefined, aggregator: 'first' }); - let isSet = false; + nodeVersionGauge + .labels(version, versionSegments[0], versionSegments[1], versionSegments[2]) + .set(1); - return () => { - if (isSet) { - return; - } - nodeVersionGauge - .labels( - version, - versionSegments[0], - versionSegments[1], - versionSegments[2] - ) - .set(1); - isSet = true; - }; + return () => {}; }; module.exports.metricNames = [NODE_VERSION_INFO]; diff --git a/lib/registry.js b/lib/registry.js index 2f56427b..6c940ff1 100644 --- a/lib/registry.js +++ b/lib/registry.js @@ -18,6 +18,7 @@ const defaultMetricsOpts = { class Registry { constructor() { this._metrics = {}; + this._collectors = []; this._defaultLabels = {}; } @@ -70,6 +71,8 @@ class Registry { metrics(opts) { let metrics = ''; + this.collect(); + for (const metric of this.getMetricsAsArray()) { metrics += `${this.getMetricAsPrometheusString(metric, opts)}\n\n`; } @@ -77,21 +80,34 @@ class Registry { return metrics.substring(0, metrics.length - 1); } - registerMetric(metricFn) { - if ( - this._metrics[metricFn.name] && - this._metrics[metricFn.name] !== metricFn - ) { + registerMetric(metric) { + if (this._metrics[metric.name] && this._metrics[metric.name] !== metric) { throw new Error( - `A metric with the name ${metricFn.name} has already been registered.` + `A metric with the name ${metric.name} has already been registered.` ); } - this._metrics[metricFn.name] = metricFn; + this._metrics[metric.name] = metric; + } + + registerCollector(collectorFn) { + if (this._collectors.includes(collectorFn)) { + return; // Silently ignore repeated registration. + } + this._collectors.push(collectorFn); + } + + collectors() { + return this._collectors; + } + + collect() { + this._collectors.forEach(collector => collector()); } clear() { this._metrics = {}; + this._collectors = []; this._defaultLabels = {}; } @@ -99,6 +115,8 @@ class Registry { const metrics = []; const defaultLabelNames = Object.keys(this._defaultLabels); + this.collect(); + for (const metric of this.getMetricsAsArray()) { const item = metric.get(); diff --git a/test/defaultMetricsTest.js b/test/defaultMetricsTest.js index 33bcef4e..e777cdca 100644 --- a/test/defaultMetricsTest.js +++ b/test/defaultMetricsTest.js @@ -111,9 +111,14 @@ describe('collectDefaultMetrics', () => { expect(register.getMetricsAsJSON()).toHaveLength(0); expect(registry.getMetricsAsJSON()).toHaveLength(0); - interval = collectDefaultMetrics({ register: registry }); + collectDefaultMetrics(); - expect(register.getMetricsAsJSON()).toHaveLength(0); + expect(register.getMetricsAsJSON()).not.toHaveLength(0); + expect(registry.getMetricsAsJSON()).toHaveLength(0); + + collectDefaultMetrics({ register: registry }); + + expect(register.getMetricsAsJSON()).not.toHaveLength(0); expect(registry.getMetricsAsJSON()).not.toHaveLength(0); }); }); diff --git a/test/metrics/maxFileDescriptorsTest.js b/test/metrics/maxFileDescriptorsTest.js index 84011c54..76a8f96e 100644 --- a/test/metrics/maxFileDescriptorsTest.js +++ b/test/metrics/maxFileDescriptorsTest.js @@ -39,23 +39,19 @@ describe('processMaxFileDescriptors', () => { expect(metrics[0].values).toHaveLength(1); }); - it('should have a reasonable metric value', done => { + it('should have a reasonable metric value', () => { const maxFiles = Number(exec('ulimit -Hn', { encoding: 'utf8' })); expect(register.getMetricsAsJSON()).toHaveLength(0); - processMaxFileDescriptors(register, { ready })(); + processMaxFileDescriptors(register, {})(); - function ready() { - const metrics = register.getMetricsAsJSON(); - - expect(metrics).toHaveLength(1); - expect(metrics[0].values).toHaveLength(1); + const metrics = register.getMetricsAsJSON(); - expect(metrics[0].values[0].value).toBeLessThanOrEqual(maxFiles); - expect(metrics[0].values[0].value).toBeGreaterThan(0); + expect(metrics).toHaveLength(1); + expect(metrics[0].values).toHaveLength(1); - return done(); - } + expect(metrics[0].values[0].value).toBeLessThanOrEqual(maxFiles); + expect(metrics[0].values[0].value).toBeGreaterThan(0); }); } });