From 4f1fdfe63bd7f650a6660dd6680e0b986368aaa9 Mon Sep 17 00:00:00 2001 From: kwasniew Date: Mon, 22 Sep 2025 16:57:21 +0200 Subject: [PATCH] feat: histogram impact metric ingestion --- .../metrics/impact/batch-histogram.test.ts | 56 +++++++++++++++---- .../metrics/impact/batch-histogram.ts | 13 +++-- .../metrics/impact/impact-metrics.e2e.test.ts | 2 - .../metrics/impact/metrics-translator.test.ts | 42 ++++++++++++++ .../metrics/impact/metrics-translator.ts | 20 +++++-- 5 files changed, 110 insertions(+), 23 deletions(-) diff --git a/src/lib/features/metrics/impact/batch-histogram.test.ts b/src/lib/features/metrics/impact/batch-histogram.test.ts index 6e7e0e7019..f30eeaff2f 100644 --- a/src/lib/features/metrics/impact/batch-histogram.test.ts +++ b/src/lib/features/metrics/impact/batch-histogram.test.ts @@ -117,9 +117,9 @@ describe('BatchHistogram', () => { expect(metrics).toMatch(/test_histogram_count{label1="value1"} 8/); }); - test('should record different labels separately', async () => { + test('should record different labels separately and handle special characters', async () => { histogram.recordBatch( - { service: 'api', app: 'my_app' }, + { service: 'api', url: 'http://example.com:8080/api' }, { count: 3, sum: 1.5, @@ -131,7 +131,7 @@ describe('BatchHistogram', () => { ); histogram.recordBatch( - { service: 'web', app: 'my_app' }, + { service: 'web', url: 'https://app.example.com/dashboard' }, { count: 2, sum: 3.0, @@ -145,29 +145,30 @@ describe('BatchHistogram', () => { const metrics = await registry.metrics(); expect(metrics).toMatch( - /test_histogram_bucket{app="my_app",service="api",le="1"} 2/, + /test_histogram_bucket{service="api",url="http:\/\/example\.com:8080\/api",le="1"} 2/, ); expect(metrics).toMatch( - /test_histogram_bucket{app="my_app",service="api",le="\+Inf"} 3/, + /test_histogram_bucket{service="api",url="http:\/\/example\.com:8080\/api",le="\+Inf"} 3/, ); expect(metrics).toMatch( - /test_histogram_sum{app="my_app",service="api"} 1\.5/, + /test_histogram_sum{service="api",url="http:\/\/example\.com:8080\/api"} 1\.5/, ); expect(metrics).toMatch( - /test_histogram_count{app="my_app",service="api"} 3/, + /test_histogram_count{service="api",url="http:\/\/example\.com:8080\/api"} 3/, ); + // Web service metrics (with HTTPS URL) expect(metrics).toMatch( - /test_histogram_bucket{app="my_app",service="web",le="1"} 1/, + /test_histogram_bucket{service="web",url="https:\/\/app\.example\.com\/dashboard",le="1"} 1/, ); expect(metrics).toMatch( - /test_histogram_bucket{app="my_app",service="web",le="\+Inf"} 2/, + /test_histogram_bucket{service="web",url="https:\/\/app\.example\.com\/dashboard",le="\+Inf"} 2/, ); expect(metrics).toMatch( - /test_histogram_sum{app="my_app",service="web"} 3/, + /test_histogram_sum{service="web",url="https:\/\/app\.example\.com\/dashboard"} 3/, ); expect(metrics).toMatch( - /test_histogram_count{app="my_app",service="web"} 2/, + /test_histogram_count{service="web",url="https:\/\/app\.example\.com\/dashboard"} 2/, ); }); @@ -193,4 +194,37 @@ describe('BatchHistogram', () => { expect(metrics).toMatch(/test_histogram_sum{client="sdk"} 12\.3/); expect(metrics).toMatch(/test_histogram_count{client="sdk"} 5/); }); + + test('should handle unsorted bucket input', async () => { + histogram.recordBatch( + { service: 'test' }, + { + count: 5, + sum: 7.5, + buckets: [ + { le: '+Inf', count: 5 }, // Infinity first (unsorted) + { le: 2.5, count: 4 }, // Out of order + { le: 0.5, count: 2 }, // Out of order + { le: 1, count: 3 }, // Out of order + ], + }, + ); + + const metrics = await registry.metrics(); + + expect(metrics).toMatch( + /test_histogram_bucket{service="test",le="0.5"} 2/, + ); + expect(metrics).toMatch( + /test_histogram_bucket{service="test",le="1"} 3/, + ); + expect(metrics).toMatch( + /test_histogram_bucket{service="test",le="2.5"} 4/, + ); + expect(metrics).toMatch( + /test_histogram_bucket{service="test",le="\+Inf"} 5/, + ); + expect(metrics).toMatch(/test_histogram_sum{service="test"} 7\.5/); + expect(metrics).toMatch(/test_histogram_count{service="test"} 5/); + }); }); diff --git a/src/lib/features/metrics/impact/batch-histogram.ts b/src/lib/features/metrics/impact/batch-histogram.ts index 4b604c294b..6bf3ebb52a 100644 --- a/src/lib/features/metrics/impact/batch-histogram.ts +++ b/src/lib/features/metrics/impact/batch-histogram.ts @@ -15,6 +15,7 @@ export class BatchHistogram { private name: string; private help: string; private registry: Registry; + public labelNames: string[] = []; // Store accumulated data for each label combination private store: Map< @@ -65,7 +66,7 @@ export class BatchHistogram { private createLabelKey(labels: Record): string { const sortedKeys = Object.keys(labels).sort(); - return sortedKeys.map((key) => `${key}:${labels[key]}`).join(','); + return JSON.stringify(sortedKeys.map((key) => [key, labels[key]])); } reset(): void { @@ -78,10 +79,12 @@ export class BatchHistogram { for (const [labelKey, data] of this.store) { const labels: Record = {}; if (labelKey) { - labelKey.split(',').forEach((pair) => { - const [key, value] = pair.split(':'); - labels[key] = value; - }); + const parsedLabels = JSON.parse(labelKey); + parsedLabels.forEach( + ([key, value]: [string, string | number]) => { + labels[key] = value; + }, + ); } for (const [le, cumulativeCount] of Array.from( 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 5d124ced66..724ccb8652 100644 --- a/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts +++ b/src/lib/features/metrics/impact/impact-metrics.e2e.test.ts @@ -180,7 +180,6 @@ test('should store histogram metrics with batch data', async () => { name: 'response_time', help: 'Response time histogram', type: 'histogram', - buckets: [1], samples: [ { labels: { foo: 'bar' }, @@ -200,7 +199,6 @@ test('should store histogram metrics with batch data', async () => { name: 'response_time', help: 'Response time histogram', type: 'histogram', - buckets: [1], samples: [ { labels: { foo: 'bar' }, diff --git a/src/lib/features/metrics/impact/metrics-translator.test.ts b/src/lib/features/metrics/impact/metrics-translator.test.ts index 387ad143b8..ad3cb604f9 100644 --- a/src/lib/features/metrics/impact/metrics-translator.test.ts +++ b/src/lib/features/metrics/impact/metrics-translator.test.ts @@ -193,6 +193,22 @@ describe('MetricsTranslator', () => { }, ], }, + { + name: 'histogram_with_labels', + help: 'histogram with labels', + type: 'histogram' as const, + samples: [ + { + labels: { service: 'api' }, + count: 5, + sum: 2.5, + buckets: [ + { le: 1, count: 3 }, + { le: '+Inf' as const, count: 5 }, + ], + }, + ], + }, ]; const result1 = await translator.translateAndSerializeMetrics(metrics1); @@ -202,6 +218,9 @@ describe('MetricsTranslator', () => { expect(result1).toContain( 'unleash_gauge_gauge_with_labels{unleash_env="prod",unleash_origin="sdk"} 10', ); + expect(result1).toContain( + 'unleash_histogram_histogram_with_labels_count{unleash_origin="sdk",unleash_service="api"} 5', + ); const metrics2 = [ { @@ -226,6 +245,23 @@ describe('MetricsTranslator', () => { }, ], }, + { + name: 'histogram_with_labels', + help: 'histogram with labels', + type: 'histogram' as const, + buckets: [1], + samples: [ + { + labels: { service: 'api', region: 'us-east' }, // Added a new label + count: 3, + sum: 1.8, + buckets: [ + { le: 1, count: 2 }, + { le: '+Inf' as const, count: 3 }, + ], + }, + ], + }, ]; const result2 = await translator.translateAndSerializeMetrics(metrics2); @@ -236,5 +272,11 @@ describe('MetricsTranslator', () => { expect(result2).toContain( 'unleash_gauge_gauge_with_labels{unleash_env="prod",unleash_region="us-east",unleash_origin="sdk"} 20', ); + expect(result2).toContain( + 'unleash_histogram_histogram_with_labels_count{unleash_origin="sdk",unleash_region="us-east",unleash_service="api"} 3', + ); + expect(result2).not.toContain( + 'unleash_histogram_histogram_with_labels_count{unleash_origin="sdk",unleash_service="api"} 5', + ); }); }); diff --git a/src/lib/features/metrics/impact/metrics-translator.ts b/src/lib/features/metrics/impact/metrics-translator.ts index fb9b8e81b0..bf7fb06a27 100644 --- a/src/lib/features/metrics/impact/metrics-translator.ts +++ b/src/lib/features/metrics/impact/metrics-translator.ts @@ -24,7 +24,6 @@ export interface BucketMetric { name: string; help: string; type: 'histogram'; - buckets: number[]; samples: BucketMetricSample[]; } @@ -32,7 +31,6 @@ export type Metric = NumericMetric | BucketMetric; export class MetricsTranslator { private registry: Registry; - private histograms: Map = new Map(); constructor(registry: Registry) { this.registry = registry; @@ -166,14 +164,26 @@ export class MetricsTranslator { return null; } - let histogram = this.histograms.get(prefixedName); - if (!histogram) { + let histogram: BatchHistogram; + + if (existingMetric && existingMetric instanceof BatchHistogram) { + if (this.hasNewLabels(existingMetric, labelNames)) { + this.registry.removeSingleMetric(prefixedName); + + histogram = new BatchHistogram({ + name: prefixedName, + help: metric.help, + registry: this.registry, + }); + } else { + histogram = existingMetric as BatchHistogram; + } + } else { histogram = new BatchHistogram({ name: prefixedName, help: metric.help, registry: this.registry, }); - this.histograms.set(prefixedName, histogram); } for (const sample of metric.samples) {