From 03d01de36406c4a5de6374d16cb568f0f0eb7728 Mon Sep 17 00:00:00 2001 From: Kenn Sippell Date: Wed, 28 Jun 2023 01:18:19 -0700 Subject: [PATCH 1/6] Simplified prom route when using default express route --- src/express-middleware.js | 5 ++ .../express/middleware-default-route-test.js | 56 +++++++++++++++++++ .../server/express-server-default-route.js | 21 +++++++ 3 files changed, 82 insertions(+) create mode 100644 test/integration-tests/express/middleware-default-route-test.js create mode 100644 test/integration-tests/express/server/express-server-default-route.js diff --git a/src/express-middleware.js b/src/express-middleware.js index a1ef3d4..6ae9d43 100644 --- a/src/express-middleware.js +++ b/src/express-middleware.js @@ -57,6 +57,11 @@ class ExpressMiddleware { route = route ? route + req.route.path : req.route.path; } + // #112 - aggregated express default route + if (route === '*') { + return route; + } + if (!route || route === '' || typeof route !== 'string') { route = req.originalUrl.split('?')[0]; } else { diff --git a/test/integration-tests/express/middleware-default-route-test.js b/test/integration-tests/express/middleware-default-route-test.js new file mode 100644 index 0000000..a96dbc2 --- /dev/null +++ b/test/integration-tests/express/middleware-default-route-test.js @@ -0,0 +1,56 @@ +'use strict'; +const { expect } = require('chai'); +const Prometheus = require('prom-client'); +const supertest = require('supertest'); + +let app, config; + +describe('when using express framework (default route)', function() { + this.timeout(4000); + before(() => { + config = require('./server/config'); + config.useUniqueHistogramName = true; + app = require('./server/express-server-default-route'); + }); + after(() => { + Prometheus.register.clear(); + delete require.cache[require.resolve('./server/express-server-default-route.js')]; + delete require.cache[require.resolve('../../../src/index.js')]; + delete require.cache[require.resolve('../../../src/metrics-middleware.js')]; + }); + + describe('when start up with default route', () => { + describe('when calling a GET endpoint', () => { + before(() => { + return supertest(app) + .get('/hello') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="*",code="200"'); + }); + }); + }); + describe('when calling a GET endpoint with one query param', () => { + before(() => { + return supertest(app) + .get('/hello?test=test') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="*",code="200"'); + }); + }); + }); + }); +}); diff --git a/test/integration-tests/express/server/express-server-default-route.js b/test/integration-tests/express/server/express-server-default-route.js new file mode 100644 index 0000000..a0a9dfe --- /dev/null +++ b/test/integration-tests/express/server/express-server-default-route.js @@ -0,0 +1,21 @@ +'use strict'; + +const express = require('express'); +const middleware = require('../../../../src/index.js').expressMiddleware; + +const app = express(); +app.use(middleware({ includeQueryParams: true })); + +app.all('*', (req, res, next) => { + res.status(200); + res.json({ message: 'Hello World!' }); +}); + +// Error handler +app.use((err, req, res, next) => { + res.statusCode = 500; + // Do not expose your error in production + res.json({ error: err.message }); +}); + +module.exports = app; From 86f18b3dd789430e993b2ac8599896b341ae8e6e Mon Sep 17 00:00:00 2001 From: Kenn Sippell Date: Wed, 28 Jun 2023 01:35:18 -0700 Subject: [PATCH 2/6] Simpler test server --- .../express/server/express-server-default-route.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/integration-tests/express/server/express-server-default-route.js b/test/integration-tests/express/server/express-server-default-route.js index a0a9dfe..13c454c 100644 --- a/test/integration-tests/express/server/express-server-default-route.js +++ b/test/integration-tests/express/server/express-server-default-route.js @@ -6,16 +6,9 @@ const middleware = require('../../../../src/index.js').expressMiddleware; const app = express(); app.use(middleware({ includeQueryParams: true })); -app.all('*', (req, res, next) => { +app.all('*', (req, res) => { res.status(200); res.json({ message: 'Hello World!' }); }); -// Error handler -app.use((err, req, res, next) => { - res.statusCode = 500; - // Do not expose your error in production - res.json({ error: err.message }); -}); - module.exports = app; From 849736318cd955fd46dca1da1245f6fb0c7be54f Mon Sep 17 00:00:00 2001 From: Kenn Sippell Date: Wed, 28 Jun 2023 10:58:00 -0700 Subject: [PATCH 3/6] Test case that default route is not always hit --- .../express/middleware-default-route-test.js | 22 +++++++++++++++++-- .../server/express-server-default-route.js | 6 ++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/test/integration-tests/express/middleware-default-route-test.js b/test/integration-tests/express/middleware-default-route-test.js index a96dbc2..f765910 100644 --- a/test/integration-tests/express/middleware-default-route-test.js +++ b/test/integration-tests/express/middleware-default-route-test.js @@ -20,10 +20,10 @@ describe('when using express framework (default route)', function() { }); describe('when start up with default route', () => { - describe('when calling a GET endpoint', () => { + describe('when calling default endpoint', () => { before(() => { return supertest(app) - .get('/hello') + .get('/dne') .expect(200) .then((res) => {}); }); @@ -36,6 +36,24 @@ describe('when using express framework (default route)', function() { }); }); }); + + describe('when calling existing GET endpoint', () => { + before(() => { + return supertest(app) + .get('/hello') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="/hello",code="200"'); + }); + }); + }); + describe('when calling a GET endpoint with one query param', () => { before(() => { return supertest(app) diff --git a/test/integration-tests/express/server/express-server-default-route.js b/test/integration-tests/express/server/express-server-default-route.js index 13c454c..c098caa 100644 --- a/test/integration-tests/express/server/express-server-default-route.js +++ b/test/integration-tests/express/server/express-server-default-route.js @@ -6,9 +6,13 @@ const middleware = require('../../../../src/index.js').expressMiddleware; const app = express(); app.use(middleware({ includeQueryParams: true })); +app.get('/hello', (req, res) => { + res.json({ message: 'Hello World!' }); +}) + app.all('*', (req, res) => { res.status(200); - res.json({ message: 'Hello World!' }); + res.json({ message: 'Default' }); }); module.exports = app; From 0d96f1795afbe52e8d8e6eb5f940cdc8c20f47e9 Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 28 Oct 2025 16:02:30 -0600 Subject: [PATCH 4/6] Tests for cases discussed in PR --- .../express/middleware-default-route-test.js | 34 +++++++++++++++++++ .../server/express-server-default-route.js | 10 +++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/test/integration-tests/express/middleware-default-route-test.js b/test/integration-tests/express/middleware-default-route-test.js index f765910..4467071 100644 --- a/test/integration-tests/express/middleware-default-route-test.js +++ b/test/integration-tests/express/middleware-default-route-test.js @@ -54,6 +54,40 @@ describe('when using express framework (default route)', function() { }); }); + describe('when calling a GET endpoint with route param', () => { + before(() => { + return supertest(app) + .get('/hello/test') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="/hello/:val",code="200"'); + }); + }); + }); + + describe('when calling a GET endpoint with * in route', () => { + before(() => { + return supertest(app) + .get('/hellllllo') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="/he*o",code="200"'); + }); + }); + }); + describe('when calling a GET endpoint with one query param', () => { before(() => { return supertest(app) diff --git a/test/integration-tests/express/server/express-server-default-route.js b/test/integration-tests/express/server/express-server-default-route.js index c098caa..e16e4ab 100644 --- a/test/integration-tests/express/server/express-server-default-route.js +++ b/test/integration-tests/express/server/express-server-default-route.js @@ -8,7 +8,15 @@ app.use(middleware({ includeQueryParams: true })); app.get('/hello', (req, res) => { res.json({ message: 'Hello World!' }); -}) +}); + +app.get('/he*o', (req, res) => { + res.json({ message: 'Hello World!' }); +}); + +app.get('/hello/:val', (req, res) => { + res.json({ message: 'Hello World!' }); +}); app.all('*', (req, res) => { res.status(200); From 76f5179b747db7bb0eded2b9cc68095bd1f6fa0b Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 28 Oct 2025 16:04:24 -0600 Subject: [PATCH 5/6] One more case --- .../express/middleware-default-route-test.js | 17 +++++++++++++++++ .../server/express-server-default-route.js | 6 +++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/test/integration-tests/express/middleware-default-route-test.js b/test/integration-tests/express/middleware-default-route-test.js index 4467071..51a277d 100644 --- a/test/integration-tests/express/middleware-default-route-test.js +++ b/test/integration-tests/express/middleware-default-route-test.js @@ -88,6 +88,23 @@ describe('when using express framework (default route)', function() { }); }); + describe('when calling a GET endpoint with * in route and route param', () => { + before(() => { + return supertest(app) + .get('/hellllllo/val') + .expect(200) + .then((res) => {}); + }); + it('should add it to the histogram', () => { + return supertest(app) + .get('/metrics') + .expect(200) + .then((res) => { + expect(res.text).to.contain('method="GET",route="/he*o/:val",code="200"'); + }); + }); + }); + describe('when calling a GET endpoint with one query param', () => { before(() => { return supertest(app) diff --git a/test/integration-tests/express/server/express-server-default-route.js b/test/integration-tests/express/server/express-server-default-route.js index e16e4ab..e34c299 100644 --- a/test/integration-tests/express/server/express-server-default-route.js +++ b/test/integration-tests/express/server/express-server-default-route.js @@ -10,11 +10,15 @@ app.get('/hello', (req, res) => { res.json({ message: 'Hello World!' }); }); +app.get('/hello/:val', (req, res) => { + res.json({ message: 'Hello World!' }); +}); + app.get('/he*o', (req, res) => { res.json({ message: 'Hello World!' }); }); -app.get('/hello/:val', (req, res) => { +app.get('/he*o/:val', (req, res) => { res.json({ message: 'Hello World!' }); }); From 37fb0a25dda4690b85821c9a897e3cb610e75a9a Mon Sep 17 00:00:00 2001 From: kennsippell Date: Tue, 28 Oct 2025 16:07:53 -0600 Subject: [PATCH 6/6] Avoid nest nonsense --- test/integration-tests/express/middleware-default-route-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration-tests/express/middleware-default-route-test.js b/test/integration-tests/express/middleware-default-route-test.js index 51a277d..4c24385 100644 --- a/test/integration-tests/express/middleware-default-route-test.js +++ b/test/integration-tests/express/middleware-default-route-test.js @@ -91,7 +91,7 @@ describe('when using express framework (default route)', function() { describe('when calling a GET endpoint with * in route and route param', () => { before(() => { return supertest(app) - .get('/hellllllo/val') + .get('/hellllllo/variable') .expect(200) .then((res) => {}); });