From d082125a699608eb19a6d6ae2a4c919d511680e1 Mon Sep 17 00:00:00 2001 From: kwasniew Date: Mon, 22 Sep 2025 15:32:15 +0200 Subject: [PATCH] feat: histogram impact metric ingestion --- .../client-metrics/metrics-service-v2.ts | 2 +- .../metrics/impact/impact-metrics.e2e.test.ts | 77 ++++++++++++++++++- .../metrics/impact/metrics-translator.ts | 66 +++++++++++++--- src/lib/features/metrics/shared/schema.ts | 34 +++++++- 4 files changed, 165 insertions(+), 14 deletions(-) diff --git a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts index 996a3985f6..20f754ea5b 100644 --- a/src/lib/features/metrics/client-metrics/metrics-service-v2.ts +++ b/src/lib/features/metrics/client-metrics/metrics-service-v2.ts @@ -259,7 +259,7 @@ export default class ClientMetricsServiceV2 { this.impactMetricsTranslator.translateMetrics(value); } catch (e) { // impact metrics should not affect other metrics on failure - this.logger.warn(e); + this.logger.warn('Impact metrics registration failed:', e); } } diff --git a/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts b/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts index 7b5a92a5ee..5d124ced66 100644 --- a/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts +++ b/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts @@ -6,12 +6,15 @@ import dbInit, { type ITestDb, } from '../../../../test/e2e/helpers/database-init.js'; import getLogger from '../../../../test/fixtures/no-logger.js'; -import type { Metric } from './metrics-translator.js'; +import type { NumericMetric, BucketMetric } from './metrics-translator.js'; let app: IUnleashTest; let db: ITestDb; -const sendImpactMetrics = async (impactMetrics: Metric[], status = 202) => +const sendImpactMetrics = async ( + impactMetrics: (NumericMetric | BucketMetric)[], + status = 202, +) => app.request .post('/api/client/metrics') .send({ @@ -27,7 +30,7 @@ const sendImpactMetrics = async (impactMetrics: Metric[], status = 202) => .expect(status); const sendBulkMetricsWithImpact = async ( - impactMetrics: Metric[], + impactMetrics: (NumericMetric | BucketMetric)[], status = 202, ) => { return app.request @@ -170,3 +173,71 @@ test('should store impact metrics sent via bulk metrics endpoint', async () => { /unleash_counter_bulk_counter{unleash_source="bulk",unleash_origin="sdk"} 15/, ); }); + +test('should store histogram metrics with batch data', async () => { + await sendImpactMetrics([ + { + name: 'response_time', + help: 'Response time histogram', + type: 'histogram', + buckets: [1], + samples: [ + { + labels: { foo: 'bar' }, + count: 10, + sum: 8.5, + buckets: [ + { le: 1, count: 7 }, + { le: '+Inf', count: 10 }, + ], + }, + ], + }, + ]); + + await sendImpactMetrics([ + { + name: 'response_time', + help: 'Response time histogram', + type: 'histogram', + buckets: [1], + samples: [ + { + labels: { foo: 'bar' }, + count: 5, + sum: 3.2, + buckets: [ + { le: 1, count: 3 }, + { le: '+Inf', count: 5 }, + ], + }, + ], + }, + ]); + + const response = await app.request + .get('/internal-backstage/impact/metrics') + .expect('Content-Type', /text/) + .expect(200); + + const metricsText = response.text; + + expect(metricsText).toContain( + '# HELP unleash_histogram_response_time Response time histogram', + ); + expect(metricsText).toContain( + '# TYPE unleash_histogram_response_time histogram', + ); + expect(metricsText).toContain( + 'unleash_histogram_response_time_bucket{unleash_foo="bar",unleash_origin="sdk",le="1"} 10', + ); + expect(metricsText).toContain( + 'unleash_histogram_response_time_bucket{unleash_foo="bar",unleash_origin="sdk",le="+Inf"} 15', + ); + expect(metricsText).toContain( + 'unleash_histogram_response_time_sum{unleash_foo="bar",unleash_origin="sdk"} 11.7', + ); + expect(metricsText).toContain( + 'unleash_histogram_response_time_count{unleash_foo="bar",unleash_origin="sdk"} 15', + ); +}); diff --git a/src/lib/features/metrics/impact/metrics-translator.ts b/src/lib/features/metrics/impact/metrics-translator.ts index dcd577f141..fb9b8e81b0 100644 --- a/src/lib/features/metrics/impact/metrics-translator.ts +++ b/src/lib/features/metrics/impact/metrics-translator.ts @@ -1,19 +1,38 @@ import { Counter, Gauge, type Registry } from 'prom-client'; +import { BatchHistogram } from './batch-histogram.js'; -export interface MetricSample { +export interface NumericMetricSample { labels?: Record; value: number; } -export interface Metric { +export interface BucketMetricSample { + labels?: Record; + count: number; + sum: number; + buckets: Array<{ le: number | '+Inf'; count: number }>; +} + +export interface NumericMetric { name: string; help: string; type: 'counter' | 'gauge'; - samples: MetricSample[]; + samples: NumericMetricSample[]; } +export interface BucketMetric { + name: string; + help: string; + type: 'histogram'; + buckets: number[]; + samples: BucketMetricSample[]; +} + +export type Metric = NumericMetric | BucketMetric; + export class MetricsTranslator { private registry: Registry; + private histograms: Map = new Map(); constructor(registry: Registry) { this.registry = registry; @@ -22,13 +41,11 @@ export class MetricsTranslator { sanitizeName(name: string): string { const regex = /[^a-zA-Z0-9_]/g; - const sanitized = name.replace(regex, '_'); - - return sanitized; + return name.replace(regex, '_'); } private hasNewLabels( - existingMetric: Counter | Gauge, + existingMetric: Counter | Gauge | BatchHistogram, newLabelNames: string[], ): boolean { const existingLabelNames = (existingMetric as any).labelNames || []; @@ -50,7 +67,7 @@ export class MetricsTranslator { } private addOriginLabel( - sample: MetricSample, + sample: NumericMetricSample, ): Record { return { ...(sample.labels || {}), @@ -58,7 +75,9 @@ export class MetricsTranslator { }; } - translateMetric(metric: Metric): Counter | Gauge | null { + translateMetric( + metric: Metric, + ): Counter | Gauge | BatchHistogram | null { const sanitizedName = this.sanitizeName(metric.name); const prefixedName = `unleash_${metric.type}_${sanitizedName}`; const existingMetric = this.registry.getSingleMetric(prefixedName); @@ -142,6 +161,35 @@ export class MetricsTranslator { } return gauge; + } else if (metric.type === 'histogram') { + if (!metric.samples || metric.samples.length === 0) { + return null; + } + + let histogram = this.histograms.get(prefixedName); + if (!histogram) { + histogram = new BatchHistogram({ + name: prefixedName, + help: metric.help, + registry: this.registry, + }); + this.histograms.set(prefixedName, histogram); + } + + for (const sample of metric.samples) { + const transformedLabels = this.transformLabels({ + ...sample.labels, + origin: sample.labels?.origin || 'sdk', + }); + + histogram.recordBatch(transformedLabels, { + count: sample.count, + sum: sample.sum, + buckets: sample.buckets, + }); + } + + return histogram; } return null; diff --git a/src/lib/features/metrics/shared/schema.ts b/src/lib/features/metrics/shared/schema.ts index 6670e5630d..9b7a0c0021 100644 --- a/src/lib/features/metrics/shared/schema.ts +++ b/src/lib/features/metrics/shared/schema.ts @@ -99,6 +99,33 @@ export const metricSampleSchema = joi .optional(), }); +export const histogramSampleSchema = joi + .object() + .options({ stripUnknown: true }) + .keys({ + count: joi.number().required(), + sum: joi.number().required(), + buckets: joi + .array() + .items( + joi.object({ + le: joi + .alternatives() + .try(joi.number(), joi.string().valid('+Inf')) + .required(), + count: joi.number().required(), + }), + ) + .required(), + labels: joi + .object() + .pattern( + joi.string(), + joi.alternatives().try(joi.string(), joi.number()), + ) + .optional(), + }); + export const impactMetricSchema = joi .object() .options({ stripUnknown: true }) @@ -106,7 +133,12 @@ export const impactMetricSchema = joi name: joi.string().required(), help: joi.string().required(), type: joi.string().required(), - samples: joi.array().items(metricSampleSchema).required(), + buckets: joi.array().items(joi.number()).optional(), + samples: joi.when('type', { + is: 'histogram', + then: joi.array().items(histogramSampleSchema).required(), + otherwise: joi.array().items(metricSampleSchema).required(), + }), }); export const impactMetricsSchema = joi