From 51e8511d3cf475adfef979750d3f29d780b06e6c Mon Sep 17 00:00:00 2001 From: Diego Sampaio Date: Fri, 23 May 2025 22:10:51 +0000 Subject: [PATCH] fix: prometheus' REST API metrics (#36070) --- .changeset/silent-shoes-repeat.md | 5 + apps/meteor/app/api/server/api.ts | 2 +- .../api/server/middlewares/metrics.spec.ts | 106 +++++++++++++++++- .../app/api/server/middlewares/metrics.ts | 31 +++-- 4 files changed, 132 insertions(+), 12 deletions(-) create mode 100644 .changeset/silent-shoes-repeat.md diff --git a/.changeset/silent-shoes-repeat.md b/.changeset/silent-shoes-repeat.md new file mode 100644 index 0000000000000..7859758cdf463 --- /dev/null +++ b/.changeset/silent-shoes-repeat.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixes an issue with REST API metrics sent to Prometheus not grouping endpoints by route diff --git a/apps/meteor/app/api/server/api.ts b/apps/meteor/app/api/server/api.ts index 2898624f191cf..1739aaa74d3f2 100644 --- a/apps/meteor/app/api/server/api.ts +++ b/apps/meteor/app/api/server/api.ts @@ -1217,7 +1217,7 @@ export const startRestAPI = () => { .use(remoteAddressMiddleware) .use(cors(settings)) .use(loggerMiddleware(logger)) - .use(metricsMiddleware(API.v1, settings, metrics.rocketchatRestApi)) + .use(metricsMiddleware({ basePathRegex: new RegExp(/^\/api\/v1\//), api: API.v1, settings, summary: metrics.rocketchatRestApi })) .use(tracerSpanMiddleware) .use(API.v1.router) .use(API.default.router).router, diff --git a/apps/meteor/app/api/server/middlewares/metrics.spec.ts b/apps/meteor/app/api/server/middlewares/metrics.spec.ts index f5974c135e2f8..59a1b00f55606 100644 --- a/apps/meteor/app/api/server/middlewares/metrics.spec.ts +++ b/apps/meteor/app/api/server/middlewares/metrics.spec.ts @@ -25,7 +25,7 @@ describe('Metrics middleware', () => { const mockEndTimer = jest.fn(); summary.startTimer.mockReturnValue(mockEndTimer); - api.use(metricsMiddleware({ version: 1 } as any, settings, summary as any)).get( + api.use(metricsMiddleware({ api: { version: 1 } as any, settings, summary: summary as any })).get( '/test', { response: { @@ -52,6 +52,108 @@ describe('Metrics middleware', () => { expect(response.body).toHaveProperty('message', 'Metrics test successful'); expect(summary.startTimer).toHaveBeenCalledTimes(1); - expect(mockEndTimer).toHaveBeenCalledWith({ status: 200 }); + expect(mockEndTimer).toHaveBeenCalledWith({ status: 200, method: 'get', version: 1, user_agent: 'test', entrypoint: '/api/test' }); + }); + + it('should strip path from metrics', async () => { + const ajv = new Ajv(); + const app = express(); + const api = new Router('/api'); + const settings = new CachedSettings(); + settings.set({ + _id: 'Prometheus_API_User_Agent', + value: true, + } as any); + + const summary = { + startTimer: jest.fn().mockImplementation(() => jest.fn()), + }; + + // Get the mock startTimer function + const mockEndTimer = jest.fn(); + summary.startTimer.mockReturnValue(mockEndTimer); + + api + .use(metricsMiddleware({ basePathRegex: new RegExp(/^\/api\//), api: { version: 1 } as any, settings, summary: summary as any })) + .get( + '/test', + { + response: { + 200: ajv.compile({ + type: 'object', + properties: { + message: { type: 'string' }, + }, + }), + }, + }, + async () => { + return { + statusCode: 200, + body: { + message: 'Metrics test successful', + }, + }; + }, + ); + app.use(api.router); + const response = await request(app).get('/api/test').set('user-agent', 'test'); + expect(response.statusCode).toBe(200); + expect(response.body).toHaveProperty('message', 'Metrics test successful'); + + expect(summary.startTimer).toHaveBeenCalledTimes(1); + expect(mockEndTimer).toHaveBeenCalledWith({ status: 200, method: 'get', version: 1, user_agent: 'test', entrypoint: 'test' }); + }); + + it('should decode path for method.call endpoints', async () => { + const ajv = new Ajv(); + const app = express(); + const settings = new CachedSettings(); + + const api = new Router('/api'); + + const summary = { + startTimer: jest.fn().mockImplementation(() => jest.fn()), + }; + + // Get the mock startTimer function + const mockEndTimer = jest.fn(); + summary.startTimer.mockReturnValue(mockEndTimer); + + api + .use(metricsMiddleware({ basePathRegex: new RegExp(/^\/api\//), api: { version: 1 } as any, settings, summary: summary as any })) + .get( + '/method.call/:id', + { + response: { + 200: ajv.compile({ + type: 'object', + properties: { + message: { type: 'string' }, + }, + }), + }, + }, + async () => { + return { + statusCode: 200, + body: { + message: `Metrics test successful`, + }, + }; + }, + ); + app.use(api.router); + const response = await request(app).get('/api/method.call/get%3Aparam').set('user-agent', 'test'); + expect(response.statusCode).toBe(200); + expect(response.body).toHaveProperty('message', 'Metrics test successful'); + + expect(summary.startTimer).toHaveBeenCalledTimes(1); + expect(mockEndTimer).toHaveBeenCalledWith({ + status: 200, + method: 'get', + version: 1, + entrypoint: 'method.call/get:param', + }); }); }); diff --git a/apps/meteor/app/api/server/middlewares/metrics.ts b/apps/meteor/app/api/server/middlewares/metrics.ts index 518febc7132a4..3557146e6c574 100644 --- a/apps/meteor/app/api/server/middlewares/metrics.ts +++ b/apps/meteor/app/api/server/middlewares/metrics.ts @@ -5,19 +5,32 @@ import type { CachedSettings } from '../../../settings/server/CachedSettings'; import type { APIClass } from '../api'; export const metricsMiddleware = - (api: APIClass, settings: CachedSettings, summary: Summary): MiddlewareHandler => + ({ + basePathRegex, + api, + settings, + summary, + }: { + basePathRegex?: RegExp; + api: APIClass; + settings: CachedSettings; + summary: Summary; + }): MiddlewareHandler => async (c, next) => { - const { method, path } = c.req; - - const rocketchatRestApiEnd = summary.startTimer({ - method, - version: api.version, - ...(settings.get('Prometheus_API_User_Agent') && { user_agent: c.req.header('user-agent') }), - entrypoint: path.startsWith('method.call') ? decodeURIComponent(c.req.url.slice(8)) : path, - }); + const rocketchatRestApiEnd = summary.startTimer(); await next(); + + const { method, path, routePath } = c.req; + + // get rid of the base path (i.e.: /api/v1/) + const entrypoint = basePathRegex ? routePath.replace(basePathRegex, '') : routePath; + rocketchatRestApiEnd({ status: c.res.status, + method: method.toLowerCase(), + version: api.version, + ...(settings.get('Prometheus_API_User_Agent') && { user_agent: c.req.header('user-agent') }), + entrypoint: basePathRegex && entrypoint.startsWith('method.call') ? decodeURIComponent(path.replace(basePathRegex, '')) : entrypoint, }); };