Skip to content
Merged
43 changes: 41 additions & 2 deletions apps/meteor/server/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { type APIActionHandler, RocketChatAPIRouter } from './router';
import { metrics } from '../lib/metrics';
import { settings } from '../settings';
import { cors } from './v1/middlewares/cors';
import { experimentalWarningMiddleware } from './v1/middlewares/experimental';
import { loggerMiddleware } from './v1/middlewares/logger';
import { metricsMiddleware } from './v1/middlewares/metrics';
import { remoteAddressMiddleware } from './v1/middlewares/remoteAddressMiddleware';
Expand Down Expand Up @@ -42,6 +43,7 @@ const createApi = function _createApi(options: { version?: string; useDefaultAut
export const API: {
api: Router<'/api', any, APIActionHandler>;
v1: APIClass<'/v1'>;
experimental: APIClass<'/experimental'>;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
default: APIClass;
ApiClass: typeof APIClass;
channels?: {
Expand Down Expand Up @@ -73,6 +75,10 @@ export const API: {
version: 'v1',
useDefaultAuth: true,
}),
experimental: createApi({
Comment thread
sampaiodiego marked this conversation as resolved.
version: 'experimental',
useDefaultAuth: true,
}),
default: createApi({}),
};

Expand All @@ -89,14 +95,19 @@ settings.watch<string>('Accounts_CustomFields', (value) => {
}
});

const reloadRoutesToRefreshRateLimiter = () => {
API.v1.reloadRoutesToRefreshRateLimiter();
API.experimental.reloadRoutesToRefreshRateLimiter();
};

settings.watch<number>('API_Enable_Rate_Limiter_Limit_Time_Default', (value) => {
defaultRateLimiterOptions.intervalTimeInMS = value;
API.v1.reloadRoutesToRefreshRateLimiter();
reloadRoutesToRefreshRateLimiter();
});

settings.watch<number>('API_Enable_Rate_Limiter_Limit_Calls_Default', (value) => {
defaultRateLimiterOptions.numRequestsAllowed = value;
API.v1.reloadRoutesToRefreshRateLimiter();
reloadRoutesToRefreshRateLimiter();
});

export const startRestAPI = () => {
Expand All @@ -113,11 +124,39 @@ export const startRestAPI = () => {
activeRequestsGauge: metrics.rocketchatRestApiActiveRequests,
}),
)
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/experimental\//),
Comment thread
sampaiodiego marked this conversation as resolved.
api: API.experimental,
settings,
endpointTimeSummary: metrics.rocketchatRestApi,
endpointTimeHistogram: metrics.rocketchatRestApiSeconds,
responseSizeHistogram: metrics.rocketchatRestApiResponseSizeBytes,
activeRequestsGauge: metrics.rocketchatRestApiActiveRequests,
}),
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.use(
// Catch-all sampler for the default router (`/api/info`, `/api/docs/json`) and for
// unmatched `/api/*` paths, which belong to none of the versioned prefixes above.
// Add any new versioned namespace to `excludePathRegex` as well, or it gets counted twice.
metricsMiddleware({
excludePathRegex: new RegExp(/^\/api\/(v1|experimental|apps)\//),
// `API.default` has no `version`; label it explicitly so the series is not blank.
api: { version: 'default' },
settings,
endpointTimeSummary: metrics.rocketchatRestApi,
endpointTimeHistogram: metrics.rocketchatRestApiSeconds,
responseSizeHistogram: metrics.rocketchatRestApiResponseSizeBytes,
activeRequestsGauge: metrics.rocketchatRestApiActiveRequests,
}),
)
.use(tracerSpanMiddleware)
.use(remoteAddressMiddleware)
.use(experimentalWarningMiddleware({ basePathRegex: new RegExp(/^\/api\/experimental(\/|$)/) }))
.use(cors(settings))
.use(loggerMiddleware(logger))
.use(API.v1.router)
.use(API.experimental.router)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.use(API.default.router).router,
);
};
Expand Down
86 changes: 86 additions & 0 deletions apps/meteor/server/api/v1/middlewares/experimental.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { Router } from '@rocket.chat/http-router';
import Ajv from 'ajv';
import express from 'express';
import request from 'supertest';

