From 37fdce4bf3ed8cdc7759f901054b611305c3d46d Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Tue, 20 Jan 2026 13:45:26 +0000
Subject: [PATCH] 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<void>((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) {