add Prometheus best-practices metric duplicates (Histograms + _total counters)"

Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com>

Author: Copilot

apps/meteor/app/api/server/api.ts

@@ -105,7 +105,7 @@ export const startRestAPI = () => {
 			.use(remoteAddressMiddleware)
 			.use(cors(settings))
 			.use(loggerMiddleware(logger))
-			.use(metricsMiddleware({ basePathRegex: new RegExp(/^\/api\/v1\//), api: API.v1, settings, summary: metrics.rocketchatRestApi }))
+			.use(metricsMiddleware({ basePathRegex: new RegExp(/^\/api\/v1\//), api: API.v1, settings, summary: metrics.rocketchatRestApi, histogram: metrics.rocketchatRestApiSeconds }))
 			.use(tracerSpanMiddleware)
 			.use(API.v1.router)
 			.use(API.default.router).router,

apps/meteor/app/api/server/middlewares/metrics.ts

@@ -1,5 +1,5 @@
 import type { MiddlewareHandler } from 'hono';
-import type { Summary } from 'prom-client';
+import type { Histogram, Summary } from 'prom-client';
 
 import type { CachedSettings } from '../../../settings/server/CachedSettings';
 import type { APIClass } from '../ApiClass';
@@ -10,14 +10,17 @@ export const metricsMiddleware =
 		api,
 		settings,
 		summary,
+		histogram,
 	}: {
 		basePathRegex?: RegExp;
 		api: APIClass;
 		settings: CachedSettings;
 		summary: Summary;
+		histogram?: Histogram;
 	}): MiddlewareHandler =>
 	async (c, next) => {
 		const rocketchatRestApiEnd = summary.startTimer();
+		const rocketchatRestApiHistEnd = histogram?.startTimer();
 
 		await next();
 
@@ -26,11 +29,14 @@ export const metricsMiddleware =
 		// get rid of the base path (i.e.: /api/v1/)
 		const entrypoint = basePathRegex ? routePath.replace(basePathRegex, '') : routePath;
 
-		rocketchatRestApiEnd({
+		const labels = {
 			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,
-		});
+		};
+
+		rocketchatRestApiEnd(labels);
+		rocketchatRestApiHistEnd?.(labels);
 	};

apps/meteor/app/integrations/server/api/api.ts

@@ -428,7 +428,7 @@ const Api = new WebHookAPI({
 
 Api.router
 	.use(loggerMiddleware(integrationLogger))
-	.use(metricsMiddleware({ basePathRegex: new RegExp(/^\/hooks\//), api: Api, settings, summary: metrics.rocketchatRestApi }))
+	.use(metricsMiddleware({ basePathRegex: new RegExp(/^\/hooks\//), api: Api, settings, summary: metrics.rocketchatRestApi, histogram: metrics.rocketchatRestApiSeconds }))
 	.use(tracerSpanMiddleware);
 
 Api.addRoute(

apps/meteor/app/lib/server/functions/notifications/desktop.ts

@@ -67,6 +67,7 @@ export async function notifyDesktopUser({
 	};
 
 	metrics.notificationsSent.inc({ notification_type: 'desktop' });
+	metrics.notificationsSentTotal.inc({ notification_type: 'desktop' });
 
 	void api.broadcast('notify.desktop', userId, payload);
 }

apps/meteor/app/lib/server/functions/notifications/email.js

@@ -184,11 +184,13 @@ export async function getEmailData({ message, receiver, sender, subscription, ro
 	}
 
 	metrics.notificationsSent.inc({ notification_type: 'email' });
+	metrics.notificationsSentTotal.inc({ notification_type: 'email' });
 	return email;
 }
 
 export function sendEmailFromData(data) {
 	metrics.notificationsSent.inc({ notification_type: 'email' });
+	metrics.notificationsSentTotal.inc({ notification_type: 'email' });
 	return Mailer.send(data);
 }
 

apps/meteor/app/lib/server/lib/debug.js

@@ -57,11 +57,14 @@ const wrapMethods = function (name, originalHandler, methodsMap) {
 
 		const method = name === 'stream' ? `${name}:${originalArgs[0]}` : name;
 
-		const end = metrics.meteorMethods.startTimer({
+		const meteorMethodLabels = {
 			method,
 			has_connection: this.connection != null,
 			has_user: this.userId != null,
-		});
+		};
+
+		const end = metrics.meteorMethods.startTimer(meteorMethodLabels);
+		const endHistogram = metrics.meteorMethodsSeconds.startTimer(meteorMethodLabels);
 
 		logger.method({
 			method,
@@ -84,6 +87,7 @@ const wrapMethods = function (name, originalHandler, methodsMap) {
 			async () => {
 				const result = await originalHandler.apply(this, originalArgs);
 				end();
+				endHistogram();
 				return result;
 			},
 		);
@@ -115,10 +119,12 @@ Meteor.publish = function (name, func) {
 		});
 
 		const end = metrics.meteorSubscriptions.startTimer({ subscription: name });
+		const endHistogram = metrics.meteorSubscriptionsSeconds.startTimer({ subscription: name });
 
 		const originalReady = this.ready;
 		this.ready = function () {
 			end();
+			endHistogram();
 			return originalReady.apply(this, args);
 		};
 

apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts

@@ -40,6 +40,7 @@ export const apiDeprecationLogger = ((logger) => {
 			writeDeprecationHeader(res, 'endpoint-deprecation', message, version);
 
 			metrics.deprecations.inc({ type: 'deprecation', kind: 'endpoint', name: endpoint });
+			metrics.deprecationsTotal.inc({ type: 'deprecation', kind: 'endpoint', name: endpoint });
 
 			logger.warn({ msg: message, endpoint, version, info });
 		},
@@ -63,6 +64,7 @@ export const apiDeprecationLogger = ((logger) => {
 			}
 
 			metrics.deprecations.inc({ type: 'parameter-deprecation', kind: 'endpoint', name: endpoint, params: parameter });
+			metrics.deprecationsTotal.inc({ type: 'parameter-deprecation', kind: 'endpoint', name: endpoint, params: parameter });
 
 			writeDeprecationHeader(res, 'parameter-deprecation', message, version);
 
@@ -92,6 +94,7 @@ export const apiDeprecationLogger = ((logger) => {
 			compareVersions(version, message);
 
 			metrics.deprecations.inc({ type: 'invalid-usage', kind: 'endpoint', name: endpoint, params: parameter });
+			metrics.deprecationsTotal.inc({ type: 'invalid-usage', kind: 'endpoint', name: endpoint, params: parameter });
 
 			writeDeprecationHeader(res, 'invalid-usage', message, version);
 
@@ -114,6 +117,7 @@ export const methodDeprecationLogger = ((logger) => {
 			}
 			compareVersions(version, message);
 			metrics.deprecations.inc({ type: 'deprecation', name: method, kind: 'method' });
+			metrics.deprecationsTotal.inc({ type: 'deprecation', name: method, kind: 'method' });
 			logger.warn({ msg: message, method, version, replacement });
 		},
 		parameter: (method: string, parameter: string, version: DeprecationLoggerNextPlannedVersion) => {
@@ -123,6 +127,7 @@ export const methodDeprecationLogger = ((logger) => {
 			}
 
 			metrics.deprecations.inc({ type: 'parameter-deprecation', name: method, params: parameter });
+			metrics.deprecationsTotal.inc({ type: 'parameter-deprecation', name: method, params: parameter });
 
 			compareVersions(version, message);
 			logger.warn({ msg: message, method, parameter, version });
@@ -149,6 +154,7 @@ export const methodDeprecationLogger = ((logger) => {
 			compareVersions(version, message);
 
 			metrics.deprecations.inc({ type: 'invalid-usage', name: method, params: parameter, kind: 'method' });
+			metrics.deprecationsTotal.inc({ type: 'invalid-usage', name: method, params: parameter, kind: 'method' });
 
 			logger.warn({ msg: message, method, parameter, version });
 		},

apps/meteor/app/lib/server/lib/processDirectEmail.ts

@@ -110,6 +110,7 @@ export const processDirectEmail = async function (email: ParsedMail): Promise<vo
 	}
 
 	metrics.messagesSent.inc(); // TODO This line needs to be moved to it's proper place. See the comments on: https://github.com/RocketChat/Rocket.Chat/pull/5736
+	metrics.messagesSentTotal.inc();
 
 	const message: Pick<IMessage, 'ts' | 'msg' | 'groupable' | 'rid' | 'sentByEmail' | 'tmid'> = {
 		ts: tsDiff < 10000 ? ts : new Date(),

apps/meteor/app/lib/server/methods/sendMessage.ts

@@ -106,6 +106,7 @@ export async function executeSendMessage(
 		}
 
 		metrics.messagesSent.inc(); // TODO This line needs to be moved to it's proper place. See the comments on: https://github.com/RocketChat/Rocket.Chat/pull/5736
+		metrics.messagesSentTotal.inc();
 		return await sendMessage(user, message, room, { previewUrls: extraInfo?.previewUrls });
 	} catch (err: any) {
 		SystemLogger.error({ msg: 'Error sending message:', err });

apps/meteor/app/lib/server/startup/rateLimiter.js

@@ -134,6 +134,14 @@ const callback = (msg, name) => async (reply, input) => {
 			name: input.name,
 			connection_id: input.connectionId,
 		});
+		metrics.ddpRateLimitExceededTotal.inc({
+			limit_name: name,
+			user_id: input.userId,
+			client_address: input.clientAddress,
+			type: input.type,
+			name: input.name,
+			connection_id: input.connectionId,
+		});
 		// sleep before sending the error to slow down next requests
 		if (slowDownRate > 0 && reply.numInvocationsExceeded) {
 			await sleep(slowDownRate * reply.numInvocationsExceeded);