From 2f35db4087bb0e4dcf6d6c0556896a01cdd763de Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 16 Sep 2020 16:45:56 -0400 Subject: [PATCH] Fix startup crash Version 3.2.0 crashes on startup because it calls buildPromKey() without arguments. To avoid this kind of obvious bug to happen again, I added some basic acceptance tests. --- libraries/metrics/.circleci/config.yml | 11 ++ libraries/metrics/index.js | 2 +- libraries/metrics/package.json | 4 +- .../metrics/test/acceptance/metrics_tests.js | 115 ++++++++++++++++++ 4 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 libraries/metrics/.circleci/config.yml create mode 100644 libraries/metrics/test/acceptance/metrics_tests.js diff --git a/libraries/metrics/.circleci/config.yml b/libraries/metrics/.circleci/config.yml new file mode 100644 index 0000000000..d8eed2a65a --- /dev/null +++ b/libraries/metrics/.circleci/config.yml @@ -0,0 +1,11 @@ +version: 2.1 + +orbs: + node: circleci/node@3.0.0 + +workflows: + test: + jobs: + - node/test: + version: "10.22" + override-ci-command: npm install diff --git a/libraries/metrics/index.js b/libraries/metrics/index.js index 7ebd876c37..c76ab35c3c 100644 --- a/libraries/metrics/index.js +++ b/libraries/metrics/index.js @@ -28,7 +28,7 @@ function configure(opts = {}) { */ function initialize(_name, opts = {}) { configure({ ...opts, appName: _name }) - collectDefaultMetrics({ timeout: 5000, prefix: buildPromKey() }) + collectDefaultMetrics({ timeout: 5000, prefix: '' }) console.log(`ENABLE_TRACE_AGENT set to ${process.env.ENABLE_TRACE_AGENT}`) if (process.env.ENABLE_TRACE_AGENT === 'true') { diff --git a/libraries/metrics/package.json b/libraries/metrics/package.json index b73f17358e..b79b74b320 100644 --- a/libraries/metrics/package.json +++ b/libraries/metrics/package.json @@ -35,7 +35,9 @@ }, "scripts": { "lint": "eslint --max-warnings 0 .", - "test": "mocha --reporter spec --recursive --exit --grep=$MOCHA_GREP test/unit", + "test:unit": "mocha --reporter spec --recursive --exit --grep=$MOCHA_GREP test/unit", + "test:acceptance": "mocha --reporter spec --recursive --exit --grep=$MOCHA_GREP test/acceptance", + "test": "npm run test:unit && npm run test:acceptance", "format": "prettier-eslint $PWD'/**/*.js' --list-different", "format:fix": "prettier-eslint $PWD'/**/*.js' --write" } diff --git a/libraries/metrics/test/acceptance/metrics_tests.js b/libraries/metrics/test/acceptance/metrics_tests.js new file mode 100644 index 0000000000..e7f6cdd6cc --- /dev/null +++ b/libraries/metrics/test/acceptance/metrics_tests.js @@ -0,0 +1,115 @@ +const os = require('os') +const { expect } = require('chai') +const Metrics = require('../..') + +const HOSTNAME = os.hostname() +const APP_NAME = 'test-app' + +describe('Metrics module', function() { + before(function() { + Metrics.initialize(APP_NAME) + }) + + describe('at startup', function() { + it('increments the process_startup counter', async function() { + await expectMetricValue('process_startup', 1) + }) + + it('collects default metrics', async function() { + const metric = await getMetric('process_cpu_user_seconds_total') + expect(metric).to.exist + }) + }) + + describe('inc()', function() { + it('increments counts by 1', async function() { + Metrics.inc('duck_count') + await expectMetricValue('duck_count', 1) + Metrics.inc('duck_count') + Metrics.inc('duck_count') + await expectMetricValue('duck_count', 3) + }) + + it('escapes special characters in the key', async function() { + Metrics.inc('show.me the $!!') + await expectMetricValue('show_me_the____', 1) + }) + }) + + describe('count()', function() { + it('increments counts by the given count', async function() { + Metrics.count('rabbit_count', 5) + await expectMetricValue('rabbit_count', 5) + Metrics.count('rabbit_count', 6) + Metrics.count('rabbit_count', 7) + await expectMetricValue('rabbit_count', 18) + }) + }) + + describe('summary()', function() { + it('collects observations', async function() { + Metrics.summary('oven_temp', 200) + Metrics.summary('oven_temp', 300) + Metrics.summary('oven_temp', 450) + const sum = await getSummarySum('oven_temp') + expect(sum).to.equal(950) + }) + }) + + describe('timing()', function() { + it('collects timings', async function() { + Metrics.timing('sprint_100m', 10) + Metrics.timing('sprint_100m', 20) + Metrics.timing('sprint_100m', 30) + const sum = await getSummarySum('timer_sprint_100m') + expect(sum).to.equal(60) + }) + }) + + describe('gauge()', function() { + it('records values', async function() { + Metrics.gauge('water_level', 1.5) + await expectMetricValue('water_level', 1.5) + Metrics.gauge('water_level', 4.2) + await expectMetricValue('water_level', 4.2) + }) + }) + + describe('globalGauge()', function() { + it('records values without a host label', async function() { + Metrics.globalGauge('tire_pressure', 99.99) + const { value, labels } = await getMetricValue('tire_pressure') + expect(value).to.equal(99.99) + expect(labels.host).to.equal('') + expect(labels.app).to.equal(APP_NAME) + }) + }) +}) + +function getMetric(key) { + return Metrics.register.getSingleMetric(key) +} + +async function getSummarySum(key) { + const metric = getMetric(key) + const item = await metric.get() + for (const value of item.values) { + if (value.metricName === `${key}_sum`) { + return value.value + } + } + return null +} + +async function getMetricValue(key) { + const metric = getMetric(key) + const item = await metric.get() + return item.values[0] +} + +async function expectMetricValue(key, expectedValue) { + const value = await getMetricValue(key) + expect(value.value).to.equal(expectedValue) + expect(value.labels.host).to.equal(HOSTNAME) + expect(value.labels.app).to.equal(APP_NAME) +}