From ea9f34aa3e361cb312ca4b7ff24b5f5d23cb6dd3 Mon Sep 17 00:00:00 2001 From: Henrik Karlsson Date: Fri, 9 Dec 2022 17:19:10 +0100 Subject: [PATCH 1/2] Add new option bypassOnFinish, for when response is required in bypass callback --- README.md | 1 + spec/index.spec.js | 38 ++++++++++++++++++++++++++++++++++++++ src/index.js | 4 ++++ types/index.d.ts | 1 + 4 files changed, 44 insertions(+) diff --git a/README.md b/README.md index 88f6404..86133a6 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ Which labels to include in `http_request_duration_seconds` metric: * **metricsPath**: replace the `/metrics` route with a **regex** or exact **string**. Note: it is highly recommended to just stick to the default * **metricType**: histogram/summary selection. See more details below * **bypass**: function taking express request as an argument and determines whether the given request should be excluded in the metrics, default: **() => false** +* **bypassOnFinish**: function taking express request and response as arguments and determines whether the given request should be excluded in the metrics, default: **() => false**. Prefer using **bypass** if you don't need the response object. * **httpDurationMetricName**: Allows you change the name of HTTP duration metric, default: **`http_request_duration_seconds`**. ### metricType option ### diff --git a/spec/index.spec.js b/spec/index.spec.js index 3236696..181457a 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -240,6 +240,44 @@ describe('index', () => { }); }); + it('bypass requests, checking res', done => { + const app = express(); + const instance = bundle({ + bypassOnFinish: (req, res) => res.statusCode === 404, + }); + app.use(instance); + app.use('/200', (req, res) => res.send('')); + app.use('/404', (req, res) => res.status(404).send('')); + app.use('/500', (req, res) => res.status(500).send('')); + + const agent = supertest(app); + agent.get('/200') + .expect(200) + .then(() => { + return agent + .get('/404') + .expect(404); + }) + .then(() => { + return agent + .get('/500') + .expect(500); + }) + .then(() => { + const metricHashMap = instance.metrics.http_request_duration_seconds.hashMap; + + // only /200 and /500 should be counted + expect(metricHashMap['status_code:200'].count).toBe(1); + expect(metricHashMap['status_code:404']).not.toBeDefined(); + expect(metricHashMap['status_code:500'].count).toBe(1); + + return agent + .get('/metrics') + .expect(200); + }) + .then(done); + }); + it('complains about deprecated options', () => { expect(() => bundle({prefix: 'hello'})).toThrow(); }); diff --git a/src/index.js b/src/index.js index 88e80bc..bad1eeb 100644 --- a/src/index.js +++ b/src/index.js @@ -182,6 +182,10 @@ function main(opts) { const timer = metrics[httpMetricName].startTimer(labels); onFinished(res, () => { + if (opts.bypassOnFinish && opts.bypassOnFinish(req, res)) { + return; + } + if (opts.includeStatusCode) { labels.status_code = opts.formatStatusCode(res, opts); } diff --git a/types/index.d.ts b/types/index.d.ts index 6bdc5a7..bec8583 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -28,6 +28,7 @@ declare namespace express_prom_bundle { includeUp?: boolean; bypass?: (req: Request) => boolean; + bypassOnFinish?: (req: Request, res: Response) => boolean; excludeRoutes?: Array; From 1171fb5be17955816744fd80b219d8fb3fe4d5f2 Mon Sep 17 00:00:00 2001 From: Henrik Karlsson Date: Wed, 14 Dec 2022 17:00:11 +0100 Subject: [PATCH 2/2] Combined bypass and bypassOnFinish into one option --- README.md | 15 +++++++++++++-- spec/index.spec.js | 4 +++- src/index.js | 12 ++++++++++-- types/index.d.ts | 8 ++++++-- 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 86133a6..187566a 100644 --- a/README.md +++ b/README.md @@ -52,8 +52,6 @@ Which labels to include in `http_request_duration_seconds` metric: * **includeUp**: include an auxiliary "up"-metric which always returns 1, default: **true** * **metricsPath**: replace the `/metrics` route with a **regex** or exact **string**. Note: it is highly recommended to just stick to the default * **metricType**: histogram/summary selection. See more details below -* **bypass**: function taking express request as an argument and determines whether the given request should be excluded in the metrics, default: **() => false** -* **bypassOnFinish**: function taking express request and response as arguments and determines whether the given request should be excluded in the metrics, default: **() => false**. Prefer using **bypass** if you don't need the response object. * **httpDurationMetricName**: Allows you change the name of HTTP duration metric, default: **`http_request_duration_seconds`**. ### metricType option ### @@ -91,6 +89,19 @@ Additional options for **summary**: see [advanced example](https://github.com/jochen-schweizer/express-prom-bundle/blob/master/advanced-example.js) * **promRegistry**: Optional `promClient.Registry` instance to attach metrics to. Defaults to global `promClient.register`. * **metricsApp**: Allows you to attach the metrics endpoint to a different express app. You probably want to use it in combination with `autoregister: false`. +* **bypass**: An object that takes onRequest and onFinish callbacks that determines whether the given request should be excluded in the metrics. Default: + + ```js + { + onRequest: (req) => false, + onFinish: (req, res) => false + } + ``` + + `onRequest` is run directly in the middleware chain, before the request is processed. `onFinish` is run after the request has been processed, and has access to the express response object in addition to the request object. Both callbacks are optional, and if one or both returns true the request is excluded. + + As a shorthand, just the onRequest callback can be used instead of the object. + ### More details on includePath option diff --git a/spec/index.spec.js b/spec/index.spec.js index 181457a..1e09568 100644 --- a/spec/index.spec.js +++ b/spec/index.spec.js @@ -243,7 +243,9 @@ describe('index', () => { it('bypass requests, checking res', done => { const app = express(); const instance = bundle({ - bypassOnFinish: (req, res) => res.statusCode === 404, + bypass: { + onFinish: (req, res) => res.statusCode === 404 + } }); app.use(instance); app.use('/200', (req, res) => res.send('')); diff --git a/src/index.js b/src/index.js index bad1eeb..0c96dfb 100644 --- a/src/index.js +++ b/src/index.js @@ -161,6 +161,14 @@ function main(opts) { const metricsMatch = opts.metricsPath instanceof RegExp ? opts.metricsPath : new RegExp('^' + (opts.metricsPath || '/metrics') + '/?$'); + if (typeof opts.bypass === 'function') { + opts.bypass = { + onRequest: opts.bypass + }; + } else if (!opts.bypass) { + opts.bypass = {}; + } + const middleware = function (req, res, next) { const path = req.originalUrl || req.url; // originalUrl gets lost in koa-connect? @@ -170,7 +178,7 @@ function main(opts) { // bypass() is checked only after /metrics was processed // if you wish to disable /metrics use autoregister:false instead - if (opts.bypass && opts.bypass(req)) { + if (opts.bypass.onRequest && opts.bypass.onRequest(req)) { return next(); } @@ -182,7 +190,7 @@ function main(opts) { const timer = metrics[httpMetricName].startTimer(labels); onFinished(res, () => { - if (opts.bypassOnFinish && opts.bypassOnFinish(req, res)) { + if (opts.bypass.onFinish && opts.bypass.onFinish(req, res)) { return; } diff --git a/types/index.d.ts b/types/index.d.ts index bec8583..a024a89 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -27,8 +27,12 @@ declare namespace express_prom_bundle { includePath?: boolean; includeUp?: boolean; - bypass?: (req: Request) => boolean; - bypassOnFinish?: (req: Request, res: Response) => boolean; + bypass?: + | ((req: Request) => boolean) + | { + onRequest?: (req: Request) => boolean; + onFinish?: (req: Request, res: Response) => boolean; + }; excludeRoutes?: Array;