feat: validate impact metrics (#10181)

src/lib/features/metrics/client-metrics/metrics-service-v2.ts

@@ -6,7 +6,7 @@ import type {
     IClientMetricsEnv,
     IClientMetricsStoreV2,
 } from './client-metrics-store-v2-type.js';
-import { clientMetricsSchema } from '../shared/schema.js';
+import { clientMetricsSchema, impactMetricsSchema } from '../shared/schema.js';
 import { compareAsc, secondsToMilliseconds } from 'date-fns';
 import {
     CLIENT_METRICS,
@@ -30,6 +30,11 @@ import {
     MAX_UNKNOWN_FLAGS,
     type UnknownFlagsService,
 } from '../unknown-flags/unknown-flags-service.js';
+import {
+    type Metric,
+    MetricsTranslator,
+} from '../impact/metrics-translator.js';
+import { impactRegister } from '../impact/impact-register.js';
 
 export default class ClientMetricsServiceV2 {
     private config: IUnleashConfig;
@@ -46,6 +51,8 @@ export default class ClientMetricsServiceV2 {
 
     private logger: Logger;
 
+    private impactMetricsTranslator: MetricsTranslator;
+
     private cachedFeatureNames: () => Promise<string[]>;
 
     constructor(
@@ -69,6 +76,7 @@ export default class ClientMetricsServiceV2 {
                 maxAge: secondsToMilliseconds(10),
             },
         );
+        this.impactMetricsTranslator = new MetricsTranslator(impactRegister);
     }
 
     async clearMetrics(hoursAgo: number) {
@@ -187,6 +195,11 @@ export default class ClientMetricsServiceV2 {
         this.lastSeenService.updateLastSeen(metrics);
     }
 
+    async registerImpactMetrics(impactMetrics: Metric[]) {
+        const value = await impactMetricsSchema.validateAsync(impactMetrics);
+        this.impactMetricsTranslator.translateMetrics(value);
+    }
+
     async registerClientMetrics(
         data: ClientMetricsSchema,
         clientIp: string,

src/lib/features/metrics/impact/impact-metrics.e2e.test.ts

@@ -11,7 +11,7 @@ import type { Metric } from './metrics-translator.js';
 let app: IUnleashTest;
 let db: ITestDb;
 
-const sendImpactMetrics = async (impactMetrics: Metric[]) =>
+const sendImpactMetrics = async (impactMetrics: Metric[], status = 202) =>
     app.request
         .post('/api/client/metrics')
         .send({
@@ -24,7 +24,7 @@ const sendImpactMetrics = async (impactMetrics: Metric[]) =>
             },
             impactMetrics,
         })
-        .expect(202);
+        .expect(status);
 
 beforeAll(async () => {
     db = await dbInit('impact_metrics', getLogger);
@@ -71,6 +71,25 @@ test('should store impact metrics in memory and be able to retrieve them', async
         },
     ]);
 
+    await sendImpactMetrics([]);
+    // missing help
+    await sendImpactMetrics(
+        [
+            // @ts-expect-error
+            {
+                name: 'labeled_counter',
+                type: 'counter',
+                samples: [
+                    {
+                        labels: { foo: 'bar' },
+                        value: 10,
+                    },
+                ],
+            },
+        ],
+        400,
+    );
+
     const response = await app.request
         .get('/internal-backstage/impact/metrics')
         .expect('Content-Type', /text/)

src/lib/features/metrics/instance/metrics.ts

@@ -58,7 +58,6 @@ export default class ClientMetricsController extends Controller {
             | 'customMetricsService'
         >,
         config: IUnleashConfig,
-        metricsTranslator: MetricsTranslator,
     ) {
         super(config);
         const { getLogger } = config;
@@ -69,7 +68,6 @@ export default class ClientMetricsController extends Controller {
         this.metricsV2 = clientMetricsServiceV2;
         this.customMetricsService = customMetricsService;
         this.flagResolver = config.flagResolver;
-        this.metricsTranslator = metricsTranslator;
 
         this.route({
             method: 'post',
@@ -171,7 +169,7 @@ export default class ClientMetricsController extends Controller {
                     this.flagResolver.isEnabled('impactMetrics') &&
                     impactMetrics
                 ) {
-                    this.metricsTranslator.translateMetrics(impactMetrics);
+                    await this.metricsV2.registerImpactMetrics(impactMetrics);
                 }
 
                 res.getHeaderNames().forEach((header) =>

src/lib/features/metrics/shared/schema.ts

@@ -85,6 +85,35 @@ export const customMetricsSchema = joi
         metrics: joi.array().items(customMetricSchema).required(),
     });
 
+export const metricSampleSchema = joi
+    .object()
+    .options({ stripUnknown: true })
+    .keys({
+        value: joi.number().required(),
+        labels: joi
+            .object()
+            .pattern(
+                joi.string(),
+                joi.alternatives().try(joi.string(), joi.number()),
+            )
+            .optional(),
+    });
+
+export const impactMetricSchema = joi
+    .object()
+    .options({ stripUnknown: true })
+    .keys({
+        name: joi.string().required(),
+        help: joi.string().required(),
+        type: joi.string().required(),
+        samples: joi.array().items(metricSampleSchema).required(),
+    });
+
+export const impactMetricsSchema = joi
+    .array()
+    .items(impactMetricSchema)
+    .empty();
+
 export const batchMetricsSchema = joi
     .object()
     .options({ stripUnknown: true })

src/lib/routes/client-api/index.ts

@@ -4,20 +4,13 @@ import MetricsController from '../../features/metrics/instance/metrics.js';
 import RegisterController from '../../features/metrics/instance/register.js';
 import type { IUnleashConfig } from '../../types/index.js';
 import type { IUnleashServices } from '../../services/index.js';
-import { impactRegister } from '../../features/metrics/impact/impact-register.js';
-import { MetricsTranslator } from '../../features/metrics/impact/metrics-translator.js';
 
 export default class ClientApi extends Controller {
     constructor(config: IUnleashConfig, services: IUnleashServices) {
         super(config);
 
-        const metricsTranslator = new MetricsTranslator(impactRegister);
-
         this.use('/features', new FeatureController(services, config).router);
-        this.use(
+        this.use('/metrics', new MetricsController(services, config).router);
-            '/metrics',
-            new MetricsController(services, config, metricsTranslator).router,
-        );
         this.use('/register', new RegisterController(services, config).router);
     }
 }