feature: update prom-client (#123)

* test(integration): record metric names and ensure descriptions

* feat(metrics): update prom-client for more dimensions

also removes the need for gc-stats, using the Node.JS API instead

* test(integration): use a positive scenario for metric help

Co-authored-by: Mike Tobia <Francois-Esquire@users.noreply.github.com>

__tests__/integration/__snapshots__/one-app.spec.js.snap

@@ -42,6 +42,46 @@ Object {
 }
 `;
 
+exports[`Tests that require Docker setup one-app successfully started metrics has all metrics 1`] = `
+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"`;

__tests__/integration/helpers/testRunner.js

@@ -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
@@ -46,7 +46,8 @@ const setUpTestRunner = async ({ oneAppLocalPortToUse } = {}) => {
           'one-app': {
             ports: [
               `${oneAppLocalPortToUse}:8443`,
-            ],
+              oneAppMetricsLocalPortToUse ? `${oneAppMetricsLocalPortToUse}:3005` : undefined,
+            ].filter(Boolean),
           },
         },
       },

__tests__/integration/one-app.spec.js

@@ -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';
@@ -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',
     };
 
@@ -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 () => {
@@ -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`,

__tests__/server/metricsServer.spec.js

@@ -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;
 
@@ -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', () => {

package.json

@@ -99,8 +99,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",
@@ -160,6 +159,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",
@@ -179,7 +179,6 @@
     }
   },
   "optionalDependencies": {
-    "gc-stats": "^1.4.0",
     "heapdump": "^0.3.15"
   },
   "standard-version": {

prod-sample/docker-compose.yml

@@ -14,6 +14,7 @@ services:
     image: one-app:at-test
     expose:
       - "8443"
+      - "3005"
     volumes:
       - ./one-app/one-app-cert.pem:/opt/cert.pem
       - ./one-app/one-app-privkey.pem:/opt/key.pem

src/server/metricsServer.js

@@ -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();