From f04dd454d98cab2cdbbb4187794110d69910b172 Mon Sep 17 00:00:00 2001
From: Mateusz Kwasniewski <kwasniewski.mateusz@gmail.com>
Date: Thu, 17 Jul 2025 12:59:50 +0200
Subject: [PATCH] feat: sanitize impact metrics (#10364)

---
 .../metrics/impact/metrics-translator.test.ts | 85 +++++++++++++++++++
 .../metrics/impact/metrics-translator.ts      | 17 +++-
 2 files changed, 99 insertions(+), 3 deletions(-)

diff --git a/src/lib/features/metrics/impact/metrics-translator.test.ts b/src/lib/features/metrics/impact/metrics-translator.test.ts
index 302da2dbaf..387ad143b8 100644
--- a/src/lib/features/metrics/impact/metrics-translator.test.ts
+++ b/src/lib/features/metrics/impact/metrics-translator.test.ts
@@ -2,6 +2,38 @@ import { MetricsTranslator } from './metrics-translator.js';
 import { Registry } from 'prom-client';
 
 describe('MetricsTranslator', () => {
+    describe('Sanitize name', () => {
+        let translator: MetricsTranslator;
+
+        beforeEach(() => {
+            const registry = new Registry();
+            translator = new MetricsTranslator(registry);
+        });
+
+        it('should not modify valid names', () => {
+            expect(translator.sanitizeName('valid_name')).toBe('valid_name');
+            expect(translator.sanitizeName('validName')).toBe('validName');
+            expect(translator.sanitizeName('_valid_name')).toBe('_valid_name');
+        });
+
+        it('should replace invalid characters with underscores', () => {
+            expect(translator.sanitizeName('invalid-name')).toBe(
+                'invalid_name',
+            );
+            expect(translator.sanitizeName('invalid.name')).toBe(
+                'invalid_name',
+            );
+            expect(translator.sanitizeName('invalid@name')).toBe(
+                'invalid_name',
+            );
+            expect(translator.sanitizeName('invalid name')).toBe(
+                'invalid_name',
+            );
+            expect(translator.sanitizeName('invalid:name')).toBe(
+                'invalid_name',
+            );
+        });
+    });
     it('should handle metrics with labels', async () => {
         const metrics = [
             {
@@ -81,6 +113,59 @@ describe('MetricsTranslator', () => {
         expect(result).not.toContain('unsupported');
     });
 
+    it('should sanitize metric names and label names', async () => {
+        const registry = new Registry();
+        const translator = new MetricsTranslator(registry);
+
+        const metrics = [
+            {
+                name: 'invalid-metric-name',
+                help: 'metric with invalid name',
+                type: 'counter' as const,
+                samples: [
+                    {
+                        labels: { 'invalid-label': 'value', '1numeric': 123 },
+                        value: 5,
+                    },
+                ],
+            },
+            {
+                name: '1numeric-metric',
+                help: 'metric with numeric prefix',
+                type: 'gauge' as const,
+                samples: [
+                    {
+                        labels: {
+                            'invalid:colon': 'value',
+                            'space label': 'test',
+                        },
+                        value: 10,
+                    },
+                ],
+            },
+        ];
+
+        const result = await translator.translateAndSerializeMetrics(metrics);
+
+        expect(result).toContain(
+            '# HELP unleash_counter_invalid_metric_name metric with invalid name',
+        );
+        expect(result).toContain(
+            '# TYPE unleash_counter_invalid_metric_name counter',
+        );
+        expect(result).toContain(
+            '# HELP unleash_gauge_1numeric_metric metric with numeric prefix',
+        );
+        expect(result).toContain('# TYPE unleash_gauge_1numeric_metric gauge');
+
+        expect(result).toContain(
+            'unleash_counter_invalid_metric_name{unleash_invalid_label="value",unleash_1numeric="123",unleash_origin="sdk"} 5',
+        );
+        expect(result).toContain(
+            'unleash_gauge_1numeric_metric{unleash_invalid_colon="value",unleash_space_label="test",unleash_origin="sdk"} 10',
+        );
+    });
+
     it('should handle re-labeling of metrics', async () => {
         const registry = new Registry();
         const translator = new MetricsTranslator(registry);
diff --git a/src/lib/features/metrics/impact/metrics-translator.ts b/src/lib/features/metrics/impact/metrics-translator.ts
index 8f93cadf21..0e343f0caf 100644
--- a/src/lib/features/metrics/impact/metrics-translator.ts
+++ b/src/lib/features/metrics/impact/metrics-translator.ts
@@ -19,6 +19,14 @@ export class MetricsTranslator {
         this.registry = registry;
     }
 
+    sanitizeName(name: string): string {
+        const regex = /[^a-zA-Z0-9_]/g;
+
+        const sanitized = name.replace(regex, '_');
+
+        return sanitized;
+    }
+
     private hasNewLabels(
         existingMetric: Counter<string> | Gauge<string>,
         newLabelNames: string[],
@@ -35,7 +43,7 @@ export class MetricsTranslator {
     ): Record<string, string | number> {
         return Object.fromEntries(
             Object.entries(labels).map(([labelKey, value]) => [
-                `unleash_${labelKey}`,
+                `unleash_${this.sanitizeName(labelKey)}`,
                 value,
             ]),
         );
@@ -51,7 +59,8 @@ export class MetricsTranslator {
     }
 
     translateMetric(metric: Metric): Counter<string> | Gauge<string> | null {
-        const prefixedName = `unleash_${metric.type}_${metric.name}`;
+        const sanitizedName = this.sanitizeName(metric.name);
+        const prefixedName = `unleash_${metric.type}_${sanitizedName}`;
         const existingMetric = this.registry.getSingleMetric(prefixedName);
 
         const allLabelNames = new Set<string>();
@@ -63,7 +72,9 @@ export class MetricsTranslator {
                 );
             }
         }
-        const labelNames = Array.from(allLabelNames);
+        const labelNames = Array.from(allLabelNames).map((labelName) =>
+            this.sanitizeName(labelName),
+        );
 
         if (metric.type === 'counter') {
             let counter: Counter<string>;