Skip to content

Commit

Permalink
Remove all default healthchecks
Browse files Browse the repository at this point in the history
This is a change that just kept on giving!

So I started out wanting to remove the health check that verifies
whether next-metrics is configured properly. We don't need this any
more, I couldn't care less if an app stops sending metrics 🙈 we will
rethink this in future, the right way is probably a heartbeat metric in
OpenTelemetry.

Anyway, once I removed the metrics health check there were no more
default checks. This means there are no more ticking checks at all as
far as I can tell. This allowed me to remove a bunch of redundant code
from the internals.

I tested this by symlinking it into several different n-express apps and
everything starts up fine, including for apps that define their own
healthchecks.
  • Loading branch information
rowanmanning committed Aug 22, 2024
1 parent 8dddb67 commit 3dd435f
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 87 deletions.
5 changes: 2 additions & 3 deletions main.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* @typedef {import("./typings/n-express").Callback} Callback
* @typedef {import("./typings/n-express").AppOptions} AppOptions
* @typedef {import("./typings/n-express").AppContainer} AppContainer
* @typedef {import("./typings/metrics").TickingMetric} TickingMetric
*/

require('isomorphic-fetch');
Expand All @@ -26,7 +25,7 @@ const metrics = require('next-metrics');
const logger = require('@dotcom-reliability-kit/logger');

// utils
const healthChecks = require('./src/lib/health-checks');
const setupHealthEndpoint = require('./src/lib/health-checks');
const InstrumentListen = require('./src/lib/instrument-listen');
const guessAppDetails = require('./src/lib/guess-app-details');

Expand Down Expand Up @@ -105,7 +104,7 @@ const getAppContainer = (options) => {
app.use(vary);

if (!options.demo) {
instrumentListen.addMetrics(healthChecks(app, options, meta));
setupHealthEndpoint(app, options, meta);
}

// Debug related headers.
Expand Down
27 changes: 2 additions & 25 deletions src/lib/health-checks.js
Original file line number Diff line number Diff line change
@@ -1,47 +1,26 @@
/**
* @typedef {import("express").Application} ExpressApp
* @typedef {import("../../typings/metrics").Healthcheck} Healthcheck
* @typedef {import("../../typings/metrics").TickingMetric} TickingMetric
* @typedef {import("../../typings/n-express").AppOptions} AppOptions
*/

const logger = require('@dotcom-reliability-kit/logger');

const metricsHealthCheck = require('./metrics-healthcheck');

/**
* @param {ExpressApp} app
* @param {AppOptions} options
* @param {{name: string, description: string}} meta
* @returns {(Healthcheck & TickingMetric)[]}
* @returns {void}
*/
module.exports = (app, options, meta) => {
const defaultAppName = `Next FT.com ${meta.name} in ${
process.env.REGION || 'unknown region'
}`;
/**
* Add checks to this array if they use an interval or similar
* to poll for data. This allows them to be properly stopped
* alongside the n-express app.
*
* @type {(Healthcheck & TickingMetric)[]}
*/
const tickingMetricChecks = [];

/** @type {Healthcheck[]} */
const defaultChecks = [
...tickingMetricChecks,
metricsHealthCheck(meta.name)
];

/** @type {Healthcheck[]} */
const healthChecks = options.healthChecks.concat(defaultChecks);

app.get(
/\/__health(?:\.([123]))?$/,
/** @type {ExpressApp} */ (req, res) => {
res.set({ 'Cache-Control': 'private, no-cache, max-age=0' });
const checks = healthChecks.map((check) => check.getStatus());
const checks = options.healthChecks.map((check) => check.getStatus());

checks.forEach(check => {if(!check.id){
logger.warn({
Expand Down Expand Up @@ -86,6 +65,4 @@ module.exports = (app, options, meta) => {
);
}
);

return tickingMetricChecks;
};
11 changes: 0 additions & 11 deletions src/lib/instrument-listen.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
/**
* @typedef {import("../../typings/metrics").TickingMetric} TickingMetric
*/
// @ts-nocheck

const metrics = require('next-metrics');
Expand All @@ -18,8 +15,6 @@ const readFile = denodeify(fs.readFile);
module.exports = class InstrumentListen {
constructor (app, meta, initPromises) {
this.app = app;
/** @type {TickingMetric[]} */
this.tickingMetrics = [];
this.server = null;
this.initApp(meta, initPromises);
}
Expand Down Expand Up @@ -118,15 +113,9 @@ module.exports = class InstrumentListen {
* Attempts to clean up the ticking checks and close the server
*/
this.app.close = (callback) => {
this.tickingMetrics.forEach(check => check.stop());
if (this.server) {
this.server.close(() => callback && callback());
}
};
}

addMetrics (item) {
const items = (Array.isArray(item) ? item : [item]);
this.tickingMetrics.push(...items);
}
};
27 changes: 0 additions & 27 deletions src/lib/metrics-healthcheck.js

This file was deleted.

17 changes: 0 additions & 17 deletions test/app/clear-interval.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,9 @@ const express = require('express');
const nextExpress = require('../../main');
const InstrumentListen = require('../../src/lib/instrument-listen');
const expect = require('chai').expect;
const healthChecks = require('../../src/lib/health-checks');
const sinon = require('sinon');

describe('clears intervals', () => {
it('should return an array of objects with stop functions in health-checks', () => {
const app = express();

const checks = healthChecks(app, {healthChecks: []}, {});
expect(checks).to.be.an('array');
for (const check of checks) {
expect(check.stop).to.be.a('function');
}

//ensure the start() function is called before the stop(),
//because healthChecks(...) eventually calls n-health runCheck(), but runCheck() doesn't await on start()
setTimeout(() => {
checks.forEach(check => check.stop());
}, 0);
});

it('should close server in app.close if there is a live server', () => {
const app = express();
const instrumentListen = new InstrumentListen(app, {}, [Promise.resolve()]);
Expand Down
4 changes: 0 additions & 4 deletions typings/metrics.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ export interface HealthcheckStatus {
technicalSummary: string;
}

export type TickingMetric = {
stop: () => void;
}

export type Healthcheck = {
getStatus: () => HealthcheckStatus;
};

0 comments on commit 3dd435f

Please sign in to comment.