diff --git a/src/Utils/prometheus-metrics.ts b/src/Utils/prometheus-metrics.ts index 91871b6f..b9d6893c 100644 --- a/src/Utils/prometheus-metrics.ts +++ b/src/Utils/prometheus-metrics.ts @@ -20,13 +20,16 @@ * - Circuit breaker metrics * - Environment variable configuration * - * Configuration via environment variables: + * Configuration via environment variables (supports BAILEYS_PROMETHEUS_* and METRICS_* prefixes): * - BAILEYS_PROMETHEUS_ENABLED: Enable/disable metrics (default: false) * - BAILEYS_PROMETHEUS_PORT: Port for HTTP metrics server (default: 9092) + * - BAILEYS_PROMETHEUS_HOST: Host/IP to bind (default: 127.0.0.1) * - BAILEYS_PROMETHEUS_PATH: Path for metrics endpoint (default: /metrics) * - BAILEYS_PROMETHEUS_PREFIX: Prefix for all metrics (default: baileys) * - BAILEYS_PROMETHEUS_LABELS: JSON string with default labels (e.g. {"environment":"production"}) * - BAILEYS_PROMETHEUS_COLLECT_DEFAULT: Collect default Node.js metrics (default: true) + * - BAILEYS_PROMETHEUS_INCLUDE_SYSTEM: Include system metrics like CPU/memory (default: true) + * - BAILEYS_PROMETHEUS_COLLECT_INTERVAL_MS: Interval for system metrics collection (default: 10000) * * @module Utils/prometheus-metrics */ @@ -140,8 +143,11 @@ export function loadMetricsConfig(): MetricsConfig { host: process.env.BAILEYS_PROMETHEUS_HOST || process.env.METRICS_HOST || '127.0.0.1', path: process.env.BAILEYS_PROMETHEUS_PATH || process.env.METRICS_PATH || '/metrics', prefix: process.env.BAILEYS_PROMETHEUS_PREFIX || process.env.METRICS_PREFIX || 'baileys', - defaultLabels: parseLabelsFromEnv(process.env.BAILEYS_PROMETHEUS_LABELS), - includeSystem: (process.env.BAILEYS_PROMETHEUS_COLLECT_DEFAULT ?? process.env.METRICS_INCLUDE_SYSTEM) !== 'false', + // Support both env prefixes for labels + defaultLabels: parseLabelsFromEnv(process.env.BAILEYS_PROMETHEUS_LABELS ?? process.env.METRICS_LABELS), + // Separate flag for system metrics (CPU/memory) - independent from collectDefaultMetrics + includeSystem: (process.env.BAILEYS_PROMETHEUS_INCLUDE_SYSTEM ?? process.env.METRICS_INCLUDE_SYSTEM) !== 'false', + // Flag for prom-client default Node.js metrics collectDefaultMetrics: (process.env.BAILEYS_PROMETHEUS_COLLECT_DEFAULT ?? process.env.METRICS_COLLECT_DEFAULT) !== 'false', collectIntervalMs: parseInt(process.env.BAILEYS_PROMETHEUS_COLLECT_INTERVAL_MS || process.env.METRICS_COLLECT_INTERVAL_MS || '10000', 10), } @@ -161,6 +167,19 @@ export type MetricType = 'counter' | 'gauge' | 'histogram' | 'summary' */ export type Labels = Record +/** + * Convert labels to a stable key string for lookups + * Sorts keys to ensure consistent ordering regardless of object property order + */ +function stableLabelsToKey(labels: Labels): string { + const sortedKeys = Object.keys(labels).sort() + const sortedObj: Labels = {} + for (const key of sortedKeys) { + sortedObj[key] = labels[key]! + } + return JSON.stringify(sortedObj) +} + /** * Default histogram buckets (in ms) */ @@ -312,7 +331,7 @@ export class Counter implements BaseMetric { } private labelsToKey(labels: Labels): string { - return JSON.stringify(labels) + return stableLabelsToKey(labels) } /** @@ -462,7 +481,7 @@ export class Gauge implements BaseMetric { } private labelsToKey(labels: Labels): string { - return JSON.stringify(labels) + return stableLabelsToKey(labels) } /** @@ -662,7 +681,7 @@ export class Histogram implements BaseMetric { } private labelsToKey(labels: Labels): string { - return JSON.stringify(labels) + return stableLabelsToKey(labels) } /** @@ -831,7 +850,7 @@ export class Summary implements BaseMetric { } private labelsToKey(labels: Labels): string { - return JSON.stringify(labels) + return stableLabelsToKey(labels) } /** @@ -1202,7 +1221,12 @@ export class MetricsServer { } this.server = createServer(async (req: IncomingMessage, res: ServerResponse) => { - if (req.url === this.config.path && req.method === 'GET') { + // Parse URL to handle querystrings and trailing slashes + const parsedUrl = new URL(req.url || '/', `http://${req.headers.host || 'localhost'}`) + const pathname = parsedUrl.pathname.replace(/\/+$/, '') || '/' // Normalize trailing slashes + const configPath = this.config.path.replace(/\/+$/, '') || '/' + + if (pathname === configPath && req.method === 'GET') { try { // Collect system metrics before responding this.systemCollector?.collect() @@ -1211,7 +1235,6 @@ export class MetricsServer { res.writeHead(200, { 'Content-Type': this.registry.contentType() }) res.end(metricsOutput) } catch (error) { - // FIX: More descriptive error message const errorMessage = error instanceof Error ? error.message : 'Unknown error' console.error(`[Prometheus] Error collecting metrics: ${errorMessage}`) res.writeHead(500, { 'Content-Type': 'application/json' }) @@ -1221,7 +1244,7 @@ export class MetricsServer { timestamp: new Date().toISOString() })) } - } else if (req.url === '/health' && req.method === 'GET') { + } else if (pathname === '/health' && req.method === 'GET') { res.writeHead(200, { 'Content-Type': 'application/json' }) res.end(JSON.stringify({ status: 'ok', timestamp: new Date().toISOString() })) } else { diff --git a/src/Utils/retry-utils.ts b/src/Utils/retry-utils.ts index e3be6ce0..dfd209b9 100644 --- a/src/Utils/retry-utils.ts +++ b/src/Utils/retry-utils.ts @@ -131,15 +131,17 @@ export function calculateDelay( multiplier: number, jitter: number ): number { + // Normalize attempt to ensure valid calculation (must be >= 1) + const normalizedAttempt = Math.max(attempt, 1) let delay: number switch (strategy) { case 'exponential': - delay = baseDelay * Math.pow(multiplier, attempt - 1) + delay = baseDelay * Math.pow(multiplier, normalizedAttempt - 1) break case 'linear': - delay = baseDelay * attempt + delay = baseDelay * normalizedAttempt break case 'constant': @@ -147,7 +149,7 @@ export function calculateDelay( break case 'fibonacci': { - const fib = fibonacciNumber(attempt) + const fib = fibonacciNumber(normalizedAttempt) delay = baseDelay * fib break } @@ -155,7 +157,7 @@ export function calculateDelay( case 'stepped': { // Uses pre-defined delay array directly (ignores baseDelay/multiplier) // Falls back to last delay if attempt exceeds array length - const index = Math.min(attempt - 1, RETRY_BACKOFF_DELAYS.length - 1) + const index = Math.min(normalizedAttempt - 1, RETRY_BACKOFF_DELAYS.length - 1) delay = RETRY_BACKOFF_DELAYS[index] ?? RETRY_BACKOFF_DELAYS[0] ?? baseDelay break } @@ -164,15 +166,15 @@ export function calculateDelay( delay = baseDelay } - // Apply max delay cap - delay = Math.min(delay, maxDelay) - - // Apply jitter + // Apply jitter BEFORE capping to maxDelay if (jitter > 0) { const jitterAmount = delay * jitter delay = delay + (Math.random() * 2 - 1) * jitterAmount } + // Apply max delay cap AFTER jitter to ensure we never exceed maxDelay + delay = Math.min(delay, maxDelay) + return Math.max(0, Math.round(delay)) } @@ -196,7 +198,15 @@ function fibonacciNumber(n: number): number { */ async function sleep(ms: number, signal?: AbortSignal): Promise { return new Promise((resolve, reject) => { - const timer = setTimeout(resolve, ms) + let abortHandler: (() => void) | undefined + + const timer = setTimeout(() => { + // Cleanup abort listener on normal completion to prevent memory leak + if (signal && abortHandler) { + signal.removeEventListener('abort', abortHandler) + } + resolve() + }, ms) if (signal) { if (signal.aborted) { @@ -205,7 +215,7 @@ async function sleep(ms: number, signal?: AbortSignal): Promise { return } - const abortHandler = () => { + abortHandler = () => { clearTimeout(timer) reject(new RetryAbortedError(0)) } @@ -324,8 +334,9 @@ export async function retry( result = await operation(context) } - // Success - if (config.collectMetrics) { + // Success - only count as retry success if this wasn't the first attempt + if (config.collectMetrics && attempt > 1) { + // This was a successful retry (not first attempt) metrics.retries.inc({ operation: config.operationName }) } @@ -339,9 +350,9 @@ export async function retry( const shouldRetry = await config.shouldRetry(lastError, attempt) if (!shouldRetry || attempt >= config.maxAttempts) { - // Final failure + // Final failure - use dedicated retry exhausted metric if (config.collectMetrics) { - metrics.errors.inc({ category: 'retry', code: 'exhausted' }) + metrics.retryExhausted.inc({ operation: config.operationName }) } config.onFailure(lastError, attempt) diff --git a/src/__tests__/Utils/retry-utils.test.ts b/src/__tests__/Utils/retry-utils.test.ts index 6ded3a5b..038d4f9c 100644 --- a/src/__tests__/Utils/retry-utils.test.ts +++ b/src/__tests__/Utils/retry-utils.test.ts @@ -16,6 +16,8 @@ import { retryConfigs, getRetryDelayWithJitter, getAllRetryDelaysWithJitter, + RETRY_BACKOFF_DELAYS, + RETRY_JITTER_FACTOR, type RetryContext, } from '../../Utils/retry-utils.js' @@ -615,3 +617,29 @@ describe('getAllRetryDelaysWithJitter', () => { expect(allSame).toBe(false) }) }) + +describe('retryConfigs.rsocket consistency', () => { + /** + * This test ensures the hardcoded values in retryConfigs.rsocket + * stay synchronized with RETRY_BACKOFF_DELAYS and RETRY_JITTER_FACTOR. + * + * The values are hardcoded to avoid ESM initialization order issues, + * but this test will fail if someone updates the constants without + * updating retryConfigs.rsocket. + */ + it('should have maxAttempts equal to RETRY_BACKOFF_DELAYS.length', () => { + expect(retryConfigs.rsocket.maxAttempts).toBe(RETRY_BACKOFF_DELAYS.length) + }) + + it('should have baseDelay equal to RETRY_BACKOFF_DELAYS[0]', () => { + expect(retryConfigs.rsocket.baseDelay).toBe(RETRY_BACKOFF_DELAYS[0]) + }) + + it('should have maxDelay equal to last RETRY_BACKOFF_DELAYS element', () => { + expect(retryConfigs.rsocket.maxDelay).toBe(RETRY_BACKOFF_DELAYS[RETRY_BACKOFF_DELAYS.length - 1]) + }) + + it('should have jitter equal to RETRY_JITTER_FACTOR', () => { + expect(retryConfigs.rsocket.jitter).toBe(RETRY_JITTER_FACTOR) + }) +})