Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

feature: update prom-client #123

Merged
merged 8 commits into from
May 22, 2020
40 changes: 40 additions & 0 deletions __tests__/integration/__snapshots__/one-app.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,46 @@ Object {
}
`;

exports[`Tests that require Docker setup one-app successfully started metrics has all metrics 1`] = `
Copy link
Contributor Author

@PixnBits PixnBits Apr 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd encourage looking at the diff of this file across the two commits to see the difference in metric names

Array [
"circuit",
"circuit_perf",
"nodejs_active_handles",
"nodejs_active_handles_total",
"nodejs_active_requests",
"nodejs_active_requests_total",
"nodejs_eventloop_lag_max_seconds",
"nodejs_eventloop_lag_mean_seconds",
"nodejs_eventloop_lag_min_seconds",
"nodejs_eventloop_lag_p50_seconds",
"nodejs_eventloop_lag_p90_seconds",
"nodejs_eventloop_lag_p99_seconds",
"nodejs_eventloop_lag_seconds",
"nodejs_eventloop_lag_stddev_seconds",
"nodejs_external_memory_bytes",
"nodejs_gc_duration_seconds",
"nodejs_heap_size_total_bytes",
"nodejs_heap_size_used_bytes",
"nodejs_heap_space_size_available_bytes",
"nodejs_heap_space_size_total_bytes",
"nodejs_heap_space_size_used_bytes",
"nodejs_version_info",
"oneapp_holocron_module_map_poll_consecutive_errors_total",
"oneapp_holocron_module_map_poll_total",
"oneapp_holocron_module_map_poll_wait_seconds",
"oneapp_version_info",
"process_cpu_seconds_total",
"process_cpu_system_seconds_total",
"process_cpu_user_seconds_total",
"process_heap_bytes",
"process_max_fds",
"process_open_fds",
"process_resident_memory_bytes",
"process_start_time_seconds",
"process_virtual_memory_bytes",
]
`;

exports[`Tests that require Docker setup one-app successfully started one-app server provides reporting routes client reported errors logs errors when reported to /_/report/errors 1`] = `"reported client error"`;

exports[`Tests that require Docker setup one-app successfully started one-app server provides reporting routes csp-violations reported to server logs violations reported to /_/report/errors 1`] = `"CSP Violation: {\\\\n \\\\\\"csp-report\\\\\\": {\\\\n \\\\\\"document-uri\\\\\\": \\\\\\"bad.example.com"`;
5 changes: 3 additions & 2 deletions __tests__/integration/helpers/testRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const deepMergeObjects = require('../../../src/server/utils/deepMergeObjects');
const prodSampleDir = path.resolve('./prod-sample/');
const pathToDockerComposeTestFile = path.resolve(prodSampleDir, 'docker-compose.test.yml');

const setUpTestRunner = async ({ oneAppLocalPortToUse } = {}) => {
const setUpTestRunner = async ({ oneAppLocalPortToUse, oneAppMetricsLocalPortToUse } = {}) => {
const pathToBaseDockerComposeFile = path.resolve(prodSampleDir, 'docker-compose.yml');
const seleniumServerPort = getRandomPortNumber();
// create docker compose file from base with changes needed for tests
Expand All @@ -46,7 +46,8 @@ const setUpTestRunner = async ({ oneAppLocalPortToUse } = {}) => {
'one-app': {
ports: [
`${oneAppLocalPortToUse}:8443`,
],
oneAppMetricsLocalPortToUse ? `${oneAppMetricsLocalPortToUse}:3005` : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamesSingleton the 3005 port has to be exposed because it is mapped here like from ramdomPort to 3005

].filter(Boolean),
},
},
},
Expand Down
34 changes: 33 additions & 1 deletion __tests__/integration/one-app.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
/* eslint-disable no-underscore-dangle */
import fetch from 'cross-fetch';
import yargs, { argv } from 'yargs';
import parsePrometheusTextFormat from 'parse-prometheus-text-format';

