From 82dce29dbd712ae6b553166859570e0b8890b40f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 13:25:21 +0000 Subject: [PATCH 1/2] fix(metrics): address PR #13 code review feedback - Apply configured prefix to all metric names via getFullMetricName() - Configure customRegistry with defaultLabels at initialization - Prevent memory leak: collectDefaultMetrics only called once via flag - collectDefaultMetrics now uses customRegistry instead of global - Initialize prefix/defaultLabels before creating baileysMetrics All metrics now properly include prefix (e.g., baileys_connection_attempts) and inherit defaultLabels configured via environment variables. --- src/Utils/prometheus-metrics.ts | 59 +++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/src/Utils/prometheus-metrics.ts b/src/Utils/prometheus-metrics.ts index 262b19a5..7acfde93 100644 --- a/src/Utils/prometheus-metrics.ts +++ b/src/Utils/prometheus-metrics.ts @@ -38,6 +38,33 @@ import * as promClient from 'prom-client' // Create a custom registry to avoid conflicts with global registry const customRegistry = new promClient.Registry() +// Flag to prevent multiple collectDefaultMetrics calls (memory leak prevention) +let defaultMetricsCollected = false + +/** + * Configure the custom registry with prefix and default labels + * Should be called once during initialization + */ +function configureRegistry(config: { prefix?: string; defaultLabels?: Labels }): void { + if (config.defaultLabels && Object.keys(config.defaultLabels).length > 0) { + customRegistry.setDefaultLabels(config.defaultLabels) + } +} + +/** + * Get the configured prefix for metric names + * Stored globally after first configuration + */ +let configuredPrefix = '' + +function setMetricPrefix(prefix: string): void { + configuredPrefix = prefix +} + +function getFullMetricName(name: string): string { + return configuredPrefix ? `${configuredPrefix}_${name}` : name +} + // ============================================ // Configuration // ============================================ @@ -186,8 +213,9 @@ export class Counter implements BaseMetric { public help: string, public labelNames: string[] = [] ) { + const fullName = getFullMetricName(name) this.promCounter = new promClient.Counter({ - name, + name: fullName, help, labelNames, registers: [customRegistry], // Use custom registry, not global @@ -290,8 +318,9 @@ export class Gauge implements BaseMetric { public help: string, public labelNames: string[] = [] ) { + const fullName = getFullMetricName(name) this.promGauge = new promClient.Gauge({ - name, + name: fullName, help, labelNames, registers: [customRegistry], // Use custom registry, not global @@ -460,8 +489,9 @@ export class Histogram implements BaseMetric { buckets: number[] = DEFAULT_BUCKETS ) { this.buckets = [...buckets].sort((a, b) => a - b) + const fullName = getFullMetricName(name) this.promHistogram = new promClient.Histogram({ - name, + name: fullName, help, labelNames, buckets: this.buckets, @@ -645,8 +675,9 @@ export class Summary implements BaseMetric { options: { percentiles?: number[]; maxAgeSeconds?: number; ageBuckets?: number } = {} ) { this.percentiles = options.percentiles ?? DEFAULT_PERCENTILES + const fullName = getFullMetricName(name) this.promSummary = new promClient.Summary({ - name, + name: fullName, help, labelNames, percentiles: this.percentiles, @@ -1118,11 +1149,20 @@ export class MetricsServer { // Cache the promise so concurrent calls get the same one this.startPromise = new Promise((resolve, reject) => { - // Enable prom-client's default Node.js metrics collection - if (this.config.collectDefaultMetrics) { + // Configure registry with prefix and defaultLabels (once) + setMetricPrefix(this.config.prefix) + configureRegistry({ + prefix: this.config.prefix, + defaultLabels: this.config.defaultLabels, + }) + + // Enable prom-client's default Node.js metrics collection (only once to prevent memory leak) + if (this.config.collectDefaultMetrics && !defaultMetricsCollected) { + defaultMetricsCollected = true promClient.collectDefaultMetrics({ prefix: this.config.prefix ? `${this.config.prefix}_` : '', labels: this.config.defaultLabels, + register: customRegistry, // Use custom registry, not global }) } @@ -1228,6 +1268,13 @@ export class MetricsServer { */ const metricsConfig = loadMetricsConfig() +// Initialize prefix and defaultLabels for all metrics created after this point +setMetricPrefix(metricsConfig.prefix) +configureRegistry({ + prefix: metricsConfig.prefix, + defaultLabels: metricsConfig.defaultLabels, +}) + /** * Global registry for Baileys metrics * Uses prefix and default labels from environment variables From 37fdce4bf3ed8cdc7759f901054b611305c3d46d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Jan 2026 13:45:26 +0000 Subject: [PATCH 2/2] fix(metrics): address PR #14 code review feedback - Add registryConfigured flag to ensure single execution of configureRegistry - Fix race condition by moving setMetricPrefix/configureRegistry before Promise - Remove unused prefix parameter from configureRegistry function - Consolidate prefix to single source of truth (configuredPrefix global) - Allow clearing default labels by passing empty object - Add resetMetricsConfiguration() and getConfiguredPrefix() for testing --- src/Utils/prometheus-metrics.ts | 60 +++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/src/Utils/prometheus-metrics.ts b/src/Utils/prometheus-metrics.ts index 7acfde93..1d618c4c 100644 --- a/src/Utils/prometheus-metrics.ts +++ b/src/Utils/prometheus-metrics.ts @@ -38,33 +38,60 @@ import * as promClient from 'prom-client' // Create a custom registry to avoid conflicts with global registry const customRegistry = new promClient.Registry() -// Flag to prevent multiple collectDefaultMetrics calls (memory leak prevention) +// Flags to prevent multiple configuration calls (thread-safety and memory leak prevention) let defaultMetricsCollected = false +let registryConfigured = false +let configuredPrefix = '' /** - * Configure the custom registry with prefix and default labels - * Should be called once during initialization + * Configure the custom registry with default labels + * Thread-safe: Only executes once, subsequent calls are no-ops + * To clear default labels, pass an empty object {} explicitly */ -function configureRegistry(config: { prefix?: string; defaultLabels?: Labels }): void { - if (config.defaultLabels && Object.keys(config.defaultLabels).length > 0) { - customRegistry.setDefaultLabels(config.defaultLabels) +function configureRegistry(defaultLabels?: Labels): void { + if (registryConfigured) { + return // Already configured, ignore subsequent calls + } + registryConfigured = true + + // Allow setting empty labels to clear previous (useful for testing/reconfiguration) + if (defaultLabels !== undefined) { + customRegistry.setDefaultLabels(defaultLabels) } } /** - * Get the configured prefix for metric names - * Stored globally after first configuration + * Set the metric prefix - thread-safe, only first call takes effect */ -let configuredPrefix = '' - function setMetricPrefix(prefix: string): void { - configuredPrefix = prefix + if (configuredPrefix === '') { + configuredPrefix = prefix + } } function getFullMetricName(name: string): string { return configuredPrefix ? `${configuredPrefix}_${name}` : name } +/** + * Reset all configuration flags - FOR TESTING ONLY + * Allows reconfiguration in test environments + */ +export function resetMetricsConfiguration(): void { + registryConfigured = false + defaultMetricsCollected = false + configuredPrefix = '' + // Clear all metrics from custom registry + customRegistry.clear() +} + +/** + * Get the current configured prefix - FOR TESTING/DEBUGGING + */ +export function getConfiguredPrefix(): string { + return configuredPrefix +} + // ============================================ // Configuration // ============================================ @@ -1147,14 +1174,13 @@ export class MetricsServer { return this.startPromise } + // Configure prefix and registry BEFORE creating Promise to avoid race conditions + // These functions are thread-safe and only execute once + setMetricPrefix(this.config.prefix) + configureRegistry(this.config.defaultLabels) + // Cache the promise so concurrent calls get the same one this.startPromise = new Promise((resolve, reject) => { - // Configure registry with prefix and defaultLabels (once) - setMetricPrefix(this.config.prefix) - configureRegistry({ - prefix: this.config.prefix, - defaultLabels: this.config.defaultLabels, - }) // Enable prom-client's default Node.js metrics collection (only once to prevent memory leak) if (this.config.collectDefaultMetrics && !defaultMetricsCollected) {