import { cors } from './cors';
import { experimentalWarningMiddleware } from './experimental';
import { CachedSettings } from '../../../settings/CachedSettings';

const WARNING_HEADER = '299 - "experimental: endpoint is unstable and may change without notice"';

const buildApp = ({ corsEnabled }: { corsEnabled: boolean }) => {
const ajv = new Ajv();
const settings = new CachedSettings();
settings.set({ _id: 'API_Enable_CORS', value: corsEnabled } as any);
settings.set({ _id: 'API_CORS_Origin', value: 'https://allowed.example' } as any);

const route = (router: Router<any, any, any>) =>
router.get('/test', { response: { 200: ajv.compile({ type: 'object' }) } }, async () => ({
statusCode: 200 as const,
body: {},
}));

const api = new Router('/api')
.use(experimentalWarningMiddleware({ basePathRegex: new RegExp(/^\/api\/experimental(\/|$)/) }))
.use(cors(settings))
.use(route(new Router('/v1')))
.use(route(new Router('/experimental')));

const app = express();
app.use(api.router);
return app;
};

const preflight = (app: express.Express, path: string, origin: string) =>
request(app).options(path).set('Origin', origin).set('Access-Control-Request-Method', 'GET');

describe('Experimental middleware', () => {
it('should stamp the unstable signal headers on experimental responses', async () => {
const res = await request(buildApp({ corsEnabled: true })).get('/api/experimental/test');

expect(res.statusCode).toBe(200);
expect(res.headers['x-experimental']).toBe('true');
expect(res.headers.warning).toBe(WARNING_HEADER);
});

it('should not stamp responses from other versions', async () => {
const res = await request(buildApp({ corsEnabled: true })).get('/api/v1/test');

expect(res.statusCode).toBe(200);
expect(res.headers['x-experimental']).toBeUndefined();
expect(res.headers.warning).toBeUndefined();
});

it('should stamp 404s for unmatched experimental paths', async () => {
const res = await request(buildApp({ corsEnabled: true })).get('/api/experimental/nope');

expect(res.statusCode).toBe(404);
expect(res.headers['x-experimental']).toBe('true');
});

// cors answers rejected preflights without calling next(), so these only carry the headers
// while the middleware stays registered ahead of it
it('should stamp preflight rejections when CORS is disabled', async () => {
const res = await preflight(buildApp({ corsEnabled: false }), '/api/experimental/test', 'https://allowed.example');

expect(res.statusCode).toBe(405);
expect(res.headers['x-experimental']).toBe('true');
expect(res.headers.warning).toBe(WARNING_HEADER);
});

it('should stamp preflight rejections from disallowed origins', async () => {
const res = await preflight(buildApp({ corsEnabled: true }), '/api/experimental/test', 'https://evil.example');

expect(res.statusCode).toBe(403);
expect(res.headers['x-experimental']).toBe('true');
expect(res.headers.warning).toBe(WARNING_HEADER);
});

it('should not stamp preflight rejections from other versions', async () => {
const res = await preflight(buildApp({ corsEnabled: true }), '/api/v1/test', 'https://evil.example');

expect(res.statusCode).toBe(403);
expect(res.headers['x-experimental']).toBeUndefined();
});
});
29 changes: 29 additions & 0 deletions apps/meteor/server/api/v1/middlewares/experimental.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { MiddlewareHandler } from 'hono';

// `x-experimental` is the supported programmatic signal. `Warning: 299` is emitted for
// legacy tooling only — warn code 299 came from RFC 7234, which RFC 9111 has obsoleted
// along with the `Warning` header itself.
const WARNING_HEADER = '299 - "experimental: endpoint is unstable and may change without notice"';

