Skip to content

Commit 43e32aa

Browse files
wrn14897wrn14897
authored
feat: revisit Otel metrics semantic convention migration logics (#1267)
Since users can still switch to the new metric name using feature gate Follow up #1248
1 parent 79af32e commit 43e32aa

File tree

7 files changed

+53
-44
lines changed

7 files changed

+53
-44
lines changed

‎.changeset/lucky-plums-sort.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@hyperdx/common-utils": patch
3+
"@hyperdx/api": patch
4+
"@hyperdx/app": patch
5+
---
6+
7+
fix: handle metrics semantic convention upgrade (feature gate)

‎packages/api/src/clickhouse/__tests__/renderChartConfig.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,7 +1431,7 @@ describe('renderChartConfig', () => {
14311431
aggFn: 'avg',
14321432
metricName: 'k8s.pod.cpu.utilization',
14331433
metricNameSql:
1434-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
1434+
"MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')",
14351435
metricType: MetricsDataType.Gauge,
14361436
valueExpression: 'Value',
14371437
},
@@ -1452,10 +1452,10 @@ describe('renderChartConfig', () => {
14521452
expect(res.length).toBeGreaterThan(0);
14531453
expect(res).toMatchSnapshot();
14541454

1455-
// Verify the SQL contains the dynamic metric name condition
1456-
expect(query.sql).toContain('if(greaterOrEquals(ScopeVersion');
1455+
// Verify the SQL contains the IN-based metric name condition
14571456
expect(query.sql).toContain('k8s.pod.cpu.usage');
14581457
expect(query.sql).toContain('k8s.pod.cpu.utilization');
1458+
expect(query.sql).toMatch(/MetricName IN /);
14591459
});
14601460

14611461
it('should handle gauge metric with metricNameSql and groupBy', async () => {
@@ -1466,7 +1466,7 @@ describe('renderChartConfig', () => {
14661466
aggFn: 'avg',
14671467
metricName: 'k8s.pod.cpu.utilization',
14681468
metricNameSql:
1469-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
1469+
"MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')",
14701470
metricType: MetricsDataType.Gauge,
14711471
valueExpression: 'Value',
14721472
},
@@ -1516,9 +1516,9 @@ describe('renderChartConfig', () => {
15161516
// Should only return data from old metric name (k8s.pod.cpu.utilization)
15171517
expect(res).toMatchSnapshot();
15181518

1519-
// Verify the SQL uses simple string comparison
1519+
// Verify the SQL uses simple string comparison (not IN-based)
15201520
expect(query.sql).toContain("MetricName = 'k8s.pod.cpu.utilization'");
1521-
expect(query.sql).not.toContain('if(greaterOrEquals(ScopeVersion');
1521+
expect(query.sql).not.toMatch(/MetricName IN /);
15221522
});
15231523
});
15241524
});

‎packages/app/src/__tests__/otelSemanticConventions.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,21 @@ describe('otelSemanticConventions', () => {
55
it('should return SQL for k8s.pod.cpu.utilization migration', () => {
66
const result = getMetricNameSql('k8s.pod.cpu.utilization');
77
expect(result).toBe(
8-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
8+
"MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')",
99
);
1010
});
1111

1212
it('should return SQL for k8s.node.cpu.utilization migration', () => {
1313
const result = getMetricNameSql('k8s.node.cpu.utilization');
1414
expect(result).toBe(
15-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.node.cpu.usage', 'k8s.node.cpu.utilization')",
15+
"MetricName IN ('k8s.node.cpu.utilization', 'k8s.node.cpu.usage')",
1616
);
1717
});
1818

1919
it('should return SQL for container.cpu.utilization migration', () => {
2020
const result = getMetricNameSql('container.cpu.utilization');
2121
expect(result).toBe(
22-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'container.cpu.usage', 'container.cpu.utilization')",
22+
"MetricName IN ('container.cpu.utilization', 'container.cpu.usage')",
2323
);
2424
});
2525

‎packages/app/src/otelSemanticConventions.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,38 +2,37 @@
22
* OpenTelemetry Semantic Conventions utilities
33
* Handles transformations between different versions of OTel semantic conventions
44
*/
5+
import SqlString from 'sqlstring';
56

67
/**
78
* Mapping of old metric names to new metric names based on semantic convention version
9+
* The key is the old metric name for easy lookup
810
*/
911
const METRIC_NAME_MIGRATIONS: Record<
1012
string,
1113
{
12-
oldName: string;
1314
newName: string;
1415
versionThreshold: string;
1516
}
1617
> = {
1718
'k8s.pod.cpu.utilization': {
18-
oldName: 'k8s.pod.cpu.utilization',
1919
newName: 'k8s.pod.cpu.usage',
2020
versionThreshold: '0.125.0',
2121
},
2222
'k8s.node.cpu.utilization': {
23-
oldName: 'k8s.node.cpu.utilization',
2423
newName: 'k8s.node.cpu.usage',
2524
versionThreshold: '0.125.0',
2625
},
2726
'container.cpu.utilization': {
28-
oldName: 'container.cpu.utilization',
2927
newName: 'container.cpu.usage',
3028
versionThreshold: '0.125.0',
3129
},
3230
};
3331

3432
/**
35-
* Generates SQL expression to dynamically select metric name based on ScopeVersion
36-
* @param metricName - The metric name to check for migrations
33+
* Generates SQL expression to coerce metric name to handle both old and new conventions
34+
* Matches metrics using either the old or new naming convention
35+
* @param metricName - The metric name to check for migrations (should be the old name)
3736
* @returns SQL expression if migration exists, undefined otherwise
3837
*/
3938
export function getMetricNameSql(metricName: string): string | undefined {
@@ -43,5 +42,7 @@ export function getMetricNameSql(metricName: string): string | undefined {
4342
return undefined;
4443
}
4544

46-
return `if(greaterOrEquals(ScopeVersion, '${migration.versionThreshold}'), '${migration.newName}', '${migration.oldName}')`;
45+
return SqlString.format('MetricName IN (?)', [
46+
[metricName, migration.newName],
47+
]);
4748
}

‎packages/common-utils/src/__tests__/__snapshots__/renderChartConfig.test.ts.snap‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ exports[`renderChartConfig k8s semantic convention migrations should generate SQ
296296
cityHash64(ExplicitBounds) AS bounds_hash,
297297
CAST(BucketCounts AS Array(Int64)) counts
298298
FROM default.otel_metrics_histogram
299-
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 2 minute) - INTERVAL 2 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 2 minute) + INTERVAL 2 minute) AND ((MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'container.cpu.usage', 'container.cpu.utilization')))
299+
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 2 minute) - INTERVAL 2 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 2 minute) + INTERVAL 2 minute) AND ((MetricName IN ('container.cpu.utilization', 'container.cpu.usage')))
300300
ORDER BY attr_hash, TimeUnix ASC
301301
)
302302
)
@@ -380,7 +380,7 @@ exports[`renderChartConfig k8s semantic convention migrations should generate SQ
380380
cityHash64(ExplicitBounds) AS bounds_hash,
381381
CAST(BucketCounts AS Array(Int64)) counts
382382
FROM default.otel_metrics_histogram
383-
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 1 minute) - INTERVAL 1 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 1 minute) + INTERVAL 1 minute) AND ((MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')))
383+
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 1 minute) - INTERVAL 1 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 1 minute) + INTERVAL 1 minute) AND ((MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')))
384384
ORDER BY attr_hash, TimeUnix ASC
385385
)
386386
)
@@ -435,7 +435,7 @@ exports[`renderChartConfig k8s semantic convention migrations should generate SQ
435435
) AS Rate,
436436
IF(AggregationTemporality = 1, Rate, Value) AS Sum
437437
FROM default.otel_metrics_sum
438-
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 5 minute) - INTERVAL 5 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 5 minute) + INTERVAL 5 minute) AND ((MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.node.cpu.usage', 'k8s.node.cpu.utilization')))),Bucketed AS (
438+
WHERE (TimeUnix >= toStartOfInterval(fromUnixTimestamp64Milli(1739318400000), INTERVAL 5 minute) - INTERVAL 5 minute AND TimeUnix <= toStartOfInterval(fromUnixTimestamp64Milli(1765670400000), INTERVAL 5 minute) + INTERVAL 5 minute) AND ((MetricName IN ('k8s.node.cpu.utilization', 'k8s.node.cpu.usage')))),Bucketed AS (
439439
SELECT
440440
toStartOfInterval(toDateTime(TimeUnix), INTERVAL 5 minute) AS \`__hdx_time_bucket2\`,
441441
AttributesHash,
@@ -473,7 +473,7 @@ exports[`renderChartConfig k8s semantic convention migrations should generate SQ
473473
*,
474474
cityHash64(mapConcat(ScopeAttributes, ResourceAttributes, Attributes)) AS AttributesHash
475475
FROM default.otel_metrics_gauge
476-
WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1765670400000)) AND ((MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')))
476+
WHERE (TimeUnix >= fromUnixTimestamp64Milli(1739318400000) AND TimeUnix <= fromUnixTimestamp64Milli(1765670400000)) AND ((MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')))
477477
),Bucketed AS (
478478
SELECT
479479
toStartOfInterval(toDateTime(TimeUnix), INTERVAL 1 minute) AS \`__hdx_time_bucket2\`,

‎packages/common-utils/src/__tests__/renderChartConfig.test.ts‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ describe('renderChartConfig', () => {
434434
valueExpression: 'Value',
435435
metricName: 'k8s.pod.cpu.utilization',
436436
metricNameSql:
437-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
437+
"MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')",
438438
metricType: MetricsDataType.Gauge,
439439
},
440440
],
@@ -449,10 +449,10 @@ describe('renderChartConfig', () => {
449449
const generatedSql = await renderChartConfig(config, mockMetadata);
450450
const actual = parameterizedQueryToSql(generatedSql);
451451

452-
// Verify the SQL contains the dynamic metric name condition
453-
expect(actual).toContain(
454-
"MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
455-
);
452+
// Verify the SQL contains the IN-based metric name condition
453+
expect(actual).toContain('k8s.pod.cpu.utilization');
454+
expect(actual).toContain('k8s.pod.cpu.usage');
455+
expect(actual).toMatch(/MetricName IN /);
456456
expect(actual).toMatchSnapshot();
457457
});
458458

@@ -479,7 +479,7 @@ describe('renderChartConfig', () => {
479479
valueExpression: 'Value',
480480
metricName: 'k8s.node.cpu.utilization',
481481
metricNameSql:
482-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.node.cpu.usage', 'k8s.node.cpu.utilization')",
482+
"MetricName IN ('k8s.node.cpu.utilization', 'k8s.node.cpu.usage')",
483483
metricType: MetricsDataType.Sum,
484484
},
485485
],
@@ -494,9 +494,9 @@ describe('renderChartConfig', () => {
494494
const generatedSql = await renderChartConfig(config, mockMetadata);
495495
const actual = parameterizedQueryToSql(generatedSql);
496496

497-
expect(actual).toContain(
498-
"MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.node.cpu.usage', 'k8s.node.cpu.utilization')",
499-
);
497+
expect(actual).toContain('k8s.node.cpu.utilization');
498+
expect(actual).toContain('k8s.node.cpu.usage');
499+
expect(actual).toMatch(/MetricName IN /);
500500
expect(actual).toMatchSnapshot();
501501
});
502502

@@ -522,7 +522,7 @@ describe('renderChartConfig', () => {
522522
valueExpression: 'Value',
523523
metricName: 'container.cpu.utilization',
524524
metricNameSql:
525-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'container.cpu.usage', 'container.cpu.utilization')",
525+
"MetricName IN ('container.cpu.utilization', 'container.cpu.usage')",
526526
metricType: MetricsDataType.Histogram,
527527
},
528528
],
@@ -537,9 +537,9 @@ describe('renderChartConfig', () => {
537537
const generatedSql = await renderChartConfig(config, mockMetadata);
538538
const actual = parameterizedQueryToSql(generatedSql);
539539

540-
expect(actual).toContain(
541-
"MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'container.cpu.usage', 'container.cpu.utilization')",
542-
);
540+
expect(actual).toContain('container.cpu.utilization');
541+
expect(actual).toContain('container.cpu.usage');
542+
expect(actual).toMatch(/MetricName IN /);
543543
expect(actual).toMatchSnapshot();
544544
});
545545

@@ -565,7 +565,7 @@ describe('renderChartConfig', () => {
565565
valueExpression: 'Value',
566566
metricName: 'k8s.pod.cpu.utilization',
567567
metricNameSql:
568-
"if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
568+
"MetricName IN ('k8s.pod.cpu.utilization', 'k8s.pod.cpu.usage')",
569569
metricType: MetricsDataType.Histogram,
570570
},
571571
],
@@ -581,9 +581,9 @@ describe('renderChartConfig', () => {
581581
const generatedSql = await renderChartConfig(config, mockMetadata);
582582
const actual = parameterizedQueryToSql(generatedSql);
583583

584-
expect(actual).toContain(
585-
"MetricName = if(greaterOrEquals(ScopeVersion, '0.125.0'), 'k8s.pod.cpu.usage', 'k8s.pod.cpu.utilization')",
586-
);
584+
expect(actual).toContain('k8s.pod.cpu.utilization');
585+
expect(actual).toContain('k8s.pod.cpu.usage');
586+
expect(actual).toMatch(/MetricName IN /);
587587
expect(actual).toMatchSnapshot();
588588
});
589589

@@ -624,9 +624,9 @@ describe('renderChartConfig', () => {
624624
const generatedSql = await renderChartConfig(config, mockMetadata);
625625
const actual = parameterizedQueryToSql(generatedSql);
626626

627-
// Should use the simple string comparison for regular metrics
627+
// Should use the simple string comparison for regular metrics (not IN-based)
628628
expect(actual).toContain("MetricName = 'some.regular.metric'");
629-
expect(actual).not.toContain('if(greaterOrEquals(ScopeVersion');
629+
expect(actual).not.toMatch(/MetricName IN /);
630630
expect(actual).toMatchSnapshot();
631631
});
632632
});

‎packages/common-utils/src/renderChartConfig.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@ import { CustomSchemaSQLSerializerV2, SearchQueryBuilder } from '@/queryParser';
88

99
/**
1010
* Helper function to create a MetricName filter condition.
11-
* Uses metricNameSql if available (for dynamic SQL), otherwise falls back to metricName.
11+
* Uses metricNameSql if available (which handles both old and new metric names via OR),
12+
* otherwise falls back to a simple equality check.
1213
*/
1314
function createMetricNameFilter(
1415
metricName: string,
1516
metricNameSql?: string,
1617
): string {
17-
return SqlString.format(
18-
'MetricName = ?',
19-
metricNameSql ? SqlString.raw(metricNameSql) : [metricName],
20-
);
18+
if (metricNameSql) {
19+
return metricNameSql;
20+
}
21+
return SqlString.format('MetricName = ?', [metricName]);
2122
}
2223
import {
2324
AggregateFunction,

0 commit comments

Comments
 (0)