import { setUpTestRunner, tearDownTestRunner } from './helpers/testRunner';
import { waitFor } from './helpers/wait';
Expand Down Expand Up @@ -49,8 +50,10 @@ describe('Tests that require Docker setup', () => {
const defaultFetchOptions = createFetchOptions();
let originalModuleMap;
const oneAppLocalPortToUse = getRandomPortNumber();
const oneAppMetricsLocalPortToUse = getRandomPortNumber();
const appAtTestUrls = {
fetchUrl: `https://localhost:${oneAppLocalPortToUse}`,
fetchMetricsUrl: `http://localhost:${oneAppMetricsLocalPortToUse}/metrics`,
browserUrl: 'https://one-app:8443',
};

Expand All @@ -59,7 +62,7 @@ describe('Tests that require Docker setup', () => {
beforeAll(async () => {
removeModuleFromModuleMap('late-frank');
originalModuleMap = readModuleMap();
({ browser } = await setUpTestRunner({ oneAppLocalPortToUse }));
({ browser } = await setUpTestRunner({ oneAppLocalPortToUse, oneAppMetricsLocalPortToUse }));
});

afterAll(async () => {
Expand All @@ -85,6 +88,35 @@ describe('Tests that require Docker setup', () => {
expect(rawHeaders).not.toHaveProperty('access-control-allow-credentials');
});

describe('metrics', () => {
it('connects', async () => {
expect.assertions(1);
const response = await fetch(appAtTestUrls.fetchMetricsUrl);
expect(response).toHaveProperty('status', 200);
});

it('has all metrics', async () => {
expect.assertions(1);
const response = await fetch(appAtTestUrls.fetchMetricsUrl);
const parsedMetrics = parsePrometheusTextFormat(await response.text());
expect(parsedMetrics.map((metric) => metric.name).sort()).toMatchSnapshot();
});

it('has help information on each metric', async () => {
expect.assertions(1);
const response = await fetch(appAtTestUrls.fetchMetricsUrl);
const parsedMetrics = parsePrometheusTextFormat(await response.text());
const allMetricNames = parsedMetrics
.map((metric) => metric.name);

const metricsNamesWithHelpInfo = parsedMetrics
.filter((metric) => metric.help && metric.help.length > 0)
.map((metric) => metric.name);

expect(metricsNamesWithHelpInfo).toEqual(allMetricNames);
});
});

test('app rejects CORS OPTIONS pre-flight requests for POST', async () => {
const response = await fetch(
`${appAtTestUrls.fetchUrl}/success`,
Expand Down
7 changes: 0 additions & 7 deletions __tests__/server/__snapshots__/metricsServer.spec.js.snap

This file was deleted.

31 changes: 1 addition & 30 deletions __tests__/server/metricsServer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,17 @@ describe('metricsServer', () => {
jest.spyOn(console, 'warn').mockImplementation(() => {});

let client;
let promGcStats;
let healthCheck;
let logging;

function load({ gcStatsError = false } = {}) {
function load() {
jest.resetModules();

jest.mock('prom-client');
if (gcStatsError) {
jest.mock('gc-stats', () => {
throw new Error('unable to resolve gc-stats');
}, { virtual: true });
} else {
jest.mock('gc-stats', () => jest.fn(), { virtual: true });
}
jest.mock('prometheus-gc-stats', () => jest.fn(() => () => {}));
jest.mock('../../src/server/utils/logging/serverMiddleware', () => jest.fn((req, res, next) => next()));
jest.mock('../../src/server/middleware/healthCheck');

client = require('prom-client');
promGcStats = require('prometheus-gc-stats');
logging = require('../../src/server/utils/logging/serverMiddleware');
healthCheck = require('../../src/server/middleware/healthCheck').default;

Expand All @@ -53,25 +43,6 @@ describe('metricsServer', () => {
load();
expect(client.collectDefaultMetrics).toHaveBeenCalledTimes(1);
});

it('collects default metrics every ten seconds', () => {
load();
expect(client.collectDefaultMetrics).toHaveBeenCalledTimes(1);
expect(client.collectDefaultMetrics.mock.calls[0][0]).toHaveProperty('timeout', 10 * 1e3);
});

it('collects garbage collection metrics', () => {
load();
expect(promGcStats).toHaveBeenCalledTimes(1);
expect(promGcStats).toHaveBeenCalledWith(client.register);
});

it('warns if unable to collect garbage collection metrics', () => {
console.warn.mockClear();
load({ gcStatsError: true });
expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn.mock.calls[0]).toMatchSnapshot();
});
});

describe('unknown routes', () => {
Expand Down
21 changes: 18 additions & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@
"opossum": "^5.0.0",
"opossum-prometheus": "^0.1.0",
"pidusage": "^2.0.17",
"prom-client": "^11.5.3",
"prometheus-gc-stats": "^0.6.2",
"prom-client": "^12.0.0",
"prop-types": "^15.7.2",
"react": "^16.13.1",
"react-dom": "^16.13.1",
Expand Down Expand Up @@ -161,6 +160,7 @@
"node-mocks-http": "1.8.0",
"nodemon": "^1.19.4",
"ora": "^4.0.3",
"parse-prometheus-text-format": "^1.1.1",
"react-test-renderer": "^16.13.1",
"rimraf": "^3.0.0",
"rollup": "^2.2.0",
Expand All @@ -180,7 +180,6 @@
}
},
"optionalDependencies": {
"gc-stats": "^1.4.0",
"heapdump": "^0.3.15"
},
"standard-version": {
Expand Down
1 change: 1 addition & 0 deletions prod-sample/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ services:
image: one-app:at-test
expose:
- "8443"
- "3005"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it is the sample, but what expose 3005 now?

volumes:
- ./one-app/one-app-cert.pem:/opt/cert.pem
- ./one-app/one-app-privkey.pem:/opt/key.pem
Expand Down
15 changes: 1 addition & 14 deletions src/server/metricsServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,11 @@
import express from 'express';
import helmet from 'helmet';
import { register as metricsRegister, collectDefaultMetrics } from 'prom-client';
import gcStats from 'prometheus-gc-stats';

import logging from './utils/logging/serverMiddleware';
import healthCheck from './middleware/healthCheck';

// Probe every 10th second.
collectDefaultMetrics({ timeout: 10e3 });
gcStats(metricsRegister)();

// prometheus-gc-stats uses gc-stats but swallows the error if not importable
// try importing ourselves so we can log a warning
try {
/* eslint import/no-extraneous-dependencies: ["error", {"optionalDependencies": true}] */
// eslint-disable-next-line global-require, import/no-unresolved
require('gc-stats');
} catch (err) {
console.warn('Unable to set up garbage collection monitor. This is not an issue for local development.');
}
collectDefaultMetrics();

export function createMetricsServer() {
const app = express();
Expand Down