/**
* Stamps every experimental response with the unstable signal headers.
*
* Registered on the shared `/api` mount ahead of `cors`, and scoped by path rather than by
* router: `cors` answers rejected preflights with 403/405 without calling `next()`, so a
* middleware living on `API.experimental.router` would never run for those responses.
*
* The headers are set on `c.res.headers` before the downstream handlers run; Hono merges them
* into whatever response is produced later, so 404s and CORS rejections are covered too.
*/
export const experimentalWarningMiddleware =
({ basePathRegex }: { basePathRegex: RegExp }): MiddlewareHandler =>
async (c, next) => {
if (!basePathRegex.test(c.req.path)) {
return next();
}

c.res.headers.set('x-experimental', 'true');
c.res.headers.set('Warning', WARNING_HEADER);

await next();
};
147 changes: 147 additions & 0 deletions apps/meteor/server/api/v1/middlewares/metrics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,4 +199,151 @@ describe('Metrics middleware', () => {
entrypoint: 'method.call/get:param',
});
});

it('should only record requests matching its own base path', async () => {
const ajv = new Ajv();
const app = express();
const settings = new CachedSettings();

const makeMetrics = () => {
const endTimer = jest.fn();
return {
endTimer,
summary: { startTimer: jest.fn().mockReturnValue(endTimer) },
histogram: { startTimer: jest.fn().mockReturnValue(jest.fn()) },
responseSizeHistogram: { observe: jest.fn() },
activeRequestsGauge: { inc: jest.fn(), dec: jest.fn() },
};
};

const v1Metrics = makeMetrics();
const experimentalMetrics = makeMetrics();

const route = (router: Router<any, any, any>) =>
router.get(
'/test',
{
response: {
200: ajv.compile({
type: 'object',
properties: {
message: { type: 'string' },
},
}),
},
},
async () => ({
statusCode: 200,
body: { message: 'Metrics test successful' },
}),
);

const api = new Router('/api');

api
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/v1\//),
api: { version: 'v1' } as any,
settings,
endpointTimeSummary: v1Metrics.summary as any,
endpointTimeHistogram: v1Metrics.histogram as any,
responseSizeHistogram: v1Metrics.responseSizeHistogram as any,
activeRequestsGauge: v1Metrics.activeRequestsGauge as any,
}),
)
.use(
metricsMiddleware({
basePathRegex: new RegExp(/^\/api\/experimental\//),
api: { version: 'experimental' } as any,
settings,
endpointTimeSummary: experimentalMetrics.summary as any,
endpointTimeHistogram: experimentalMetrics.histogram as any,
responseSizeHistogram: experimentalMetrics.responseSizeHistogram as any,
activeRequestsGauge: experimentalMetrics.activeRequestsGauge as any,
}),
)
.use(route(new Router('/v1')))
.use(route(new Router('/experimental')));

app.use(api.router);

expect((await request(app).get('/api/v1/test')).statusCode).toBe(200);

expect(v1Metrics.summary.startTimer).toHaveBeenCalledTimes(1);
expect(v1Metrics.endTimer).toHaveBeenCalledWith({ status: 200, method: 'get', version: 'v1', entrypoint: 'test' });
expect(experimentalMetrics.summary.startTimer).not.toHaveBeenCalled();
expect(experimentalMetrics.activeRequestsGauge.inc).not.toHaveBeenCalled();

expect((await request(app).get('/api/experimental/test')).statusCode).toBe(200);

expect(experimentalMetrics.summary.startTimer).toHaveBeenCalledTimes(1);
expect(experimentalMetrics.endTimer).toHaveBeenCalledWith({
status: 200,
method: 'get',
version: 'experimental',
entrypoint: 'test',
});
expect(v1Metrics.summary.startTimer).toHaveBeenCalledTimes(1);
});

it('should sample the default router and unmatched paths exactly once', async () => {
const ajv = new Ajv();
const app = express();
const settings = new CachedSettings();

const makeMetrics = () => {
const endTimer = jest.fn();
return {
endTimer,
summary: { startTimer: jest.fn().mockReturnValue(endTimer) },
histogram: { startTimer: jest.fn().mockReturnValue(jest.fn()) },
responseSizeHistogram: { observe: jest.fn() },
activeRequestsGauge: { inc: jest.fn(), dec: jest.fn() },
};
};

const v1Metrics = makeMetrics();
const defaultMetrics = makeMetrics();

const wire = (metrics: ReturnType<typeof makeMetrics>, extra: { basePathRegex?: RegExp; excludePathRegex?: RegExp }, version: string) =>
metricsMiddleware({
...extra,
api: { version },
settings,
endpointTimeSummary: metrics.summary as any,
endpointTimeHistogram: metrics.histogram as any,
responseSizeHistogram: metrics.responseSizeHistogram as any,
activeRequestsGauge: metrics.activeRequestsGauge as any,
});

const api = new Router('/api')
.use(wire(v1Metrics, { basePathRegex: new RegExp(/^\/api\/v1\//) }, 'v1'))
.use(wire(defaultMetrics, { excludePathRegex: new RegExp(/^\/api\/(v1|experimental|apps)\//) }, 'default'));

const route = (router: Router<any, any, any>, subpath: string) =>
router.get(subpath, { response: { 200: ajv.compile({ type: 'object' }) } }, async () => ({ statusCode: 200 as const, body: {} }));

// the catch-all router is mounted first on purpose: the exclusion guard has to hold
// regardless of the order the versioned routers happen to be registered in
api.use(route(new Router(''), 'info'));
api.use(route(new Router('/v1'), '/test'));

expect((await request(app.use(api.router)).get('/api/info')).statusCode).toBe(200);

expect(v1Metrics.summary.startTimer).not.toHaveBeenCalled();
expect(defaultMetrics.endTimer).toHaveBeenCalledWith({ status: 200, method: 'get', version: 'default', entrypoint: '/api/info' });

defaultMetrics.endTimer.mockClear();

expect((await request(app).get('/api/v1/test')).statusCode).toBe(200);

expect(v1Metrics.summary.startTimer).toHaveBeenCalledTimes(1);
expect(defaultMetrics.summary.startTimer).toHaveBeenCalledTimes(1); // still just the /api/info call
expect(defaultMetrics.endTimer).not.toHaveBeenCalled();

expect((await request(app).get('/api/bogus')).statusCode).toBe(404);

expect(defaultMetrics.endTimer).toHaveBeenCalledWith({ status: 404, method: 'get', version: 'default', entrypoint: '/api/*' });
});
});
16 changes: 14 additions & 2 deletions apps/meteor/server/api/v1/middlewares/metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import type { MiddlewareHandler } from 'hono';
import type { Gauge, Histogram, Summary } from 'prom-client';

import type { CachedSettings } from '../../../settings/CachedSettings';
import type { APIClass } from '../../ApiClass';

export const metricsMiddleware =
({
basePathRegex,
excludePathRegex,
api,
settings,
endpointTimeSummary,
Expand All @@ -15,14 +15,26 @@ export const metricsMiddleware =
activeRequestsGauge,
}: {
basePathRegex?: RegExp;
api: APIClass;
excludePathRegex?: RegExp;
api: { version?: string };
settings: CachedSettings;
endpointTimeSummary: Summary;
endpointTimeHistogram: Histogram;
responseSizeHistogram: Histogram;
activeRequestsGauge: Gauge;
}): MiddlewareHandler =>
async (c, next) => {
// Several metrics middlewares share the same `/api` mount (v1, experimental, apps, default), so
// each one has to ignore the paths that belong to the others or a request gets sampled more than
// once. The versioned ones opt in by prefix; the catch-all opts out of the prefixes it does not own.
if (basePathRegex && !basePathRegex.test(c.req.path)) {
Comment thread
sampaiodiego marked this conversation as resolved.
return next();
}

if (excludePathRegex?.test(c.req.path)) {
return next();
}

const rocketchatRestApiEnd = endpointTimeSummary.startTimer();
const rocketchatRestApiHistEnd = endpointTimeHistogram.startTimer();

Expand Down
Loading
Loading